WIP: Narrow which in pkg context#15381
Draft
punchagan wants to merge 7 commits into
Draft
Conversation
punchagan
force-pushed
the
narrow-pkg-which
branch
5 times, most recently
from
344e923 to
e258dde
Compare
punchagan
force-pushed
the
narrow-pkg-which
branch
2 times, most recently
from
8aaa6fd to
e186b0f
Compare
Extend the test as a precursor to fixing PATH to include only the bin layouts of packages that the owning package of a stanza explicitly depend on. Signed-off-by: Puneeth Chaganti <punchagan@muse-amuse.in>
This commit extends the resolve-program-from-undeclared-pkg.t cram test in preparation for narrowing of the lockdir packages on PATH based on the dependency closure of a stanza's owning package. This PR is similar to ocaml#15222 Signed-off-by: Puneeth Chaganti <punchagan@muse-amuse.in>
Signed-off-by: Puneeth Chaganti <punchagan@muse-amuse.in>
Signed-off-by: Puneeth Chaganti <punchagan@muse-amuse.in>
We only add the bin layouts of the packages in the dependencies closure of a stanza to the env's PATH. Signed-off-by: Puneeth Chaganti <punchagan@muse-amuse.in>
punchagan
force-pushed
the
narrow-pkg-which
branch
from
c3c9a0e to
2ea1389
Compare
Signed-off-by: Puneeth Chaganti <punchagan@muse-amuse.in>
Signed-off-by: Puneeth Chaganti <punchagan@muse-amuse.in>
punchagan
force-pushed
the
narrow-pkg-which
branch
from
2ea1389 to
985e8af
Compare
rgrinberg
reviewed
Jul 5, 2026
| val which : Context_name.t -> (Filename.t -> Path.t option Memo.t) Staged.t | ||
|
|
||
| val which_for_packages | ||
| : ?packages:Package.Name.Set.t option |
Member
There was a problem hiding this comment.
This is still a WIP, but this argument being optional just obscures the scope of the changes.
Alizter
added a commit
that referenced
this pull request
Jul 6, 2026
<!-- Thank you for contributing to dune! For general guidelines on contributing to dune, see https://github.com/ocaml/dune/blob/main/CONTRIBUTING.md#developing-dune --> ## Description Extend the test as a precursor to fixing PATH to include only the bin layouts of packages that the owning package of a stanza explicitly depend on. ## Related Issue and Motivation <!-- Why is this change required? What problem does it solve? --> <!-- If it closes an open issue, link to the issue here. --> <!-- Non-trivial contributions are expected to be preceded by an issue, as per CONTRIBUTING.md. --> This change is a preparatory improvement to the test for the changes in #15381
Alizter
added a commit
that referenced
this pull request
Jul 6, 2026
## Description This commit extends the resolve-program-from-undeclared-pkg.t cram test in preparation for narrowing of the lockdir packages on PATH based on the dependency closure of a stanza's owning package. This PR is similar to #15222 ## Related Issue and Motivation <!-- Why is this change required? What problem does it solve? --> <!-- If it closes an open issue, link to the issue here. --> <!-- Non-trivial contributions are expected to be preceded by an issue, as per CONTRIBUTING.md. --> This is a preparatory change in the resolve-program test case for narrowing of the lockdir package binaries available on PATH implemented in #15381.
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.
Stacked on #15412 and #15413
In the Lock-kind contexts,
%{bin:X}and%{bin-available:X}fall through toPkg_rules.whichviaContext.whichwhich runsArtifacts_and_deps.of_closureon all the lock-dir packages. The function adds a dependency on thecookie, which is a build artifact, of all the lock-dir packages. This means we add a dependency on building all the packages in the lock-dir to resolve these pforms, leading to one of the reasons for the in-and-out cycles as described here. #14736 added a test, which was extended in #15222, to document the behavior when expanding these pforms.This PR narrows the closure of packages passed to
Artifacts_and_deps.of_closureto the dependency closure of the package that owns the stanza's directory, when expanding these pforms. When there is no owning package, the previous behavior of using the whole lockdir closure is still retained.Previously, all lock-dir packages' bin layouts were also added to
PATH. This PR applies the same narrowing as the pforms. Only the bin layouts of the owning package's dependency closure are added to PATH. So, a stanza can run only binaries from the packages it depends on. If there's no owning package for a stanza, the old behavior is retained.