Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions bin/exec.ml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,20 @@ let program_not_built_yet prog =
]
;;

(* Cons the workspace's install staging dir ([_build/install/<ctx>/]) onto
the env's path-like vars (OCAMLPATH, PATH, etc.) for [dune exec]. This
does NOT trigger any build; if the staging dir isn't populated (no
[@install] was run), the entries on the path simply don't resolve. The
point is to preserve the old [dune exec] contract that workspace
libraries are visible via OCAMLPATH to anything the binary delegates to
(findlib, ocamlfind, dune-site's plugin lookup, etc.). Per-action rule
envs are unaffected — only [dune exec]'s own env extension. *)
let extend_with_staging_env context base =
let staging = Install.Context.dir ~context in
let roots = Install.Roots.opam_from_prefix staging ~relative:Path.Build.relative in
Install.Roots.add_to_env roots base
;;

let build_prog ~no_rebuild ~prog p =
if no_rebuild
then
Expand Down Expand Up @@ -176,6 +190,7 @@ let step ~prog ~args ~common ~no_rebuild ~context ~on_exit () =
Memo.parallel_map args ~f:(Cmd_arg.expand ~root:(Common.root common) ~sctx)
in
let* env = Super_context.context_env sctx in
let env = extend_with_staging_env context env in
Memo.of_non_reproducible_fiber
@@ Dune_engine.Process.run_inherit_std_in_out
~dir:(Path.of_string Fpath.initial_cwd)
Expand Down Expand Up @@ -305,6 +320,7 @@ let exec_building_directly ~common ~config ~context ~prog ~args ~no_rebuild =
and* args =
Memo.parallel_map ~f:(Cmd_arg.expand ~root:(Common.root common) ~sctx) args
in
let env = extend_with_staging_env context env in
Util.restore_cwd_and_execve (Common.root common) prog args env)
;;

Expand Down
5 changes: 5 additions & 0 deletions doc/changes/changed/14373.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- `(deps (package ...))` now exposes only the directly declared packages to
the action's environment (`OCAMLPATH`, `PATH`, etc.). Previously, other
packages in the workspace could be discoverable via the shared install
staging area. Actions that relied on undeclared packages being visible
must declare them explicitly. (#14373, @Alizter)
215 changes: 215 additions & 0 deletions doc/dev/install-layouts.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
# Install Layouts for Package Sets

## Overview

`(deps (package ...))` materializes a scoped install layout under
`_build/install/<context>/.packages/<digest>/` containing only the declared
package dependencies. This replaces the old alias-based mechanism where
`(deps (package foo))` depended on the `.foo-files` install alias, which
populated the `_build/install/` staging area shared by all packages.

## Motivation

The `_build/install/` staging area shared by all packages causes several
problems:

1. Actions can silently depend on packages they did not declare via the shared
environment variables (OCAMLPATH, PATH, etc.). Whether an action succeeds
can depend on what other packages happened to be built, making builds
non-deterministic. `(strict_package_deps)` validates that dependencies are
declared but does not prevent undeclared packages from being visible at
runtime.

2. The shared staging area can cause rule collisions and dependency cycles.
Multiple packages installing directory targets to the same section root
crash ([#13307]). Computing install artifacts globally creates cycles in
monorepo builds ([#13500]) and Coq compositions ([#7908]).

3. Lock-dir packages that depend on workspace packages need a scoped install
prefix to build against. The shared staging area does not provide this. This
is the "in-and-out" problem ([#8652]): when a lock-dir package depends on a
workspace package which depends on another lock-dir package, dune cannot
resolve the dependency without per-package install layouts.

[#7908]: https://github.com/ocaml/dune/issues/7908
[#8652]: https://github.com/ocaml/dune/issues/8652
[#13307]: https://github.com/ocaml/dune/issues/13307
[#13500]: https://github.com/ocaml/dune/issues/13500

## Design

### Package kinds

`Package_db.find_package` classifies packages into three kinds:

- `Local`: workspace packages defined in `dune-project`. These get layout
file deps and env vars.

- `Build`: lock-dir packages from `dune.lock`. `(deps (package foo))` runs
the package's build action but does not set up env vars for the consuming
action. This is a known bug. See the TODO section below.

- `Installed`: externally installed packages (found via findlib). Depended
on directly. Already on the system OCAMLPATH.

### Immediate deps only

The layout includes only the immediate packages listed in `(deps (package
...))`. No transitive expansion is performed. Actions should declare what
they need explicitly.

This is a deliberate design choice:

1. Transitive closure cannot traverse lock-dir packages (they are not
workspace packages), so it gives incomplete results in mixed
workspace/lock-dir setups. Immediate deps avoid this inconsistency.

2. Workspace package compilation is handled by dune internally via `Lib.DB`,
not via OCAMLPATH. The only consumer of OCAMLPATH in the layout is
user-written rule actions, where explicit deps are appropriate.

3. Immediate deps keep the layout tractable for the "in-and-out" problem
([#8652]): when a lock-dir package depends on a workspace package, the
layout for that workspace package can be provided to the lock-dir
package's build env without needing to chase transitive deps through
other lock-dir packages.

### Environment variables

The layout sets PATH, OCAMLPATH, CAML_LD_LIBRARY_PATH,
OCAMLFIND_IGNORE_DUPS_IN, OCAMLTOP_INCLUDE_PATH, and MANPATH from the
layout's directory structure. These are defined by `Install.Roots.path_vars`
and consed onto the directory environment in `extend_action`
(super_context.ml), so external/system paths remain visible. The env vars
are propagated to rule actions, cram tests, cinaps, mdx, and inline tests.

### Staging area

`(deps (package ...))` no longer populates the `_build/install/` staging
area. The staging area is only populated by `dune build @install` /
`dune install`. Code that hardcodes staging area paths must use `@install`
or `%{bin:foo}`.

### Environment hermeticity

`make_root_env` returns only `Context.installed_env` with no staging paths
attached. Actions only see declared dependencies via layout env vars consed
in `extend_action`. This is what makes the strict-deps property hold:
nothing in the workspace is reachable unless declared.

### `dune exec` and the staging area

`dune exec` runs binaries outside the rule machinery, so it cannot inherit
per-action env from `combined_package_deps_builder`. To preserve the
documented `dune exec` contract that workspace libraries are reachable via
OCAMLPATH (see the man page: "you will have access to the libraries defined
in the workspace using your usual directives"), `bin/exec.ml` conses
`_build/install/<context>/{lib,bin,...}` onto the path-like env vars of
the executed process.

This cons is unconditional and does not trigger any build. If
`dune build @install` (or any rule that drags install entries in) was run
beforehand, the staging dir is populated and workspace libraries are
visible to anything the binary delegates to (findlib, ocamlfind, dune-site
plugin lookup, etc.). If not, the entries on the path simply don't
resolve. Per-action rule envs are unaffected; only `dune exec`'s own env
extension is consed.

### Sites and plugins

The dune-site mechanism uses `DUNE_DIR_LOCATIONS` to tell binaries where
their sites live. `Site_env.add_packages_env` walks workspace stanzas to
discover site/plugin packages and sets the env var to point at the
staging area (`_build/install/<context>/lib/<pkg>/<section>/`). Combined
with the `dune exec` staging cons above, this means:

- A binary built locally and run via `dune exec foo.exe` sees plugins iff
the staging dir is populated. The dune-site / sites cram tests
consequently run `dune build @install` before `dune exec`.
- Cram tests that exercise dune-site libraries (`(libraries dune-site
dune-site.plugins)`) must declare both `(package dune-site)` and
`(package dune-private-libs)` in their cram-level `dune` setup.
`dune-site` re-exports `dune-private-libs.dune-section`, and the layout
does not auto-expand transitive package deps (see "Immediate deps only"
above). The same pattern applies to other re-exporting libraries (e.g.
`(package stdune)` requires `(package dyn) (package ordering) (package
pp) (package top-closure) (package csexp) (package fs-io)`).

The current staging cons is sufficient for the existing test surface and
matches pre-install-layouts behaviour.

### Package set structure and `_root` section collisions

The layout merges all packages' install entries into a single directory tree.
For scoped sections (`lib`, `share`, `doc`, `etc`), each package installs
under its own subdirectory (`lib/<pkgname>/`), so collisions are impossible
by construction. The unordered set is the correct data structure.

Collisions can only occur in `_root` sections (`lib_root`, `share_root`,
`libexec_root`), which install directly to the section root without package
namespacing ([opam manual][opam-install-format], [opam#2153]). In opam,
`_root` sections serve specific use cases:

- `ocamlfind` installs `topfind` into the compiler's stdlib directory via
`lib_root` ([opam#45]).
- Cross-compiled C libraries install `.pc` files to `lib/pkgconfig/` via
`lib_root`.
- `share_root` is used for shared data directories (e.g. emacs site-lisp).

These are rare. Collisions in `_root` sections are currently errors. To
support opam-style override semantics in the future, the layout would
resolve `_root` collisions using dependency edges: if B depends on A, B's
`_root` entries take priority. Diamond collisions (incomparable packages)
would remain errors. This only affects `_root` handling and does not
require changing the core layout mechanism.

[opam-install-format]: https://opam.ocaml.org/doc/Manual.html
[opam#2153]: https://github.com/ocaml/opam/issues/2153
[opam#45]: https://github.com/ocaml/opam/issues/45

## Future work

### Lock-dir build layouts

The layout mechanism can support the "in-and-out" problem ([#8652]): when a
lock-dir package depends on a workspace package, a layout containing the
workspace package's artifacts can be provided to the lock-dir package's
build env. The opam-like directory structure matches what build systems
expect at an install prefix.

Longer term, the layout overlay mechanism could replace isolated per-package
install directories for lock-dir builds entirely. Packages would build
against a layout that overlays all their dependencies' installed files into
a single virtual prefix, using dependency structure for overlay ordering.

### Incremental layout construction

If A depends on B depends on C, the layouts for their builds are {C}, {B,C},
and {A,B,C}. Instead of computing each independently, layouts can be built
incrementally: {B,C} hardlinks from {C} and adds B's entries, {A,B,C}
hardlinks from {B,C} and adds A's entries. This makes `_root` overrides
natural: each step replaces entries from the smaller layout. The key
question is whether the digest/key scheme can encode this incrementality or
whether a structural (DAG-based) key is needed.

## TODO

- Lock-dir package deps don't set up env vars for the consuming action.
`Package_db.find_package` classifies lock-dir packages as `Build` and
runs their build action but does not set up layout env vars. `dune exec
lockdir-tool` continues to work via `bin/exec.ml`'s unconditional
staging cons.
- Sandbox PATH non-relocation: layout dirs on PATH/OCAMLPATH point at
un-sandboxed `_build/`. Works locally; would break remote execution.
Same mechanism as the bin-layout limitation in #14432.
- `Install_layout.Key.reverse_table`, `Bin_layout.Key.reverse_table`, and
`Ppx_driver.Key.reverse_table` are three copies of the same
digest+reverse-table pattern. Worth a `Digest_key` functor; non-blocking.
- Factor the `Context.for_host` fallback used in `make_bin_env`,
`combined_package_deps_builder`, and `super_context.ml:make_root_env`
into a single helper.
- Missing test: mixing workspace and lock-dir package deps in a single
`(deps ...)`.
- `overlapping-directory-targets.t` halves test different things: the old
lang section tests `dune build @install` (not package deps), while the
new lang section tests layout rule collision. Should be split.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
These tests show how various pkg-config invocations get quotes (and test specifying a custom PKG_CONFIG):
$ dune build @install
$ PKG_CONFIG=$PWD/_build/install/default/bin/pkg-config dune build 2>&1 | awk '/run:.*bin\/pkg-config/{a=1}/stderr/{a=0}a'
run: $TESTCASE_ROOT/_build/install/default/bin/pkg-config --print-errors gtk+-quartz-3.0
-> process exited with code 0
Expand Down
3 changes: 2 additions & 1 deletion otherlibs/dune-site/test/dune
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
(applies_to :whole_subtree)
(deps
(package dune)
(package dune-site)))
(package dune-site)
(package dune-private-libs)))

; The test being broken on CI, we deactivate it when
; we detect that the CI environment variable is not
Expand Down
10 changes: 4 additions & 6 deletions src/dune_rules/bin_layout.ml
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,10 @@ let make_dispatch ~dir subdirs f =

let gen_rules context_name ~dir rest =
match rest with
| [] -> Memo.return (make_dispatch ~dir Subdir_set.all (fun () -> Memo.return ()))
| [] -> make_dispatch ~dir Subdir_set.all (fun () -> Memo.return ())
| [ key ] ->
Memo.return
(make_dispatch ~dir Subdir_set.empty (fun () ->
symlink_rules_for_key context_name ~dir key))
make_dispatch ~dir Subdir_set.empty (fun () ->
symlink_rules_for_key context_name ~dir key)
| _ :: _ :: _ ->
Memo.return
(Build_config.Gen_rules.redirect_to_parent Build_config.Gen_rules.Rules.empty)
Build_config.Gen_rules.redirect_to_parent Build_config.Gen_rules.Rules.empty
;;
2 changes: 1 addition & 1 deletion src/dune_rules/bin_layout.mli
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ val gen_rules
: Context_name.t
-> dir:Path.Build.t
-> string list
-> Build_config.Gen_rules.result Memo.t
-> Build_config.Gen_rules.result
31 changes: 14 additions & 17 deletions src/dune_rules/cram/cram_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Spec = struct
; extra_aliases : Alias.Name.Set.t
; deps : unit Action_builder.t list
; sandbox : Sandbox_config.t Action_builder.t
; bin_env : Env.t Action_builder.t
; env : Env.t Action_builder.t
; enabled_if : (Expander.t * Blang.t) list
; locks : Path.Set.t Action_builder.t
; packages : Package.Name.Set.t
Expand All @@ -26,7 +26,7 @@ module Spec = struct
; locks = Action_builder.return Path.Set.empty
; deps = []
; sandbox = Action_builder.return Sandbox_config.needs_sandboxing
; bin_env = Action_builder.return Env.empty
; env = Action_builder.return Env.empty
; packages = Package.Name.Set.empty
; timeout = None
; conflict_markers = Ignore
Expand Down Expand Up @@ -64,7 +64,7 @@ let test_rule
; deps
; locks
; sandbox
; bin_env
; env
; packages = _
; timeout
; conflict_markers
Expand Down Expand Up @@ -140,8 +140,8 @@ let test_rule
let+ (_ : Path.Set.t) = Action_builder.dyn_memo_deps deps in
()
and+ () = Action_builder.paths setup_scripts
and+ sandbox = sandbox
and+ bin_env = bin_env
and+ sandbox
and+ env
and+ locks = locks >>| Path.Set.to_list in
Cram_exec.run
~src:(Path.build script)
Expand All @@ -156,7 +156,7 @@ let test_rule
~setup_scripts
shell
|> Action.Full.make ~locks ~sandbox
|> Action.Full.add_env bin_env)
|> Action.Full.add_env env)
|> Action_builder.with_file_targets ~file_targets:[ output ]
|> Super_context.add_rule sctx ~dir ~loc
in
Expand Down Expand Up @@ -261,11 +261,11 @@ let rules ~sctx ~dir tests project =
| false -> Memo.return (runtest_alias, acc)
| true ->
let+ expander = Super_context.expander sctx ~dir in
let deps, sandbox, bin_env =
let deps, sandbox, env =
match stanza.deps with
| None -> acc.deps, acc.sandbox, acc.bin_env
| None -> acc.deps, acc.sandbox, acc.env
| Some deps ->
let action_env, _, sandbox =
let env, _, sandbox =
Dep_conf_eval.named
~expander
Sandbox_config.no_special_requirements
Expand All @@ -277,16 +277,13 @@ let rules ~sctx ~dir tests project =
and+ sandbox = sandbox in
Sandbox_config.inter acc sandbox
in
let bin_env =
let env =
let open Action_builder.O in
let+ acc = acc.bin_env
and+ env = action_env in
let+ acc = acc.env
and+ env in
Env_path.extend_env_concat_path acc env
in
(* [action_env]'s deps are registered when [bin_env] is
consumed at the rule site (it folds the env's
action_builder evaluation into [bin_env]). *)
acc.deps, sandbox, bin_env
acc.deps, sandbox, env
in
let locks =
let open Action_builder.O in
Expand Down Expand Up @@ -369,7 +366,7 @@ let rules ~sctx ~dir tests project =
; extra_aliases
; packages
; sandbox
; bin_env
; env
; timeout
; conflict_markers
; setup_scripts
Expand Down
Loading
Loading