Skip to content

fix(root_module): missing dependencies in the sandbox#13842

Merged
anmonteiro merged 4 commits intoocaml:mainfrom
anmonteiro:anmonteiro/fix-missing-deps-root-module
Apr 6, 2026
Merged

fix(root_module): missing dependencies in the sandbox#13842
anmonteiro merged 4 commits intoocaml:mainfrom
anmonteiro:anmonteiro/fix-missing-deps-root-module

Conversation

@anmonteiro
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread src/dune_rules/module_compilation.ml Outdated
@anmonteiro anmonteiro force-pushed the anmonteiro/fix-missing-deps-root-module branch 8 times, most recently from 8685b78 to f34d57b Compare March 22, 2026 22:49
@anmonteiro anmonteiro force-pushed the anmonteiro/fix-missing-deps-root-module branch 4 times, most recently from f0585ad to 0bb7a2b Compare April 2, 2026 23:09
@anmonteiro anmonteiro requested a review from rgrinberg April 3, 2026 01:38
Comment thread src/dune_rules/dep_rules.ml
Comment thread src/dune_rules/dep_graph.ml Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Dune’s OCaml module dependency-graph handling so sandboxed builds correctly include interface-derived dependencies (fixing a regression where builds could succeed without sandboxing but fail under sandboxing).

Changes:

  • Adjust dependency-graph construction to incorporate additional dependencies needed for module compilation.
  • Update blackbox regression test to expect successful sandboxed builds for the root-module shadowing scenario.
  • Update ocamldep blackbox test expected cycle output to match the new dependency ordering.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/blackbox-tests/test-cases/root-module/missing-dependencies-regression-test.t Updates regression to assert sandboxed build succeeds (no longer expects the interface/impl mismatch error).
test/blackbox-tests/test-cases/ocamldep/ocamldep-7018.t Updates expected dependency-cycle chain output ordering.
src/dune_rules/dep_rules.ml Maps computed dep graphs through a new normalization step intended for module compilation.
src/dune_rules/dep_graph.mli Exposes a new Ml_kind.for_module_compilation helper in the public interface.
src/dune_rules/dep_graph.ml Implements Ml_kind.for_module_compilation by merging impl/intf dependency sets for modules with interfaces and deduping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/dune_rules/dep_graph.ml
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
@anmonteiro anmonteiro force-pushed the anmonteiro/fix-missing-deps-root-module branch 3 times, most recently from 7c5ce78 to 24961a7 Compare April 6, 2026 02:33
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
@anmonteiro anmonteiro force-pushed the anmonteiro/fix-missing-deps-root-module branch from 24961a7 to cfe18cb Compare April 6, 2026 02:51
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
@anmonteiro anmonteiro merged commit 91078b2 into ocaml:main Apr 6, 2026
30 checks passed
@anmonteiro anmonteiro deleted the anmonteiro/fix-missing-deps-root-module branch April 6, 2026 04:29
@rgrinberg
Copy link
Copy Markdown
Member

I still find this fix mysterious. A PR description would make this a lot easier to understand.

avsm pushed a commit to ocaml/opam-repository that referenced this pull request May 5, 2026
CHANGES:

### Fixed

- Auto-inject `"menhir" {>= "20180523"}` into generated opam files when the
  menhir extension is used. Dune's menhir rules rely on `--infer-write-query`
  and `--infer-read-reply`, which require at least this version; without the
  lower bound, builds fail with older menhir installations.
  (ocaml/dune#10707, @robinbb)

- Fix `--display=quiet` not suppressing "Entering directory" and "Leaving
  directory" messages when using `--root`. These messages are now deferred
  until there is actual output, so silent builds produce no noise.
  (ocaml/dune#12974, fixes ocaml/dune#12854, @Alizter)

- Fix an internal error when a module is shared between a `rocq.theory` and
  `rocq.extraction` stanza. The error now includes a hint pointing to the
  conflicting stanza. (ocaml/dune#13733, @Durbatuluk1701)

- Fix cookies defined on `ppx_rewriter` being lost when that rewriter was used
  as a dependency of another `ppx_rewriter`. (ocaml/dune#13737, fixes ocaml/dune#3426, @Alizter)

- Improve opam file generation for packages that set `(dir ..)`. Such packages
  will now have a test target that is limited to this directory. (ocaml/dune#13778, @rgrinberg)

- Stop duplicating dune diagnostics to subscribers over RPC (ocaml/dune#13816,
  @rgrinberg)

- Fix dependency cycle when using the `package` with a library
  conditionally enabled via `enabled_if`. (ocaml/dune#13833, @toots)

- Fix underspecification of dependencies in the sandbox for modules with
  interfaces. (ocaml/dune#13842, @anmonteiro)

- Honor sandbox settings specified inside `(:include ..)` expressions in the `deps` field
  (ocaml/dune#13898, @rgrinberg)

- Fix `dune pkg lock` failing when a `pin` stanza contains a `file://` URL with
  a relative path outside the workspace (ocaml/dune#13915, fixes ocaml/dune#10254, @shunueda)

- Use all stat attributes to detect patch back source tree changes (ocaml/dune#13986, @rgrinberg)

- Use device and inode number for invalidating cached digests (ocaml/dune#13991, @rgrinberg)

- Stop trying to set SO_REUSEADDR on unix sockets when setting up dune rpc
  (ocaml/dune#14056, @rgrinberg)

- Fix autolocking to correctly detect changes to `(depends)` in watch mode
  (ocaml/dune#14066, fixes ocaml/dune#13234, @Alizter)

- Fix incremental builds for libraries using `(wrapped (transition ...))`.
  The compat shim modules were never recompiled when the inner module's
  interface changed, causing "inconsistent assumptions" errors.
  (ocaml/dune#14090, fixes ocaml/dune#14089, @Alizter)

- Fix Rocq configuration detection to use `rocq c --config` subcommand
  instead of `rocq --config` (ocaml/dune#14093, fixes ocaml/dune#13774, @Durbatuluk1701)

- Bump dune rpc's request pending request limit 10 to 100 (ocaml/dune#14094, @rginberg)

- Invalidate stale cached digests in all build commands (ocaml/dune#14126, @rgrinberg)

- Fix `root_module` generating duplicate module definitions when a
  findlib sub-package shares a directory with its parent (e.g.,
  `logs.lwt` alongside `logs`). (ocaml/dune#14135, fixes ocaml/dune#6148, @robinbb)

- Fix `dune subst` prepending a duplicate `version:` field to opam
  files that already contain one, instead of replacing it in place.
  (ocaml/dune#14136, fixes ocaml/dune#878, @robinbb)

- Fix `@ocaml-index` alias being generated for all contexts, causing build
  errors in multi-context workspaces where some libraries are disabled in
  non-default contexts. The alias is now only generated for the merlin context.
  (ocaml/dune#14137, fixes ocaml/dune#12007, @robinbb)

- Remove the warning about the deprecation of the coq stanza for
  dune lang before 3.21 since it has been introduced in this
  version (ocaml/dune#14187, @bobot)

- `$ dune init` now generates projects with correct .opam files (ocaml/dune#14192, @rgrinberg)

- Send SIGTERM before SIGKILL when cancelling child processes in watch
  mode, giving cleanup handlers a chance to run before escalating.
  (ocaml/dune#14224, ocaml/dune#14170, fixes ocaml/dune#2445, @robinbb)

- Correctly specify dependencies for `generate_runner` in inline tests (ocaml/dune#14276, @rgrinberg)

- Fix duplicate dune version bounds in generated opam files. When users
  declare `(dune (>= X.Y))` matching or exceeding the `(lang dune X.Y)`
  version, the generated opam `depends` no longer contains redundant
  constraints like `{>= "2.7" & >= "2.7"}`. (ocaml/dune#3916, ocaml/dune#11106, @robinbb)

- Fix inline tests not being executed for byte-only libraries. When a library
  has `(modes byte)`, the inline test runner is now linked in byte mode so the
  library's test code is included. (ocaml/dune#9757, @robinbb)

### Added

- Add support for `c_library_flags` in foreign_stubs (ocaml/dune#13484, @madroach)

- Move the management of Jsoo config details out of dune (ocaml/dune#13613, @vouillon)

- Js_of_ocaml: share standalone runtimes (ocaml/dune#13621, @vouillon)

- Allow multiple `(dirs ..)` stanzas in the same Dune file. Starting with
  `(lang dune 3.23)`, Dune takes the union of the specified directories
  (ocaml/dune#13734, fixes ocaml/dune#6249, @anmonteiro)

- Sandboxed rules are now allowed to produce diff promotions when `(corrections
  produce)` is set on the rules. Whenever such an action produces `foo.corrected`, it will
  be automatically registered as a promotion for `foo` (ocaml/dune#13813, @rgrinberg)

- `$ dune clean` now accepts arguments to only remove certain targets from the
  _build directory (ocaml/dune#13875, @rgrinberg)

- `$ dune promote` now promotes all paths that are a prefix of the path provided.
  For example, `$ dune promote foo` will promote `foo`, `foo/bar`, `foo/bar/baz`
  (but not `foobar`). (ocaml/dune#13876, @rgrinberg)

- Allow for the `diff` action to diff entire directories (ocaml/dune#13880, fixes ocaml/dune#3567, @rgrinberg)

- A hint is now emitted when trying to build a target inside a directory that
  was excluded by a `(dirs ...)` stanza (ocaml/dune#13919, @mefyl).

- Add --debug-backtraces to `$ dune {promote,trace,cache}` (ocaml/dune#13933, @rgrinberg)

- Actions are now able to observe their project directory via
  `DUNE_PROJECT_ROOT` (ocaml/dune#13934, @rgrinberg)

- Support hardlink sandboxing on Windows. (ocaml/dune#13987, addresses part of ocaml/dune#4362, @Alizter)

- `rocq.extraction`: Add `extracted_files` field in `(rocq 0.13)`, replacing
  `extracted_modules`, supporting extraction to languages other than OCaml
  (ocaml/dune#13997, @Durbatuluk1701).

- Infer source directories as dependencies when they're as reference
  directories in `diff` actions.

- Add trace events for build start/stop/restarts (ocaml/dune#14163, ocaml/dune#14166, @rgrinberg)

- Generate `compile_commands.json` for C/C++ foreign stubs when building
  `@check` with `(lang dune 3.23)`. This enables `clangd`, `ccls`, and other
  tools that consume compilation databases. (ocaml/dune#14185, fixes ocaml/dune#3531, @Alizter)

- Add trace events for accepting clients and shutting down RPC (ocaml/dune#14232, @rgrinberg)

- Back up the default trace file as `trace.csexp.old` before each build,
  preserving the previous trace for comparison. This does not apply when
  `--trace-file` is used. (ocaml/dune#14269, @Alizter)

### Changed

- Dune digests target files and directories in background threads (ocaml/dune#13341,
  @rgrinbreg)

- `Dyn.pp` now prints valid OCaml literals for more constructors:
  - `Char` is quoted (e.g., `'a'`)
  - `Int32`/`Int64`/`Nativeint` include literal suffixes (`l`/`L`/`n`)
  - `Float` handles `infinity`, `neg_infinity`, and `nan`
  (ocaml/dune#13394, grants ocaml/dune#13378, @Alizter)

- Bump the minimal OCaml version required to build Dune from 4.08 to 4.14.
  (ocaml/dune#13533, @Alizter)

- Starting from 3.23, user rules are sandboxed by default (ocaml/dune#13805, @rgrinberg)

- Dune will no longer stat paths in the _build directory. This was done as protection
  from user rules accidentally touching things in the _build directory. This is forbidden,
  and dune offers rule sandboxing to prevent this (ocaml/dune#13875, @rgrinberg)

- Improve mtime precision by no longer approximating them with floats (ocaml/dune#13962,
  @rgrinberg).

- Remove `--makefile` (or `-m`) in `$ dune rules`. The sexp output is the only
  supported way of reflecting dune rules (ocaml/dune#14069, @rgrinberg)

- Do not automatically promote files. Now, opam files must be manually
  promoted with `$ dune promote`. Their generation is triggered by `@install`,
  `@runtest`, and `@opam`. In release mode, .opam files aren't generated at all
  and whatever is in the source is used (ocaml/dune#14108, @rgrinberg)

- Inline test runner generation is now sandboxed (ocaml/dune#14257, @rgrinberg)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants