feat: bin-layout for %{bin:...} deps #14432
Draft
Alizter wants to merge 1 commit into
Draft
Conversation
3dc397b to
f5576c4
Compare
44701f1 to
b9f314d
Compare
78a86fb to
dea3003
Compare
Alizter
added a commit
that referenced
this pull request
May 10, 2026
In this PR we organise all the tests about `%{bin:...}` and increase our
test coverage. This is in preparation of:
- #14432
- #14373
Add `test/blackbox-tests/test-cases/bin-pform/` with focused tests for
the `%{bin:NAME}` pform.
New tests:
- `cram-path.t` `%{bin:...}` in cram `(deps ...)`
- `env-binaries.t` `%{bin:...}` for `(env (binaries ...))` entries
- `include-deps.t` `%{bin:...}` inside `(include ...)` deps
- `inline-tests.t` `%{bin:...}` in `(inline_tests (deps ...))`
- `multiple-bins.t` multiple `%{bin:...}` deps in a single rule
- `public-name.t` `%{bin:...}` expansion for `(public_name)`
- `run-action.t` `%{bin:...}` in `(run ...)` and `PATH` visibility
- `run-literal-vs-pform.t` `(run NAME)` vs `(run %{bin:NAME})` record
different deps (build artifact vs install staging)
- `system.t` `%{bin:...}` for a system binary on `PATH`
- `test-stanza.t` `%{bin:...}` in a test stanza
- `missing-bin.t` `%{bin:...}` and `(run LITERAL)` where the name
doesn't resolve
Plus a cross-compilation analog of
`lib-pform-context-expansion-gh9199.t`
at `custom-cross-compilation/bin-pform-context-expansion.t`.
Renamed:
- `bin-eager-deps.t -> bin-pform/lazy-failure.t`
- `adversarial-binary-name.t -> bin-pform/colon-in-name.t`
- `build-percent-form-bin.t -> bin-pform/cli-build.t`
Split `missing-lib-bin-pform-errors-gh1541.t`: bin parts moved into
`bin-pform/missing-bin.t`, lib parts stay in
`missing-lib-pform-errors-gh1541.t`.
`setup-script.sh`: add env_added helper.
b499421 to
d2626f9
Compare
2 tasks
4fc638f to
018fbbe
Compare
Resolve %{bin:...} to the build artifact path (Original_path) instead
of the install staging area. When %{bin:...} appears in (deps ...),
a bin-layout directory is created under _build/install/<context>/
.bin-layout/<digest>/ with correctly-named symlinks, and added to PATH.
The legacy staging paths (_build/install/default/bin, lib, etc.) remain
on the root env for now. They cannot be removed until (deps (package ...))
provides its own scoped layout (the install-layout PR), since actions
using (package ...) deps still rely on the staging area for PATH and
OCAMLPATH visibility.
Env binaries (from (env (binaries ...))) are excluded from the bin-layout
since they already use .bin/ on PATH.
- Fixes ocaml#3324 (dune build %{bin:foo} returns an error)
- Related: ocaml#12996 (dune exec and install stanza interaction)
- Related: ocaml#9496 (original introduction of Install_dir/Original_path)
Signed-off-by: Ali Caglayan <alizter@gmail.com>
---
TODO before merge:
- Expand the CHANGES entry to flag this as a breaking change:
- %{bin:NAME} pform now resolves to the build artifact path
(was: install staging path). Affects pforms used as string values
(e.g. (echo %{bin:foo})), as run targets, and as deps.
- argv[0] differs between (run %{bin:foo}) (build artifact path)
and PATH-callable foo (bin-layout symlink basename).
- Note: not gated by lang version. This is intentional. nkm removes
the always-on staging area, so an Install_dir-gated %{bin:NAME}
would point at a path that nothing builds. Flipping universally
keeps the pform value coherent across the chain.
- Add a paragraph to the dune reference manual where %{bin:...} is
described, explaining the new resolution target and the bin-layout
on PATH for actions with %{bin:...} deps.
- Comment on Action.Full.add_env (src/dune_engine/action.ml:498) noting
that env values overwrite for shared keys, so multiple PATH-prepend
hints must be concat-merged with Env_path.extend_env_concat_path
before calling add_env. The current convention is implicit
(cram_rules.ml is the example).
Known limitations (deferred to follow-up):
- dune's sandbox does not relocate the PATH environment variable.
The bin-layout dir on PATH points at the un-sandboxed _build/.
This works locally because absolute paths are still readable from
inside the sandbox, but would break under remote action execution.
Tracked by test/blackbox-tests/test-cases/bin-pform/sandbox.t.
- Bin_layout.Key.reverse_table grows unboundedly in long-running
--watch sessions (one entry per unique sorted bin set). Pattern
inherited from Ppx_driver.Key. Bounded by the number of distinct
bin sets the workspace evaluates; unlikely to be a real problem.
The earlier (include ...) propagation limitation is fixed by the
follow-up commit "fix: propagate bin_env across (include ...) deps".
Signed-off-by: Ali Caglayan <alizter@gmail.com>
018fbbe to
9bd7512
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
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.
Resolve
%{bin:...}to the build artifact path (Original_path) instead of the install staging area. When%{bin:...}appears in(deps ...), a bin-layout directory is created under_build/install/<context>/.bin-layout/<digest>/with correctly-named symlinks, and added toPATH.The legacy staging paths (
_build/install/default/bin,lib, etc.) remain on the root env for now. They cannot be removed until(deps (package ...))provides its own scoped layout (the install-layout PR #14373), since actions using(package ...)deps still rely on the staging area forPATHandOCAMLPATHvisibility.Env binaries (from
(env (binaries ...))) are excluded from the bin-layout since they already use.bin/onPATH.TODO before merge
Expand the CHANGES entry to flag this as a breaking change:
%{bin:NAME}pform now resolves to the build artifact path (was: install staging path). Affects pforms used as string values (e.g.(echo %{bin:foo})), as run targets, and as deps.argv[0]differs between(run %{bin:foo})(build artifact path) and PATH-callablefoo(bin-layout symlink basename).Install_dir-gated%{bin:NAME}would point at a path that nothing builds. Flipping universally keeps the pform value coherent across the chain.Add a paragraph to the dune reference manual where
%{bin:...}is described, explaining the new resolution target and the bin-layout on PATH for actions with%{bin:...}deps.Comment on
Action.Full.add_env(src/dune_engine/action.ml:498) noting that env values overwrite for shared keys, so multiplePATH-prepend hints must be concat-merged withEnv_path.extend_env_concat_pathbefore callingadd_env. The current convention is implicit (cram_rules.mlis the example).Known limitations (deferred to follow-up)
dune's sandbox does not relocate the
PATHenvironment variable. The bin-layout dir onPATHpoints at the un-sandboxed_build/. This works locally because absolute paths are still readable from inside the sandbox, but would break under remote action execution. Tracked bytest/blackbox-tests/test-cases/bin-pform/sandbox.t.Bin_layout.Key.reverse_tablegrows unboundedly in long-running--watchsessions (one entry per unique sorted bin set). Pattern inherited fromPpx_driver.Key. Bounded by the number of distinct bin sets the workspace evaluates; unlikely to be a real problem.The earlier
(include ...)propagation limitation is fixed by a follow-up commit between this PR and #14373 (separate PR forthcoming).