feat: finer dependency analysis between libraries (#4572)

[robinbb]

Implements per-module library dependency filtering as proposed in #4572. When
library A depends on library B, Dune currently assumes every module in A
depends on every module in B — causing full recompilation of A whenever any
module in B changes. This PR uses ocamldep's per-module output to declare
cross-library file dependencies only for libraries a module actually
references.

This PR builds on prerequisite PRs:

• test: document inter-library recompilation behavior (#4572) #14017 — baseline tests documenting current recompilation behavior (merged)
• test: document module name shadowing between stanza and library (#4572) #14031 — test documenting module name shadowing between stanzas and libraries
• test: verify library file deps in compilation rules and sandboxed builds (#4572) #14100 — test verifying library file deps in compilation rules and sandboxed builds
• test: verify transparent alias incremental build safety (#4572) #14101 — safety test for incremental builds with transparent aliases
• refactor: move library file deps from Includes to per-module in build_cm (#4572) #14030 — refactor moving Hidden_deps from Includes.make to per-module in build_cm

Depends on #14030.

The changes in this PR (on top of the above):

• A new Lib_index (in lib_file_deps.ml) maps library entry module names
  back to their libraries. It is computed once per stanza and stored in
  Compilation_context.t.
• read_immediate_deps_raw_of reads raw ocamldep output (including
  cross-library references that were previously silently dropped), filters
  out stanza-internal names, and looks up the remainder in the index to
  determine which libraries are actually needed.
• Conservative fallbacks ensure correctness: special module kinds, link-time
  generated modules, and unresolvable references fall back to all-library
  deps.
• The nine baseline tests are updated to show the improved behaviour.

Resolves #4572

[rgrinberg]

Thanks for working on this. I expect users are going to be quite pleased to have this feature. Some preliminary questions:

Includes.make no longer bundles Hidden_deps with -I flags. Include
flags remain shared across all modules in a stanza; file dependencies are
now declared per-module in build_cm.

Why are the include flags are needed if we don't expect the library to be used by the module?

For each module, read_immediate_deps_raw_of reads the raw ocamldep output
(including cross-library references that were previously silently dropped),
filters out stanza-internal names, and looks up the remainder in the index
to determine which libraries are actually needed.

Do we have tests for when an internal module name is shadowing a library?

Conservative fallbacks ensure correctness: alias and wrapped-compat modules
skip library deps entirely; special module kinds, link-time generated
modules, and unresolvable references fall back to all-library deps.

Is it possible to move these changes to a separate PR and rebase this work on top of that?

[robinbb]

Is it possible to move these changes to a separate PR and rebase this work on top of that?

@rgrinberg I have tried to do so with this PR #14030, depending on what you really meant by "these changes". :-)

[robinbb]

Thanks for working on this. I expect users are going to be quite pleased to have this feature.

You're very welcome, and thank you for your prompt attention to it!

[robinbb]

Do we have tests for when an internal module name is shadowing a library?

I guess not. I have made #14031 for this. Please have a look.

[nojb]

Thanks for working on this patch @robinbb!

I tried this in our codebase by adding a new (unused) module to a library which is close to the root of the dependency tree, and doing dune build. Then I modified the unused module and redid dune build. At this point I expected for there not to be any recompilation at all, since the modified module is not used anywhere in the codebase. However, I observe some recompilation of modules (in other libraries) which do not depend on it. The libraries involved are all unwrapped.

Do you have any insight as to what may be causing the extra recompilation?

[nojb]

Do you have any insight as to what may be causing the extra recompilation?

I think one case where unexpected extra recompilation is triggered is when there a library consisting of a single module. If memory serves there is an optimization that avoids calling ocamldep in this case, but that may be defeating the optimization in this patch.

[Alizter]

Transparent module aliases hide library references from ocamldep when a module re-exports a library wrapper via module M = Mylib, ocamldep will miss the dependency on Mylib.

Here is the regression:

  $ cat > dune-project << EOF
  > (lang dune 3.0)
  > EOF
  $ mkdir lib && cat > lib/dune << EOF
  > (library (name mylib))
  > EOF
  $ cat > lib/mylib.ml << EOF
  > let v = 42
  > EOF
  $ cat > lib/mylib.mli << EOF
  > val v : int
  > EOF
  $ cat > dune << EOF
  > (executable (name main) (libraries mylib))
  > EOF
  $ cat > re.ml << EOF
  > module M = Mylib
  > EOF
  $ cat > re.mli << EOF
  > module M = Mylib
  > EOF
  $ cat > main.ml << EOF
  > let () = print_int Re.M.v
  > EOF

  $ dune build ./main.exe

Change mylib's interface:

  $ cat > lib/mylib.mli << EOF
  > val v : int
  > val w : string
  > EOF
  $ cat > lib/mylib.ml << EOF
  > let v = 42
  > let w = ""
  > EOF

The incremental build must succeed:

  $ dune build ./main.exe
  File "_none_", line 1:
  Error: Files .main.eobjs/native/dune__exe__Main.cmx and lib/mylib.cmxa
         make inconsistent assumptions over interface Mylib
  [1]

This probably also occurs for any module from Mylib not just the wrapper, so affects unwrapped, wrapped and singleton buildable stanzas. Before this change, the coarse glob would handle this unknowingly.

[nojb]

Do you have any insight as to what may be causing the extra recompilation?

I think one case where unexpected extra recompilation is triggered is when there a library consisting of a single module. If memory serves there is an optimization that avoids calling ocamldep in this case, but that may be defeating the optimization in this patch.

Continuing to investigate this, an .mli in a library A containing only

val foo: Buffer.t

triggers the recompilation of the .cmi every time when a module in a library on which A depends on is changed. From a distance this looks like some missing logic to handle dependencies on stdlib modules.

[Alizter]

The removal of this error is strange, because it means we will be able to build packages together in a workspace that otherwise would have cycle errors and cause the (opam and dune) solvers to fail to find a solution.

I think we need to do cycle detection at the level of packages in order to avoid such situations. Before it wasn't needed because the dependencies were coarse enough to imply the sufficient conditions to avoid a cycle.

@rgrinberg any advice on where and how this detection should be done?

[Leonidas-from-XIV]

Can we also run a meaningful revdep check on this? This has the potential to affect a bunch of downstream packages.

Yes, we have a (manually triggered) workflow for this, so provided the branch is rebased on a reasonably recent version of main (I fixed some smaller issues in it recently) it should be possible to trigger this workflow by

• picking @robinbb's branch from the dropdown
• setting the list of packages to some meaningful set (or all but it takes a long time, testing it at time of writing)
• keeping the Dune version as dev (which means it will take the version of the branch itself, not main)

For smaller package sets this takes about 10 minutes, bigger sets might take several hours I assume.

[robinbb]

I am chasing down a CI failure in this branch. The failure is good, in the sense that it has caught legitimate regressions introduced by prior versions of this branch - ones that were not caught by the cram test suite.