fix(pkg): unify workspace and lockdir dependency traversal - #15636
Open
Alizter wants to merge 1 commit into
Open
fix(pkg): unify workspace and lockdir dependency traversal#15636Alizter wants to merge 1 commit into
Alizter wants to merge 1 commit into
Conversation
Alizter
force-pushed
the
push-svuqwnmomsll
branch
2 times, most recently
from
July 31, 2026 15:32
c21ac99 to
01cab0f
Compare
Member
|
The fact that this change seems to remove a bunch of code without dropping functionality is good, but the description makes no sense. Can you rewrite it to something that describes what is accomplished without so much references to mechanical changes? |
Alizter
changed the title
Alizter
marked this pull request as ready for review
August 3, 2026 08:34
Alizter
force-pushed
the
push-svuqwnmomsll
branch
2 times, most recently
from
August 3, 2026 13:13
94962af to
ce74cb1
Compare
Alizter
changed the title
Alizter
force-pushed
the
push-svuqwnmomsll
branch
from
August 4, 2026 07:33
ce74cb1 to
e0ac898
Compare
Store the platform-filtered lock packages in the universe and compute transitive closures with one reachability traversal over a combined graph whose nodes are local packages (via their resolved dependency formulas) and lock packages (via their platform-selected dependency lists), instead of stitching a local-only closure onto Lock_dir.transitive_dependency_closure. The unnecessary-packages check is converted to the same traversal, which leaves Lock_dir.transitive_dependency_closure without callers; it is deleted together with Packages.choose_pkg_for_platform, which it was the last user of. Caching the platform-filtered packages also confines the platform projection of the package set to the universe constructor instead of reprojecting at every consumer. For valid input this preserves behaviour. Lock-directory packages cannot name workspace packages, so from local roots the combined traversal can only discover the local closure followed by the lockdir closure of its non-local frontier: exactly the union the two-phase code computed. Names that resolve to neither package map are leaves; the previous code ignored them during lockdir traversal (dune, platform-disabled packages) and its missing-seed Code_error was unreachable for validated input. For cyclic input the traversal is plain reachability, so dependency cycles between workspace packages are now tolerated like lockdir cycles instead of crashing the local-only closure with a Code_error, as pinned by the updated lockdir-dependency-cycle test. Rejecting both kinds of cycle properly is the subject of a follow-up commit. Signed-off-by: Ali Caglayan <alizter@gmail.com>
Alizter
force-pushed
the
push-svuqwnmomsll
branch
from
August 5, 2026 13:23
e0ac898 to
e5db0d5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Dune currently computes dependency reachability separately for workspace
packages and lock-directory packages. This works only because locked packages
are currently forbidden from depending back on workspace packages. It cannot
represent a dependency path such as:
This refactor makes
Package_universeown a single view of dependencies acrossboth package sets. Dependency queries and lock-directory reachability checks now
use that view, removing the assumption that dependency traversal ends after
entering the lock directory.
This does not yet permit lock-directory packages to depend on workspace
packages. Follow-up changes use this graph to produce dependency-first build
plans, reject cycles across the combined graph, and ultimately enable those
dependencies.
Existing valid lock directories produce the same dependency closures. The only
observable change is that querying a cycle between workspace packages no longer
raises an internal error; the following PR replaces this temporary tolerance
with a user-facing cycle diagnostic.
Related Issue and Motivation
This is the dependency-graph foundation for supporting lock-directory packages
that depend on workspace packages in #8652.