Filter inter-library dependencies per module#14492
Open
robinbb wants to merge 2 commits into
Open
Conversation
robinbb
added a commit
that referenced
this pull request
May 11, 2026
Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Collaborator
|
There seems to be a regression in the latest version of this patch vs previous versions: when a library cat >dune-project <<EOF
(lang dune 3.22)
EOF
cat >dune <<EOF
(library
(name libA)
(wrapped false)
(instrumentation (backend foo))
(modules modA))
(library
(name libB)
(wrapped false)
(modules modB)
(libraries libA))
EOF
cat >modA.ml <<EOF
let x = 42
EOF
cat >modB.ml <<EOF
let x = ModA.x
EOF
dune=~/dune/dune-pr
echo "dune version: $($dune --version)"
echo 'There should not be a "glob" entry next:'
opam exec -- $dune rules --format json --deps _build/default/.libB.objs/byte/modB.cmo | jq '.[].[].glob'Output: nicolasojedabar@LEXIFI-L58:~/sample2$ bash setup.sh
dune version: 3.23.0-203-ga5cd5fa
There should not be a "glob" entry next:
null
null
{
"dir": [
"In_build_dir",
"_build/default/.libA.objs/byte"
],
"predicate": "*.cmi",
"only_generated_files": false
} |
robinbb
force-pushed
the
robinbb-issue-4572-rebased
branch
from
a5cd5fa to
410fa53
Compare
robinbb
added a commit
that referenced
this pull request
May 12, 2026
Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Collaborator
Author
|
@nojb I really appreciate you watching this matter, and helping with test cases. I recognise the effort that you must be putting in to give the minimal test cases as you do. tl;dr I'll investigate adding a fix for this, now, in this PR. Longer explanation: I (in b531a47) wanted to fix a different soundness bug - preprocessed libs whose .mli leaks a transitive type the consumer never names. My fix correctly handles the Ordinary ppx case (read ocamldep on .pp.ml), but for the instrumentation-only case, the choice was: rather than work out whether |
robinbb
added a commit
that referenced
this pull request
May 12, 2026
is disabled at build time [build_lib_index]'s [post_pp_module] returned [None] (non-tight- eligible) for any [Pps] consisting only of [Instrumentation_backend] entries, on the assumption that no [.pp.ml] is produced. That sent such libs down the wide-glob fallback path, regressing the per- module narrow on consumers of instrumentation-decorated libs. A [.pp.ml] is actually produced only when the build's [--instrument-with] argument names a backend that the lib declares; otherwise the lib's compile pipeline reads the raw [.ml]. [Context.instrument_with] gives that argument list; consult it when classifying each [Instrumentation_backend] entry as active or not. If no entry is active, return [Some (Module.ml_source m)] - raw, tight-eligible, narrow path. Extend [cross-lib-instrumentation-barrier.t] with a [dune rules --format=json --deps ...] assertion that the consumer's compile rule has no glob over the instrumented lib's objdir. The original [$ dune build consumer/consumer.exe] assertion only proved the build succeeded - the wide-glob fallback also makes it succeed, so the precision regression went uncaught. Reported by @nojb on the PR: #14492 (comment) Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Collaborator
Author
Collaborator
Author
Tests Added/Changed for this PR
Why each category relates to 14492
|
robinbb
force-pushed
the
robinbb-issue-4572-rebased
branch
2 times, most recently
from
4fa889a to
b80622a
Compare
There was a problem hiding this comment.
Pull request overview
Implements per-module inter-library dependency filtering using ocamldep results, and aligns per-module -I/-H include flags and rule file-dependencies with that filtered dependency set to reduce unnecessary recompilation (fixes #4572).
Changes:
- Add per-module, cross-library dependency narrowing (including a cross-library BFS for tight-eligible unwrapped local libs) and use it to compute module compile-rule deps.
- Filter per-module include flags (
-I/-H) to match the dependency filter, and adjust library file-dep computation to support both glob and per-entry-module dep shapes. - Add/adjust extensive blackbox regression tests and ship a changelog entry describing the behavior change.
Reviewed changes
Copilot reviewed 68 out of 68 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/blackbox-tests/test-cases/watching/multiple-errors-output.t/run.t | Update expected watch-mode output to include an additional reported syntax error. |
| test/blackbox-tests/test-cases/strict-package-deps.t | Make test sources actually reference dependencies to exercise strict package dep inference. |
| test/blackbox-tests/test-cases/reporting-of-cycles.t/b/x.ml | Add module to help exercise/report cycle behavior. |
| test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a2/x.ml | Add module to help exercise/report cycle behavior. |
| test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a1/x.ml | Add module to help exercise/report cycle behavior. |
| test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-transition-soundness.t | New regression guard for wrapped (transition) soundness with per-module deps. |
| test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-internal-leak.t | Assert wrapped-internal mangled-module access no longer compiles under narrowed deps/includes. |
| test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-from-vlib-soundness.t | New regression guard for wrapped-setting inherited from virtual libs. |
| test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-compat.t | Update expected rebuild counts under per-module filtering. |
| test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped.t | Update narrative/expectations for unwrapped per-module rebuild behavior. |
| test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-tight-deps.t | Update expectations to reflect tight per-module deps (empty rebuild target lists). |
| test/blackbox-tests/test-cases/per-module-lib-deps/transitive.t | Update expected rebuild counts for transitive unreferenced consumers. |
| test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-module.t | Update expectations to no rebuild when a transitively-unreferenced module changes. |
| test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-lib.t | Update expectations to no rebuild when a transitively-unreferenced library changes. |
| test/blackbox-tests/test-cases/per-module-lib-deps/stdlib-modules.t | Update expected rebuild counts for stdlib-only consumer. |
| test/blackbox-tests/test-cases/per-module-lib-deps/single-module-unreferenced-lib.t | Update expectations for single-module consumer with zero references. |
| test/blackbox-tests/test-cases/per-module-lib-deps/single-module-lib.t | Update expectations for single-module consumer where ocamldep is now used when deps exist. |
| test/blackbox-tests/test-cases/per-module-lib-deps/sibling-unreferenced-lib.t | Update expectations for non-referencing sibling module in a library stanza. |
| test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t | Assert per-module -I filtering drops unrelated lib objdirs. |
| test/blackbox-tests/test-cases/per-module-lib-deps/opaque.t | Update expected rebuild counts in opaque scenarios. |
| test/blackbox-tests/test-cases/per-module-lib-deps/opaque-mli-change.t | Update expected rebuild target lists for .mli changes under opaque rules. |
| test/blackbox-tests/test-cases/per-module-lib-deps/multiple-libraries.t | Update expectations for multiple direct deps with per-module filtering. |
| test/blackbox-tests/test-cases/per-module-lib-deps/modules-without-implementation-cross-lib.t | Minor test text update; keeps regression guard intent. |
| test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t/run.t | New reproducer for mixed per-module preprocessing soundness edge case. |
| test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t/run.t | New guard ensuring mixed-pp libs stay precise when only Some entries are referenced. |
| test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-wrapped.t | Update expected rebuild counts for wrapped lib-to-lib case. |
| test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-unwrapped.t | Update expected rebuild counts for unwrapped lib-to-lib case. |
| test/blackbox-tests/test-cases/per-module-lib-deps/lib-deps-preserved.t | Update assertions around cm_kind/-opaque behavior for library deps. |
| test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t | Update expectations under (implicit_transitive_deps false) with narrowed deps. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-walk-pre-pp-source.t | Assert per-module deps use correct .cmi basename and avoid pre-pp source deps. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-preprocess-barrier.t | New guard for transitive .cmi reads through a preprocessed intermediate library. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-pps-runtime-no-ocamldep-barrier.t/run.t | New guard ensuring ppx runtime deps prevent “no-ocamldep” misclassification. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/run.t | New reproducer/guard for instrumentation-disabled .pp.ml mapping and deps globbing. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello.ml | Add runtime lib for instrumentation test fixture. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello_ppx.ml | Add ppx fixture for instrumentation test. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune-project | Add dune-project for ppx fixture. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune | Add dune file for ppx fixture. |
| test/blackbox-tests/test-cases/per-module-lib-deps/consumer-is-virtual-impl.t | Adjust wording to keep regression guard relevant after refactoring. |
| test/blackbox-tests/test-cases/per-module-lib-deps/cmx-native-tight-deps.t | New guard exercising native .cmx per-module deps under release (opaque=false). |
| test/blackbox-tests/test-cases/per-module-lib-deps/basic-wrapped.t | Update expected rebuild counts for wrapped dependency case. |
| test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t | Update expectations: adding an unreferenced lib no longer rebuilds other modules. |
| test/blackbox-tests/test-cases/inline-tests/alias-cycle.t | Make cycle test assert only stable invariants of the dependency-cycle error. |
| test/blackbox-tests/test-cases/custom-cross-compilation/cross-compilation-ocamlfind.t | Update expected trace to include an additional -modules invocation. |
| src/dune_rules/virtual_rules.mli | Expose helper to detect virtual/parameter implementers. |
| src/dune_rules/virtual_rules.ml | Implement is_virtual_or_parameter. |
| src/dune_rules/parameterised_rules.ml | Pass has_library_deps into dep-graph rule generation. |
| src/dune_rules/ocamldep.mli | Add API to read raw ocamldep module-name output. |
| src/dune_rules/ocamldep.ml | Add shared ocamldep output caching and raw dependency reading. |
| src/dune_rules/ocaml_flags.mli | Add API to extract module names from -open flags. |
| src/dune_rules/ocaml_flags.ml | Implement extraction of -open module names. |
| src/dune_rules/modules.mli | Add Modules.as_singleton and document wrapped entry modules. |
| src/dune_rules/modules.ml | Include wrapped-compat shims in wrapped entry modules and add as_singleton. |
| src/dune_rules/module_compilation.ml | Core implementation: per-module lib deps + per-module include-flag filtering. |
| src/dune_rules/lib.mli | Document closure memoization/key considerations. |
| src/dune_rules/lib.ml | Memoize Lib.closure by (linking, for_, libs). |
| src/dune_rules/lib_rules.ml | Compute and pass ppx runtime libs into compilation context. |
| src/dune_rules/lib_file_deps.mli | Add per-entry-module dep APIs and Lib_index for cross-lib filtering. |
| src/dune_rules/lib_file_deps.ml | Implement per-entry-module deps and Lib_index classification/lookup helpers. |
| src/dune_rules/exe_rules.ml | Compute and pass ppx runtime libs into compilation context for executables. |
| src/dune_rules/dep_rules.mli | Extend dep-graph rule API with has_library_deps. |
| src/dune_rules/dep_rules.ml | Only skip ocamldep for singleton stanzas when safe (no lib deps / Melange). |
| src/dune_rules/dep_graph.mli | Add dir and mem helpers for safety checks in filtering. |
| src/dune_rules/dep_graph.ml | Implement dir and mem. |
| src/dune_rules/compilation_context.mli | Add raw-refs/include filtering APIs; add ppx runtime libs plumbing. |
| src/dune_rules/compilation_context.ml | Build lib index, memoize raw refs and filtered include flags, wire into dep-graph creation. |
| src/dune_lang/lib_mode.mli | Add Lib_mode.hash. |
| src/dune_lang/lib_mode.ml | Implement Lib_mode.hash. |
| doc/changes/added/14492.md | Changelog entry describing the new per-module inter-library dep filtering behavior. |
This was referenced May 13, 2026
Collaborator
Collaborator
Author
|
Performance status: null builds of Dune take 3.8 seconds on 'main' branch, and 5.9 seconds on this branch, 55% longer. |
Collaborator
Author
|
Result of Dune Dev meeting today: maintainers want this PR merged piece-wise, and are willing to accept pieces that themselves would not be justified to merge without being part of the solution that this PR proves exists. |
Collaborator
Author
|
I will produce a sequence of 9 PRs that "sum" to this PR, with the following layers:
|
This was referenced May 13, 2026
robinbb
added a commit
that referenced
this pull request
May 15, 2026
Restores correctness for three cases the bare BFS filter mishandles: - Deps that implement a virtual library: dep-graph through them is computed elsewhere ([Dep_rules.imported_vlib_deps]); the per-module filter can miss cmi changes. Gate: fall through to glob whenever the cctx has [has_virtual_impl]. - Wrapped local libs the consumer references through the wrapper name: the ocamldep walk can't see the alias chain into the lib's [wrapped_compat] / inner modules. Reach: glob the wrapped lib's [Lib.closure]. - [ppx_runtime_libraries] introduced by [pps] in the consumer's preprocessor: their modules appear in the post-pp source which ocamldep can't see. Reach: glob their [Lib.closure]. [Module_compilation.lib_deps_for_module]: - After [can_filter], read [Compilation_context.has_virtual_impl]; if true, fall back to glob. - Read [Compilation_context.pps_runtime_libs] and include them in [direct_libs] so [Lib.closure] sees them. - Compute [wrapped_libs_referenced] from the consumer's [referenced_modules] (BFS-initial frontier — pre-cross-lib-walk). Take the [Lib.closure] of that set union [pps_runtime_libs] to get [must_glob_libs]; the classification fold sends every member to the glob path. [Modules]: - [Wrapped.entry_modules]: new function. Returns the wrapper ([lib_interface]) plus every [wrapped_compat] shim. Mirrors what [(wrapped (transition ...))] libraries expose to consumers. - [entry_modules]'s wrapped case switches to use it. Net effect: in transition wrapped libs, consumers can resolve any of the bare module names the lib exposes; this lifts a false-negative in the index that previously hid the consumer's reference to a [wrapped_compat] shim from the per-module filter. Tests (cherry-picked from #14492): - New soundness fixtures land here: [cross-lib-instrumentation-barrier.t], [cross-lib-preprocess-barrier.t], [cross-lib-pps-runtime-no-ocamldep-barrier.t], [wrapped-from-vlib-soundness.t], [wrapped-transition-soundness.t], [mixed-per-module-preprocess.t], [mixed-per-module-preprocess-precision.t], [cmx-native-tight-deps.t]. - The five pre-existing tests broken by L4 ([auto-wrapped-child-reexport.t], [ppx-runtime-libraries.t], [virtual-library.t], [wrapped-closure-precision.t], [wrapped-reexport-via-open-flag.t]) pass again — soundness recovery restores their original behavior; no test file change in #14492's diff for them. Changelog: [doc/changes/added/14492.md] lands now. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 15, 2026
Activates the tight branch in [lib_deps_for_module]: a per-module BFS over the cross-library dependency graph (built from each lib's [ocamldep -modules] output, normalised through [build_lib_index]'s post-pp module map) produces the set of dep-lib modules actually referenced by the consumer module. The compile rule sees only those [.cmi]/[.cmx] files; sibling-module recompilations on unreferenced dep-lib cmi changes drop out. Include flags are still the cctx-wide [-I]/[-H] in this layer; the filtered include flags ship separately. Wrapped-lib soundness recovery, virtual-impl gating on the deps side, ppx-runtime force-glob, and the new soundness test fixtures ship in a follow-up — this commit leaves five existing cram tests broken ([auto-wrapped-child-reexport.t], [ppx-runtime-libraries.t], [virtual-library.t], [wrapped-closure-precision.t], [wrapped-reexport-via-open-flag.t]) that the soundness recovery restores. [Module_compilation]: - [union_module_name_sets_mapped]: parallel fold over a list of [Module_name.Set.t] producers. - [module_kind_is_filterable]: predicate excluding kinds whose dep story is handled outside the BFS ([Root], [Wrapped_compat], [Impl_vmodule], [Virtual], [Parameter]). - [cross_lib_tight_set]: BFS expanding through the lib_index's [(lib, entry)] pairs, reading each entry's impl + intf [ocamldep] output. Non-tight-eligible libs terminate chains. - [lib_deps_for_module]: replaces the scaffold body. A [can_filter] guard (consumer-side virtual / parameter, dummy dep graph, module kind, [Module.has m ~ml_kind]) falls back to glob; otherwise runs the BFS, classifies libs via [Lib_file_deps.Lib_index.filter_libs_with_modules], and emits specific-file deps for tight libs + glob deps for non-tight / unreached-non-eligible libs. Returns the cctx-wide [Includes]; filtered include flags follow in a later layer. [Compilation_context.create]: peek [direct_requires] / [hidden_requires] and pass [has_library_deps] to [Dep_rules.rules]. Single-module stanzas with library deps now produce real dep graphs (the filter needs them). [Dep_rules.rules]: gate the singleton short-circuit on [(not has_library_deps) || for_ = Melange]. Other singletons fall through to the full dep-graph build. [Ocaml_flags]: [extract_open_module_names] surfaces [-open Foo] references that ocamldep doesn't see; they join [BFS]'s initial frontier. [Virtual_rules]: [is_virtual_or_parameter] — true for virtual impls and parameter cctxs; used by [can_filter] to suppress per-module filtering on consumer cctxs whose dep story [Dep_rules] handles specially. [Parameterised_rules]: pass [~has_library_deps:true] to the [Dep_rules.rules] call; conservative — the dep-rules path here serves external parameterised libs whose dep set is built from generated sources. Tests: rebuild-precision promotions for the existing modified-test set in #14492 — cram outputs reflect the tighter dep / rebuild behavior that L4 already produces. New soundness test fixtures (and the two tests gated on filtered include flags, [per-module-include-flags.t] / [add-unreferenced-sibling-lib.t]) are deferred to their respective follow-ups. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 15, 2026
Restores correctness for three cases the bare BFS filter mishandles: - Deps that implement a virtual library: dep-graph through them is computed elsewhere ([Dep_rules.imported_vlib_deps]); the per-module filter can miss cmi changes. Gate: fall through to glob whenever the cctx has [has_virtual_impl]. - Wrapped local libs the consumer references through the wrapper name: the ocamldep walk can't see the alias chain into the lib's [wrapped_compat] / inner modules. Reach: glob the wrapped lib's [Lib.closure]. - [ppx_runtime_libraries] introduced by [pps] in the consumer's preprocessor: their modules appear in the post-pp source which ocamldep can't see. Reach: glob their [Lib.closure]. [Module_compilation.lib_deps_for_module]: - After [can_filter], read [Compilation_context.has_virtual_impl]; if true, fall back to glob. - Read [Compilation_context.pps_runtime_libs] and include them in [direct_libs] so [Lib.closure] sees them. - Compute [wrapped_libs_referenced] from the consumer's [referenced_modules] (BFS-initial frontier — pre-cross-lib-walk). Take the [Lib.closure] of that set union [pps_runtime_libs] to get [must_glob_libs]; the classification fold sends every member to the glob path. [Modules]: - [Wrapped.entry_modules]: new function. Returns the wrapper ([lib_interface]) plus every [wrapped_compat] shim. Mirrors what [(wrapped (transition ...))] libraries expose to consumers. - [entry_modules]'s wrapped case switches to use it. Net effect: in transition wrapped libs, consumers can resolve any of the bare module names the lib exposes; this lifts a false-negative in the index that previously hid the consumer's reference to a [wrapped_compat] shim from the per-module filter. Tests (cherry-picked from #14492): - New soundness fixtures land here: [cross-lib-instrumentation-barrier.t], [cross-lib-preprocess-barrier.t], [cross-lib-pps-runtime-no-ocamldep-barrier.t], [wrapped-from-vlib-soundness.t], [wrapped-transition-soundness.t], [mixed-per-module-preprocess.t], [mixed-per-module-preprocess-precision.t], [cmx-native-tight-deps.t]. - The five pre-existing tests broken by L4 ([auto-wrapped-child-reexport.t], [ppx-runtime-libraries.t], [virtual-library.t], [wrapped-closure-precision.t], [wrapped-reexport-via-open-flag.t]) pass again — soundness recovery restores their original behavior; no test file change in #14492's diff for them. Changelog: [doc/changes/added/14492.md] lands now. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-issue-4572-rebased
branch
from
0a16e54 to
ea13ad6
Compare
robinbb
added a commit
that referenced
this pull request
May 16, 2026
Activates the tight branch in [lib_deps_for_module]: a per-module BFS over the cross-library dependency graph (built from each lib's [ocamldep -modules] output, normalised through [build_lib_index]'s post-pp module map) produces the set of dep-lib modules actually referenced by the consumer module. The compile rule sees only those [.cmi]/[.cmx] files; sibling-module recompilations on unreferenced dep-lib cmi changes drop out. Include flags are still the cctx-wide [-I]/[-H] in this layer; the filtered include flags ship separately. Wrapped-lib soundness recovery, virtual-impl gating on the deps side, ppx-runtime force-glob, and the new soundness test fixtures ship in a follow-up — this commit leaves five existing cram tests broken ([auto-wrapped-child-reexport.t], [ppx-runtime-libraries.t], [virtual-library.t], [wrapped-closure-precision.t], [wrapped-reexport-via-open-flag.t]) that the soundness recovery restores. [Module_compilation]: - [union_module_name_sets_mapped]: parallel fold over a list of [Module_name.Set.t] producers. - [module_kind_is_filterable]: predicate excluding kinds whose dep story is handled outside the BFS ([Root], [Wrapped_compat], [Impl_vmodule], [Virtual], [Parameter]). - [cross_lib_tight_set]: BFS expanding through the lib_index's [(lib, entry)] pairs, reading each entry's impl + intf [ocamldep] output. Non-tight-eligible libs terminate chains. - [lib_deps_for_module]: replaces the scaffold body. A [can_filter] guard (consumer-side virtual / parameter, dummy dep graph, module kind, [Module.has m ~ml_kind]) falls back to glob; otherwise runs the BFS, classifies libs via [Lib_file_deps.Lib_index.filter_libs_with_modules], and emits specific-file deps for tight libs + glob deps for non-tight / unreached-non-eligible libs. Returns the cctx-wide [Includes]; filtered include flags follow in a later layer. [Compilation_context.create]: peek [direct_requires] / [hidden_requires] and pass [has_library_deps] to [Dep_rules.rules]. Single-module stanzas with library deps now produce real dep graphs (the filter needs them). [Dep_rules.rules]: gate the singleton short-circuit on [(not has_library_deps) || for_ = Melange]. Other singletons fall through to the full dep-graph build. [Ocaml_flags]: [extract_open_module_names] surfaces [-open Foo] references that ocamldep doesn't see; they join [BFS]'s initial frontier. [Virtual_rules]: [is_virtual_or_parameter] — true for virtual impls and parameter cctxs; used by [can_filter] to suppress per-module filtering on consumer cctxs whose dep story [Dep_rules] handles specially. [Parameterised_rules]: pass [~has_library_deps:true] to the [Dep_rules.rules] call; conservative — the dep-rules path here serves external parameterised libs whose dep set is built from generated sources. Tests: rebuild-precision promotions for the existing modified-test set in #14492 — cram outputs reflect the tighter dep / rebuild behavior that L4 already produces. New soundness test fixtures (and the two tests gated on filtered include flags, [per-module-include-flags.t] / [add-unreferenced-sibling-lib.t]) are deferred to their respective follow-ups. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 16, 2026
Restores correctness for three cases the bare BFS filter mishandles: - Deps that implement a virtual library: dep-graph through them is computed elsewhere ([Dep_rules.imported_vlib_deps]); the per-module filter can miss cmi changes. Gate: fall through to glob whenever the cctx has [has_virtual_impl]. - Wrapped local libs the consumer references through the wrapper name: the ocamldep walk can't see the alias chain into the lib's [wrapped_compat] / inner modules. Reach: glob the wrapped lib's [Lib.closure]. - [ppx_runtime_libraries] introduced by [pps] in the consumer's preprocessor: their modules appear in the post-pp source which ocamldep can't see. Reach: glob their [Lib.closure]. [Module_compilation.lib_deps_for_module]: - After [can_filter], read [Compilation_context.has_virtual_impl]; if true, fall back to glob. - Read [Compilation_context.pps_runtime_libs] and include them in [direct_libs] so [Lib.closure] sees them. - Compute [wrapped_libs_referenced] from the consumer's [referenced_modules] (BFS-initial frontier — pre-cross-lib-walk). Take the [Lib.closure] of that set union [pps_runtime_libs] to get [must_glob_libs]; the classification fold sends every member to the glob path. [Modules]: - [Wrapped.entry_modules]: new function. Returns the wrapper ([lib_interface]) plus every [wrapped_compat] shim. Mirrors what [(wrapped (transition ...))] libraries expose to consumers. - [entry_modules]'s wrapped case switches to use it. Net effect: in transition wrapped libs, consumers can resolve any of the bare module names the lib exposes; this lifts a false-negative in the index that previously hid the consumer's reference to a [wrapped_compat] shim from the per-module filter. Tests (cherry-picked from #14492): - New soundness fixtures land here: [cross-lib-instrumentation-barrier.t], [cross-lib-preprocess-barrier.t], [cross-lib-pps-runtime-no-ocamldep-barrier.t], [wrapped-from-vlib-soundness.t], [wrapped-transition-soundness.t], [mixed-per-module-preprocess.t], [mixed-per-module-preprocess-precision.t], [cmx-native-tight-deps.t]. - The five pre-existing tests broken by L4 ([auto-wrapped-child-reexport.t], [ppx-runtime-libraries.t], [virtual-library.t], [wrapped-closure-precision.t], [wrapped-reexport-via-open-flag.t]) pass again — soundness recovery restores their original behavior; no test file change in #14492's diff for them. Changelog: [doc/changes/added/14492.md] lands now. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 16, 2026
Add [doc/dev/per-module-narrowing.md] describing the per-module library file dependency narrowing introduced in #14492 (split into PRs #14513..#14521 as layers L1..L9): - The motivation and soundness model. - The [can_filter] precondition and [has_virtual_impl] early-out. - The narrowing pipeline: read ocamldep raw refs → [referenced] → [Lib.closure] → cross-library BFS → classification → emit per-lib deps and filtered include flags. - The data structures used ([Lib_index], the per-cctx [cached_raw_refs] / [Filtered_includes] / [Lib.closure] memos). - Soundness fallbacks (wrapped libs, virtual impls, ppx runtime). - A source map locating each concern in [src/dune_rules/]. - A layer-by-layer summary of #14513..#14521. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 16, 2026
Activates the tight branch in [lib_deps_for_module]: a per-module BFS over the cross-library dependency graph (built from each lib's [ocamldep -modules] output, normalised through [build_lib_index]'s post-pp module map) produces the set of dep-lib modules actually referenced by the consumer module. The compile rule sees only those [.cmi]/[.cmx] files; sibling-module recompilations on unreferenced dep-lib cmi changes drop out. Include flags are still the cctx-wide [-I]/[-H] in this layer; the filtered include flags ship separately. Wrapped-lib soundness recovery, virtual-impl gating on the deps side, ppx-runtime force-glob, and the new soundness test fixtures ship in a follow-up — this commit leaves five existing cram tests broken ([auto-wrapped-child-reexport.t], [ppx-runtime-libraries.t], [virtual-library.t], [wrapped-closure-precision.t], [wrapped-reexport-via-open-flag.t]) that the soundness recovery restores. [Module_compilation]: - [union_module_name_sets_mapped]: parallel fold over a list of [Module_name.Set.t] producers. - [module_kind_is_filterable]: predicate excluding kinds whose dep story is handled outside the BFS ([Root], [Wrapped_compat], [Impl_vmodule], [Virtual], [Parameter]). - [cross_lib_tight_set]: BFS expanding through the lib_index's [(lib, entry)] pairs, reading each entry's impl + intf [ocamldep] output. Non-tight-eligible libs terminate chains. - [lib_deps_for_module]: replaces the scaffold body. A [can_filter] guard (consumer-side virtual / parameter, dummy dep graph, module kind, [Module.has m ~ml_kind]) falls back to glob; otherwise runs the BFS, classifies libs via [Lib_file_deps.Lib_index.filter_libs_with_modules], and emits specific-file deps for tight libs + glob deps for non-tight / unreached-non-eligible libs. Returns the cctx-wide [Includes]; filtered include flags follow in a later layer. [Compilation_context.create]: peek [direct_requires] / [hidden_requires] and pass [has_library_deps] to [Dep_rules.rules]. Single-module stanzas with library deps now produce real dep graphs (the filter needs them). [Dep_rules.rules]: gate the singleton short-circuit on [(not has_library_deps) || for_ = Melange]. Other singletons fall through to the full dep-graph build. [Ocaml_flags]: [extract_open_module_names] surfaces [-open Foo] references that ocamldep doesn't see; they join [BFS]'s initial frontier. [Virtual_rules]: [is_virtual_or_parameter] — true for virtual impls and parameter cctxs; used by [can_filter] to suppress per-module filtering on consumer cctxs whose dep story [Dep_rules] handles specially. [Parameterised_rules]: pass [~has_library_deps:true] to the [Dep_rules.rules] call; conservative — the dep-rules path here serves external parameterised libs whose dep set is built from generated sources. Tests: rebuild-precision promotions for the existing modified-test set in #14492 — cram outputs reflect the tighter dep / rebuild behavior that L4 already produces. New soundness test fixtures (and the two tests gated on filtered include flags, [per-module-include-flags.t] / [add-unreferenced-sibling-lib.t]) are deferred to their respective follow-ups. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 16, 2026
Restores correctness for three cases the bare BFS filter mishandles: - Deps that implement a virtual library: dep-graph through them is computed elsewhere ([Dep_rules.imported_vlib_deps]); the per-module filter can miss cmi changes. Gate: fall through to glob whenever the cctx has [has_virtual_impl]. - Wrapped local libs the consumer references through the wrapper name: the ocamldep walk can't see the alias chain into the lib's [wrapped_compat] / inner modules. Reach: glob the wrapped lib's [Lib.closure]. - [ppx_runtime_libraries] introduced by [pps] in the consumer's preprocessor: their modules appear in the post-pp source which ocamldep can't see. Reach: glob their [Lib.closure]. [Module_compilation.lib_deps_for_module]: - After [can_filter], read [Compilation_context.has_virtual_impl]; if true, fall back to glob. - Read [Compilation_context.pps_runtime_libs] and include them in [direct_libs] so [Lib.closure] sees them. - Compute [wrapped_libs_referenced] from the consumer's [referenced_modules] (BFS-initial frontier — pre-cross-lib-walk). Take the [Lib.closure] of that set union [pps_runtime_libs] to get [must_glob_libs]; the classification fold sends every member to the glob path. [Modules]: - [Wrapped.entry_modules]: new function. Returns the wrapper ([lib_interface]) plus every [wrapped_compat] shim. Mirrors what [(wrapped (transition ...))] libraries expose to consumers. - [entry_modules]'s wrapped case switches to use it. Net effect: in transition wrapped libs, consumers can resolve any of the bare module names the lib exposes; this lifts a false-negative in the index that previously hid the consumer's reference to a [wrapped_compat] shim from the per-module filter. Tests (cherry-picked from #14492): - New soundness fixtures land here: [cross-lib-instrumentation-barrier.t], [cross-lib-preprocess-barrier.t], [cross-lib-pps-runtime-no-ocamldep-barrier.t], [wrapped-from-vlib-soundness.t], [wrapped-transition-soundness.t], [mixed-per-module-preprocess.t], [mixed-per-module-preprocess-precision.t], [cmx-native-tight-deps.t]. - The five pre-existing tests broken by L4 ([auto-wrapped-child-reexport.t], [ppx-runtime-libraries.t], [virtual-library.t], [wrapped-closure-precision.t], [wrapped-reexport-via-open-flag.t]) pass again — soundness recovery restores their original behavior; no test file change in #14492's diff for them. Changelog: [doc/changes/added/14492.md] lands now. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-issue-4572-rebased
branch
from
ea13ad6 to
698f3e4
Compare
robinbb
added a commit
that referenced
this pull request
May 18, 2026
Restores correctness for three cases the bare BFS filter mishandles: - Deps that implement a virtual library: dep-graph through them is computed elsewhere ([Dep_rules.imported_vlib_deps]); the per-module filter can miss cmi changes. Gate: fall through to glob whenever the cctx has [has_virtual_impl]. - Wrapped local libs the consumer references through the wrapper name: the ocamldep walk can't see the alias chain into the lib's [wrapped_compat] / inner modules. Reach: glob the wrapped lib's [Lib.closure]. - [ppx_runtime_libraries] introduced by [pps] in the consumer's preprocessor: their modules appear in the post-pp source which ocamldep can't see. Reach: glob their [Lib.closure]. [Module_compilation.lib_deps_for_module]: - After [can_filter], read [Compilation_context.has_virtual_impl]; if true, fall back to glob. - Read [Compilation_context.pps_runtime_libs] and include them in [direct_libs] so [Lib.closure] sees them. - Compute [wrapped_libs_referenced] from the consumer's [referenced_modules] (BFS-initial frontier — pre-cross-lib-walk). Take the [Lib.closure] of that set union [pps_runtime_libs] to get [must_glob_libs]; the classification fold sends every member to the glob path. [Modules]: - [Wrapped.entry_modules]: new function. Returns the wrapper ([lib_interface]) plus every [wrapped_compat] shim. Mirrors what [(wrapped (transition ...))] libraries expose to consumers. - [entry_modules]'s wrapped case switches to use it. Net effect: in transition wrapped libs, consumers can resolve any of the bare module names the lib exposes; this lifts a false-negative in the index that previously hid the consumer's reference to a [wrapped_compat] shim from the per-module filter. Tests (cherry-picked from #14492): - New soundness fixtures land here: [cross-lib-instrumentation-barrier.t], [cross-lib-preprocess-barrier.t], [cross-lib-pps-runtime-no-ocamldep-barrier.t], [wrapped-from-vlib-soundness.t], [wrapped-transition-soundness.t], [mixed-per-module-preprocess.t], [mixed-per-module-preprocess-precision.t], [cmx-native-tight-deps.t]. - The five pre-existing tests broken by L4 ([auto-wrapped-child-reexport.t], [ppx-runtime-libraries.t], [virtual-library.t], [wrapped-closure-precision.t], [wrapped-reexport-via-open-flag.t]) pass again — soundness recovery restores their original behavior; no test file change in #14492's diff for them. Changelog: [doc/changes/added/14492.md] lands now. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 18, 2026
Add [doc/dev/per-module-narrowing.md] describing the per-module library file dependency narrowing introduced in #14492 (split into PRs #14513..#14521 as layers L1..L9): - The motivation and soundness model. - The [can_filter] precondition and [has_virtual_impl] early-out. - The narrowing pipeline: read ocamldep raw refs → [referenced] → [Lib.closure] → cross-library BFS → classification → emit per-lib deps and filtered include flags. - The data structures used ([Lib_index], the per-cctx [cached_raw_refs] / [Filtered_includes] / [Lib.closure] memos). - Soundness fallbacks (wrapped libs, virtual impls, ppx runtime). - A source map locating each concern in [src/dune_rules/]. - A layer-by-layer summary of #14513..#14521. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
This was referenced May 18, 2026
robinbb
added a commit
that referenced
this pull request
May 18, 2026
Four fixtures pinning the consumer-compile cmi-dep behaviour today, when the cctx-wide cmi glob over a dep lib's objdir covers the consumer's transitive `.cmi` needs regardless of whether `ocamldep` can see the chain syntactically. Each fixture hides the leaf's name from `ocamldep` through a different mechanism: - `cross-lib-preprocess-barrier.t` — `(preprocess (action ...))` - `cross-lib-pps-runtime-no-ocamldep-barrier.t` — `(preprocess (pps X))` + `ppx_runtime_libraries` - `cross-lib-instrumentation-barrier.t` — `(instrumentation (backend X))` - `cross-lib-open-flag-barrier.t` — `(flags (-open M))` Three of the four assert build success under `--sandbox=copy`. The instrumentation case additionally pins today's wide cmi glob over `middle`'s objdir in `dune rules` output. The forthcoming per-module narrowing work (#14492) will validate that each construct continues to work soundly under tighter per-module dep tracking, and will flip the instrumentation case's `jq` expected output from "glob present" to "glob absent". Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 18, 2026
Pins today's behaviour for a `(wrapped false)` library with an explicit `(modules a b)` clause: editing one sibling's `.mli` rebuilds every sibling's compile rule because the consumer's compile depends on the lib's whole-objdir `.cmi` glob. The test asserts (via `dune trace cat` + `jq`) the rebuild lists explicitly. The forthcoming per-module narrowing work (#14492) will flip both expected outputs to `[]` (sibling rebuilds suppressed by per-module deps). Matches the exact lib shape raised in #14492's review feedback; the explicit `(modules ...)` clause routes through a different parse path from the implicit form covered by `lib-to-lib-unwrapped.t`, so a separate observational test pins the equivalence empirically. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 18, 2026
Two co-designed fixtures covering an unwrapped lib with per-module preprocessing where one module is default-pp (Some-entry in the per-lib index) and one is staged-pps (None-entry): - `mixed-per-module-preprocess.t` (soundness): consumer references the Some-entry module; build succeeds under `--sandbox=copy` because today's wide cmi glob over `mylib`'s objdir covers the staged-pps module's cmi. - `mixed-per-module-preprocess-precision.t` (precision): consumer references the Some-entry module while the staged-pps module's source contains an unresolvable identifier. Today, the wide glob pulls the bad module into consumer's deps and the build fails on the unbound identifier — pinning the current over-invalidation. The forthcoming per-module narrowing work (#14492) will flip the precision test's expected output from the compile error back to silent exit-0, demonstrating that the staged-pps None-entry module is no longer pulled in when the consumer doesn't reference it. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 18, 2026
Four fixtures pinning the consumer-compile cmi-dep behaviour today, when the cctx-wide cmi glob over a dep lib's objdir covers the consumer's transitive `.cmi` needs regardless of whether `ocamldep` can see the chain syntactically. Each fixture hides the leaf's name from `ocamldep` through a different mechanism: - `cross-lib-preprocess-barrier.t` — `(preprocess (action ...))` - `cross-lib-pps-runtime-no-ocamldep-barrier.t` — `(preprocess (pps X))` + `ppx_runtime_libraries` - `cross-lib-instrumentation-barrier.t` — `(instrumentation (backend X))` - `cross-lib-open-flag-barrier.t` — `(flags (-open M))` Three of the four assert build success under `--sandbox=copy`. The instrumentation case additionally pins today's wide cmi glob over `middle`'s objdir in `dune rules` output. The forthcoming per-module narrowing work (#14492) will validate that each construct continues to work soundly under tighter per-module dep tracking, and will flip the instrumentation case's `jq` expected output from "glob present" to "glob absent". Signed-off-by: Robin Bate Boerop <me@robinbb.com>
This was referenced May 19, 2026
This was referenced May 20, 2026
Narrow each module compile's library .cmi dependencies to those actually reachable — via ocamldep raw references plus effective `-open` flags — rather than globbing every dependency library's objdir. Per-module `-I`/`-H` include flags are scoped to the kept libraries, with caches for the filtered includes, per-cctx raw ocamldep references, and `Lib.closure`. Soundness recoveries cover wrapped libraries, ppx-runtime dependencies, virtual-impl dependencies, and stanza/env `-open` barriers. Squashed reconstruction of the L1–L9 stack (#14513–#14521) onto current main; see those PRs for per-layer detail. Tests live under test/blackbox-tests/test-cases/per-module-lib-deps/. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
## Summary Lifts the `can_filter` Melange opt-out installed by #14516 (L4). The BFS per-module narrowing pipeline now activates for Melange consumer compiles on the same terms as OCaml — same `Lib_index`, same wrapped-lib soundness recovery, same `must_glob_set` / `tight_set` split. `Module_compilation.lib_deps_for_module` drops the `match Lib_mode.of_cm_kind cm_kind` arm from `can_filter`. `Dep_rules.rules` drops the `|| Compilation_mode.equal for_ Melange` disjunct from the singleton-stanza short-circuit. `Lib_file_deps.deps_of_entry_modules` gains a `want_cmj` arm symmetric with `want_cmx`, so `Melange Cmj` consumers see per-module `.cmj` deps in addition to `.cmi`. Six Melange cram tests are promoted for output drift (additional `ocamldep (internal)` trace lines + one duplicated `melc not found` error in `missing-melc.t`). No artefact changes; same exit codes. ## Stack Rebases on #14521 (L9). Part of #14492. ## Validation - All 112 `test/blackbox-tests/test-cases/melange/*.t` pass after promotion. - melange-re/melange 6.0.1-54 unit-test suite: 84/84 OUnit tests pass against this branch with melange-re/melange's `test/unit-tests/ounit_unicode_tests.ml` re-routed through `Melange_ppx.String_interp` (a 3-line patch that's a no-op for upstream Melange and that the project is welcome to absorb). - Pristine 6.0.1-54 fails with `module String_interp is an alias for module Melange_ppx__String_interp, which is missing` — the same alias-form issue already documented by `wrapped-internal-leak.t` on L4. The pattern is essentially absent outside Melange's own ppx (zero GitHub code-search hits for `= Core__`, `= Async__`, `= Ppxlib__`, `= Bonsai__`). ## Fixes Part of #14492. --------- Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Collaborator
Author
|
@Alizter Be aware that this is L1..L9, which you reviewed, plus L10 (#14732) which was reviewed by @anmonteiro. If we merge this PR, it all goes into 'main'. |
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.
Summary
Per-module library dependency filtering for #4572. A consumer module's compile rule now sees only the artefacts of libraries it actually references, so unrelated sibling modules no longer recompile when an unreferenced dependency library's cmi changes. User-facing detail is in the shipped changelog entry under
doc/changes/added/.Predecessor PRs (now closed)
-I/-Hinclude flags matching the filter.Raw_refsAction_buildercache to deduplicate ocamldep results across cctxs sharing the same module sources.Fixes #4572.