From a70c06e5bd99dc4f6241f576db76880402913023 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Mon, 11 May 2026 10:21:10 -0700 Subject: [PATCH 01/12] test: guards for wrapped-lib soundness under non-trivial wrapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two forward-looking guard files in `test/blackbox-tests/test-cases/ per-module-lib-deps/`. Each file mixes soundness cases (must keep rebuilding) with a forward-looking pin on current per-library filter behaviour (currently rebuilds the consumer when an unreferenced sibling module changes; will need promotion once per-module narrowing within a library lands): - `wrapped-transition-soundness.t`: a consumer reaches an inner module of a `(wrapped (transition ...))` library through the wrapper alias `Wrapped_lib.Inner_a.x`. Case 1 (soundness) pins that the consumer rebuilds when the referenced inner module's interface changes — its compile-rule deps must cover `wrapped_lib__Inner_a.cmi` (the mangled inner-module artifact, not the un-prefixed transition compat shim), not only the wrapper's `.cmi`. Case 2 (narrowing pin) covers an unreferenced sibling (`inner_b`). - `wrapped-from-vlib-soundness.t`: a consumer depends on a virtual-library implementation that inherits its `(wrapped ...)` setting from the vlib (the impl does not redeclare `wrapped`). Cases 1–2 (soundness) pin that the consumer rebuilds when a concrete vlib module (`helper`) or a virtual module (`virt_iface`) has its interface change — its compile-rule deps must cover the impl's `.cmi` directory. Case 3 (narrowing pin) covers an unreferenced sibling (`unused`). The soundness cases hold trivially today via the cctx-wide compile-rule glob over each dep library's `.cmi` directory. Future changes that narrow compile-rule deps per-module must keep that coverage for the referenced-module / inherited-wrapped-library edge cases (the soundness cases); the narrowing pins will flip when the narrowing lands and should be promoted then. Test structure: jq regexes are anchored to the objdir (`\.consumer\.objs/byte/consumer\.cm` and `consumer/\.main\.eobjs/byte/`) rather than relying on dune's internal mangling. Signed-off-by: Robin Bate Boerop --- .../wrapped-from-vlib-soundness.t | 152 ++++++++++++++++++ .../wrapped-transition-soundness.t | 105 ++++++++++++ 2 files changed, 257 insertions(+) create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-from-vlib-soundness.t create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-transition-soundness.t diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-from-vlib-soundness.t b/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-from-vlib-soundness.t new file mode 100644 index 00000000000..a2f0a56edb6 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-from-vlib-soundness.t @@ -0,0 +1,152 @@ +Regression guards for soundness, with a forward-looking pin on +current behaviour, all when a consumer depends on an implementation +that inherits its `(wrapped ...)` setting from the virtual library +(the implementation does not redeclare `wrapped`). + +`vlib` declares `(wrapped true)` with a virtual module `virt_iface` +and concrete siblings `helper` and `unused`. `impl` implements +`vlib` without redeclaring `wrapped`. The executable `main` depends +on `impl` and reaches `virt_iface` and `helper` via the vlib +wrapper: `Vlib.Virt_iface.x` and `Vlib.Helper.h`. `main` does not +reference `unused`. + +The implementation's closure includes `virt_iface`'s impl and +`vlib`'s concrete modules. `main`'s compile rule must therefore +cover `vlib__Virt_iface.cmi` and `vlib__Helper.cmi`. Any future +per-module narrowing that treats inherited-wrapped libraries as +ordinary local libraries must still keep that coverage; otherwise +a change to either module's interface fails to invalidate `main`. + + $ make_dune_project 3.24 + + $ mkdir vlib impl consumer + + $ cat > vlib/dune < (library + > (name vlib) + > (wrapped true) + > (virtual_modules virt_iface)) + > EOF + $ cat > vlib/virt_iface.mli < val x : string + > EOF + $ cat > vlib/helper.ml < let h = "h" + > let z = 42 + > EOF + $ cat > vlib/helper.mli < val h : string + > EOF + $ cat > vlib/unused.ml < let u = "u" + > let w = "w" + > EOF + $ cat > vlib/unused.mli < val u : string + > EOF + + $ cat > impl/dune < (library + > (name impl) + > (implements vlib)) + > EOF + $ cat > impl/virt_iface.ml < let x = "impl" + > let z = 42 + > EOF + + $ cat > consumer/dune < (executable + > (name main) + > (libraries impl)) + > EOF + $ cat > consumer/main.ml < let () = print_string Vlib.Virt_iface.x; print_string Vlib.Helper.h + > EOF + + $ dune build @check + +Glob coverage on `main.cmi`'s compile rule: + + $ dune rules --root . --format=json --deps '%{cmi:consumer/main}' | + > jq -r 'include "dune"; .[] | depsGlobs | "\(.dir_kind) \(.dir) \(.predicate)"' + In_build_dir _build/default/impl/.impl.objs/byte *.cmi + +Case 1 (soundness): edit `helper`'s interface to expose `z`. `main` +reaches `helper` through the vlib wrapper; the compile-rule deps +must cover `vlib__Helper.cmi`, so `main` rebuilds: + + $ cat > vlib/helper.mli < val h : string + > val z : int + > EOF + $ dune build @check + $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("consumer/\\.main\\.eobjs/byte/"))]' + [ + { + "target_files": [ + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmi", + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmti" + ] + }, + { + "target_files": [ + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmo", + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmt" + ] + } + ] + +Case 2 (soundness): edit `virt_iface`'s interface to expose `z`. +`main` reaches `virt_iface` through the vlib wrapper; the compile- +rule deps must cover `vlib__Virt_iface.cmi`, so `main` rebuilds: + + $ cat > vlib/virt_iface.mli < val x : string + > val z : int + > EOF + $ dune build @check + $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("consumer/\\.main\\.eobjs/byte/"))]' + [ + { + "target_files": [ + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmi", + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmti" + ] + }, + { + "target_files": [ + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmo", + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmt" + ] + } + ] + +Case 3 (forward-looking pin on current behaviour): edit `unused`'s +interface to expose `w`. `main` does not reference `unused`, so +under a future per-module narrowing this edit would not rebuild +`main`. Today, the per-library filter rebuilds `main` anyway +because `impl`'s `.cmi` glob covers every module of the +implementation's closure. Promote when per-module narrowing within +a library lands. + + $ cat > vlib/unused.mli < val u : string + > val w : string + > EOF + $ dune build @check + $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("consumer/\\.main\\.eobjs/byte/"))]' + [ + { + "target_files": [ + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmi", + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmti" + ] + }, + { + "target_files": [ + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmo", + "_build/default/consumer/.main.eobjs/byte/dune__exe__Main.cmt" + ] + } + ] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-transition-soundness.t b/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-transition-soundness.t new file mode 100644 index 00000000000..08b4118ba01 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-transition-soundness.t @@ -0,0 +1,105 @@ +Regression guard for wrapped-library soundness, with a forward- +looking pin on current behaviour, both under +`(wrapped (transition ...))`. + +`wrapped_lib` uses `(wrapped (transition ...))` with inner modules +`inner_a` and `inner_b` plus a hand-written wrapper module that +aliases both. The library `consumer` depends on `wrapped_lib` and +writes `Wrapped_lib.Inner_a.x` — naming the wrapper and one inner +module but not the other. + +The wrapper's `.cmi` only carries an alias name; the type lives in +the inner module's mangled artifact `wrapped_lib__Inner_a.cmi` (not +the `inner_a.cmi` transition shim). So `consumer`'s compile rule +must cover `wrapped_lib__Inner_a.cmi` alongside `wrapped_lib.cmi`. +Any future per-module narrowing of compile-rule deps must keep that +coverage; otherwise a change to `inner_a`'s interface fails to +invalidate `consumer`. + + $ make_dune_project 3.24 + + $ cat > dune < (library + > (name wrapped_lib) + > (wrapped (transition "use Wrapped_lib.X instead of X")) + > (modules wrapped_lib inner_a inner_b)) + > (library + > (name consumer) + > (modules consumer) + > (libraries wrapped_lib)) + > EOF + + $ cat > wrapped_lib.ml < module Inner_a = Inner_a + > module Inner_b = Inner_b + > EOF + $ cat > inner_a.ml < let x = "a" + > let z = 42 + > EOF + $ cat > inner_a.mli < val x : string + > EOF + $ cat > inner_b.ml < let y = "b" + > let w = "w" + > EOF + $ cat > inner_b.mli < val y : string + > EOF + + $ cat > consumer.ml < let _ = Wrapped_lib.Inner_a.x + > EOF + + $ dune build @check + +Glob coverage on `consumer.cmi`'s compile rule: + + $ dune rules --root . --format=json --deps '%{cmi:consumer}' | + > jq -r 'include "dune"; .[] | depsGlobs | "\(.dir_kind) \(.dir) \(.predicate)"' + In_build_dir _build/default/.wrapped_lib.objs/byte *.cmi + +Case 1 (soundness): edit `inner_a`'s interface to expose `z`. +`consumer` reaches `inner_a` through the wrapper `Wrapped_lib`; +the compile-rule deps must cover `wrapped_lib__Inner_a.cmi`, so +`consumer` rebuilds: + + $ cat > inner_a.mli < val x : string + > val z : int + > EOF + $ dune build @check + $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("\\.consumer\\.objs/byte/consumer\\.cm"))]' + [ + { + "target_files": [ + "_build/default/.consumer.objs/byte/consumer.cmi", + "_build/default/.consumer.objs/byte/consumer.cmo", + "_build/default/.consumer.objs/byte/consumer.cmt" + ] + } + ] + +Case 2 (forward-looking pin on current behaviour): edit `inner_b`'s +interface to expose `w`. `consumer` does not reference `inner_b`, +so under a future per-module narrowing this edit would not rebuild +`consumer`. Today, the per-library filter rebuilds `consumer` +anyway because `wrapped_lib`'s `.cmi` glob covers every module. +Promote when per-module narrowing within a library lands. + + $ cat > inner_b.mli < val y : string + > val w : string + > EOF + $ dune build @check + $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("\\.consumer\\.objs/byte/consumer\\.cm"))]' + [ + { + "target_files": [ + "_build/default/.consumer.objs/byte/consumer.cmi", + "_build/default/.consumer.objs/byte/consumer.cmo", + "_build/default/.consumer.objs/byte/consumer.cmt" + ] + } + ] From b4b8fb17d5c7887b77cbbd0c65b6c2c3c1fdfce6 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 09:58:38 -0700 Subject: [PATCH 02/12] scaffold: thread Lib_index, has_virtual_impl, pps_runtime_libs through cctx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure-additive scaffolding for #4572's per-module inter-library filter. Fields are populated but no consumer reads them yet; the per-module filter (which is the only caller) lands in a follow-up. [Compilation_context]: - [build_lib_index]: builds a [Lib_file_deps.Lib_index.t] from the cctx's direct + hidden libs. Each entry carries [Some Module.t] for unwrapped locals (tight-eligible) and [None] otherwise (wrapped locals / externals). Local libs whose source is preprocessed by a non-staged ppx are indexed on the post-pp module so the cross-lib walker reads ocamldep on the same source the dep lib's compile pipeline produces. - Three new fields on [t]: [lib_index] (Memo.Lazy.t computing the index), [has_virtual_impl] (Memo.Lazy.t flag — true iff any dep lib implements a virtual lib), and [pps_runtime_libs] (closure of ppx_runtime_libraries introduced by [pps] in this stanza, threaded through [create]). - Accessors for each. [for_module_generated_at_link_time] populates [lib_index] with a [Code_error.raise] sentinel — the per-module filter's [can_filter] guard prevents reaching it from synthesised link-time cctxs. [Lib_rules] / [Exe_rules]: compute [pps_runtime_libs] from the stanza's [compile_info] and thread it through [Compilation_context.create]. [Dep_graph]: expose [dir] and [mem]; the cross-lib walker's [can_filter] guard uses these to detect synthesised dummy graphs. [Modules]: add [as_singleton] returning [Some m] iff the module set is a single user-written module. Used by [build_lib_index] to detect the single-module-no-deps short-circuit case. Signed-off-by: Robin Bate Boerop --- src/dune_rules/compilation_context.ml | 102 +++++++++++++++++++++++++ src/dune_rules/compilation_context.mli | 16 ++++ src/dune_rules/dep_graph.ml | 2 + src/dune_rules/dep_graph.mli | 2 + src/dune_rules/exe_rules.ml | 6 ++ src/dune_rules/lib_file_deps.ml | 5 +- src/dune_rules/lib_file_deps.mli | 6 +- src/dune_rules/lib_rules.ml | 6 ++ src/dune_rules/modules.ml | 6 ++ src/dune_rules/modules.mli | 7 ++ 10 files changed, 153 insertions(+), 5 deletions(-) diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index de6a2e4cfc1..85d6c16a43b 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -91,7 +91,10 @@ type t = ; parameters : Module_name.t list Resolve.Memo.t ; instances : Parameterised_instances.t Resolve.Memo.t option ; includes : Includes.t + ; lib_index : Lib_file_deps.Lib_index.t Resolve.t Memo.Lazy.t + ; has_virtual_impl : bool Resolve.t Memo.Lazy.t ; preprocessing : Pp_spec.t + ; pps_runtime_libs : Lib.t list Resolve.Memo.t ; opaque : bool ; js_of_ocaml : Js_of_ocaml.In_context.t option Js_of_ocaml.Mode.Pair.t ; sandbox : Sandbox_config.t @@ -118,7 +121,10 @@ let requires_hidden t = t.requires_hidden let requires_link t = Memo.Lazy.force t.requires_link let parameters t = t.parameters let includes t = t.includes +let lib_index t = Memo.Lazy.force t.lib_index +let has_virtual_impl t = Memo.Lazy.force t.has_virtual_impl let preprocessing t = t.preprocessing +let pps_runtime_libs t = t.pps_runtime_libs let opaque t = t.opaque let js_of_ocaml t = t.js_of_ocaml let sandbox t = t.sandbox @@ -147,6 +153,74 @@ let parameters_main_modules parameters = [ "param", Lib.to_dyn param ]) ;; +(* Hidden libs must be indexed: otherwise unreached ones fall to the glob branch + and over-invalidate. *) +let build_lib_index ~super_context ~libs ~for_ = + let open Resolve.Memo.O in + let instrument_with = Context.instrument_with (Super_context.context super_context) in + let+ per_lib = + Resolve.Memo.List.map libs ~f:(fun lib -> + match Lib_info.entry_modules (Lib.info lib) ~for_ with + | External (Ok names) -> + Resolve.Memo.return (List.map names ~f:(fun n -> n, lib, None), None) + | External (Error e) -> Resolve.Memo.of_result (Error e) + | Local -> + let* mods = + Resolve.Memo.lift_memo + (Dir_contents.modules_of_local_lib + super_context + (Lib.Local.of_lib_exn lib) + ~for_) + in + (* [no_ocamldep_lib] tags libs that are walker-terminal: running + ocamldep on their entry module via the cross-lib walk can't + propagate anywhere, so the walker skips them. A singleton lib is + terminal only when its resolved requires are empty; otherwise the + walker must read its post-pp ocamldep to discover transitive refs + (incl. pps runtime libs added via [add_pp_runtime_deps]). *) + let+ requires_resolved = Lib.requires lib ~for_ in + (* [Some m] only for unwrapped locals (tight-eligible); wrapped locals + and externals → [None]. *) + let unwrapped = + match Lib_info.wrapped (Lib.info lib) with + | Some (This w) -> not (Wrapped.to_bool w) + | Some (From _) | None -> false + in + (* Mirror [Pp_spec.pped_modules_map] so the cross-lib walker reads + ocamldep on the source the dep lib's compile pipeline produces. *) + let preprocess = Lib_info.preprocess (Lib.info lib) ~for_ in + let post_pp_module m = + match Preprocess.Per_module.find (Module.name m) preprocess with + | No_preprocessing | Future_syntax _ -> Some (Module.ml_source m) + | Action _ -> Some (Module.ml_source (Module.pped m)) + | Pps { staged = false; pps; _ } -> + let any_active = + List.exists pps ~f:(function + | Preprocess.With_instrumentation.Ordinary _ -> true + | Instrumentation_backend { libname = _, name; _ } -> + List.mem instrument_with name ~equal:Lib_name.equal) + in + if any_active + then Some (Module.pped (Module.ml_source m)) + else Some (Module.ml_source m) + | Pps { staged = true; _ } -> None + in + let entries = + List.map (Modules.entry_modules mods) ~f:(fun m -> + Module.name m, lib, if unwrapped then post_pp_module m else None) + in + let no_ocamldep_lib = + match Modules.as_singleton mods with + | Some _ when List.is_empty requires_resolved -> Some lib + | _ -> None + in + entries, no_ocamldep_lib) + in + let entries = List.concat_map per_lib ~f:fst in + let no_ocamldep = List.filter_map per_lib ~f:snd |> Lib.Set.of_list in + Lib_file_deps.Lib_index.create ~no_ocamldep entries +;; + let create ~super_context ~scope @@ -155,6 +229,7 @@ let create ~flags ~requires_compile ~requires_link + ?(pps_runtime_libs = Resolve.Memo.return []) ?(preprocessing = Pp_spec.dummy) ~opaque ~js_of_ocaml @@ -246,7 +321,20 @@ let create ; parameters ; includes = Includes.make ~project ~opaque ~direct_requires ~hidden_requires ocaml.lib_config + ; lib_index = + Memo.lazy_ (fun () -> + let open Resolve.Memo.O in + let* d = direct_requires + and* h = hidden_requires in + build_lib_index ~super_context ~libs:(d @ h) ~for_) + ; has_virtual_impl = + Memo.lazy_ (fun () -> + let open Resolve.Memo.O in + let+ direct = direct_requires + and+ hidden = hidden_requires in + List.exists (direct @ hidden) ~f:(fun lib -> Option.is_some (Lib.implements lib))) ; preprocessing + ; pps_runtime_libs ; opaque ; js_of_ocaml ; sandbox @@ -347,7 +435,21 @@ let for_module_generated_at_link_time cctx ~requires ~module_ = ; flags = Ocaml_flags.empty ; requires_link = Memo.lazy_ (fun () -> requires) ; requires_compile = requires + ; requires_hidden = Resolve.Memo.return [] ; includes + ; lib_index = + Memo.lazy_ (fun () -> + (* Unreachable: synthesised modules use [Dep_graph.dummy] (whose [dir] + is [Path.Build.root]), so [lib_deps_for_module]'s [can_filter] + dir-equality guard fails and the non-filter fallback is taken. *) + Code_error.raise + "Compilation_context.lib_index forced for a module synthesised at link time; \ + this should be unreachable." + []) + ; (* Link-time-generated modules cannot participate in virtual-impl + relationships; override the parent's value so the accessor reflects + this cctx's [requires_compile]/[requires_link]. *) + has_virtual_impl = Memo.Lazy.of_val (Resolve.return false) ; modules } ;; diff --git a/src/dune_rules/compilation_context.mli b/src/dune_rules/compilation_context.mli index da50ceb82db..c917486e0cf 100644 --- a/src/dune_rules/compilation_context.mli +++ b/src/dune_rules/compilation_context.mli @@ -27,6 +27,7 @@ val create -> flags:Ocaml_flags.t -> requires_compile:Lib.t list Resolve.Memo.t -> requires_link:Lib.t list Resolve.t Memo.Lazy.t + -> ?pps_runtime_libs:Lib.t list Resolve.Memo.t -> ?preprocessing:Pp_spec.t -> opaque:opaque -> js_of_ocaml:Js_of_ocaml.In_context.t option Js_of_ocaml.Mode.Pair.t @@ -62,7 +63,22 @@ val requires_hidden : t -> Lib.t list Resolve.Memo.t val requires_compile : t -> Lib.t list Resolve.Memo.t val parameters : t -> Module_name.t list Resolve.Memo.t val includes : t -> Command.Args.without_targets Command.Args.t Lib_mode.Cm_kind.Map.t +val lib_index : t -> Lib_file_deps.Lib_index.t Resolve.Memo.t + +(** [true] iff any library in the compilation context's direct or hidden + requires implements a virtual library. Memoized per cctx. *) +val has_virtual_impl : t -> bool Resolve.Memo.t + val preprocessing : t -> Pp_spec.t + +(** Direct [ppx_runtime_deps] of every [pps] in this stanza's preprocessor + (a flat concatenation; not closed transitively — consumers that need + closure semantics wrap with [Lib.closure]). These libraries' modules are + visible only to ppx-rewritten output, so the per-module dependency + filter cannot reason about which of them are referenced — they must be + treated as opaque [must-glob] dependencies. *) +val pps_runtime_libs : t -> Lib.t list Resolve.Memo.t + val opaque : t -> bool val js_of_ocaml : t -> Js_of_ocaml.In_context.t option Js_of_ocaml.Mode.Pair.t val sandbox : t -> Sandbox_config.t diff --git a/src/dune_rules/dep_graph.ml b/src/dune_rules/dep_graph.ml index 8ff3f084366..dd8b8b2a8cd 100644 --- a/src/dune_rules/dep_graph.ml +++ b/src/dune_rules/dep_graph.ml @@ -7,6 +7,8 @@ type t = } let make ~dir ~per_module = { dir; per_module } +let dir t = t.dir +let mem t (m : Module.t) = Module_name.Unique.Map.mem t.per_module (Module.obj_name m) let deps_of t (m : Module.t) = match Module_name.Unique.Map.find t.per_module (Module.obj_name m) with diff --git a/src/dune_rules/dep_graph.mli b/src/dune_rules/dep_graph.mli index d5f663b222f..d8b5ff3726a 100644 --- a/src/dune_rules/dep_graph.mli +++ b/src/dune_rules/dep_graph.mli @@ -9,6 +9,8 @@ val make -> per_module:Module.t list Action_builder.t Module_name.Unique.Map.t -> t +val dir : t -> Path.Build.t +val mem : t -> Module.t -> bool val deps_of : t -> Module.t -> Module.t list Action_builder.t val top_closed_implementations : t -> Module.t list -> Module.t list Action_builder.t diff --git a/src/dune_rules/exe_rules.ml b/src/dune_rules/exe_rules.ml index e032e212171..6dbc907ead6 100644 --- a/src/dune_rules/exe_rules.ml +++ b/src/dune_rules/exe_rules.ml @@ -193,6 +193,11 @@ let executables_rules let* cctx = let requires_compile = Lib.Compile.direct_requires compile_info ~for_ in let requires_link = Lib.Compile.requires_link compile_info ~for_ in + let pps_runtime_libs = + let open Resolve.Memo.O in + let* pps = Lib.Compile.pps compile_info ~for_ in + Resolve.Memo.List.concat_map pps ~f:(Lib.ppx_runtime_deps ~for_) + in let instances = Parameterised_instances.instances ~sctx @@ -215,6 +220,7 @@ let executables_rules ~flags ~requires_link ~requires_compile + ~pps_runtime_libs ~preprocessing:pp ~js_of_ocaml ~opaque:Inherit_from_settings diff --git a/src/dune_rules/lib_file_deps.ml b/src/dune_rules/lib_file_deps.ml index 13048f499e4..c5fb609e974 100644 --- a/src/dune_rules/lib_file_deps.ml +++ b/src/dune_rules/lib_file_deps.ml @@ -118,8 +118,9 @@ module Lib_index = struct { by_module_name : (Lib.t * Module.t option) list Module_name.Map.t ; tight_eligible : Lib.Set.t ; no_ocamldep : Lib.Set.t - (* Local libs short-circuited by [Dep_rules.skip_ocamldep] — no [.d] rules - exist; the cross-library walk must skip them. *) + (* Local libs that are walker-terminal: running ocamldep on their entry + module via the cross-library walk can't propagate anywhere (no + resolved requires to chase), so the walker skips them. *) } (* Tight-eligibility is encoded in the entry shape: [(_, lib, Some _)] means diff --git a/src/dune_rules/lib_file_deps.mli b/src/dune_rules/lib_file_deps.mli index 83ef20403c0..75683ac1ec2 100644 --- a/src/dune_rules/lib_file_deps.mli +++ b/src/dune_rules/lib_file_deps.mli @@ -42,9 +42,9 @@ module Lib_index : sig (** Third tuple element is [Some m] for local + unwrapped libs (with the entry's [Module.t]) and [None] otherwise (wrapped locals, externals). - [no_ocamldep] names local libs whose [.d] files don't exist - (short-circuited by [Dep_rules.skip_ocamldep]); the cross-library walk - skips them. *) + [no_ocamldep] names local libs that are walker-terminal (singletons + with no resolved requires) — the cross-library walk would gain nothing + by running ocamldep on them, so it skips them. *) val create : no_ocamldep:Lib.Set.t -> (Module_name.t * Lib.t * Module.t option) list diff --git a/src/dune_rules/lib_rules.ml b/src/dune_rules/lib_rules.ml index d6c79a1a5e7..44a1f6de602 100644 --- a/src/dune_rules/lib_rules.ml +++ b/src/dune_rules/lib_rules.ml @@ -502,6 +502,11 @@ let cctx let modules = Virtual_rules.impl_modules implements modules in let requires_compile = Lib.Compile.direct_requires compile_info ~for_ in let requires_link = Lib.Compile.requires_link compile_info ~for_ in + let pps_runtime_libs = + let open Resolve.Memo.O in + let* pps = Lib.Compile.pps compile_info ~for_ in + Resolve.Memo.List.concat_map pps ~f:(Lib.ppx_runtime_deps ~for_) + in let instances = Parameterised_instances.instances ~sctx ~db:(Scope.libs scope) lib.buildable.libraries in @@ -532,6 +537,7 @@ let cctx ~flags ~requires_compile ~requires_link + ~pps_runtime_libs ~implements ~parameters ~preprocessing:pp diff --git a/src/dune_rules/modules.ml b/src/dune_rules/modules.ml index 6a42fed4e9a..eed14fdbd33 100644 --- a/src/dune_rules/modules.ml +++ b/src/dune_rules/modules.ml @@ -918,6 +918,12 @@ let wrapped t = | Stdlib _ -> Simple true ;; +let as_singleton t = + match t.modules with + | Singleton m -> Some m + | Unwrapped _ | Wrapped _ | Stdlib _ -> None +;; + let is_user_written m = match Module.kind m with | Root | Wrapped_compat | Alias _ -> false diff --git a/src/dune_rules/modules.mli b/src/dune_rules/modules.mli index 57ff2ed5ca2..6dc81116e0c 100644 --- a/src/dune_rules/modules.mli +++ b/src/dune_rules/modules.mli @@ -57,6 +57,13 @@ val obj_map : t -> Sourced_module.t Module_name.Unique.Map.t val virtual_module_names : t -> Module_name.Path.Set.t val wrapped : t -> Wrapped.t + +(** [Some m] iff the library's module set is a singleton — the single + user-written module [m]. This covers both unwrapped stanzas with exactly one + module and wrapped stanzas whose only module is the main module (in which + case the wrapper is elided). Mirrors [With_vlib.as_singleton]. *) +val as_singleton : t -> Module.t option + val source_dirs : t -> Path.Set.t val compat_for_exn : t -> Module.t -> Module.t From acc84297823db90735e194a8930115415fafbe9e Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:05:38 -0700 Subject: [PATCH 03/12] refactor: route lib file deps through [lib_deps_for_module] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavior-equivalent restructuring that opens the lib-deps computation to per-module filtering in a follow-up. No test promotions. [Compilation_context.Includes.make] previously emitted both [-I]/[-H] include flags AND [Hidden_deps] for the cctx's libs in a single [Command.Args.t]. The opaque-aware [Cmx] case duplicated the deps logic that already exists in [Lib_file_deps.deps_of_entries]. Simplified: [Includes.t] now carries only the include flags; the [~opaque] parameter (and the [for_module_generated_at_link_time] call site) drops out. [Module_compilation]: - [lib_deps_for_module]: scaffold form, returns [(cctx_includes, deps_of_entries libs)] where [libs = requires_compile @ requires_hidden]. The per-module tight filter activates in a follow-up; arguments [obj_dir], [for_], [dep_graph], [ml_kind], [mode] are threaded but ignored here. - [lib_cm_deps]: wraps [lib_deps_for_module] with [Action_builder.dyn_deps], yielding the include args and registering the lib file deps. - [build_cm]: gated route — [Alias _] (non-stdlib) and [Wrapped_compat] modules short-circuit to the cctx's now-flag-only [Includes] (no lib deps, matching prior behavior since [Includes.empty] was used for these); all other module kinds call [lib_cm_deps]. Replace the in-line [Includes] lookup at the [Command.run] site with [Command.Args.Dyn lib_cm_deps]. - [ocamlc_i]: same swap. Combined: every consumer that previously read [-I]/[-H] + [Hidden_deps] from [Includes] now reads [-I]/[-H] from [Includes] and the deps from [deps_of_entries] (via [lib_cm_deps]). Same flags, same deps. The [Alias]/[Wrapped_compat] short-circuit preserves the prior "no-lib-deps" behavior for those module kinds. Signed-off-by: Robin Bate Boerop --- src/dune_rules/compilation_context.ml | 44 +++------------- src/dune_rules/lib_file_deps.ml | 1 - src/dune_rules/lib_file_deps.mli | 2 - src/dune_rules/module_compilation.ml | 73 ++++++++++++++++++++++++--- 4 files changed, 75 insertions(+), 45 deletions(-) diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index 85d6c16a43b..18ce63a29a0 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -4,55 +4,29 @@ open Memo.O module Includes = struct type t = Command.Args.without_targets Command.Args.t Lib_mode.Cm_kind.Map.t - let make ~project ~opaque ~direct_requires ~hidden_requires lib_config + let make ~project ~direct_requires ~hidden_requires lib_config : _ Lib_mode.Cm_kind.Map.t = - (* TODO: some of the requires can filtered out using [ocamldep] info *) let open Resolve.Memo.O in let iflags direct_libs hidden_libs mode = Lib_flags.L.include_flags ~project ~direct_libs ~hidden_libs mode lib_config in - let make_includes_args ~mode groups = + let make_includes_args ~mode = (let+ direct_libs = direct_requires and+ hidden_libs = hidden_requires in - Command.Args.S - [ iflags direct_libs hidden_libs mode - ; Hidden_deps (Lib_file_deps.deps (direct_libs @ hidden_libs) ~groups) - ]) + iflags direct_libs hidden_libs mode) |> Resolve.Memo.args |> Command.Args.memo in { ocaml = - (let cmi_includes = make_includes_args ~mode:(Ocaml Byte) [ Ocaml Cmi ] in + (let cmi_includes = make_includes_args ~mode:(Ocaml Byte) in { cmi = cmi_includes ; cmo = cmi_includes - ; cmx = - (let+ direct_libs = direct_requires - and+ hidden_libs = hidden_requires in - Command.Args.S - [ iflags direct_libs hidden_libs (Ocaml Native) - ; Hidden_deps - (let libs = direct_libs @ hidden_libs in - if opaque - then - List.map libs ~f:(fun lib -> - ( lib - , if Lib.is_local lib - then [ Lib_file_deps.Group.Ocaml Cmi ] - else [ Ocaml Cmi; Ocaml Cmx ] )) - |> Lib_file_deps.deps_with_exts - else - Lib_file_deps.deps - libs - ~groups:[ Lib_file_deps.Group.Ocaml Cmi; Ocaml Cmx ]) - ]) - |> Resolve.Memo.args - |> Command.Args.memo + ; cmx = make_includes_args ~mode:(Ocaml Native) }) ; melange = - { cmi = make_includes_args ~mode:Melange [ Melange Cmi ] - ; cmj = make_includes_args ~mode:Melange [ Melange Cmi; Melange Cmj ] - } + (let mel_includes = make_includes_args ~mode:Melange in + { cmi = mel_includes; cmj = mel_includes }) } ;; @@ -319,8 +293,7 @@ let create ; requires_link ; implements ; parameters - ; includes = - Includes.make ~project ~opaque ~direct_requires ~hidden_requires ocaml.lib_config + ; includes = Includes.make ~project ~direct_requires ~hidden_requires ocaml.lib_config ; lib_index = Memo.lazy_ (fun () -> let open Resolve.Memo.O in @@ -425,7 +398,6 @@ let for_module_generated_at_link_time cctx ~requires ~module_ = let direct_requires = requires in Includes.make ~project:(Scope.project cctx.scope) - ~opaque ~direct_requires ~hidden_requires cctx.ocaml.lib_config diff --git a/src/dune_rules/lib_file_deps.ml b/src/dune_rules/lib_file_deps.ml index c5fb609e974..875060da506 100644 --- a/src/dune_rules/lib_file_deps.ml +++ b/src/dune_rules/lib_file_deps.ml @@ -55,7 +55,6 @@ let deps_of_lib (lib : Lib.t) ~groups = |> Dep.Set.of_list ;; -let deps_with_exts = Dep.Set.union_map ~f:(fun (lib, groups) -> deps_of_lib lib ~groups) let deps libs ~groups = Dep.Set.union_map libs ~f:(deps_of_lib ~groups) let groups_for_cm_kind ~opaque ~(cm_kind : Lib_mode.Cm_kind.t) lib = diff --git a/src/dune_rules/lib_file_deps.mli b/src/dune_rules/lib_file_deps.mli index 75683ac1ec2..876a99fa4b1 100644 --- a/src/dune_rules/lib_file_deps.mli +++ b/src/dune_rules/lib_file_deps.mli @@ -15,8 +15,6 @@ end with extension [files] of libraries [libs]. *) val deps : Lib.t list -> groups:Group.t list -> Dep.Set.t -val deps_with_exts : (Lib.t * Group.t list) list -> Dep.Set.t - (** [deps_of_entries ~opaque ~cm_kind libs] computes the file dependencies (glob deps on .cmi/.cmx files) for the given libraries. *) val deps_of_entries : opaque:bool -> cm_kind:Lib_mode.Cm_kind.t -> Lib.t list -> Dep.Set.t diff --git a/src/dune_rules/module_compilation.ml b/src/dune_rules/module_compilation.ml index d73cef2d651..3bfd86b54f8 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -1,6 +1,54 @@ open Import open Memo.O +let all_libs cctx = + let open Resolve.Memo.O in + let+ d = Compilation_context.requires_compile cctx + and+ h = Compilation_context.requires_hidden cctx in + d @ h +;; + +(* Returns the include flags and lib-file deps for compiling [m]. The scaffold + form is glob-only: include flags come from the cctx's [Includes] (-I/-H over + the full lib list) and deps come from [deps_of_entries] over the same list. + The per-module tight filter activates in a follow-up; arguments unused by + this form are reserved for that filter. *) +let lib_deps_for_module + ~cctx + ~obj_dir:_ + ~for_:_ + ~dep_graph:_ + ~opaque + ~cm_kind + ~ml_kind:(_ : Ml_kind.t) + ~mode:(_ : Lib_mode.t) + _m + = + let open Action_builder.O in + let* libs = Resolve.Memo.read (all_libs cctx) in + Action_builder.return + ( Lib_mode.Cm_kind.Map.get (Compilation_context.includes cctx) cm_kind + , Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs ) +;; + +let lib_cm_deps ~cctx ~cm_kind ~ml_kind ~mode m = + let obj_dir = Compilation_context.obj_dir cctx in + let opaque = Compilation_context.opaque cctx in + let for_ = Compilation_context.for_ cctx in + let dep_graph = Ml_kind.Dict.get (Compilation_context.dep_graphs cctx) ml_kind in + Action_builder.dyn_deps + (lib_deps_for_module + ~cctx + ~obj_dir + ~for_ + ~dep_graph + ~opaque + ~cm_kind + ~ml_kind + ~mode + m) +;; + (* Arguments for the compiler to prevent it from being too clever. The compiler creates the cmi when it thinks a .ml file has no corresponding @@ -281,6 +329,20 @@ let build_cm | Some All | None -> Hidden_targets [ obj ]) in let opaque = Compilation_context.opaque cctx in + let skip_lib_deps = + match Module.kind m with + | Alias _ -> + not (Modules.With_vlib.is_stdlib_alias (Compilation_context.modules cctx) m) + | Wrapped_compat -> true + | Intf_only | Virtual | Impl | Impl_vmodule | Root | Parameter -> false + in + let lib_cm_args = + if skip_lib_deps + then + Action_builder.return + (Lib_mode.Cm_kind.Map.get (Compilation_context.includes cctx) cm_kind) + else lib_cm_deps ~cctx ~cm_kind ~ml_kind ~mode m + in let other_cm_files = let dep_graph = Ml_kind.Dict.get (Compilation_context.dep_graphs cctx) ml_kind in let module_deps = Dep_graph.deps_of dep_graph m in @@ -407,8 +469,7 @@ let build_cm ; cmt_args ; cms_args ; Command.Args.S obj_dirs - ; Command.Args.as_any - (Lib_mode.Cm_kind.Map.get (Compilation_context.includes cctx) cm_kind) + ; Command.Args.Dyn lib_cm_args ; extra_args ; As as_parameter_arg ; as_argument_for @@ -524,6 +585,9 @@ let ocamlc_i ~deps cctx (m : Module.t) ~output = List.concat_map deps ~f:(fun m -> [ Path.build (Obj_dir.Module.cm_file_exn obj_dir m ~kind:(Ocaml Cmi)) ])) in + let lib_cm_args = + lib_cm_deps ~cctx ~cm_kind:(Ocaml Cmo) ~ml_kind:Impl ~mode:(Ocaml Byte) m + in let ocaml_flags = Ocaml_flags.get (Compilation_context.flags cctx) (Ocaml Byte) in let modules = Compilation_context.modules cctx in let ocaml = Compilation_context.ocaml cctx in @@ -541,10 +605,7 @@ let ocamlc_i ~deps cctx (m : Module.t) ~output = [ Command.Args.dyn ocaml_flags ; A "-I" ; Path (Path.build (Obj_dir.byte_dir obj_dir)) - ; Command.Args.as_any - (Lib_mode.Cm_kind.Map.get - (Compilation_context.includes cctx) - (Ocaml Cmo)) + ; Command.Args.Dyn lib_cm_args ; opens modules m ; A "-short-paths" ; A "-i" From 2b9037d80d9ae8253329440f342c74ec0dca6501 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:12:26 -0700 Subject: [PATCH 04/12] feat: per-module library dependency filter via ocamldep BFS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/dune_rules/compilation_context.ml | 14 ++ src/dune_rules/dep_rules.ml | 10 +- src/dune_rules/dep_rules.mli | 4 + src/dune_rules/module_compilation.ml | 191 ++++++++++++++++-- src/dune_rules/ocaml_flags.ml | 15 ++ src/dune_rules/ocaml_flags.mli | 3 + src/dune_rules/parameterised_rules.ml | 1 + src/dune_rules/virtual_rules.ml | 5 + src/dune_rules/virtual_rules.mli | 1 + .../cross-compilation-ocamlfind.t | 25 +++ .../test-cases/inline-tests/alias-cycle.t | 40 ++-- .../per-module-lib-deps/basic-wrapped.t | 4 +- .../consumer-is-virtual-impl.t | 2 +- .../cross-lib-walk-pre-pp-source.t | 18 +- .../implicit-transitive-deps-false.t | 43 ++-- .../per-module-lib-deps/lib-deps-preserved.t | 17 +- .../lib-to-lib-unwrapped.t | 14 +- .../per-module-lib-deps/lib-to-lib-wrapped.t | 4 +- ...modules-without-implementation-cross-lib.t | 2 +- .../per-module-lib-deps/multiple-libraries.t | 8 +- .../per-module-lib-deps/opaque-mli-change.t | 12 -- .../test-cases/per-module-lib-deps/opaque.t | 4 +- .../sibling-unreferenced-lib.t | 29 ++- .../per-module-lib-deps/single-module-lib.t | 34 +--- .../single-module-unreferenced-lib.t | 20 +- .../singleton-with-requires.t | 61 ++++++ .../per-module-lib-deps/stdlib-modules.t | 4 +- .../transitive-unreferenced-lib.t | 34 +--- .../transitive-unreferenced-module.t | 22 +- .../per-module-lib-deps/transitive.t | 4 +- .../unwrapped-explicit-modules.t | 78 +++++++ .../unwrapped-tight-deps.t | 35 +--- .../per-module-lib-deps/unwrapped.t | 16 +- .../per-module-lib-deps/wrapped-compat.t | 4 +- .../wrapped-internal-leak.t | 53 ++--- .../reporting-of-cycles.t/a/a1/x.ml | 1 + .../reporting-of-cycles.t/a/a2/x.ml | 1 + .../test-cases/reporting-of-cycles.t/b/x.ml | 1 + .../test-cases/strict-package-deps.t | 11 +- .../watching/multiple-errors-output.t/run.t | 12 +- 40 files changed, 581 insertions(+), 276 deletions(-) create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/singleton-with-requires.t create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-explicit-modules.t diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index 18ce63a29a0..6de2be794e4 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -259,6 +259,19 @@ let create let profile = Context.profile context in eval_opaque ocaml profile opaque in + let* has_library_deps = + (* Single-module stanzas still run ocamldep when they have library deps so + the per-module filter can inform the result. *) + let open Resolve.Memo.O in + let+ direct = direct_requires + and+ hidden = hidden_requires in + match direct, hidden with + | [], [] -> false + | _ -> true + in + (* Resolution errors surface later through the normal compilation rules; + assume deps are present here. *) + let has_library_deps = Resolve.peek has_library_deps |> Result.value ~default:true in let+ dep_graphs = Dep_rules.rules ~dir:(Obj_dir.dir obj_dir) @@ -268,6 +281,7 @@ let create ~impl:implements ~modules ~for_ + ~has_library_deps and+ bin_annot = match bin_annot with | Some b -> Memo.return b diff --git a/src/dune_rules/dep_rules.ml b/src/dune_rules/dep_rules.ml index b9e70e4768d..5dc2272211b 100644 --- a/src/dune_rules/dep_rules.ml +++ b/src/dune_rules/dep_rules.ml @@ -504,10 +504,14 @@ let for_module ~obj_dir ~modules ~sandbox ~impl ~dir ~sctx ~for_ module_ = (deps_of ~modules ~transitive_deps ~imported_vlib_deps (Normal module_)) ;; -let rules ~obj_dir ~modules ~sandbox ~impl ~sctx ~dir ~for_ = +let rules ~obj_dir ~modules ~sandbox ~impl ~sctx ~dir ~for_ ~has_library_deps = match Modules.With_vlib.as_singleton modules with - | Some m -> Memo.return (Dep_graph.Ml_kind.dummy m) - | None -> + | Some m when (not has_library_deps) || Compilation_mode.equal for_ Melange -> + (* Single-module stanzas have no intra-stanza deps; the dep graph is only + consumed by the per-module filter in [lib_deps_for_module]. That filter + doesn't activate for Melange (see its [can_filter]) — skip ocamldep. *) + Memo.return (Dep_graph.Ml_kind.dummy m) + | Some _ | None -> let transitive_deps, imported_vlib_deps = make_transitive_deps ~obj_dir ~modules ~sandbox ~impl ~dir ~sctx ~for_ in diff --git a/src/dune_rules/dep_rules.mli b/src/dune_rules/dep_rules.mli index 8e1324d6050..7390f136291 100644 --- a/src/dune_rules/dep_rules.mli +++ b/src/dune_rules/dep_rules.mli @@ -13,6 +13,9 @@ val for_module -> Module.t -> Module.t list Action_builder.t Ml_kind.Dict.t Memo.t +(** Single-module stanzas short-circuit ocamldep when [not has_library_deps] or + [for_ = Melange]: the per-module filter that consumes the dep graph doesn't + activate in those cases. *) val rules : obj_dir:Path.Build.t Obj_dir.t -> modules:Modules.With_vlib.t @@ -21,6 +24,7 @@ val rules -> sctx:Super_context.t -> dir:Path.Build.t -> for_:Compilation_mode.t + -> has_library_deps:bool -> Dep_graph.Ml_kind.t Memo.t val read_immediate_deps_of diff --git a/src/dune_rules/module_compilation.ml b/src/dune_rules/module_compilation.ml index 3bfd86b54f8..ba8fa04158a 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -8,27 +8,180 @@ let all_libs cctx = d @ h ;; -(* Returns the include flags and lib-file deps for compiling [m]. The scaffold - form is glob-only: include flags come from the cctx's [Includes] (-I/-H over - the full lib list) and deps come from [deps_of_entries] over the same list. - The per-module tight filter activates in a follow-up; arguments unused by - this form are reserved for that filter. *) -let lib_deps_for_module - ~cctx - ~obj_dir:_ - ~for_:_ - ~dep_graph:_ - ~opaque - ~cm_kind - ~ml_kind:(_ : Ml_kind.t) - ~mode:(_ : Lib_mode.t) - _m - = +let union_module_name_sets_mapped xs ~f = + Action_builder.List.map xs ~f + |> Action_builder.map + ~f:(List.fold_left ~init:Module_name.Set.empty ~f:Module_name.Set.union) +;; + +let module_kind_is_filterable m = + match Module.kind m with + | Root | Wrapped_compat | Impl_vmodule | Virtual | Parameter -> false + | Intf_only | Impl | Alias _ -> true +;; + +(* BFS over tight-eligible entries: each (lib, entry) pair's impl+intf ocamldep + names extend the frontier. Non-tight-eligible libs (wrapped locals, + externals, virtual-impls, staged-Pps libs) are skipped by + [lookup_tight_entries], terminating chains through them. The [Module.t] + supplied here is the post-pp form (constructed in [build_lib_index]), so + ocamldep runs on the dep lib's [.pp.ml] (action / non-staged Pps) or directly + on the source (no preprocessing / future-syntax). *) +let cross_lib_tight_set ~sandbox ~sctx ~lib_index ~initial_refs = + let open Action_builder.O in + let read_entry_deps (lib, m) = + let obj_dir = Lib.info lib |> Lib_info.obj_dir |> Obj_dir.as_local_exn in + let* impl_deps = + Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Impl m + in + let+ intf_deps = + Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf m + in + Module_name.Set.union impl_deps intf_deps + in + let rec loop ~seen ~frontier = + if Module_name.Set.is_empty frontier + then Action_builder.return seen + else ( + let pairs = + Module_name.Set.fold frontier ~init:[] ~f:(fun name acc -> + Lib_file_deps.Lib_index.lookup_tight_entries lib_index name @ acc) + in + let* discovered = union_module_name_sets_mapped pairs ~f:read_entry_deps in + let seen = Module_name.Set.union seen frontier in + let frontier = Module_name.Set.diff discovered seen in + loop ~seen ~frontier) + in + loop ~seen:Module_name.Set.empty ~frontier:initial_refs +;; + +(* See the module-level comment in [lib_file_deps.ml] for the two dep shapes + ([deps_of_entry_modules] vs [deps_of_entries]) and why wrapped dep libraries + always take the glob path. *) +let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kind ~mode m = let open Action_builder.O in + let cctx_includes_for_cm_kind () = + Lib_mode.Cm_kind.Map.get (Compilation_context.includes cctx) cm_kind + in + let can_filter = + (match Lib_mode.of_cm_kind cm_kind with + | Melange -> false + | Ocaml _ -> true) + (* Dummy dep graph (alias/root/link-time-synthesised module); cannot supply + transitive deps. *) + && Path.Build.equal (Dep_graph.dir dep_graph) (Obj_dir.dir obj_dir) + (* Modules synthesised outside the stanza, handed to [ocamlc_i]. *) + && Dep_graph.mem dep_graph m + && module_kind_is_filterable m + && Module.has m ~ml_kind + (* Consumer-stanza virtual-impl: handled by [Dep_rules]. The deps-side + counterpart ([has_virtual_impl] below) covers the case where a lib in + [requires] is a virtual impl. *) + && not (Virtual_rules.is_virtual_or_parameter (Compilation_context.implements cctx)) + in let* libs = Resolve.Memo.read (all_libs cctx) in - Action_builder.return - ( Lib_mode.Cm_kind.Map.get (Compilation_context.includes cctx) cm_kind - , Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs ) + if not can_filter + then + Action_builder.return + (cctx_includes_for_cm_kind (), Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs) + else + let* lib_index = Resolve.Memo.read (Compilation_context.lib_index cctx) in + let sandbox = Compilation_context.sandbox cctx in + let sctx = Compilation_context.super_context cctx in + let* trans_deps = Dep_graph.deps_of dep_graph m in + (* Read [dep_m]'s [.ml]-side ocamldep only when its references can + propagate to the consumer: + + | [dep_m] is | [cm_kind] | [opaque] | read [.ml]? | + | ----------------------- | ----------- | -------- | ------------ | + | consumer ([m] itself) | any | any | iff [Impl] | + | trans_dep, no [.mli] | any | any | yes | + | trans_dep, has [.mli] | [Cmx] | false | yes (inline) | + | trans_dep, has [.mli] | [Cmx] | true | no | + | trans_dep, has [.mli] | [Cmi]/[Cmo] | any | no | *) + let need_impl_deps_of dep_m ~is_consumer = + if is_consumer + then ( + match ml_kind with + | Ml_kind.Impl -> true + | Intf -> false) + else + (not (Module.has dep_m ~ml_kind:Intf)) + || + match cm_kind with + | Ocaml Cmx -> not opaque + | Ocaml (Cmi | Cmo) | Melange _ -> false + in + let read_dep_m_raw dep_m ~is_consumer = + let* impl_deps = + if need_impl_deps_of dep_m ~is_consumer + then + Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Impl dep_m + else Action_builder.return Module_name.Set.empty + in + let+ intf_deps = + Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf dep_m + in + Module_name.Set.union impl_deps intf_deps + in + let* m_raw = read_dep_m_raw m ~is_consumer:true in + let* trans_raw = + union_module_name_sets_mapped trans_deps ~f:(read_dep_m_raw ~is_consumer:false) + in + let all_raw = Module_name.Set.union m_raw trans_raw in + let* flags = Ocaml_flags.get (Compilation_context.flags cctx) mode in + let open_modules = Ocaml_flags.extract_open_module_names flags in + let referenced = Module_name.Set.union all_raw open_modules in + let { Lib_file_deps.Lib_index.tight; non_tight } = + Lib_file_deps.Lib_index.filter_libs_with_modules + lib_index + ~referenced_modules:referenced + in + let direct_libs = + List.sort_uniq ~compare:Lib.compare (Lib.Map.keys tight @ Lib.Set.to_list non_tight) + in + (* Close transitively over transparent aliases that ocamldep doesn't + report. *) + let* all_libs = Resolve.Memo.read (Lib.closure direct_libs ~linking:false ~for_) in + let* tight_set = + cross_lib_tight_set ~sandbox ~sctx ~lib_index ~initial_refs:referenced + in + (* Classify each lib in [all_libs]: - lib has [None]-entry referenced + (mixed-entry or wrapped) → glob (covers the None entries' [.cmi]s); - + lib has only [Some] entries referenced → per-module deps; - lib + unreached but tight-eligible → drop (link rule pulls it in via + [requires_link]); - lib unreached and not tight-eligible → glob. + [kept_libs] gets every lib that contributes a tight or glob dep — used + by [Compilation_context.filtered_include_flags] to scope the consumer's + [-I]/[-H] flags. *) + let { Lib_file_deps.Lib_index.tight = tight_modules; non_tight = non_tight_set } = + Lib_file_deps.Lib_index.filter_libs_with_modules + lib_index + ~referenced_modules:tight_set + in + let tight_deps, glob_libs, _kept_libs = + List.fold_left + all_libs + ~init:(Dep.Set.empty, [], Lib.Set.empty) + ~f:(fun (td, gl, kl) lib -> + if Lib.Set.mem non_tight_set lib + then td, lib :: gl, Lib.Set.add kl lib + else ( + match Lib.Map.find tight_modules lib with + | Some modules -> + ( Dep.Set.union + td + (Lib_file_deps.deps_of_entry_modules ~opaque ~cm_kind lib modules) + , gl + , Lib.Set.add kl lib ) + | None -> + if Lib_file_deps.Lib_index.is_tight_eligible lib_index lib + then td, gl, kl + else td, lib :: gl, Lib.Set.add kl lib)) + in + let glob_deps = Lib_file_deps.deps_of_entries ~opaque ~cm_kind glob_libs in + Action_builder.return + (cctx_includes_for_cm_kind (), Dep.Set.union tight_deps glob_deps) ;; let lib_cm_deps ~cctx ~cm_kind ~ml_kind ~mode m = diff --git a/src/dune_rules/ocaml_flags.ml b/src/dune_rules/ocaml_flags.ml index b8c818790c8..56faa95279c 100644 --- a/src/dune_rules/ocaml_flags.ml +++ b/src/dune_rules/ocaml_flags.ml @@ -197,3 +197,18 @@ let allow_only_melange t = let open_flags modules = List.concat_map modules ~f:(fun name -> [ "-open"; Module_name.to_string name ]) ;; + +let extract_open_module_names flags = + let rec loop acc = function + | "-open" :: name :: rest -> + let acc = + match Module_name.of_string_opt name with + | Some m -> Module_name.Set.add acc m + | None -> acc + in + loop acc rest + | _ :: rest -> loop acc rest + | [] -> acc + in + loop Module_name.Set.empty flags +;; diff --git a/src/dune_rules/ocaml_flags.mli b/src/dune_rules/ocaml_flags.mli index c83d8c78cfe..e2dd7648565 100644 --- a/src/dune_rules/ocaml_flags.mli +++ b/src/dune_rules/ocaml_flags.mli @@ -30,3 +30,6 @@ val with_vendored_alerts : t -> t val dump : t -> Dune_lang.t list Action_builder.t val with_vendored_flags : t -> ocaml_version:Version.t -> t val open_flags : Module_name.t list -> string list + +(** Extract module names from [-open Foo] pairs in compiler flags. *) +val extract_open_module_names : string list -> Module_name.Set.t diff --git a/src/dune_rules/parameterised_rules.ml b/src/dune_rules/parameterised_rules.ml index 29ca30bf6f4..74d7489fcdb 100644 --- a/src/dune_rules/parameterised_rules.ml +++ b/src/dune_rules/parameterised_rules.ml @@ -416,6 +416,7 @@ let external_dep_rules ~sctx ~dir ~scope lib_name = ~impl:Virtual_rules.no_implements ~for_ ~modules + ~has_library_deps:true in () ;; diff --git a/src/dune_rules/virtual_rules.ml b/src/dune_rules/virtual_rules.ml index d353ae5b1a1..0bd98e72047 100644 --- a/src/dune_rules/virtual_rules.ml +++ b/src/dune_rules/virtual_rules.ml @@ -11,6 +11,11 @@ type t = let no_implements = No_implements +let is_virtual_or_parameter = function + | Virtual _ | Parameter _ -> true + | No_implements -> false +;; + let setup_copy_rules_for_impl ~sctx ~dir t = match t with | No_implements | Parameter _ -> Memo.return () diff --git a/src/dune_rules/virtual_rules.mli b/src/dune_rules/virtual_rules.mli index a56d595d1de..b59009e4e8d 100644 --- a/src/dune_rules/virtual_rules.mli +++ b/src/dune_rules/virtual_rules.mli @@ -9,6 +9,7 @@ val setup_copy_rules_for_impl -> unit Memo.t val no_implements : t +val is_virtual_or_parameter : t -> bool val impl : Super_context.t diff --git a/test/blackbox-tests/test-cases/custom-cross-compilation/cross-compilation-ocamlfind.t b/test/blackbox-tests/test-cases/custom-cross-compilation/cross-compilation-ocamlfind.t index 792010cdb9a..9da6dbd471c 100644 --- a/test/blackbox-tests/test-cases/custom-cross-compilation/cross-compilation-ocamlfind.t +++ b/test/blackbox-tests/test-cases/custom-cross-compilation/cross-compilation-ocamlfind.t @@ -107,6 +107,31 @@ Dune should be able to find it too "user_cpu_time" ] } + { + "process_args": [ + "-modules", + "-impl", + "repro.ml-gen" + ], + "categories": [], + "prog": "$TESTCASE_ROOT/notocamldep-foo", + "dir": "_build/default.foo", + "exit": 0, + "target_files": [ + "_build/.actions/default.foo/$ACTION" + ], + "rusage": [ + "inblock", + "majflt", + "maxrss", + "minflt", + "nivcsw", + "nvcsw", + "oublock", + "system_cpu_time", + "user_cpu_time" + ] + } Library is built in the target context diff --git a/test/blackbox-tests/test-cases/inline-tests/alias-cycle.t b/test/blackbox-tests/test-cases/inline-tests/alias-cycle.t index 685c8966e67..94b05957ac2 100644 --- a/test/blackbox-tests/test-cases/inline-tests/alias-cycle.t +++ b/test/blackbox-tests/test-cases/inline-tests/alias-cycle.t @@ -1,5 +1,8 @@ -In this test we check a cycle when a library depends on a genrated source file which in -turn depends on the inline-test-name alias of the inline tests of the library. +A library with inline tests depends on a generated source file +(`bar.ml`) whose rule depends on the library's inline-test +runtest alias. Building the library requires `bar.ml`; `bar.ml` +requires the runtest alias; the runtest alias requires the +library — a dependency cycle. $ make_dune_project 3.18 @@ -26,18 +29,27 @@ turn depends on the inline-test-name alias of the inline tests of the library. > (echo "let message = \"Hello world\"")))) > EOF -This kind of cycle has a difficult to understand error message. - $ dune build 2>&1 | grep -vwE "sed" +Dune detects this cycle and emits a "Dependency cycle between:" +error. The walk's node sequence depends on rule-scheduling order +and varies across environments, so the test filters walk nodes +via `dune_cmd delete` and asserts only the invariants: the +error header, the runtest alias node, and exit status `[1]`. + +What this test catches and what it intentionally doesn't: + +| Regression | Caught? | +|--------------------------------------------------|---------| +| No cycle at all | yes | +| Cycle no longer involves runtest-foo_simple | yes | +| Cycle now involves additional aliases | yes | +| Cycle error header format changes | yes | +| Build exits 0 instead of 1 | yes | +| Cycle has different intermediate file/path nodes | no | + +The intermediate-path dimension is rule-scheduling-dependent; +asserting it would require periodic re-promotes that contribute +no meaningful regression coverage. + $ dune build 2>&1 | dune_cmd delete '^(File |\d+ \|| _build|-> _build|-> required by|-> %\{|.*sed: )' Error: Dependency cycle between: - transitive deps of foo_simple__Bar.impl in _build/default - -> _build/default/.foo_simple.objs/byte/foo_simple__Bar.cmi - -> _build/default/.foo_simple.inline-tests/.t.eobjs/native/dune__exe__Main.cmx - -> _build/default/.foo_simple.inline-tests/inline-test-runner.exe -> alias runtest-foo_simple in dune:9 - -> _build/default/bar.ml - -> transitive deps of foo_simple__Bar.impl in _build/default - -> required by _build/default/.foo_simple.objs/byte/foo_simple__Bar.cmo - -> required by _build/default/foo_simple.cma - -> required by alias all - -> required by alias default [1] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/basic-wrapped.t b/test/blackbox-tests/test-cases/per-module-lib-deps/basic-wrapped.t index eb203482193..91ae8e96e4b 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/basic-wrapped.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/basic-wrapped.t @@ -58,8 +58,8 @@ Change mylib's interface: > let new_function () = "hello" > EOF -No_use_lib is recompiled even though it doesn't reference Mylib: +No_use_lib is not recompiled because it doesn't reference Mylib: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("No_use_lib"))] | length' - 2 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/consumer-is-virtual-impl.t b/test/blackbox-tests/test-cases/per-module-lib-deps/consumer-is-virtual-impl.t index c201b556adf..6c85db51004 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/consumer-is-virtual-impl.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/consumer-is-virtual-impl.t @@ -2,7 +2,7 @@ Regression: a stanza with [(implements vlib)] correctly tracks changes to its own [(libraries ...)] deps. For a virtual-library implementation, future per-module -dependency filtering work (#14116) must preserve deps from the +dependency filtering work must preserve deps from the implementation's own [(libraries ...)] closure — otherwise the implementation may fail to rebuild when one of those libraries' interfaces changes. This test checks that an [(implements vlib)] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-walk-pre-pp-source.t b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-walk-pre-pp-source.t index 2bbb9c26970..f793818da21 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-walk-pre-pp-source.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-walk-pre-pp-source.t @@ -83,13 +83,15 @@ this test will fail with a syntax error from [dep/foo.ml]. $ dune build @check -Confirm that [consumer/c.cmi] depends on [dep]'s objdir for cmis, -and not on any path that names the pre-pp source. The dep is -registered as a [*.cmi] glob over the dep's objdir; a future -regression that switched to a per-file dep on the wrong basename -(e.g. [dep/.dep.objs/byte/foo.pp.cmi] instead of [foo.cmi]) would -surface here as a different recorded dep: +Confirm that [consumer/c.cmi] depends specifically on +[dep/.dep.objs/byte/foo.cmi] — the per-module filter records only +the cmis the consumer's ocamldep output names (here, [Foo]), not +a glob over the whole library's objdir, and not [bar.cmi] which +the consumer does not reference. A regression that resurrected +the broad-glob form, or that switched to the wrong basename +(e.g. [foo.pp.cmi] instead of [foo.cmi]), would surface here as a +different recorded dep: $ dune rules --root . --format=json --deps _build/default/consumer/.consumer.objs/byte/c.cmi | - > jq -r 'include "dune"; .[] | depsGlobPredicates' - *.cmi + > jq -r 'include "dune"; .[] | depsFilePaths | select(test("dep/.dep.objs"))' + _build/default/dep/.dep.objs/byte/foo.cmi diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t b/test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t index 6f468f2e5d5..2c31508c419 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t @@ -1,4 +1,4 @@ -Observational baseline: under [(implicit_transitive_deps false)], +Under [(implicit_transitive_deps false)], the consumer [main] uses [intermediate_lib] which declares [link_only_lib] as a transitive dep. [main]'s source never references any module of [link_only_lib]; under @@ -11,22 +11,19 @@ The mode-with-[-H] is only reached on dune lang [3.17+] with an OCaml compiler that supports hidden includes (5.2+); older dune lang versions fall back to mode [Disabled] without [-H]. This test pins [(lang dune 3.23)] below to keep the [-H]-glob path the -one exercised — that is the path the future per-module filter is -expected to tighten. +one exercised — that is the path this PR's per-module filter +tightens. -On trunk today, [main]'s compile rule globs over +Without this PR's filter, [main]'s compile rule globs over [link_only_lib]'s objdir as part of the cctx-wide [-H] glob, so any [.cmi] content change in [link_only_lib] invalidates [main]. -A tighter per-module filter could observe that no -[link_only_lib] entry name reaches [main]'s reference closure -and drop the lib from [main]'s compile-rule deps. +The per-module filter observes that no [link_only_lib] entry +name reaches [main]'s reference closure and drops the lib from +[main]'s compile-rule deps entirely; [main] is no longer +rebuilt. -Reported by @nojb in -https://github.com/ocaml/dune/pull/14116#issuecomment-4323883194 -and again, after a partial fix, in -https://github.com/ocaml/dune/pull/14116#issuecomment-4331209820. -Records [main]'s current rebuild count so a future per-module -filter can flip it to 0. +Reported by @nojb (first as a baseline case, then again after a +partial fix). $ cat > dune-project < (lang dune 3.23) @@ -64,24 +61,10 @@ filter can flip it to 0. $ dune build ./main.exe Empty [link_only_module.ml] so its [.cmi] loses the [val x : int] -binding. The cctx-wide glob over [link_only_lib]'s objdir fires -on the [.cmi] content change, invalidating [main] — both the -[.cmi]/[.cmti] rule and the [.cmx]/[.o] rule re-run: +binding. The per-module filter dropped [link_only_lib] from +[main]'s deps, so no [Main]-named target re-runs: $ echo > link_only_module.ml $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("dune__exe__Main"))]' - [ - { - "target_files": [ - "_build/default/.main.eobjs/byte/dune__exe__Main.cmi", - "_build/default/.main.eobjs/byte/dune__exe__Main.cmti" - ] - }, - { - "target_files": [ - "_build/default/.main.eobjs/native/dune__exe__Main.cmx", - "_build/default/.main.eobjs/native/dune__exe__Main.o" - ] - } - ] + [] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/lib-deps-preserved.t b/test/blackbox-tests/test-cases/per-module-lib-deps/lib-deps-preserved.t index 86c419fc590..6fbe5a43b99 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/lib-deps-preserved.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/lib-deps-preserved.t @@ -1,7 +1,9 @@ -Verify that library file deps are declared for module compilation rules. +Verify the cm_kind/-opaque rules for library file deps in compile rules. -Every non-alias module should declare glob deps on its library -dependencies' .cmi files. +[mylib] is a wrapped library exposing one entry [Mylib]. The +executable has two modules: [Uses_lib] which references [Mylib] in +its [.ml] (but not its [.mli]) and [Main] which references only +[Uses_lib]. [Main] has no [.mli]. $ cat > dune-project < (lang dune 3.23) @@ -33,12 +35,17 @@ dependencies' .cmi files. $ dune build ./main.exe -Both modules declare glob deps on mylib's .cmi files: +[Uses_lib.cmx] keeps the glob: [Uses_lib.ml] references [Mylib], a +wrapped library that falls back to a directory glob over its public +cmi dir. $ dune rules --root . --format=json --deps _build/default/.main.eobjs/native/dune__exe__Uses_lib.cmx | > jq -r 'include "dune"; .[] | depsGlobPredicates' *.cmi +[Main.cmx] has no inter-library deps. Under [-opaque] (the default +profile), [Uses_lib]'s references to [Mylib] are sealed behind +[Uses_lib]'s [.cmi] and don't propagate to [Main]'s native compile. + $ dune rules --root . --format=json --deps _build/default/.main.eobjs/native/dune__exe__Main.cmx | > jq -r 'include "dune"; .[] | depsGlobPredicates' - *.cmi diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-unwrapped.t b/test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-unwrapped.t index 7ca6696af99..b1e9a77851d 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-unwrapped.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-unwrapped.t @@ -1,8 +1,8 @@ -Baseline: library-to-library recompilation (unwrapped). +Per-module library-to-library filtering (unwrapped). When an unwrapped library A depends on an unwrapped library B with multiple -modules, and one module in B changes, all modules in A are recompiled due to -coarse dependency analysis. +modules, modules of A that do not reference a given module of B must not be +recompiled when that module of B changes. See: https://github.com/ocaml/dune/issues/4572 @@ -80,11 +80,11 @@ Change only alpha.mli: > let new_alpha_fn () = "alpha" > EOF -uses_beta is recompiled even though it only references Beta, not Alpha: +uses_beta references Beta only, not Alpha, so it is not recompiled: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("uses_beta"))] | length' - 2 + 0 Change only beta.mli: @@ -97,8 +97,8 @@ Change only beta.mli: > let new_beta_fn () = "beta" > EOF -uses_alpha is recompiled even though it only references Alpha, not Beta: +uses_alpha references Alpha only, not Beta, so it is not recompiled: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("uses_alpha"))] | length' - 2 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-wrapped.t b/test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-wrapped.t index 72cbb718d9f..bc690d0beeb 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-wrapped.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/lib-to-lib-wrapped.t @@ -62,8 +62,8 @@ See: https://github.com/ocaml/dune/issues/4572 > let new_base_fn () = "hello" > EOF -Standalone in middle_lib is recompiled even though it doesn't use base_lib: +Standalone in middle_lib is not recompiled because it doesn't use base_lib: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Standalone"))] | length' - 2 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/modules-without-implementation-cross-lib.t b/test/blackbox-tests/test-cases/per-module-lib-deps/modules-without-implementation-cross-lib.t index 2f9e8c93121..e99a3b43067 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/modules-without-implementation-cross-lib.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/modules-without-implementation-cross-lib.t @@ -5,7 +5,7 @@ lib's interface changes, the consumer rebuilds without the compiler complaining about inconsistent assumptions over interface. -Reported by @art-w on #14116. The concern was that +Reported by @art-w. The concern was that [(modules_without_implementation)] entries' aliases to other libs might escape the per-module dep filter (the intra-stanza [trans_deps] graph skips them), leaving the consumer's compile diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/multiple-libraries.t b/test/blackbox-tests/test-cases/per-module-lib-deps/multiple-libraries.t index da2810d728b..84683c7bd2e 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/multiple-libraries.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/multiple-libraries.t @@ -78,11 +78,11 @@ Change only mylib's interface: > let new_function () = "hello" > EOF -Uses_other is recompiled even though it only uses Otherlib, not Mylib: +Uses_other is not recompiled because it only uses Otherlib, not Mylib: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Uses_other"))] | length' - 2 + 0 Change only otherlib's interface: @@ -95,8 +95,8 @@ Change only otherlib's interface: > let new_other_fn s = s ^ "!" > EOF -Uses_lib is recompiled even though it only uses Mylib, not Otherlib: +Uses_lib is not recompiled because it only uses Mylib, not Otherlib: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Uses_lib"))] | length' - 2 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/opaque-mli-change.t b/test/blackbox-tests/test-cases/per-module-lib-deps/opaque-mli-change.t index 0468d615b62..3e47725476a 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/opaque-mli-change.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/opaque-mli-change.t @@ -51,12 +51,6 @@ left unexported, which would trip warning 32 under dev): $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("dune__exe__Main"))]' [ - { - "target_files": [ - "_build/default/.main.eobjs/byte/dune__exe__Main.cmi", - "_build/default/.main.eobjs/byte/dune__exe__Main.cmti" - ] - }, { "target_files": [ "_build/default/.main.eobjs/native/dune__exe__Main.cmx", @@ -90,12 +84,6 @@ Add another paired declaration: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("dune__exe__Main"))]' [ - { - "target_files": [ - "_build/default/.main.eobjs/byte/dune__exe__Main.cmi", - "_build/default/.main.eobjs/byte/dune__exe__Main.cmti" - ] - }, { "target_files": [ "_build/default/.main.eobjs/native/dune__exe__Main.cmx", diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/opaque.t b/test/blackbox-tests/test-cases/per-module-lib-deps/opaque.t index 17507352124..741aa8e25c7 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/opaque.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/opaque.t @@ -62,11 +62,11 @@ Change ONLY the implementation (not the interface): > let value = 43 > EOF -No_use_lib is recompiled even though it doesn't reference Mylib: +No_use_lib is not recompiled because it doesn't reference Mylib: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("No_use_lib"))] | length' - 1 + 0 --- Dev profile (opaque=true): .cmx deps are NOT tracked for local libs --- diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/sibling-unreferenced-lib.t b/test/blackbox-tests/test-cases/per-module-lib-deps/sibling-unreferenced-lib.t index 56874d5ec51..fb46248a83c 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/sibling-unreferenced-lib.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/sibling-unreferenced-lib.t @@ -7,23 +7,21 @@ of the dependency library [dep_lib] has its interface edited. [consumer_lib] is an unwrapped library with two modules. [consumes_dep] references [Referenced_dep]; [spurious_rebuild] -references nothing from [dep_lib]. On current main, editing -[referenced_dep]'s interface rebuilds [spurious_rebuild] even -though it references nothing from [dep_lib]: the consumer depends -on a glob over [dep_lib]'s object directory, which is invalidated -by the cmi change. +references nothing from [dep_lib]. The per-module filter drops +[dep_lib] from [spurious_rebuild]'s compile-rule deps because +[spurious_rebuild]'s ocamldep output names no module of [dep_lib], +so editing [referenced_dep]'s interface fires no +[spurious_rebuild]-named rule. The zero-reference-sibling-in-a-library corner is distinct from scenarios covered by existing tests: [lib-to-lib-unwrapped.t] probes siblings that reference a different (non-edited) module of the dep; [transitive.t] and [unwrapped.t] probe zero-reference modules but -within executable stanzas, not library stanzas. A future fix -implementing library-level dep filtering at consumer-module -granularity would drop [dep_lib] entirely from [spurious_rebuild]'s -deps, tightening this corner to zero rebuilds. +within executable stanzas, not library stanzas. Previously the +consumer fell back to a glob over [dep_lib]'s objdir, which the +cmi change invalidated. See: https://github.com/ocaml/dune/issues/4572 -See: https://github.com/ocaml/dune/pull/14116#issuecomment-4301275263 $ cat > dune-project < (lang dune 3.23) @@ -59,9 +57,10 @@ See: https://github.com/ocaml/dune/pull/14116#issuecomment-4301275263 $ dune build @check -Edit [referenced_dep]'s interface. [spurious_rebuild] references -nothing in [dep_lib]. Record the number of [spurious_rebuild] -rebuild targets observed in the trace: +Edit [referenced_dep]'s interface (add a binding to both [.mli] +and [.ml] so the [.cmi] content actually changes). [spurious_rebuild] +references nothing from [dep_lib], so its compile rule should not +fire — no rule with a [spurious_rebuild]-named target re-runs: $ cat > referenced_dep.mli < val x : int @@ -72,5 +71,5 @@ rebuild targets observed in the trace: > let y = "hello" > EOF $ dune build @check - $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("spurious_rebuild"))] | length' - 1 + $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("spurious_rebuild"))]' + [] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/single-module-lib.t b/test/blackbox-tests/test-cases/per-module-lib-deps/single-module-lib.t index fde4d0693a8..c608c55a858 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/single-module-lib.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/single-module-lib.t @@ -1,11 +1,10 @@ -Single-module library consumers and recompilation behavior. +Per-module filtering for single-module library consumers. -When a consumer library has only one module, dune skips ocamldep for that -stanza as an optimization (no intra-stanza deps to compute). This means -per-module library dependency filtering cannot determine which libraries -the module actually references, so the consumer depends on all library -files via glob. Modifying an unused module in a dependency triggers -unnecessary recompilation of the consumer module's .cmo/.cmx. +A consumer library with a single module still runs ocamldep when it has +library dependencies, and the per-module filter uses that output to +dep on only the specific entry modules of an unwrapped dependency that +the consumer actually references. Modifying an unreferenced module of +the dependency does not recompile the consumer. See: https://github.com/ocaml/dune/issues/4572 @@ -61,25 +60,8 @@ Modify only the unused module: > let new_fn () = "new" > EOF -uses_alpha.cmx is recompiled even though uses_alpha.ml only references -Alpha, not Unused. This is because the consumer_lib stanza has a single -module, so dune skips ocamldep for it and falls back to glob deps on all -of base_lib's .cmi files. The trace shows compilation targets for -uses_alpha being rebuilt: +uses_alpha.ml references Alpha only, not Unused, so it is not recompiled: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("uses_alpha"))]' - [ - { - "target_files": [ - "_build/default/consumer_lib/.consumer_lib.objs/byte/uses_alpha.cmi", - "_build/default/consumer_lib/.consumer_lib.objs/byte/uses_alpha.cmti" - ] - }, - { - "target_files": [ - "_build/default/consumer_lib/.consumer_lib.objs/native/uses_alpha.cmx", - "_build/default/consumer_lib/.consumer_lib.objs/native/uses_alpha.o" - ] - } - ] + [] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/single-module-unreferenced-lib.t b/test/blackbox-tests/test-cases/per-module-lib-deps/single-module-unreferenced-lib.t index 2745df4719a..510de45c2db 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/single-module-unreferenced-lib.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/single-module-unreferenced-lib.t @@ -8,22 +8,18 @@ edited. [consumer_lib] declares [(libraries dep_lib)] but [spurious_rebuild.ml] does not reference any module of [dep_lib]. -On current main this scenario still rebuilds [spurious_rebuild]: -[consumer_lib] is a single-module stanza, so dune skips ocamldep -as an optimisation and cannot discover that [spurious_rebuild] -references no module of [dep_lib]; the consumer falls back to a -glob over [dep_lib]'s object directory, which is invalidated by -the cmi change. +The per-module filter drops [dep_lib] from [spurious_rebuild]'s +compile-rule deps for this single-module consumer, so editing +[dep_module]'s interface fires no [spurious_rebuild]-named rule. The zero-reference case is a distinct corner from the single-module consumer that references some (but not all) modules of its dep, -which [single-module-lib.t] already documents. A future fix that -detects "ocamldep yields no references to dep_lib" could tighten -this corner to zero rebuilds without needing to solve the broader -single-module-consumer skip-ocamldep limitation. +which [single-module-lib.t] already documents. Previously this +overrebuilt because dune skipped ocamldep for single-module +stanzas as an optimisation and so fell back to a glob over +[dep_lib]'s objdir, which the cmi change invalidated. See: https://github.com/ocaml/dune/issues/4572 -See: https://github.com/ocaml/dune/pull/14116#issuecomment-4286949811 $ cat > dune-project < (lang dune 3.23) @@ -64,4 +60,4 @@ rebuild targets observed in the trace: > EOF $ dune build @check $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("spurious_rebuild"))] | length' - 1 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/singleton-with-requires.t b/test/blackbox-tests/test-cases/per-module-lib-deps/singleton-with-requires.t new file mode 100644 index 00000000000..7b89b387784 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/singleton-with-requires.t @@ -0,0 +1,61 @@ +A multi-module consumer of a [(wrapped false)] singleton dep that +itself has [(libraries leaf)] builds. The cross-library walker +propagates through the singleton via ocamldep on its module so +[leaf]'s cmi stays reachable from the consumer's compile. + +Sister to [no-ocamldep-leaf-lib.t]: that test covers the +singleton-with-no-requires (walker-terminal) case. This one covers +the singleton-with-requires case, where the walker must run +ocamldep on the singleton's module to discover its transitive refs. + + $ cat > dune-project < (lang dune 3.23) + > EOF + +[leaf]: transitive dep reached only through [bridge]'s ocamldep +output. The consumer never names [Mod_leaf] directly. + + $ mkdir leaf + $ cat > leaf/dune < (library (name leaf) (wrapped false)) + > EOF + $ cat > leaf/mod_leaf.ml < type t = int + > let v : t = 1 + > EOF + $ cat > leaf/mod_leaf.mli < type t = int + > val v : t + > EOF + +[bridge]: [(wrapped false)] singleton with [(libraries leaf)]. Its +interface exposes [Mod_leaf.t], so a consumer reading +[Mod_bridge.cmi] also needs [Mod_leaf.cmi] on the include path. + + $ mkdir bridge + $ cat > bridge/dune < (library (name bridge) (wrapped false) (libraries leaf)) + > EOF + $ cat > bridge/mod_bridge.ml < let v : Mod_leaf.t = Mod_leaf.v + > EOF + $ cat > bridge/mod_bridge.mli < val v : Mod_leaf.t + > EOF + +[consumer]: multi-module ([can_filter = true]). Only [uses_bridge] +names [Mod_bridge]; [leaf] reaches the consumer's compile scope +only via the BFS walker stepping through [bridge]. + + $ mkdir consumer + $ cat > consumer/dune < (library (name consumer) (wrapped false) (libraries bridge)) + > EOF + $ cat > consumer/uses_bridge.ml < let _ = Mod_bridge.v + > EOF + $ cat > consumer/sibling.ml < let _ = () + > EOF + + $ dune build @check diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/stdlib-modules.t b/test/blackbox-tests/test-cases/per-module-lib-deps/stdlib-modules.t index 81bc097ecb7..997676c152e 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/stdlib-modules.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/stdlib-modules.t @@ -55,8 +55,8 @@ See: https://github.com/ocaml/dune/issues/4572 > let new_function () = "hello" > EOF -Uses_stdlib is recompiled even though it only uses Printf, not Mylib: +Uses_stdlib is not recompiled because it only uses Printf, not Mylib: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Uses_stdlib"))] | length' - 2 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-lib.t b/test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-lib.t index d341f2da9b8..831ff87c94b 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-lib.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-lib.t @@ -4,15 +4,13 @@ any of [dep_lib]'s modules. The consumer [main] uses [intermediate_lib] and so transitively gains [dep_lib] in its compilation context. -Today every consumer module declares a glob over each transitively- -reached library's public-cmi directory, so editing -[unreferenced_dep.ml] (which no source file references) re- -invalidates [Main]. The test records the current rebuild targets -for [Main] when [unreferenced_dep.ml] is touched. - -This test is observational: a tighter dependency tracker that drops -unreferenced libraries from compile rules' deps would shrink the -recorded target list. +The per-module filter detects that no consumer module references +any [dep_lib] module — directly or transitively through +[intermediate_lib] — and drops [dep_lib] entirely from [Main]'s +compile-rule deps, so editing [unreferenced_dep.ml] leaves [Main] +untouched. Previously every consumer module declared a glob over +each transitively-reached library's public-cmi directory, so the +edit re-invalidated [Main]. [intermediate_lib] and the [main] executable each include an unused dummy module ([intermediate_dummy] / [main_dummy]) so that ocamldep @@ -76,23 +74,9 @@ references any module of [dep_lib]: Edit [unreferenced_dep.ml]. Neither [main.ml] nor [intermediate_module.ml] references [Unreferenced_dep] or any -other [dep_lib] module, so a tighter filter could leave [Main] -untouched. Today [Main] is rebuilt: +other [dep_lib] module, so the filter leaves [Main] untouched: $ echo > unreferenced_dep.ml $ dune build @check $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("dune__exe__Main\\."))]' - [ - { - "target_files": [ - "_build/default/.main.eobjs/byte/dune__exe__Main.cmi", - "_build/default/.main.eobjs/byte/dune__exe__Main.cmti" - ] - }, - { - "target_files": [ - "_build/default/.main.eobjs/byte/dune__exe__Main.cmo", - "_build/default/.main.eobjs/byte/dune__exe__Main.cmt" - ] - } - ] + [] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-module.t b/test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-module.t index ee85a682d41..83b2a49455c 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-module.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/transitive-unreferenced-module.t @@ -1,8 +1,9 @@ A consumer transitively depends upon a library through one module of an intermediate library, never naming a sibling module of the transitive lib in -source. The current but undesirable behaviour is that the consumer rebuilds -when any module of the transitively depended upon library changes, even -unreferenced ones. +source. The per-module filter walks ocamldep output across library +boundaries: editing [unreached_module] leaves [main] untouched because no +source in this test references it. Previously this overrebuilt because +the consumer's compile rule depended on a glob over [dep_lib]'s objdir. [intermediate_lib] and [main] each include an unused dummy module so neither stanza is single-module — single-module stanzas take a fast path that would @@ -58,17 +59,4 @@ test references — and record the rebuild list for [main]: > EOF $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("dune__exe__Main\\."))]' - [ - { - "target_files": [ - "_build/default/.main.eobjs/byte/dune__exe__Main.cmi", - "_build/default/.main.eobjs/byte/dune__exe__Main.cmti" - ] - }, - { - "target_files": [ - "_build/default/.main.eobjs/native/dune__exe__Main.cmx", - "_build/default/.main.eobjs/native/dune__exe__Main.o" - ] - } - ] + [] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/transitive.t b/test/blackbox-tests/test-cases/per-module-lib-deps/transitive.t index a673b3119f9..4b69eab38dc 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/transitive.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/transitive.t @@ -73,8 +73,8 @@ Change libB's interface: > let new_base_fn () = "new" > EOF -Independent is recompiled even though it doesn't reference libA or libB: +Independent is not recompiled because it doesn't reference libA or libB: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Independent"))] | length' - 2 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-explicit-modules.t b/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-explicit-modules.t new file mode 100644 index 00000000000..faffff1b126 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-explicit-modules.t @@ -0,0 +1,78 @@ +Per-module filtering on a `(wrapped false)` library whose module set +is declared explicitly via `(modules ...)`. Matches the exact shape +of the example raised in #14492's review feedback. Behaviour must +match the implicit-discovery form covered by `lib-to-lib-unwrapped.t` +— but the explicit clause routes through a different parse path, so +a separate observational test pins the equivalence empirically rather +than by inspection. + + $ make_dune_project 3.24 + +`foo` is a `(wrapped false)` library with two modules `a`, `b`, +declared explicitly. Each module's `.ml` carries `extra` from the +start so the `.mli` edits below are interface-only. + + $ mkdir foo + $ cat > foo/dune < (library + > (name foo) + > (wrapped false) + > (modules a b)) + > EOF + $ cat > foo/a.ml < let v = 1 + > let extra = "x" + > EOF + $ cat > foo/a.mli < val v : int + > EOF + $ cat > foo/b.ml < let v = 2 + > let extra = "y" + > EOF + $ cat > foo/b.mli < val v : int + > EOF + +`consumer` is a multi-module `(wrapped false)` library depending on +`foo`. `uses_a` references `A` only; `uses_b` references `B` only. +Multi-module so the consumer's compile rules satisfy `can_filter`. + + $ mkdir consumer + $ cat > consumer/dune < (library + > (name consumer) + > (wrapped false) + > (libraries foo)) + > EOF + $ cat > consumer/uses_a.ml < let _ = A.v + > EOF + $ cat > consumer/uses_b.ml < let _ = B.v + > EOF + + $ dune build @check + +Case 1: edit `A`'s interface to expose `extra`. `uses_b` references +only `B`, so the per-module filter must drop `A.cmi` from `uses_b`'s +dep set — `uses_b.cm*` must not rebuild. + + $ cat > foo/a.mli < val v : int + > val extra : string + > EOF + $ dune build @check + $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatching("uses_b.cm")]' + [] + +Case 2: edit `B`'s interface to expose `extra`. `uses_a` references +only `A` — `uses_a.cm*` must not rebuild. + + $ cat > foo/b.mli < val v : int + > val extra : string + > EOF + $ dune build @check + $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatching("uses_a.cm")]' + [] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-tight-deps.t b/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-tight-deps.t index 449f2da858d..412b2af2d43 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-tight-deps.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped-tight-deps.t @@ -7,14 +7,13 @@ the dependency library [dep_lib] has its interface edited. [consumer] references only [Referenced_dep] from [dep_lib]; [Unread_dep_a] and [Unread_dep_b] are present but unreferenced. -On current main, editing any of the three rebuilds [consumer] -because library-level dependency filtering (and, within that, -per-module tightening) is not yet in place: the consumer is -conservatively rebuilt whenever any entry module's cmi changes. -Work on https://github.com/ocaml/dune/issues/4572 is expected to -tighten this, at which point editing [Unread_dep_a] or -[Unread_dep_b] leaves [consumer] untouched and the emitted target -list becomes empty. +The per-module filter restricts [consumer]'s deps to modules of +[dep_lib] that [consumer]'s ocamldep output names — only +[Referenced_dep]. Editing [Unread_dep_a] or [Unread_dep_b] thus +leaves [consumer] untouched (empty target list); editing +[Referenced_dep] still rebuilds it. Previously all three edits +rebuilt [consumer] because the consumer was conservatively +rebuilt whenever any entry module's cmi changed. See: https://github.com/ocaml/dune/issues/4572 @@ -84,15 +83,7 @@ reference — and record the rebuild targets for [consumer]: > EOF $ dune build @check $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("consumer_lib/\\.consumer_lib\\.objs/byte/consumer\\."))]' - [ - { - "target_files": [ - "_build/default/consumer_lib/.consumer_lib.objs/byte/consumer.cmi", - "_build/default/consumer_lib/.consumer_lib.objs/byte/consumer.cmo", - "_build/default/consumer_lib/.consumer_lib.objs/byte/consumer.cmt" - ] - } - ] + [] Same for [Unread_dep_b]: @@ -106,15 +97,7 @@ Same for [Unread_dep_b]: > EOF $ dune build @check $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("consumer_lib/\\.consumer_lib\\.objs/byte/consumer\\."))]' - [ - { - "target_files": [ - "_build/default/consumer_lib/.consumer_lib.objs/byte/consumer.cmi", - "_build/default/consumer_lib/.consumer_lib.objs/byte/consumer.cmo", - "_build/default/consumer_lib/.consumer_lib.objs/byte/consumer.cmt" - ] - } - ] + [] Edit [Referenced_dep]'s interface — the one module [consumer] does reference — and record the rebuild targets ([consumer] must diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped.t b/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped.t index 908ce2e21b8..ea4aa1b697f 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/unwrapped.t @@ -1,8 +1,6 @@ -Baseline: library dependency recompilation for unwrapped libraries. - -When an unwrapped library module's interface changes, Dune currently recompiles -all modules in stanzas that depend on the library, even those referencing -different modules in the library. +Per-module filtering for unwrapped libraries: a consumer that references +only some modules of an unwrapped library must not be recompiled when +unreferenced modules in that library change. See: https://github.com/ocaml/dune/issues/4572 @@ -73,11 +71,11 @@ Change only helper.mli: > let new_helper s = s ^ "!" > EOF -Uses_utils is recompiled even though it only references Utils, not Helper: +Uses_utils references Utils only, not Helper, so it is not recompiled: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Uses_utils"))] | length' - 2 + 0 Change only utils.mli: @@ -90,8 +88,8 @@ Change only utils.mli: > let new_utils s = s ^ "?" > EOF -Uses_helper is recompiled even though it only references Helper, not Utils: +Uses_helper references Helper only, not Utils, so it is not recompiled: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Uses_helper"))] | length' - 2 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-compat.t b/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-compat.t index adc34c6f62b..92fd82b0636 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-compat.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-compat.t @@ -67,8 +67,8 @@ See: https://github.com/ocaml/dune/issues/4572 > let new_fn () = "hello" > EOF -Standalone is recompiled even though it doesn't reference Baselib: +Standalone is not recompiled because it doesn't reference Baselib: $ dune build ./main.exe $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Standalone"))] | length' - 2 + 0 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-internal-leak.t b/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-internal-leak.t index 4b781bb4f5b..4f8e515b320 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-internal-leak.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-internal-leak.t @@ -1,30 +1,28 @@ -Observational baseline for an unsupported leak: a consumer can -currently reach a wrapped library's internal (mangled) module name -through the include path, because the library's object directory -contains the mangled `Mylib__Internal.cmi` and broad `-I` flags make -that directory reachable. +A consumer that writes `Mylib__Internal.x` — using a wrapped +library's mangled internal module name directly, bypassing the +wrapper — no longer compiles. This file asserts the compile +error. -A wrapped library exposes its public API only through its wrapper -module (`Mylib.Internal`, etc). Dune mangles the on-disk cmi of the -wrapped internal module to `Mylib__Internal.cmi` as an implementation -detail. Under broad per-module `-I` flags, a consumer that writes -`Mylib__Internal.x` compiles successfully, side-stepping the wrapper. +Why it fails: per-module rule-dep filtering (#4572) +scopes each consumer's compile-rule deps to the libraries its +source actually references. The consumer's ocamldep output +names `Mylib__Internal` as a top-level module, but +`Mylib__Internal` is not a library entry — the lib index only +knows about `Mylib`. The filter therefore does not list +`Mylib__Internal.cmi` among the consumer's compile-rule deps; +dune does not order the producing rule before the consumer's +compile, and ocamlc reports `Unbound module` because the file +is absent from `mylib`'s objdir when the consumer's compile +runs. -This is not officially supported — consumers should reach a wrapped -library's internals only through the wrapper API, not through the -mangled form (see #14317 review discussion). The leak works today -only as an implementation-detail side effect of the wrapping -convention. In practice some packages currently depend on it, which -is why the baseline is recorded here so any flip is visible in CI. - -Per-module `-I` filtering (#4572 / #14186) is expected to break this -leak by scoping each consumer's `-I` paths to the libraries its -source references. `Mylib__Internal` is not an entry in the lib -index, so the filter excludes `mylib`, the objdir is dropped from -`-I`, and the compile fails with "Unbound module Mylib__Internal". -The flip is acceptable because the leak was never supported; -downstream consumers depending on the mangled form should migrate -to the wrapper API. +Why this isn't a regression: the mangled form was never +officially supported. Wrapped libraries expose their public API +through the wrapper module (`Mylib.Internal.x`); the on-disk +mangling to `Mylib__Internal.cmi` is an implementation detail. +Under broad cctx-wide compile-rule deps the leak previously +compiled by accident; per-module rule-dep filtering removes the +accidental reachability. Downstream consumers depending on the +mangled form should migrate to the wrapper API. $ cat > dune-project < (lang dune 3.0) @@ -49,3 +47,8 @@ to the wrapper API. > EOF $ dune build ./main.exe + File "main.ml", line 1, characters 23-38: + 1 | let () = print_endline Mylib__Internal.hi + ^^^^^^^^^^^^^^^ + Error: Unbound module Mylib__Internal + [1] diff --git a/test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a1/x.ml b/test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a1/x.ml index e69de29bb2d..941c54ea28c 100644 --- a/test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a1/x.ml +++ b/test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a1/x.ml @@ -0,0 +1 @@ +let value = B.X.value diff --git a/test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a2/x.ml b/test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a2/x.ml index e69de29bb2d..1b906314f93 100644 --- a/test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a2/x.ml +++ b/test/blackbox-tests/test-cases/reporting-of-cycles.t/a/a2/x.ml @@ -0,0 +1 @@ +let value = 0 diff --git a/test/blackbox-tests/test-cases/reporting-of-cycles.t/b/x.ml b/test/blackbox-tests/test-cases/reporting-of-cycles.t/b/x.ml index e69de29bb2d..4ca1eb86d04 100644 --- a/test/blackbox-tests/test-cases/reporting-of-cycles.t/b/x.ml +++ b/test/blackbox-tests/test-cases/reporting-of-cycles.t/b/x.ml @@ -0,0 +1 @@ +let value = A2.X.value diff --git a/test/blackbox-tests/test-cases/strict-package-deps.t b/test/blackbox-tests/test-cases/strict-package-deps.t index 2f724b4f08e..abc2a1ecea9 100644 --- a/test/blackbox-tests/test-cases/strict-package-deps.t +++ b/test/blackbox-tests/test-cases/strict-package-deps.t @@ -8,8 +8,8 @@ the package dependencies inferred by dune: > (package (name foo)) > EOF $ mkdir foo bar - $ touch foo/foo.ml - $ touch bar/bar.ml + $ echo 'let () = print_int Bar.value' >foo/foo.ml + $ echo 'let value = 1' >bar/bar.ml $ cat >foo/dune < (executable (public_name foo) (libraries bar) (package foo)) > EOF @@ -26,6 +26,9 @@ the package dependencies inferred by dune: $ cat >foo/dune < (library (public_name foo) (libraries bar)) > EOF + $ cat >foo/foo.ml < let use_bar () = Bar.value + > EOF $ dune build @install Error: Package foo is missing the following package dependencies - bar @@ -45,7 +48,9 @@ transitive deps. > (package (name bar) (depends baz)) > (package (name foo) (depends bar)) > EOF - $ touch baz.ml bar.ml foo.ml + $ echo 'let value = 1' >baz.ml + $ echo 'let chain = Baz.value' >bar.ml + $ echo 'let use = Bar.chain' >foo.ml $ cat >dune < (library (public_name baz) (modules baz)) > (library (public_name bar) (libraries baz) (modules bar)) diff --git a/test/blackbox-tests/test-cases/watching/multiple-errors-output.t/run.t b/test/blackbox-tests/test-cases/watching/multiple-errors-output.t/run.t index 79458d994fb..97fd398c369 100644 --- a/test/blackbox-tests/test-cases/watching/multiple-errors-output.t/run.t +++ b/test/blackbox-tests/test-cases/watching/multiple-errors-output.t/run.t @@ -15,10 +15,18 @@ We test the output of the watch mode client when we have multiple errors 1 | let y = "unknown variable" ^ what ^^^^ Unbound value what - Error: Build failed with 2 errors. + File "$TESTCASE_ROOT/src/foo.ml", line 1, characters 39-40: + 1 | let f = Bar.x + Baz.y + invalid_syntax : ? = ! + ^ + Syntax error + Error: Build failed with 3 errors. [1] $ stop_dune + File "src/foo.ml", line 1, characters 39-40: + 1 | let f = Bar.x + Baz.y + invalid_syntax : ? = ! + ^ + Error: Syntax error File "libs/bar.ml", line 1, characters 8-20: 1 | let y = "type error" + 3 ^^^^^^^^^^^^ @@ -28,4 +36,4 @@ We test the output of the watch mode client when we have multiple errors 1 | let y = "unknown variable" ^ what ^^^^ Error: Unbound value what - Had 2 errors, waiting for filesystem changes... + Had 3 errors, waiting for filesystem changes... From 569666644e92c3faff3e5ee8d8f8aaac30be2f93 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:49:40 -0700 Subject: [PATCH 05/12] fix: soundness recovery for wrapped libs, ppx-runtime, virtual-impl deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- doc/changes/added/14492.md | 7 + src/dune_rules/module_compilation.ml | 234 +++++++++++------- src/dune_rules/modules.ml | 8 +- .../cmx-native-tight-deps.t | 50 ++++ .../ppx/dune | 13 + .../ppx/dune-project | 3 + .../ppx/hello.ml | 1 + .../ppx/hello_ppx.ml | 48 ++++ .../cross-lib-instrumentation-barrier.t/run.t | 67 +++++ .../run.t | 80 ++++++ .../cross-lib-preprocess-barrier.t | 70 ++++++ .../run.t | 72 ++++++ .../mixed-per-module-preprocess.t/run.t | 91 +++++++ 13 files changed, 649 insertions(+), 95 deletions(-) create mode 100644 doc/changes/added/14492.md create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cmx-native-tight-deps.t create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune-project create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello.ml create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello_ppx.ml create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/run.t create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-pps-runtime-no-ocamldep-barrier.t/run.t create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-preprocess-barrier.t create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t/run.t create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t/run.t diff --git a/doc/changes/added/14492.md b/doc/changes/added/14492.md new file mode 100644 index 00000000000..58fa028907e --- /dev/null +++ b/doc/changes/added/14492.md @@ -0,0 +1,7 @@ +- Filter inter-library dependencies per-module using `ocamldep` output, and + match the `-I`/`-H` include flags to that filter. When a dependency + library's cmi changes, only consumer modules that reference it get rebuilt + — eliminating spurious recompilations of unrelated sibling modules. + Applies to unwrapped dependency libraries; wrapped libraries continue to + use a glob over the whole library's objdir. (#14492, fixes #4572, + @robinbb) diff --git a/src/dune_rules/module_compilation.ml b/src/dune_rules/module_compilation.ml index ba8fa04158a..501cd31d955 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -85,103 +85,149 @@ let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kin Action_builder.return (cctx_includes_for_cm_kind (), Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs) else - let* lib_index = Resolve.Memo.read (Compilation_context.lib_index cctx) in - let sandbox = Compilation_context.sandbox cctx in - let sctx = Compilation_context.super_context cctx in - let* trans_deps = Dep_graph.deps_of dep_graph m in - (* Read [dep_m]'s [.ml]-side ocamldep only when its references can - propagate to the consumer: - - | [dep_m] is | [cm_kind] | [opaque] | read [.ml]? | - | ----------------------- | ----------- | -------- | ------------ | - | consumer ([m] itself) | any | any | iff [Impl] | - | trans_dep, no [.mli] | any | any | yes | - | trans_dep, has [.mli] | [Cmx] | false | yes (inline) | - | trans_dep, has [.mli] | [Cmx] | true | no | - | trans_dep, has [.mli] | [Cmi]/[Cmo] | any | no | *) - let need_impl_deps_of dep_m ~is_consumer = - if is_consumer - then ( - match ml_kind with - | Ml_kind.Impl -> true - | Intf -> false) - else - (not (Module.has dep_m ~ml_kind:Intf)) - || - match cm_kind with - | Ocaml Cmx -> not opaque - | Ocaml (Cmi | Cmo) | Melange _ -> false + let* has_virtual_impl = + Resolve.Memo.read (Compilation_context.has_virtual_impl cctx) in - let read_dep_m_raw dep_m ~is_consumer = - let* impl_deps = - if need_impl_deps_of dep_m ~is_consumer - then - Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Impl dep_m - else Action_builder.return Module_name.Set.empty + if has_virtual_impl + then + Action_builder.return + (cctx_includes_for_cm_kind (), Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs) + else + let* lib_index = Resolve.Memo.read (Compilation_context.lib_index cctx) in + let sandbox = Compilation_context.sandbox cctx in + let sctx = Compilation_context.super_context cctx in + let* trans_deps = Dep_graph.deps_of dep_graph m in + (* Read [dep_m]'s [.ml]-side ocamldep only when its references can + propagate to the consumer: + + | [dep_m] is | [cm_kind] | [opaque] | read [.ml]? | + | ----------------------- | ----------- | -------- | ------------ | + | consumer ([m] itself) | any | any | iff [Impl] | + | trans_dep, no [.mli] | any | any | yes | + | trans_dep, has [.mli] | [Cmx] | false | yes (inline) | + | trans_dep, has [.mli] | [Cmx] | true | no | + | trans_dep, has [.mli] | [Cmi]/[Cmo] | any | no | *) + let need_impl_deps_of dep_m ~is_consumer = + if is_consumer + then ( + match ml_kind with + | Ml_kind.Impl -> true + | Intf -> false) + else + (not (Module.has dep_m ~ml_kind:Intf)) + || + match cm_kind with + | Ocaml Cmx -> not opaque + | Ocaml (Cmi | Cmo) | Melange _ -> false in - let+ intf_deps = - Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf dep_m + let read_dep_m_raw dep_m ~is_consumer = + let* impl_deps = + if need_impl_deps_of dep_m ~is_consumer + then + Ocamldep.read_immediate_deps_raw_of + ~sandbox + ~sctx + ~obj_dir + ~ml_kind:Impl + dep_m + else Action_builder.return Module_name.Set.empty + in + let+ intf_deps = + Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf dep_m + in + Module_name.Set.union impl_deps intf_deps in - Module_name.Set.union impl_deps intf_deps - in - let* m_raw = read_dep_m_raw m ~is_consumer:true in - let* trans_raw = - union_module_name_sets_mapped trans_deps ~f:(read_dep_m_raw ~is_consumer:false) - in - let all_raw = Module_name.Set.union m_raw trans_raw in - let* flags = Ocaml_flags.get (Compilation_context.flags cctx) mode in - let open_modules = Ocaml_flags.extract_open_module_names flags in - let referenced = Module_name.Set.union all_raw open_modules in - let { Lib_file_deps.Lib_index.tight; non_tight } = - Lib_file_deps.Lib_index.filter_libs_with_modules - lib_index - ~referenced_modules:referenced - in - let direct_libs = - List.sort_uniq ~compare:Lib.compare (Lib.Map.keys tight @ Lib.Set.to_list non_tight) - in - (* Close transitively over transparent aliases that ocamldep doesn't - report. *) - let* all_libs = Resolve.Memo.read (Lib.closure direct_libs ~linking:false ~for_) in - let* tight_set = - cross_lib_tight_set ~sandbox ~sctx ~lib_index ~initial_refs:referenced - in - (* Classify each lib in [all_libs]: - lib has [None]-entry referenced - (mixed-entry or wrapped) → glob (covers the None entries' [.cmi]s); - - lib has only [Some] entries referenced → per-module deps; - lib - unreached but tight-eligible → drop (link rule pulls it in via - [requires_link]); - lib unreached and not tight-eligible → glob. - [kept_libs] gets every lib that contributes a tight or glob dep — used - by [Compilation_context.filtered_include_flags] to scope the consumer's - [-I]/[-H] flags. *) - let { Lib_file_deps.Lib_index.tight = tight_modules; non_tight = non_tight_set } = - Lib_file_deps.Lib_index.filter_libs_with_modules - lib_index - ~referenced_modules:tight_set - in - let tight_deps, glob_libs, _kept_libs = - List.fold_left - all_libs - ~init:(Dep.Set.empty, [], Lib.Set.empty) - ~f:(fun (td, gl, kl) lib -> - if Lib.Set.mem non_tight_set lib - then td, lib :: gl, Lib.Set.add kl lib - else ( - match Lib.Map.find tight_modules lib with - | Some modules -> - ( Dep.Set.union - td - (Lib_file_deps.deps_of_entry_modules ~opaque ~cm_kind lib modules) - , gl - , Lib.Set.add kl lib ) - | None -> - if Lib_file_deps.Lib_index.is_tight_eligible lib_index lib - then td, gl, kl - else td, lib :: gl, Lib.Set.add kl lib)) - in - let glob_deps = Lib_file_deps.deps_of_entries ~opaque ~cm_kind glob_libs in - Action_builder.return - (cctx_includes_for_cm_kind (), Dep.Set.union tight_deps glob_deps) + let* m_raw = read_dep_m_raw m ~is_consumer:true in + let* trans_raw = + union_module_name_sets_mapped trans_deps ~f:(read_dep_m_raw ~is_consumer:false) + in + let all_raw = Module_name.Set.union m_raw trans_raw in + let* flags = Ocaml_flags.get (Compilation_context.flags cctx) mode in + let open_modules = Ocaml_flags.extract_open_module_names flags in + let referenced = Module_name.Set.union all_raw open_modules in + let { Lib_file_deps.Lib_index.tight; non_tight } = + Lib_file_deps.Lib_index.filter_libs_with_modules + lib_index + ~referenced_modules:referenced + in + (* [ppx_runtime_libraries] introduce module references through post-pp + source that ocamldep cannot see; carry them through to [all_libs] so + the classification fold sees them, and force them onto the glob path + via [must_glob_set]. *) + let* pps_runtime_libs = + Resolve.Memo.read (Compilation_context.pps_runtime_libs cctx) + in + (* [Lib.closure]'s memo key is order- and multiplicity-sensitive on the + input list. [pps_runtime_libs] can both contain duplicates (multiple + pps sharing a runtime dep) and overlap with [tight]/[non_tight] (a lib + referenced both syntactically and via [add_pp_runtime_deps]). + [sort_uniq] keeps the input canonical for memoization. *) + let direct_libs = + List.sort_uniq + ~compare:Lib.compare + (Lib.Map.keys tight @ Lib.Set.to_list non_tight @ pps_runtime_libs) + in + (* Close transitively over transparent aliases that ocamldep doesn't + report. *) + let* all_libs = Resolve.Memo.read (Lib.closure direct_libs ~linking:false ~for_) in + (* Wrapped-lib soundness recovery: when [referenced] names a wrapped local + lib's wrapper, the consumer may reach the lib's transitive closure + through aliases the cross-library walk cannot see; glob that closure + unconditionally. *) + let wrapped_referenced = + Lib_file_deps.Lib_index.wrapped_libs_referenced + lib_index + ~referenced_modules:referenced + in + let* must_glob_libs = + Resolve.Memo.read + (Lib.closure + (List.sort_uniq + ~compare:Lib.compare + (Lib.Set.to_list wrapped_referenced @ pps_runtime_libs)) + ~linking:false + ~for_) + in + let must_glob_set = Lib.Set.of_list must_glob_libs in + let* tight_set = + cross_lib_tight_set ~sandbox ~sctx ~lib_index ~initial_refs:referenced + in + (* Classify each lib in [all_libs]: - lib has [None]-entry referenced + (mixed-entry or wrapped) → glob (covers the None entries' [.cmi]s); - + lib has only [Some] entries referenced → per-module deps; - lib + unreached but tight-eligible → drop (link rule pulls it in via + [requires_link]); - lib unreached and not tight-eligible → glob. + [kept_libs] gets every lib that contributes a tight or glob dep — used + by [Compilation_context.filtered_include_flags] to scope the consumer's + [-I]/[-H] flags. *) + let { Lib_file_deps.Lib_index.tight = tight_modules; non_tight = non_tight_set } = + Lib_file_deps.Lib_index.filter_libs_with_modules + lib_index + ~referenced_modules:tight_set + in + let tight_deps, glob_libs, _kept_libs = + List.fold_left + all_libs + ~init:(Dep.Set.empty, [], Lib.Set.empty) + ~f:(fun (td, gl, kl) lib -> + if Lib.Set.mem must_glob_set lib || Lib.Set.mem non_tight_set lib + then td, lib :: gl, Lib.Set.add kl lib + else ( + match Lib.Map.find tight_modules lib with + | Some modules -> + ( Dep.Set.union + td + (Lib_file_deps.deps_of_entry_modules ~opaque ~cm_kind lib modules) + , gl + , Lib.Set.add kl lib ) + | None -> + if Lib_file_deps.Lib_index.is_tight_eligible lib_index lib + then td, gl, kl + else td, lib :: gl, Lib.Set.add kl lib)) + in + let glob_deps = Lib_file_deps.deps_of_entries ~opaque ~cm_kind glob_libs in + Action_builder.return + (cctx_includes_for_cm_kind (), Dep.Set.union tight_deps glob_deps) ;; let lib_cm_deps ~cctx ~cm_kind ~ml_kind ~mode m = diff --git a/src/dune_rules/modules.ml b/src/dune_rules/modules.ml index eed14fdbd33..8b5a6e07914 100644 --- a/src/dune_rules/modules.ml +++ b/src/dune_rules/modules.ml @@ -670,6 +670,12 @@ module Wrapped = struct let lib_interface t = Group.lib_interface t.group + (* Entry modules visible to consumers of a wrapped library: the wrapper + itself, plus any [wrapped_compat] shims (present for + [(wrapped (transition ...))] libraries, which expose bare module names in + addition to qualified ones). *) + let entry_modules t = lib_interface t :: Module_name.Map.values t.wrapped_compat + let fold_user_available { group; toplevel_module; _ } ~init ~f = let init = match toplevel_module with @@ -1011,7 +1017,7 @@ let entry_modules t = | Unwrapped m -> Unwrapped.entry_modules m | Wrapped m -> (* we assume this is never called for implementations *) - [ Wrapped.lib_interface m ]) + Wrapped.entry_modules m) ;; module With_vlib = struct diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cmx-native-tight-deps.t b/test/blackbox-tests/test-cases/per-module-lib-deps/cmx-native-tight-deps.t new file mode 100644 index 00000000000..d8813ca1c1b --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cmx-native-tight-deps.t @@ -0,0 +1,50 @@ +A consumer of a tight-eligible local library, compiled in native +mode under [--profile release], exercises the [want_cmx = true] +branch of [Lib_file_deps.deps_of_entry_modules] — emitting +per-module [.cmx] deps in addition to per-module [.cmi] deps. + +The default [dev] profile sets [opaque = true], which makes +[want_cmx = false] for local libs and the cmx branch is skipped +entirely. Switching to [release] flips [opaque] off, so the +consumer's native compile takes the per-module path with +[cm_kind = Ocaml Cmx] and the function records cmx-file deps. +Building [consumer.cmxa] explicitly forces the native compile, +unlike [@check] which only materialises [.cmi]/[.cmt] artifacts. + + $ cat > dune-project < (lang dune 3.23) + > EOF + +[dep_lib]: an unwrapped tight-eligible library with two modules, +one of them with both [.ml] and [.mli]: + + $ mkdir dep_lib + $ cat > dep_lib/dune < (library (name dep_lib) (wrapped false)) + > EOF + $ cat > dep_lib/m.ml < let v = 1 + > EOF + $ cat > dep_lib/m.mli < val v : int + > EOF + $ cat > dep_lib/n.ml < let _ = () + > EOF + +[consumer]: a library that references [M] from [dep_lib]: + + $ mkdir consumer + $ cat > consumer/dune < (library + > (name consumer) + > (libraries dep_lib)) + > EOF + $ cat > consumer/c.ml < let _ = M.v + > EOF + +Build the consumer's [.cmxa] under release profile, forcing +native compile of every module: + + $ dune build --profile release consumer/consumer.cmxa diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune new file mode 100644 index 00000000000..495828633bd --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune @@ -0,0 +1,13 @@ +(library + (name hello_ppx) + (public_name hello.ppx) + (kind ppx_rewriter) + (ppx_runtime_libraries hello) + (libraries ppxlib) + (modules hello_ppx)) + +(library + (public_name hello) + (modules hello) + (instrumentation.backend + (ppx hello.ppx))) diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune-project b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune-project new file mode 100644 index 00000000000..568df953e22 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/dune-project @@ -0,0 +1,3 @@ +(lang dune 2.7) + +(package (name hello)) diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello.ml b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello.ml new file mode 100644 index 00000000000..7f63d09714f --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello.ml @@ -0,0 +1 @@ +let hello s = print_endline (Printf.sprintf "Hello from %s!" s) diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello_ppx.ml b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello_ppx.ml new file mode 100644 index 00000000000..ffcd0b355a0 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/ppx/hello_ppx.ml @@ -0,0 +1,48 @@ +open Ast_helper + +let place = ref None +let file = ref None + +let read_file () = + match !file with + | None -> "" + | Some s -> + let ic = open_in s in + (match input_line ic with + | exception End_of_file -> + close_in ic; + "" + | s -> + close_in ic; + s) +;; + +let impl str = + let arg = + match !place with + | None -> Exp.ident (Location.mknoloc (Longident.Lident "__MODULE__")) + | Some s -> Exp.constant (Const.string (Printf.sprintf "%s (%s)" s (read_file ()))) + in + Str.eval + (Exp.apply + (Exp.ident + (Location.mknoloc + (Longident.Ldot + ( { txt = Longident.Lident "Hello"; loc = Location.none } + , { txt = "hello"; loc = Location.none } )))) + [ Nolabel, arg ]) + :: str +;; + +let () = + Ppxlib.Driver.add_arg + "-place" + (Arg.String (fun s -> place := Some s)) + ~doc:"PLACE where to say hello from"; + Ppxlib.Driver.add_arg + "-file" + (Arg.String (fun s -> file := Some s)) + ~doc:"Add info from file" +;; + +let () = Ppxlib.Driver.register_transformation_using_ocaml_current_ast ~impl "hello" diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/run.t b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/run.t new file mode 100644 index 00000000000..d7f4d5cf906 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/run.t @@ -0,0 +1,67 @@ +Reproducer for the bug introduced by storing post-pp [Module.t] in +[Lib_index]: a library declares [(instrumentation (backend X))] +without any [(preprocess ...)]. Instrumentation is *disabled* at +build time (no [--instrument-with] flag), so dune does not produce +[.pp.ml] files. But [Lib_info.preprocess] still returns +[Pps { pps = [Instrumentation_backend X]; ... }] (the static +config), and [build_lib_index] currently maps that to +[Module.pped (Module.ml_source m)] — i.e. paths like [foo.pp.ml] — +which dune then can't find a rule for when the cross-library +walker reads ocamldep on those modules. + + $ cat >dune-project < (lang dune 3.0) + > EOF + +[middle] declares an instrumentation backend but no +[(preprocess ...)]. With instrumentation disabled at build time, +no [.pp.ml] files are produced. + + $ mkdir leaf + $ cat > leaf/dune < (library (name leaf) (wrapped false)) + > EOF + $ cat > leaf/leaf.ml < type t = int + > let zero : t = 0 + > EOF + + $ mkdir middle + $ cat > middle/dune < (library + > (name middle) + > (wrapped false) + > (libraries leaf) + > (instrumentation (backend hello))) + > EOF + $ cat > middle/middle.mli < val identity : Leaf.t -> Leaf.t + > EOF + $ cat > middle/middle.ml < let identity x = x + > EOF + + $ mkdir consumer + $ cat > consumer/dune < (executable (name consumer) (libraries middle)) + > EOF + $ cat > consumer/consumer.ml < let _ = Middle.identity 0 + > EOF + +Build without [--instrument-with]: the cross-library walker +should NOT demand [middle.pp.ml] / [middle.pp.mli]. With the +buggy mapping that demands them anyway, dune reports +"No rule found for middle.pp.ml". + + $ dune build consumer/consumer.exe + +The cross-library walker should produce per-module narrow deps +on the consumer's compile rule, not a wide glob over [middle]'s +objdir. Assert no glob entry over [middle]'s objdir. + + $ dune rules --root . --format=json --deps _build/default/consumer/.consumer.eobjs/byte/dune__exe__Consumer.cmo > deps.json + + $ jq -r 'include "dune"; .[] | depsGlobs + > | select(.dir | endswith("middle/.middle.objs/byte")) + > | .dir + " " + .predicate' < deps.json diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-pps-runtime-no-ocamldep-barrier.t/run.t b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-pps-runtime-no-ocamldep-barrier.t/run.t new file mode 100644 index 00000000000..6d70add4c63 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-pps-runtime-no-ocamldep-barrier.t/run.t @@ -0,0 +1,80 @@ +Regression guard for [build_lib_index]'s [no_ocamldep_lib] +classification: it must mirror [Dep_rules.skip_ocamldep]'s +[has_library_deps] gating, which uses the *resolved* requires +(including pps runtime libs added via [add_pp_runtime_deps]), +not the static [(libraries ...)] field. A single-module local +lib with no [(libraries ...)] but with [(preprocess (pps X))] +has non-empty resolved requires (X's runtime libs), so its +ocamldep is run and classifying it as [no_ocamldep] would cause +the cross-library walk to skip its post-pp ocamldep output — +dropping transitive [.cmi] deps from the consumer's compile rule +and breaking sandboxed builds. + + $ cat > dune-project < (lang dune 3.0) + > EOF + +[hello] is the ppx runtime lib (single-module unwrapped, no library +deps of its own — correctly classified as no-ocamldep). It exposes +[Hello.t]: + + $ mkdir hello + $ cat > hello/dune < (library (name hello) (wrapped false)) + > EOF + $ cat > hello/hello.ml < type t = int + > let zero : t = 0 + > EOF + +[hello_ppx] is a no-op ppx_rewriter declaring [hello] as its +[ppx_runtime_libraries]: + + $ mkdir hello_ppx + $ cat > hello_ppx/dune < (library + > (name hello_ppx) + > (kind ppx_rewriter) + > (ppx_runtime_libraries hello) + > (libraries ppxlib)) + > EOF + $ cat > hello_ppx/hello_ppx.ml < let () = + > Ppxlib.Driver.register_transformation_using_ocaml_current_ast + > ~impl:(fun s -> s) "noop" + > EOF + +[middle] is single-module, has no [(libraries ...)], and uses +[(preprocess (pps hello_ppx))]. Its interface mentions [Hello.t]: + + $ mkdir middle + $ cat > middle/dune < (library + > (name middle) + > (wrapped false) + > (preprocess (pps hello_ppx))) + > EOF + $ cat > middle/middle.mli < val helper : Hello.t -> Hello.t + > EOF + $ cat > middle/middle.ml < let helper x = x + > EOF + +[consumer] depends on [middle]; references [Middle.helper] but +never names [Hello] in source: + + $ mkdir consumer + $ cat > consumer/dune < (executable (name consumer) (libraries middle)) + > EOF + $ cat > consumer/consumer.ml < let _ = Middle.helper 0 + > EOF + +Sandbox-forced build of the consumer succeeds: the walker visits +[middle]'s post-pp ocamldep, surfaces [Hello] in the tight set, +and the classification fold keeps [hello]'s [.cmi] as a compile- +rule dep so it is pulled into the sandbox. + + $ dune build --sandbox=copy consumer/consumer.exe diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-preprocess-barrier.t b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-preprocess-barrier.t new file mode 100644 index 00000000000..7b8c4586c49 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-preprocess-barrier.t @@ -0,0 +1,70 @@ +Regression guard for cross-library transitive [.cmi] reads +through a preprocessed intermediate. A consumer references a +module from a preprocessed library; that preprocessed module's +interface mentions a type from a leaf library that the consumer +never names syntactically. The consumer's compile must depend on +the leaf's [.cmi] so the OCaml compiler can resolve the type +reference imported via the intermediate's [.cmi]. + +[build_lib_index] stores each entry's *post-pp* [Module.t] +(mirroring [Pp_spec.pped_modules_map]), so [cross_lib_tight_set] +reads ocamldep on the dep lib's [.pp.ml] artifact. The walker +therefore reaches the leaf's name through the intermediate's +interface, the classification fold places the leaf in the +consumer's tight per-module deps, and forced sandboxing succeeds +because [leaf]'s [.cmi] is in the sandbox. + + $ cat > dune-project < (lang dune 3.23) + > EOF + +[leaf] exposes [Leaf.t]: + + $ mkdir leaf + $ cat > leaf/dune < (library (name leaf) (wrapped false)) + > EOF + $ cat > leaf/leaf.ml < type t = int + > let zero : t = 0 + > EOF + +[middle] depends on [leaf]; its single module is preprocessed via +[(preprocess (action ...))], and its interface mentions [Leaf.t]: + + $ mkdir middle + $ cat > middle/dune < (library + > (name middle) + > (wrapped false) + > (libraries leaf) + > (preprocess (action (run cat %{input-file})))) + > EOF + $ cat > middle/middle.mli < val identity : Leaf.t -> Leaf.t + > EOF + $ cat > middle/middle.ml < let identity x = x + > EOF + +[consumer] depends on [middle]; references [Middle.identity] but +never names [Leaf] in source: + + $ mkdir consumer + $ cat > consumer/dune < (executable (name consumer) (libraries middle)) + > EOF +Applying [Middle.identity] to a literal [0] forces the compiler +to unify [int] with [Leaf.t], which requires loading [Leaf.cmi]. +ocamldep on this source still reports only [Middle] as referenced, +since [Leaf] is not named. + + $ cat > consumer/consumer.ml < let _ = Middle.identity 0 + > EOF + +Sandbox-forced build of the consumer succeeds: [leaf]'s [.cmi] +is declared as a compile-rule dep through the per-module +filter's cross-lib walk and is therefore present in the sandbox. + + $ dune build --sandbox=copy consumer/consumer.exe diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t/run.t b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t/run.t new file mode 100644 index 00000000000..f7e2e590a8a --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t/run.t @@ -0,0 +1,72 @@ +Precision regression guard for the per-pair tight-eligibility +in [Lib_index]: when a consumer references only the [Some]- +entry modules of a mixed-pp lib, the [None]-entry modules must +NOT be pulled into the consumer's compile-rule deps. + +We assert precision by giving [b] (the [None]-entry module) an +unresolvable identifier and leaving [a]'s source independent of +[b]. If the consumer's compile rule listed [b.cmi] as a dep, +dune would try to compile [b.ml] and fail. Building the +consumer's [.cmo] (compile only, no link) succeeds → [b] is +correctly excluded. + + $ cat > dune-project < (lang dune 3.0) + > EOF + +A no-op staged ppx (identical to the soundness reproducer). + + $ mkdir ppx + $ cat > ppx/dune < (library + > (name ppx_noop) + > (kind ppx_rewriter) + > (ppx.driver (main Ppx_noop.main))) + > EOF + $ cat > ppx/ppx_noop.ml < let main () = + > let n = Array.length Sys.argv in + > if n < 2 then assert false; + > let input = Sys.argv.(n - 2) in + > let output = Sys.argv.(n - 1) in + > Filename.quote_command "cp" [input; output] + > |> Sys.command + > |> exit + > EOF + +[mylib]: [a] uses default (Some entry), [b] uses staged_pps +(None entry). [a]'s source is independent of [b]; [b.ml] +contains an unresolvable identifier so any attempt to compile +it will fail. + + $ mkdir mylib + $ cat > mylib/dune < (library + > (name mylib) + > (wrapped false) + > (preprocess (per_module ((staged_pps ppx_noop) b)))) + > EOF + $ cat > mylib/a.ml < let answer = 42 + > EOF + $ cat > mylib/b.ml < let bar = no_such_thing + > EOF + +[consumer] references only [A]: + + $ mkdir consumer + $ cat > consumer/dune < (executable (name consumer) (libraries mylib)) + > EOF + $ cat > consumer/consumer.ml < let () = print_int A.answer + > EOF + +Build only the consumer's [.cmo] (compile rule, not link). If +per-pair tight-eligibility holds, the consumer depends on +[a.cmi] alone and the build succeeds. If the fix regressed and +[mylib] got globbed for the consumer, dune would attempt to +compile [b.ml] and fail. + + $ dune build consumer/.consumer.eobjs/byte/dune__exe__Consumer.cmo diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t/run.t b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t/run.t new file mode 100644 index 00000000000..b0d598d129c --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t/run.t @@ -0,0 +1,91 @@ +Reproducer for the soundness bug Copilot flagged in +[Lib_index.create]: an unwrapped lib's [tight_eligible] +membership is set on "any entry has [Some _]", which under +per-module preprocessing can be true while *some* of the lib's +entries have [m_opt = None]. The classification fold then +silently drops the [None] entries' modules from the consumer's +compile-rule deps when those modules are reached via +[cross_lib_tight_set]'s BFS expansion. + +Setup. [mylib] is unwrapped with two modules. [a] uses default +(no preprocessing) — entry [(a, mylib, Some _)]. [b] uses +[(staged_pps ...)] — entry [(b, mylib, None)], since no +[.pp.ml] is statically known for staged pps. +[a]'s source references [B], so [cross_lib_tight_set]'s BFS +walks from [a]'s ocamldep into [B]. The consumer references +only [A], so [wrapped_libs_referenced] (which is keyed off the +consumer's direct ocamldep, [referenced]) does NOT add [mylib] +to [must_glob_set]. The mixed-entry bug therefore reaches the +classification fold via [tight_set], and the fold drops [B] +because [tight_modules_per_lib] skips its [None] entry. + + $ cat > dune-project < (lang dune 3.0) + > EOF + +A no-op staged ppx, modeled on +test/blackbox-tests/test-cases/staged-pps-relative-directory-gh8158.t. +The driver copies its input verbatim, so the staged stage's +[.pp.ml] equals the input source. + + $ mkdir ppx + $ cat > ppx/dune < (library + > (name ppx_noop) + > (kind ppx_rewriter) + > (ppx.driver (main Ppx_noop.main))) + > EOF + $ cat > ppx/ppx_noop.ml < let main () = + > let n = Array.length Sys.argv in + > if n < 2 then assert false; + > let input = Sys.argv.(n - 2) in + > let output = Sys.argv.(n - 1) in + > Filename.quote_command "cp" [input; output] + > |> Sys.command + > |> exit + > EOF + +[mylib] is unwrapped with two modules; only [b] is staged. +[a]'s interface mentions [B.t], so [cross_lib_tight_set]'s BFS +walks from [a]'s ocamldep into [b]'s name AND the consumer's +own compile genuinely loads [b.cmi] (otherwise the bug wouldn't +manifest at the consumer's compile site). + + $ mkdir mylib + $ cat > mylib/dune < (library + > (name mylib) + > (wrapped false) + > (preprocess (per_module ((staged_pps ppx_noop) b)))) + > EOF + $ cat > mylib/a.mli < val identity : B.t -> B.t + > EOF + $ cat > mylib/a.ml < let identity (x : B.t) = x + > EOF + $ cat > mylib/b.ml < type t = int + > let zero : t = 0 + > EOF + +[consumer] references only [A] in source — never names [B] — +but its call to [A.identity] forces ocamlc to load [B.cmi] to +resolve [B.t]. + + $ mkdir consumer + $ cat > consumer/dune < (executable (name consumer) (libraries mylib)) + > EOF + $ cat > consumer/consumer.ml < let _ = A.identity 0 + > EOF + +Sandboxed build forces the missing-dep bug to surface +deterministically: the sandbox is populated only from the +compile-rule's declared deps, so a missing dep on +[mylib/.mylib.objs/byte/b.cmi] fails the build with a +"no such file" error rather than a silent race. + + $ dune build --sandbox=copy consumer/consumer.exe From 12cd1a94274661ab880512bb3ab23d5828bf700a Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:22:53 -0700 Subject: [PATCH 06/12] feat: per-module [-I]/[-H] include flags via [kept_libs] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Compilation_context.filtered_include_flags]: new function returning the [-I]/[-H] flags restricted to [kept_libs]. The cctx's [requires_compile] and [requires_hidden] are each filtered by [Lib.Set.mem kept_libs]; the result is built as a single [Command.Args.t] under [Action_builder]. No caching yet — each call recomputes; a follow-up adds the cache. [Module_compilation.lib_deps_for_module]: the tight branch was already threading [kept_libs] through the classification fold (it had been unused at L4-L5). Now wired into [filtered_include_flags]; the returned pair is [(filtered_include_flags, tight_deps + glob_deps)] instead of [(cctx_includes_for_cm_kind (), …)]. Behavioural effect: a consumer module's compile command sees [-I] / [-H] only for libraries its ocamldep reference set actually reaches. Adding an unreferenced sibling to the cctx's [(libraries ...)] no longer changes the consumer module's compile command, so the rule does not re-execute. Tests: - [per-module-include-flags.t]: promoted — [-I] for the unreferenced [unrelated_lib] no longer appears in the consumer's compile rule. - [add-unreferenced-sibling-lib.t]: promoted — adding an unreferenced sibling lib produces no rebuild for consumer modules. Signed-off-by: Robin Bate Boerop --- src/dune_rules/compilation_context.ml | 18 +++++++++ src/dune_rules/compilation_context.mli | 11 ++++++ src/dune_rules/module_compilation.ml | 8 ++-- .../add-unreferenced-sibling-lib.t | 39 +++++-------------- .../per-module-include-flags.t | 21 ++++------ 5 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index 6de2be794e4..defe044df9d 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -340,6 +340,24 @@ let create let for_ t = t.for_ +let filtered_include_flags t ~cm_kind ~kept_libs = + let lib_mode = Lib_mode.of_cm_kind cm_kind in + let open Action_builder.O in + let* direct_requires = Resolve.Memo.read t.requires_compile in + let+ hidden_requires = Resolve.Memo.read t.requires_hidden in + let direct_filtered = List.filter direct_requires ~f:(Lib.Set.mem kept_libs) in + let hidden_filtered = List.filter hidden_requires ~f:(Lib.Set.mem kept_libs) in + let project = Scope.project t.scope in + let lib_config = t.ocaml.lib_config in + Lib_flags.L.include_flags + ~project + ~direct_libs:direct_filtered + ~hidden_libs:hidden_filtered + lib_mode + lib_config + |> Command.Args.memo +;; + let alias_and_root_module_flags = let extra = [ "-w"; "-49" ] in fun base -> Ocaml_flags.append_common base extra |> Ocaml_flags.append_nostdlib diff --git a/src/dune_rules/compilation_context.mli b/src/dune_rules/compilation_context.mli index c917486e0cf..1075762651d 100644 --- a/src/dune_rules/compilation_context.mli +++ b/src/dune_rules/compilation_context.mli @@ -63,6 +63,17 @@ val requires_hidden : t -> Lib.t list Resolve.Memo.t val requires_compile : t -> Lib.t list Resolve.Memo.t val parameters : t -> Module_name.t list Resolve.Memo.t val includes : t -> Command.Args.without_targets Command.Args.t Lib_mode.Cm_kind.Map.t + +(** Include flags ([-I]/[-H]) for compiling a module against [kept_libs]. The + cctx's [requires_compile] and [requires_hidden] are each restricted to + libraries in [kept_libs]; the kept direct entries become [-I], the kept + hidden entries become [-H]. *) +val filtered_include_flags + : t + -> cm_kind:Lib_mode.Cm_kind.t + -> kept_libs:Lib.Set.t + -> Command.Args.without_targets Command.Args.t Action_builder.t + val lib_index : t -> Lib_file_deps.Lib_index.t Resolve.Memo.t (** [true] iff any library in the compilation context's direct or hidden diff --git a/src/dune_rules/module_compilation.ml b/src/dune_rules/module_compilation.ml index 501cd31d955..109b586ca0a 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -205,7 +205,7 @@ let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kin lib_index ~referenced_modules:tight_set in - let tight_deps, glob_libs, _kept_libs = + let tight_deps, glob_libs, kept_libs = List.fold_left all_libs ~init:(Dep.Set.empty, [], Lib.Set.empty) @@ -226,8 +226,10 @@ let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kin else td, lib :: gl, Lib.Set.add kl lib)) in let glob_deps = Lib_file_deps.deps_of_entries ~opaque ~cm_kind glob_libs in - Action_builder.return - (cctx_includes_for_cm_kind (), Dep.Set.union tight_deps glob_deps) + let+ include_flags = + Compilation_context.filtered_include_flags cctx ~cm_kind ~kept_libs + in + include_flags, Dep.Set.union tight_deps glob_deps ;; let lib_cm_deps ~cctx ~cm_kind ~ml_kind ~mode m = diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t b/test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t index 7ad07e032cb..d0bcc28a18a 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t @@ -1,20 +1,17 @@ -Observational baseline: adding a library to a stanza's -[(libraries ...)] list currently rebuilds every module in the -stanza, including modules that reference nothing from the new -library. The [-I]/[-H] flags on each module's compiler invocation -are derived from the cctx-wide library list rather than from the -module's own ocamldep reference set, so adding an unreferenced -library still changes every consumer's compile-action hash and -invalidates the rule cache. +Adding a library to a stanza's [(libraries ...)] list does not +rebuild stanza modules that reference nothing from the new +library. The per-module [-I]/[-H] include filter narrows each +module's compile-action hash to the libraries its ocamldep +reference set actually reaches, so an unreferenced added lib +leaves every other consumer's cache key unchanged. [consumer_lib] starts with [(libraries existing_dep)] and two modules: [consumes_dep] references [Existing_dep_module]; [unrelated_module] references nothing from [existing_dep] or any other library. After the initial build, [added_lib] is added to the stanza's [(libraries ...)] list. Neither [consumes_dep] nor -[unrelated_module] uses anything from [added_lib], yet both -rebuild. Records the rebuild counts on each so a future change -can flip them to 0. +[unrelated_module] uses anything from [added_lib], so neither +rebuilds — the trace shows empty target lists for both. $ cat > dune-project < (lang dune 3.23) @@ -72,22 +69,6 @@ Add [added_lib] to [consumer_lib]'s [(libraries ...)]. Neither $ dune build @check $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("consumes_dep"))]' - [ - { - "target_files": [ - "_build/default/.consumer_lib.objs/byte/consumes_dep.cmi", - "_build/default/.consumer_lib.objs/byte/consumes_dep.cmo", - "_build/default/.consumer_lib.objs/byte/consumes_dep.cmt" - ] - } - ] + [] $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("unrelated_module"))]' - [ - { - "target_files": [ - "_build/default/.consumer_lib.objs/byte/unrelated_module.cmi", - "_build/default/.consumer_lib.objs/byte/unrelated_module.cmo", - "_build/default/.consumer_lib.objs/byte/unrelated_module.cmt" - ] - } - ] + [] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t b/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t index bb474822490..51eea2c23d1 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t @@ -1,15 +1,12 @@ -Observational baseline: a consumer module's compile command -currently carries [-I] flags for every library in the cctx-wide -[(libraries ...)] closure, regardless of which libraries the -module's source actually references. +A consumer module's compile command carries [-I] flags only for +the libraries its ocamldep reference set actually reaches, not +for every library in the cctx-wide [(libraries ...)] closure. [consumer_lib] declares two library dependencies, [dep_lib] and [unrelated_lib], but its only module references just [Dep_module]. -A future per-module filter could observe via ocamldep that the -module references nothing from [unrelated_lib] and drop that -library's objdir from the [-I] path. This test records today's -[-I] contents so that a future filter improvement can flip the -asserted array to a single entry. +The per-module filter observes via ocamldep that the module +references nothing from [unrelated_lib] and drops that library's +objdir from the [-I] path; only [dep_lib]'s objdir survives. $ cat > dune-project < (lang dune 3.0) @@ -40,12 +37,10 @@ asserted array to a single entry. Inspect the [-I] flags on [consumer_module]'s [.cmo] compile rule. Filter to objdir-shaped paths so we don't print the consumer's own [.consumer_lib.objs/byte] entry — that's always present and -not what this test is about. Today both [dep_lib]'s objdir and -[unrelated_lib]'s objdir appear: +not what this test is about. Only [dep_lib]'s objdir appears: $ dune rules --root . --format=json _build/default/.consumer_lib.objs/byte/consumer_module.cmo \ > | jq 'include "dune"; .[] | [ruleActionFlagValues("-I") | select(test("\\.dep_lib\\.objs|\\.unrelated_lib\\.objs"))]' [ - ".dep_lib.objs/byte", - ".unrelated_lib.objs/byte" + ".dep_lib.objs/byte" ] From 4e57a12926e173a04f50e1ab3d54a1d68f373429 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:25:19 -0700 Subject: [PATCH 07/12] perf: cache filtered include flags per [(lib_mode, kept_libs)] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Compilation_context.Filtered_includes] caches the [Action_builder.t] returned by [filtered_include_flags] keyed on [(lib_mode, kept_libs)]. Two modules in the same cctx that reach the same set of kept libs share one builder; [Action_builder.memoize] dedupes its evaluation. Cache key omits the cctx's [requires_compile] / [requires_hidden] — they're immutable on the cctx from [create]. The [for_module_generated_at_link_time] exception, where derived cctxs could in principle alter the closure, takes [can_filter = false] in [lib_deps_for_module] and so never reaches this function. [Filtered_includes.Key]: [lib_mode] + [kept_libs : Lib.t list] (the caller passes a sorted list via [Lib.Set.to_list], canonicalising for the cache). [equal] and [hash] derived from the same; [Repr]-derived [to_dyn] for diagnostics. [Lib_mode.hash]: new — used by [Filtered_includes.Key.hash]. Three constants for the three variants ([Ocaml Byte], [Ocaml Native], [Melange]). Signed-off-by: Robin Bate Boerop --- src/dune_lang/lib_mode.ml | 6 ++ src/dune_lang/lib_mode.mli | 1 + src/dune_rules/compilation_context.ml | 82 ++++++++++++++++++++++----- 3 files changed, 75 insertions(+), 14 deletions(-) diff --git a/src/dune_lang/lib_mode.ml b/src/dune_lang/lib_mode.ml index 09229cd2fb5..4b3c5c110f3 100644 --- a/src/dune_lang/lib_mode.ml +++ b/src/dune_lang/lib_mode.ml @@ -11,6 +11,12 @@ let equal x y = | Melange, Melange -> true ;; +let hash = function + | Ocaml Byte -> 0 + | Ocaml Native -> 1 + | Melange -> 2 +;; + let decode = let open Decoder in enum [ "byte", Ocaml Byte; "native", Ocaml Native; "melange", Melange ] diff --git a/src/dune_lang/lib_mode.mli b/src/dune_lang/lib_mode.mli index 7975ae286fa..7f69dfa793d 100644 --- a/src/dune_lang/lib_mode.mli +++ b/src/dune_lang/lib_mode.mli @@ -6,6 +6,7 @@ type t = val decode : t Decoder.t val equal : t -> t -> bool +val hash : t -> int val to_dyn : t -> Dyn.t module Cm_kind : sig diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index defe044df9d..7197f307ed1 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -33,6 +33,44 @@ module Includes = struct let empty = Lib_mode.Cm_kind.Map.make_all Command.Args.empty end +(* Key omits [t.requires_compile] / [t.requires_hidden] because they're + immutable on the cctx from [create]. The exception — + [for_module_generated_at_link_time]'s derived cctxs — takes the + [can_filter = false] arm in [lib_deps_for_module] and so never reaches this + cache. *) +module Filtered_includes = struct + module Key = struct + type t = + { lib_mode : Lib_mode.t + ; kept_libs : Lib.Set.t + } + + let equal a b = + Lib_mode.equal a.lib_mode b.lib_mode && Lib.Set.equal a.kept_libs b.kept_libs + ;; + + let hash { lib_mode; kept_libs } = + Lib.Set.fold kept_libs ~init:(Lib_mode.hash lib_mode) ~f:(fun lib acc -> + acc lxor Lib.hash lib) + ;; + + let repr = + Repr.record + "Filtered_includes.Key" + [ Repr.field "lib_mode" (Repr.abstract Lib_mode.to_dyn) ~get:(fun t -> t.lib_mode) + ; Repr.field "kept_libs" (Repr.abstract Lib.Set.to_dyn) ~get:(fun t -> + t.kept_libs) + ] + ;; + + let to_dyn = Repr.to_dyn repr + end + + type t = (Key.t, Command.Args.without_targets Command.Args.t Action_builder.t) Table.t + + let create () : t = Table.create (module Key) 16 +end + type opaque = | Explicit of bool | Inherit_from_settings @@ -81,6 +119,7 @@ type t = ; loc : Loc.t option ; ocaml : Ocaml_toolchain.t ; for_ : Compilation_mode.t + ; filtered_includes : Filtered_includes.t } let loc t = t.loc @@ -335,6 +374,7 @@ let create ; ocaml ; instances ; for_ + ; filtered_includes = Filtered_includes.create () } ;; @@ -342,20 +382,34 @@ let for_ t = t.for_ let filtered_include_flags t ~cm_kind ~kept_libs = let lib_mode = Lib_mode.of_cm_kind cm_kind in - let open Action_builder.O in - let* direct_requires = Resolve.Memo.read t.requires_compile in - let+ hidden_requires = Resolve.Memo.read t.requires_hidden in - let direct_filtered = List.filter direct_requires ~f:(Lib.Set.mem kept_libs) in - let hidden_filtered = List.filter hidden_requires ~f:(Lib.Set.mem kept_libs) in - let project = Scope.project t.scope in - let lib_config = t.ocaml.lib_config in - Lib_flags.L.include_flags - ~project - ~direct_libs:direct_filtered - ~hidden_libs:hidden_filtered - lib_mode - lib_config - |> Command.Args.memo + let cache_key = { Filtered_includes.Key.lib_mode; kept_libs } in + match Table.find t.filtered_includes cache_key with + | Some builder -> builder + | None -> + (* Cache the [Action_builder.t] (not the resolved args) up-front at + rule-construction time so all compile rules sharing this + [(lib_mode, kept_libs)] share one builder; [Action_builder.memoize] then + dedupes its evaluation. Mirrors the cache pattern in + [Ocamldep.read_immediate_deps_words]. *) + let builder = + let open Action_builder.O in + let* direct_requires = Resolve.Memo.read t.requires_compile in + let+ hidden_requires = Resolve.Memo.read t.requires_hidden in + let direct_filtered = List.filter direct_requires ~f:(Lib.Set.mem kept_libs) in + let hidden_filtered = List.filter hidden_requires ~f:(Lib.Set.mem kept_libs) in + let project = Scope.project t.scope in + let lib_config = t.ocaml.lib_config in + Lib_flags.L.include_flags + ~project + ~direct_libs:direct_filtered + ~hidden_libs:hidden_filtered + lib_mode + lib_config + |> Command.Args.memo + in + let builder = Action_builder.memoize "filtered_include_flags" builder in + Table.set t.filtered_includes cache_key builder; + builder ;; let alias_and_root_module_flags = From 73f13b1018abeec610ed5b9462ec30bb4b966f23 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:28:23 -0700 Subject: [PATCH 08/12] perf: cache per-cctx raw ocamldep references by (obj_name, kind) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Compilation_context.Raw_refs] caches the [Action_builder.t] computed for each ocamldep raw-deps read inside a cctx. Two consumer modules that share trans_deps (or a consumer and one of its trans deps that share an [obj_name + ml_kind]) get the same builder. The cache short-circuits before constructing the builder; on hit, no allocation. [Raw_refs.Key] distinguishes the two read patterns the per-module filter uses: [Consumer] (the cctx-driving module's own deps, keyed by [ml_kind]) and [Transitive] (a dep module's deps, keyed by [cm_kind] because the impl/intf gating in [need_impl_deps_of] varies by cm_kind on the [Cmx]/opaque path). Conservatively-distinct keying — never collapse two semantically-different reads under one cache cell. [Compilation_context.cached_raw_refs t ~key ~compute] is the thin public surface: lookup, compute on miss, store, return the builder. [Module_compilation.lib_deps_for_module]: wraps the inline [read_dep_m_raw] body that the BFS uses for both the consumer's own and each trans dep's raw refs. No semantic change — the cache only deduplicates builder construction across calls within the same cctx. Signed-off-by: Robin Bate Boerop --- src/dune_rules/compilation_context.ml | 76 ++++++++++++++++++++++++++ src/dune_rules/compilation_context.mli | 25 +++++++++ src/dune_rules/module_compilation.ml | 32 +++++++---- 3 files changed, 123 insertions(+), 10 deletions(-) diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index 7197f307ed1..fa52d8edac9 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -33,6 +33,71 @@ module Includes = struct let empty = Lib_mode.Cm_kind.Map.make_all Command.Args.empty end +module Raw_refs = struct + module Key = struct + type t = + | Consumer of + { obj_name : Module_name.Unique.t + ; ml_kind : Ml_kind.t + } + | Transitive of + { obj_name : Module_name.Unique.t + ; cm_kind : Lib_mode.Cm_kind.t + } + + let cm_kind_tag : Lib_mode.Cm_kind.t -> int = function + | Ocaml Cmi -> 0 + | Ocaml Cmo -> 1 + | Ocaml Cmx -> 2 + | Melange Cmi -> 3 + | Melange Cmj -> 4 + ;; + + let ml_kind_tag : Ml_kind.t -> int = function + | Intf -> 0 + | Impl -> 1 + ;; + + let equal a b = + match a, b with + | Consumer a, Consumer b -> + Module_name.Unique.equal a.obj_name b.obj_name + && ml_kind_tag a.ml_kind = ml_kind_tag b.ml_kind + | Transitive a, Transitive b -> + Module_name.Unique.equal a.obj_name b.obj_name + && cm_kind_tag a.cm_kind = cm_kind_tag b.cm_kind + | Consumer _, Transitive _ | Transitive _, Consumer _ -> false + ;; + + let hash = function + | Consumer { obj_name; ml_kind } -> Poly.hash (0, obj_name, ml_kind_tag ml_kind) + | Transitive { obj_name; cm_kind } -> Poly.hash (1, obj_name, cm_kind_tag cm_kind) + ;; + + let repr = + let open Repr in + let obj_name_repr = view string ~to_:Module_name.Unique.to_string in + let ml_kind_repr = view string ~to_:Ml_kind.to_string in + let cm_kind_repr = abstract Lib_mode.Cm_kind.to_dyn in + variant + "Raw_refs.Key" + [ case "Consumer" (pair obj_name_repr ml_kind_repr) ~proj:(function + | Consumer { obj_name; ml_kind } -> Some (obj_name, ml_kind) + | Transitive _ -> None) + ; case "Transitive" (pair obj_name_repr cm_kind_repr) ~proj:(function + | Transitive { obj_name; cm_kind } -> Some (obj_name, cm_kind) + | Consumer _ -> None) + ] + ;; + + let to_dyn = Repr.to_dyn repr + end + + type t = (Key.t, Module_name.Set.t Action_builder.t) Table.t + + let create () : t = Table.create (module Key) 64 +end + (* Key omits [t.requires_compile] / [t.requires_hidden] because they're immutable on the cctx from [create]. The exception — [for_module_generated_at_link_time]'s derived cctxs — takes the @@ -120,6 +185,7 @@ type t = ; ocaml : Ocaml_toolchain.t ; for_ : Compilation_mode.t ; filtered_includes : Filtered_includes.t + ; raw_refs : Raw_refs.t } let loc t = t.loc @@ -375,11 +441,21 @@ let create ; instances ; for_ ; filtered_includes = Filtered_includes.create () + ; raw_refs = Raw_refs.create () } ;; let for_ t = t.for_ +let cached_raw_refs t ~key ~compute = + match Table.find t.raw_refs key with + | Some builder -> builder + | None -> + let builder = compute () in + Table.set t.raw_refs key builder; + builder +;; + let filtered_include_flags t ~cm_kind ~kept_libs = let lib_mode = Lib_mode.of_cm_kind cm_kind in let cache_key = { Filtered_includes.Key.lib_mode; kept_libs } in diff --git a/src/dune_rules/compilation_context.mli b/src/dune_rules/compilation_context.mli index 1075762651d..c640df9f16c 100644 --- a/src/dune_rules/compilation_context.mli +++ b/src/dune_rules/compilation_context.mli @@ -64,6 +64,31 @@ val requires_compile : t -> Lib.t list Resolve.Memo.t val parameters : t -> Module_name.t list Resolve.Memo.t val includes : t -> Command.Args.without_targets Command.Args.t Lib_mode.Cm_kind.Map.t +module Raw_refs : sig + module Key : sig + type t = + | Consumer of + { obj_name : Module_name.Unique.t + ; ml_kind : Ml_kind.t + } + | Transitive of + { obj_name : Module_name.Unique.t + ; cm_kind : Lib_mode.Cm_kind.t + } + end +end + +(** Memoise the raw-refs [Action_builder.t] computed for each [Raw_refs.Key.t] + within this cctx. [compute ()] is invoked only on cache miss; subsequent + callers with the same key get the cached builder back. The cache + short-circuits before allocating, so siblings sharing [trans_deps] don't + redo construction. *) +val cached_raw_refs + : t + -> key:Raw_refs.Key.t + -> compute:(unit -> Module_name.Set.t Action_builder.t) + -> Module_name.Set.t Action_builder.t + (** Include flags ([-I]/[-H]) for compiling a module against [kept_libs]. The cctx's [requires_compile] and [requires_hidden] are each restricted to libraries in [kept_libs]; the kept direct entries become [-I], the kept diff --git a/src/dune_rules/module_compilation.ml b/src/dune_rules/module_compilation.ml index 109b586ca0a..b5a9f92b950 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -121,21 +121,33 @@ let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kin | Ocaml (Cmi | Cmo) | Melange _ -> false in let read_dep_m_raw dep_m ~is_consumer = - let* impl_deps = - if need_impl_deps_of dep_m ~is_consumer - then + let key : Compilation_context.Raw_refs.Key.t = + let obj_name = Module.obj_name dep_m in + if is_consumer + then Consumer { obj_name; ml_kind } + else Transitive { obj_name; cm_kind } + in + Compilation_context.cached_raw_refs cctx ~key ~compute:(fun () -> + let* impl_deps = + if need_impl_deps_of dep_m ~is_consumer + then + Ocamldep.read_immediate_deps_raw_of + ~sandbox + ~sctx + ~obj_dir + ~ml_kind:Impl + dep_m + else Action_builder.return Module_name.Set.empty + in + let+ intf_deps = Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir - ~ml_kind:Impl + ~ml_kind:Intf dep_m - else Action_builder.return Module_name.Set.empty - in - let+ intf_deps = - Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf dep_m - in - Module_name.Set.union impl_deps intf_deps + in + Module_name.Set.union impl_deps intf_deps) in let* m_raw = read_dep_m_raw m ~is_consumer:true in let* trans_raw = From 933d2459e4c7e690dc940c1e62b6a85187dacc22 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:51:54 -0700 Subject: [PATCH 09/12] perf: memoise [Lib.closure] keyed on (linking, for_, libs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-module filter calls [Lib.closure] twice per consumer module (once for [direct_libs], once for [must_glob_libs]) on each compile rule. Across a cctx, many modules pass overlapping inputs to these closures; without memoisation every call re-traverses the dependency graph. [Lib.closure] is now defined as [Memo.exec] over a [Memo.create] keyed on [(bool * Compilation_mode.t * t list)]. The list-of-libs key is order- and multiplicity-sensitive, so callers that share inputs need to canonicalise (sort by [Lib.compare]) for maximum cache reuse — [lib_deps_for_module] already does this at both call sites. A docstring on [val closure] notes the requirement. Signed-off-by: Robin Bate Boerop --- src/dune_rules/lib.ml | 50 +++++++++++++++++++++++++++++++++++++----- src/dune_rules/lib.mli | 4 ++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/dune_rules/lib.ml b/src/dune_rules/lib.ml index a8630f413db..4a0a76faa7e 100644 --- a/src/dune_rules/lib.ml +++ b/src/dune_rules/lib.ml @@ -2198,11 +2198,51 @@ end = struct ;; end -let closure l ~linking = - let forbidden_libraries = Map.empty in - if linking - then Resolve_names.linking_closure_with_overlap_checks None l ~forbidden_libraries - else Resolve_names.compile_closure_with_overlap_checks None l ~forbidden_libraries +let closure = + let memo = + let module Input = struct + type nonrec t = bool * Compilation_mode.t * t list + + let equal (l, m, libs) (l', m', libs') = + Bool.equal l l' && Compilation_mode.equal m m' && List.equal equal libs libs' + ;; + + let hash_for_ = function + | Compilation_mode.Ocaml -> 0 + | Melange -> 1 + ;; + + let hash (linking, for_, libs) = + Tuple.T3.hash + Bool.hash + hash_for_ + (List.hash (fun lib -> Id.hash lib.unique_id)) + (linking, for_, libs) + ;; + + let to_dyn = Dyn.opaque + end + in + Memo.create + "lib-closure" + ~input:(module Input) + (fun (linking, for_, l) -> + let forbidden_libraries = Map.empty in + if linking + then + Resolve_names.linking_closure_with_overlap_checks + None + l + ~forbidden_libraries + ~for_ + else + Resolve_names.compile_closure_with_overlap_checks + None + l + ~forbidden_libraries + ~for_) + in + fun l ~linking ~for_ -> Memo.exec memo (linking, for_, l) ;; let descriptive_closure (l : lib list) ~with_pps ~for_ : lib list Memo.t = diff --git a/src/dune_rules/lib.mli b/src/dune_rules/lib.mli index 1ab86f84d21..ec7bc3dbe5e 100644 --- a/src/dune_rules/lib.mli +++ b/src/dune_rules/lib.mli @@ -217,6 +217,10 @@ end (** {1 Transitive closure} *) +(** Memoized. The memo key is order-sensitive on the input list, so for callers + that share inputs across invocations (e.g. the hot path in + [Module_compilation.lib_deps_for_module]), a canonical sort (by + [Lib.compare]) maximises cache reuse. *) val closure : t list -> linking:bool -> for_:Compilation_mode.t -> t list Resolve.Memo.t (** [descriptive_closure ~with_pps libs] computes the smallest set of libraries From 698f3e4081fe49a20991f5943d19c1df803f03fd Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Fri, 15 May 2026 20:58:36 -0700 Subject: [PATCH 10/12] doc: per-module narrowing algorithm reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add [doc/dev/per-module-narrowing.md] describing the per-module library file dependency narrowing introduced in this PR (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 --- doc/dev/per-module-narrowing.md | 583 ++++++++++++++++++++++++++++++++ 1 file changed, 583 insertions(+) create mode 100644 doc/dev/per-module-narrowing.md diff --git a/doc/dev/per-module-narrowing.md b/doc/dev/per-module-narrowing.md new file mode 100644 index 00000000000..7ac3b2afc18 --- /dev/null +++ b/doc/dev/per-module-narrowing.md @@ -0,0 +1,583 @@ +# Per-module library dependency narrowing + +This document describes the algorithm Dune uses to narrow the +library-file dependency set of each compile rule on a per-module basis. +The narrowing was introduced in #14492 (split into PRs #14513–#14521, +referred to internally as layers L1..L9). It supersedes the prior +behaviour in which every compile rule depended on the full glob of +every interface file of every library in the compilation context's +dependency closure. + +## Motivation + +In the pre-narrowing world, a single compile rule for module `M` in +some stanza depended on the glob of every `.cmi` (and, conditionally, +`.cmx`) of every library reachable through `requires`. The glob is +correct — recompiling `M` must see fresh interfaces — but it is +maximally broad. A change to any interface in any reachable library +invalidated `M`'s compile rule, even when `M` does not mention the module +whose interface changed. + +For incremental builds in code bases with many libraries and broad +`requires` graphs, this over-invalidation produced large, avoidable +recompilation cascades. The narrowing computes, for each compile rule, +a subset of those files that the consumer module can actually be +affected by — by inspecting what its `ocamldep` says it references and +walking the cross-library reference graph until convergence. Files +outside that subset are dropped from the rule's dependency set; their +unrelated edits no longer trigger a recompile. + +The narrowing must be *sound*: every file the compiler can read while +processing `M` must remain in the dependency set, or builds will be +non-deterministic and incremental builds will silently use stale +artefacts. The algorithm therefore conservatively widens to the +original glob in a small set of edge cases described under "Soundness +recovery" below. + +## The algorithm at a glance + +For each compile rule for a module `M` in compilation context `cctx`, +parameterised by the compile artefact kind (`Lib_mode.Cm_kind.t` — one +of `Ocaml Cmi`, `Ocaml Cmo`, `Ocaml Cmx`, `Melange Cmi`, `Melange Cmj`), +the source-language kind (`Ml_kind.t` — `Impl` or `Intf`), and the mode +(`Lib_mode.t`): + +1. If a small set of preconditions fails (`can_filter = false`), fall + back to the cctx-wide glob — the same dep set every compile rule + would have had prior to this work. This avoids needing soundness + arguments for module kinds that the narrowing was not designed for, + and lets `Melange` paths pass through unchanged. + +2. Otherwise, if the consumer cctx already carries a virtual library + in `requires` (`has_virtual_impl = true`), fall back to the + cctx-wide glob. Virtual-impl deps require analysis the BFS does not + currently do. + +3. Otherwise, run the narrowing: + 1. Read the consumer module's own `ocamldep` raw references (impl + and intf), and those of every module in `M`'s within-cctx + transitive `dep_graph`. + 2. Union them with the `-open` flag modules. The result is + `referenced : Module_name.Set.t`. + 3. Use the cctx's `Lib_index` (built once per cctx) to partition + the cctx's library closure into libraries whose entry modules + appear in `referenced` (`tight`) and libraries whose `None`- + entries appear in `referenced` (`non_tight`). + 4. Compute the `Lib.closure` of `tight ∪ non_tight ∪ + pps_runtime_libs` to expand the **lib set** over the library + `requires` DAG, picking up libs reached only via transparent + aliases that ocamldep cannot see. + 5. Run a BFS over the **module-reference graph**: starting from + `referenced`, look up `(lib, entry)` pairs in the lib index + and read each entry's `ocamldep` raw refs, extending the + frontier until it converges. The fixpoint `tight_set` is the + set of *module names* (not libs) transitively reachable. + 6. Re-run `Lib_index.filter_libs_with_modules` with `tight_set` in + place of `referenced`, producing a fresh `(tight_modules, + non_tight_set)` partition over the BFS-expanded name set. + 7. Compute `must_glob_set` for the libraries that must glob for + soundness (wrapped-library wrappers reached transparently, and + `ppx_runtime_libraries` whose modules are post-pp invisible). + 8. Fold over the closed lib set, classifying each lib into one of + four buckets and emitting either specific-file deps (for tight + libs) or a full glob (for non-tight libs). + 9. Compute the include flags scoped to the `kept_libs` set — + libraries that contributed at least one dep — so that `-I`/`-H` + reflect only what the consumer can actually reach. + +The output is `(include_args, dep_set)`, which the compile rule +consumes via `Action_builder.dyn_deps`. + +## The `can_filter` precondition + +The entry point for the narrowing is +`Module_compilation.lib_deps_for_module` +(`src/dune_rules/module_compilation.ml`). Before any narrowing runs, +the function computes a `can_filter` boolean conjunction: + +- The compile is `Ocaml _` (Melange has its own cm-kind story and is + not narrowed). +- The supplied `dep_graph` is the real one for this cctx (its `dir` + equals the cctx's `obj_dir`), not a `Dep_graph.dummy` — synthesised + / link-time-generated / alias / root modules have no usable + trans-deps and short-circuit. +- The `dep_graph` contains `M`. Modules synthesised outside the stanza + (e.g. those handed to `ocamlc_i` for `-i` extraction in unrelated + flows) are also rejected here. +- `M`'s `Module.kind` is filterable: not `Root`, `Wrapped_compat`, + `Impl_vmodule`, `Virtual`, or `Parameter`. These kinds have bespoke + dep stories handled elsewhere (`Dep_rules`, `Virtual_rules`). +- `M` has a source file of the requested `ml_kind` (`Impl` or `Intf`). +- The consumer cctx is itself not a `Virtual_rules.is_virtual_or_ + parameter` cctx. Consumer-side virtual-impl behaviour is owned by + `Dep_rules`. + +If any condition fails, the function returns + +```ocaml +(cctx_includes_for_cm_kind (), + Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs) +``` + +... the cctx-wide glob, exactly equivalent to the prior behaviour. The +narrowing applies only when all conditions hold. + +## The `has_virtual_impl` early-out + +When `can_filter` holds, the next check is + +```ocaml +let* has_virtual_impl = + Resolve.Memo.read (Compilation_context.has_virtual_impl cctx) +in +if has_virtual_impl then + Action_builder.return + (cctx_includes_for_cm_kind (), + Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs) +else ... +``` + +`has_virtual_impl` is true when any library in the cctx's +`requires_compile ∪ requires_hidden` is itself a virtual-library +implementation. The BFS does not currently analyse virtual impls' +contribution to the cctx's namespace; for soundness, all such cctxs +fall back to the cctx-wide glob. The decision is cctx-level, computed +once at `Compilation_context.create` and stored as a +`bool Resolve.t Memo.Lazy.t`. + +## The narrowing pipeline + +When `can_filter` holds and `has_virtual_impl` is false, the function +proceeds to the narrowing pipeline. + +### Step 1: read ocamldep raw refs for consumer + trans deps + +```ocaml +let* trans_deps = Dep_graph.deps_of dep_graph m in +let need_impl_deps_of dep_m ~is_consumer = + if is_consumer + then (match ml_kind with Impl -> true | Intf -> false) + else + (not (Module.has dep_m ~ml_kind:Intf)) + || + match cm_kind with + | Ocaml Cmx -> not opaque + | Ocaml (Cmi | Cmo) | Melange _ -> false +in +``` + +The function reads `M`'s impl + intf raw refs, plus the impl + intf +raw refs of every module in `M`'s transitive within-stanza +dependencies. The selectivity table (which `.ml`-side refs to read) +matches the compiler's own conditional behaviour: + +| `dep_m` is | `cm_kind` | `opaque` | read `.ml`? | +| ----------------------- | ----------- | -------- | ------------ | +| consumer (`M` itself) | any | any | iff `Impl` | +| trans_dep, no `.mli` | any | any | yes | +| trans_dep, has `.mli` | `Cmx` | false | yes (inline) | +| trans_dep, has `.mli` | `Cmx` | true | no | +| trans_dep, has `.mli` | `Cmi`/`Cmo` | any | no | + +Each ocamldep raw-ref read is wrapped through +`Compilation_context.cached_raw_refs` (L8), keyed on +`Raw_refs.Key.t = Consumer { obj_name; ml_kind } | Transitive +{ obj_name; cm_kind }`. The cache short-circuits before allocating the +underlying `Action_builder.t`, so two siblings sharing a transitive +dep only construct the dependency-reading builder once per cctx per +key. + +The intf side is always read; the impl side is gated by +`need_impl_deps_of`. The union of impl+intf for each module is the +module's *raw references* — every module name that ocamldep saw it +mention. + +### Step 2: compute `referenced` + +```ocaml +let* m_raw = read_dep_m_raw m ~is_consumer:true in +let* trans_raw = + union_module_name_sets_mapped trans_deps + ~f:(read_dep_m_raw ~is_consumer:false) +in +let all_raw = Module_name.Set.union m_raw trans_raw in +let* flags = Ocaml_flags.get (Compilation_context.flags cctx) mode in +let open_modules = Ocaml_flags.extract_open_module_names flags in +let referenced = Module_name.Set.union all_raw open_modules in +``` + +`open_modules` are module names brought into scope by `-open` flags +that ocamldep does not see (because they are command-line, not source +syntax). Without them the consumer can mention a module by short name +that the raw refs would miss, opening a soundness gap. Their union +with `all_raw` produces `referenced` — the set of module names this +compile of `M` can possibly resolve. + +### Step 3: first lib classification + +```ocaml +let { Lib_file_deps.Lib_index.tight; non_tight } = + Lib_file_deps.Lib_index.filter_libs_with_modules + lib_index + ~referenced_modules:referenced +in +``` + +The cctx-level `Lib_index` (built once at `cctx` creation by +`Compilation_context.build_lib_index`) is a structure indexing each +library's entry modules with each module name. `filter_libs_with_ +modules` walks `referenced` against the index and partitions: + +- `tight : Module.t list Lib.Map.t` — libraries whose `Some`-entry + modules appear in `referenced`. The mapped list is the specific + set of entries referenced. These are candidates for per-module + deps (`deps_of_entry_modules`). +- `non_tight : Lib.Set.t` — libraries whose `None`-entry modules + appear (wrapped locals, externals, or unwrapped locals with some + staged-pps / instrumentation-only entries). These must glob. + +A library with mixed `Some`/`None` entries can appear in both — the +caller globs it from the `non_tight` side, since the `None` entries +require glob coverage. + +### Step 4: include `pps_runtime_libs` and compute `Lib.closure` + +```ocaml +let* pps_runtime_libs = + Resolve.Memo.read (Compilation_context.pps_runtime_libs cctx) +in +let direct_libs = + List.sort_uniq ~compare:Lib.compare + (Lib.Map.keys tight @ Lib.Set.to_list non_tight @ pps_runtime_libs) +in +let* all_libs = + Resolve.Memo.read (Lib.closure direct_libs ~linking:false ~for_) +in +``` + +`pps_runtime_libs` introduce module references through post-pp source +that ocamldep cannot see — they are folded into the closure input so +the classification fold sees them. `sort_uniq` canonicalises the +input list because `Lib.closure`'s memo key (L9) is order- and +multiplicity-sensitive. + +`Lib.closure` adds libraries reachable through transparent aliases +(re-exports) that ocamldep does not surface — for instance, when a +library `A` re-exports modules from `B` via an alias, references to +`B.M` in source compile to references to `A.M` and ocamldep records +only `A`. The closure pulls `B` in. + +### Step 5: compute `must_glob_set` + +```ocaml +let wrapped_referenced = + Lib_file_deps.Lib_index.wrapped_libs_referenced + lib_index ~referenced_modules:referenced +in +let* must_glob_libs = + Resolve.Memo.read + (Lib.closure + (List.sort_uniq ~compare:Lib.compare + (Lib.Set.to_list wrapped_referenced @ pps_runtime_libs)) + ~linking:false ~for_) +in +let must_glob_set = Lib.Set.of_list must_glob_libs in +``` + +Two classes of libraries must always glob even when their entry +modules appear in `tight`: + +- **Wrapped library wrappers reached transparently.** When the + consumer mentions `Lib.M`, the source-level reference is to the + wrapper module `Lib`, but the compiled output of `M` lives at + `lib__M`. The cross-library walker reads ocamldep on the wrapper + module, which only surfaces references to the wrapper's direct + children. A consumer that reaches `Lib`'s wrapper can reach any + module of `Lib` via Foo's `(open ...)` or `Lib.Bar.x` syntax; the + walker cannot enumerate those at narrowing time. Solution: glob + `Lib.closure(wrapped_referenced)` — every library transitively + reachable from any referenced wrapper. + +- **`ppx_runtime_libraries`.** ppx-rewritten output may reference + modules from a runtime library that the consumer never names in + its source. ocamldep on the pre-pp source does not see those + references. Glob the closure to cover. + +The union `Lib.Set.of_list must_glob_libs` is the *must-glob set*; +every member is forced onto the glob path regardless of its entry +classification. + +### Step 6: BFS to fixpoint + +```ocaml +let* tight_set = + cross_lib_tight_set + ~sandbox ~sctx ~lib_index ~initial_refs:referenced +in +``` + +`cross_lib_tight_set` runs a BFS over the cross-library reference +graph: + +```ocaml +let rec loop ~seen ~frontier = + if Module_name.Set.is_empty frontier then return seen + else + let pairs = + Module_name.Set.fold frontier ~init:[] ~f:(fun name acc -> + Lib_file_deps.Lib_index.lookup_tight_entries lib_index name @ acc) + in + let* discovered = + union_module_name_sets_mapped pairs ~f:read_entry_deps + in + let seen = Module_name.Set.union seen frontier in + let frontier = Module_name.Set.diff discovered seen in + loop ~seen ~frontier +``` + +Each frontier iteration: + +1. For each module name in the frontier, look up the `(lib, entry)` + tight-eligible pairs via `Lib_file_deps.Lib_index.lookup_tight_ + entries`. The lookup skips entries with `None`-module (wrapped + locals, externals) — those are non-tight-eligible and terminate + chains through them. +2. For each `(lib, entry)`, read the entry module's impl + intf + ocamldep raw refs. +3. Union all the discovered names into `discovered`. +4. Add the current frontier to `seen`; the new frontier is + `discovered \ seen`. + +The post-pp module map (built by `Compilation_context.build_lib_ +index`) ensures that the entry module passed to ocamldep is the form +the dep lib's own compile pipeline produces — so the walker reads +either the `.pp.ml` (for action-style preprocessors or non-staged +pps), or the source directly (for no-preprocessing and future-syntax +extensions, which Dune handles at parse time). + +The walker terminates because the universe of module names is +finite (bounded by the cctx's transitive library set) and `seen` +only grows. The cost is proportional to the number of distinct +entry modules transitively reachable from `referenced`. + +### Step 7: second lib classification + +```ocaml +let { Lib_file_deps.Lib_index.tight = tight_modules; + non_tight = non_tight_set } = + Lib_file_deps.Lib_index.filter_libs_with_modules + lib_index ~referenced_modules:tight_set +in +``` + +Re-partition the index using the converged `tight_set` (which is a +superset of `referenced`). This is a separate classification from +step 3 because the BFS may have brought new module names into scope +that flip a library from "unreached" to "tight" or change the +specific entries referenced. + +### Step 8: classify and emit per-lib deps + +```ocaml +let tight_deps, glob_libs, kept_libs = + List.fold_left all_libs ~init:(Dep.Set.empty, [], Lib.Set.empty) + ~f:(fun (td, gl, kl) lib -> + if Lib.Set.mem must_glob_set lib || Lib.Set.mem non_tight_set lib + then td, lib :: gl, Lib.Set.add kl lib + else ( + match Lib.Map.find tight_modules lib with + | Some modules -> + ( Dep.Set.union td + (Lib_file_deps.deps_of_entry_modules + ~opaque ~cm_kind lib modules) + , gl + , Lib.Set.add kl lib ) + | None -> + if Lib_file_deps.Lib_index.is_tight_eligible lib_index lib + then td, gl, kl + else td, lib :: gl, Lib.Set.add kl lib)) +in +let glob_deps = Lib_file_deps.deps_of_entries ~opaque ~cm_kind glob_libs in +``` + +Each library in `all_libs` (the closure-expanded set) is bucketed: + +- **Must-glob**: lib is in `must_glob_set` (wrapped-referenced or + pps-runtime), or has a `None`-entry referenced → emit the full lib + glob via `deps_of_entries`. Added to `kept_libs`. + +- **Tight**: lib has a `Some`-entry referenced and is not must-glob → + emit `deps_of_entry_modules` for just those referenced entries. + Added to `kept_libs`. + +- **Unreached + tight-eligible**: lib has no referenced entries and is + tight-eligible (has at least one `Some`-entry) → drop entirely. The + link rule still pulls it in via `requires_link` for runtime + linkage. The compile rule does not need it. + +- **Unreached + not tight-eligible**: lib has no referenced entries + but is not tight-eligible (wrapped local, external) → emit glob. + Added to `kept_libs`. + +The `kept_libs` accumulator captures every library that contributes +*any* dep to this compile rule. It is the precise set against which +include flags should be scoped (step 9). + +### Step 9: filtered include flags + +```ocaml +let+ include_flags = + Compilation_context.filtered_include_flags cctx ~cm_kind ~kept_libs +in +include_flags, Dep.Set.union tight_deps glob_deps +``` + +`Compilation_context.filtered_include_flags` (L6+L7) restricts the +cctx's `requires_compile` and `requires_hidden` lists to libraries in +`kept_libs`, then computes `-I` / `-H` over those subsets. The result +is memoised per `(lib_mode, kept_libs : Lib.Set.t)` (L7) so that two +modules with the same `kept_libs` share evaluation. + +The final return value is the include args paired with the total dep +set. `Module_compilation.lib_cm_deps` wraps the function with +`Action_builder.dyn_deps`, which registers `dep_set` as the rule's +dynamic deps and returns just the include args for the command line. + +## Data structures + +### `Lib_file_deps.Lib_index.t` + +(`src/dune_rules/lib_file_deps.{ml,mli}`.) Built once per cctx by +`Compilation_context.build_lib_index ~super_context ~libs ~for_`. The +input is a list of `(Module_name.t, Lib.t, Module.t option)` triples: +the entry module's name, the owning library, and the entry's +`Module.t` if available (`None` for wrapped locals and externals). + +The index supports: + +- `filter_libs_with_modules ~referenced_modules` — partition into + tight (per-lib `Module.t list` for the referenced entries) and + non_tight (libs with `None`-entry references). +- `lookup_tight_entries name` — the `(lib, entry)` pairs whose entry + name is `name`, skipping `no_ocamldep`-tagged libs and `None`-entry + rows. Used by the BFS frontier expansion. +- `is_tight_eligible lib` — true iff lib has at least one `Some`- + entry. Used in step 8 to distinguish "drop" from "must glob". +- `wrapped_libs_referenced ~referenced_modules` — the set of wrapped + local libraries whose wrapper module name appears. Used to compute + the wrapped-soundness must-glob input in step 5. + +### `Compilation_context.cached_raw_refs` (L8) + +A per-cctx `Hashtbl` keyed by `Raw_refs.Key.t` that memoises the +`Module_name.Set.t Action_builder.t` for a given module's ocamldep +raw refs. Two distinct keys: + +- `Consumer { obj_name; ml_kind }` — used when reading the consumer + module `M`'s own refs (`ml_kind` distinguishes `Impl` from `Intf` + contexts; the choice of what to read varies per + `need_impl_deps_of`). +- `Transitive { obj_name; cm_kind }` — used when reading a + `trans_deps` member's refs. + +Within a cctx, two compile rules computing narrowing for siblings +that share a transitive dep both hit the same key, paying the +underlying ocamldep read once. + +### `Compilation_context.filtered_includes` (L7) + +A per-cctx `Hashtbl` keyed by `Filtered_includes.Key.t = +{ lib_mode : Lib_mode.t; kept_libs : Lib.Set.t }`. Caches the include +flags Action_builder per `(lib_mode, kept_libs)` tuple. Two modules +within the same cctx whose narrowing produces identical `kept_libs` +share the include-flag computation. + +### `Lib.closure` memo (L9) + +`Lib.closure` is wrapped in a memo keyed by `(linking, for_, libs)` +where `libs` is the input lib list. Two narrowing calls within the +same cctx (or across cctxs) that produce identical `direct_libs` +share the closure computation. + +### `Dep_graph` per cctx, per `Ml_kind` + +(`src/dune_rules/dep_rules.ml`.) Built per cctx as +`Ml_kind.Dict.t Dep_graph.t`. `Dep_graph.deps_of dep_graph m` returns +the within-cctx transitive `Module.t list` for module `m`. Used at +step 1 to enumerate trans_deps before reading their ocamldep raw +refs. + +## Soundness recovery and known edge cases + +Three categories of fallback to the glob: + +1. **Module kinds outside `module_kind_is_filterable`**: `Root`, + `Wrapped_compat`, `Impl_vmodule`, `Virtual`, `Parameter`. Each has + a bespoke dep story handled elsewhere; the `can_filter` gate + rejects them. + +2. **Consumer cctx with `has_virtual_impl = true`**: the cctx itself + has a virtual-impl in `requires`. The BFS currently lacks the + analysis to safely narrow under that condition; the entire cctx + falls back. + +3. **Per-library wrapped or pps-runtime soundness**: handled within + the narrowing by `must_glob_set` (step 5 + step 8). The + classification fold force-globs these libraries even when their + tight classification would otherwise narrow them. + +`Melange` paths bypass the narrowing entirely at the `can_filter` +check; the cm_kind machinery there is different and L9 leaves it +unchanged. + +## Cost characteristics + +Per consumer module, the narrowing does: + +- One ocamldep raw-ref read for the consumer (cached by L8). +- One ocamldep raw-ref read per trans-dep module (cached by L8 — + the read is shared with sibling consumers that share a trans-dep). +- One `Lib.closure` (memoised by L9 — shared with sibling consumers + whose `direct_libs` are identical). +- One BFS over the cross-library reference graph (state machine runs + per module; the individual ocamldep reads inside it hit L8 across + modules). +- Two `filter_libs_with_modules` calls (step 3 and step 7). +- One `filtered_include_flags` lookup (memoised by L7 — shared with + siblings whose `kept_libs` match). + +The visible per-module Dune-level functions are individually small. +The cost is in their *downstream effects* — `Module_name.Set` unions, +`Lib.Set` / `Lib.Map` operations, `Dep.Set` constructions, +`Action_builder` bind chains, and the minor-GC oldification of the +intermediate sets. + +On a null build, the narrowing yields no benefit (no `.cmi` changed, +so no module would have rebuilt either way). The work runs anyway. +The narrowing trades null-build wall time for incremental-build +recompile reduction. + +## Layer-by-layer construction (#14492) + +For reference, the algorithm above is the cumulative result of: + +- **L1** (#14513): test infrastructure prerequisite (merged). +- **L2** (#14514): cctx fields `Lib_index`, `has_virtual_impl`, + `pps_runtime_libs`. +- **L3** (#14515): scaffold — route lib file deps through a per- + module function (`lib_deps_for_module`) that initially returns the + same cctx-wide glob, preparing the call site. +- **L4** (#14516): the BFS itself (`cross_lib_tight_set`, + `lib_deps_for_module` body); the `can_filter` gate is added. +- **L5** (#14517): soundness recovery — + `has_virtual_impl` early-out, `pps_runtime_libs` fold-in, + `must_glob_set` from wrapped-referenced libs. +- **L6** (#14518): `filtered_include_flags` per `kept_libs`. +- **L7** (#14519): `Filtered_includes.t` per-cctx cache keyed by + `(lib_mode, kept_libs)`. +- **L8** (#14520): `cached_raw_refs` per-cctx cache for ocamldep + raw-ref `Action_builder.t`s. +- **L9** (#14521): `Lib.closure` memo keyed by `(linking, for_, + libs)`. + +Each layer can be read in isolation in the PR stack. From 33da56ebed38cff44fc4f7f0920990fc517fbeca Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Sun, 17 May 2026 16:39:09 -0700 Subject: [PATCH 11/12] fix: BFS-expand through dep libs' stanza -open modules Closes a soundness gap in the per-module narrowing pipeline missed by the wrapped / ppx-runtime / virtual-impl recoveries: when a dependency library's stanza injects [-open M] via [(flags (...))], its source can reference [M]'s identifiers without naming [M], and ocamldep emits no token to drive the cross-library walker. Reported by RyanJamesStewart on #14517 with this fixture: an unwrapped [middle] depending on unwrapped [prelude] with [(flags (:standard -open Prelude))], exposing [val pick : unit -> color] (where [color] resolves through the open to [Prelude.color]). The consumer pattern-matches the result against bare constructors. The compile genuinely needs [prelude.cmi] to resolve the constructors; the BFS over [ocamldep -modules] cannot reach [prelude] (no syntactic [Prelude] token on either side); the three existing recoveries do not catch it ([prelude] is not wrapped, not a ppx-runtime lib, not a virtual-impl). [Module_compilation.cross_lib_tight_set]: - Add [~mode] (the consumer's compile mode) so we can expand a dep lib's stanza flags via [Ocaml_flags.get]. - Extend [read_entry_deps] to compute [read_stanza_opens] for the visited lib and union the result into the BFS frontier. Localised in the BFS rather than the initial-frontier computation so the reachability rule reads as: a module is reachable iff the consumer references it, or some reached module's ocamldep names it, or some reached module's owning lib stanza-opens it. Returns empty for [Spec.standard] (short-circuits external libs). [Lib_info]: - Add [stanza_flags : Dune_lang.Ocaml_flags.Spec.t] field plus accessor. Local libs carry their stanza's [conf.buildable.flags]; external libs ([findlib], [dune_package]) carry [Spec.standard]. Regression test: [cross-lib-open-flag-barrier.t]. Fails on L4 and L5 head before this patch (Unbound constructor Green under [--sandbox=copy]); passes after. --- src/dune_rules/dune_package.ml | 1 + src/dune_rules/findlib.ml | 1 + src/dune_rules/lib_info.ml | 5 ++ src/dune_rules/lib_info.mli | 7 +++ src/dune_rules/module_compilation.ml | 30 +++++++-- src/dune_rules/stanzas/library.ml | 1 + .../cross-lib-open-flag-barrier.t | 61 +++++++++++++++++++ 7 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-open-flag-barrier.t diff --git a/src/dune_rules/dune_package.ml b/src/dune_rules/dune_package.ml index 9e0cf07c7de..d2e0de5a9ac 100644 --- a/src/dune_rules/dune_package.ml +++ b/src/dune_rules/dune_package.ml @@ -336,6 +336,7 @@ module Lib = struct ~jsoo_runtime ~wasmoo_runtime ~preprocess + ~stanza_flags:Dune_lang.Ocaml_flags.Spec.standard ~enabled ~virtual_deps ~dune_version diff --git a/src/dune_rules/findlib.ml b/src/dune_rules/findlib.ml index f6be5b83aa8..f760c3caa82 100644 --- a/src/dune_rules/findlib.ml +++ b/src/dune_rules/findlib.ml @@ -268,6 +268,7 @@ let to_dune_library (t : Findlib.Package.t) ~dir_contents ~ext_lib ~external_loc ~jsoo_runtime ~wasmoo_runtime ~preprocess + ~stanza_flags:Dune_lang.Ocaml_flags.Spec.standard ~enabled ~virtual_deps ~dune_version diff --git a/src/dune_rules/lib_info.ml b/src/dune_rules/lib_info.ml index 9b6ecd8dbf9..947872b7c99 100644 --- a/src/dune_rules/lib_info.ml +++ b/src/dune_rules/lib_info.ml @@ -323,6 +323,7 @@ type 'path t = ; allow_unused_libraries : (Loc.t * Lib_name.t) list ; preprocess : Preprocess.With_instrumentation.t Preprocess.Per_module.t Compilation_mode.By_mode.t + ; stanza_flags : Dune_lang.Ocaml_flags.Spec.t ; enabled : Enabled_status.t Memo.t ; virtual_deps : (Loc.t * Lib_name.t) list ; dune_version : Dune_lang.Syntax.Version.t option @@ -354,6 +355,7 @@ let parameters t = t.parameters let requires t ~for_ = Compilation_mode.By_mode.get t.requires ~for_ let requires_by_mode t = t.requires let preprocess t ~for_ = Compilation_mode.By_mode.get t.preprocess ~for_ +let stanza_flags t = t.stanza_flags let ppx_runtime_deps t = t.ppx_runtime_deps let allow_unused_libraries t = t.allow_unused_libraries let sub_systems t = t.sub_systems @@ -435,6 +437,7 @@ let create ~jsoo_runtime ~wasmoo_runtime ~preprocess + ~stanza_flags ~enabled ~virtual_deps ~dune_version @@ -476,6 +479,7 @@ let create ; jsoo_runtime ; wasmoo_runtime ; preprocess + ; stanza_flags ; enabled ; virtual_deps ; dune_version @@ -575,6 +579,7 @@ let to_dyn ; jsoo_runtime ; wasmoo_runtime ; preprocess = _ + ; stanza_flags = _ ; enabled = _ ; virtual_deps ; dune_version diff --git a/src/dune_rules/lib_info.mli b/src/dune_rules/lib_info.mli index eb5113b9945..f3972d2613d 100644 --- a/src/dune_rules/lib_info.mli +++ b/src/dune_rules/lib_info.mli @@ -170,6 +170,12 @@ val preprocess -> for_:Compilation_mode.t -> Preprocess.With_instrumentation.t Preprocess.Per_module.t +(** Unexpanded [(flags ...)] from the library's stanza. [Spec.standard] for + libraries assembled from META / dune-package files. The per-module narrowing + pipeline uses this to recover [-open]-induced cross-library [.cmi] reads + that ocamldep cannot see. *) +val stanza_flags : _ t -> Dune_lang.Ocaml_flags.Spec.t + val sub_systems : _ t -> Sub_system_info.t Sub_system_name.Map.t val enabled : _ t -> Enabled_status.t Memo.t val orig_src_dir : 'path t -> 'path option @@ -239,6 +245,7 @@ val create -> preprocess: Preprocess.With_instrumentation.t Preprocess.Per_module.t Compilation_mode.By_mode.t + -> stanza_flags:Dune_lang.Ocaml_flags.Spec.t -> enabled:Enabled_status.t Memo.t -> virtual_deps:(Loc.t * Lib_name.t) list -> dune_version:Dune_lang.Syntax.Version.t option diff --git a/src/dune_rules/module_compilation.ml b/src/dune_rules/module_compilation.ml index b5a9f92b950..136ba4699b3 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -26,18 +26,38 @@ let module_kind_is_filterable m = [lookup_tight_entries], terminating chains through them. The [Module.t] supplied here is the post-pp form (constructed in [build_lib_index]), so ocamldep runs on the dep lib's [.pp.ml] (action / non-staged Pps) or directly - on the source (no preprocessing / future-syntax). *) -let cross_lib_tight_set ~sandbox ~sctx ~lib_index ~initial_refs = + on the source (no preprocessing / future-syntax). + + Each visited lib's stanza [(flags (... -open Foo))] also extends the + frontier: a dep lib whose stanza opens [Foo] can use [Foo]'s identifiers + without naming [Foo] in its source, so ocamldep on the dep lib's source + produces no token to walk through. The stanza-open names are the missing + edges. *) +let cross_lib_tight_set ~sandbox ~sctx ~lib_index ~mode ~initial_refs = let open Action_builder.O in + let read_stanza_opens lib = + let info = Lib.info lib in + let spec = Lib_info.stanza_flags info in + if Dune_lang.Ocaml_flags.Spec.equal spec Dune_lang.Ocaml_flags.Spec.standard + then Action_builder.return Module_name.Set.empty + else ( + let dir = Lib_info.src_dir info |> Path.as_in_build_dir_exn in + let* ocaml_flags = + Action_builder.of_memo (Ocaml_flags_db.ocaml_flags sctx ~dir spec) + in + let+ flag_strings = Ocaml_flags.get ocaml_flags mode in + Ocaml_flags.extract_open_module_names flag_strings) + in let read_entry_deps (lib, m) = let obj_dir = Lib.info lib |> Lib_info.obj_dir |> Obj_dir.as_local_exn in let* impl_deps = Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Impl m in - let+ intf_deps = + let* intf_deps = Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf m in - Module_name.Set.union impl_deps intf_deps + let+ stanza_opens = read_stanza_opens lib in + Module_name.Set.union impl_deps (Module_name.Set.union intf_deps stanza_opens) in let rec loop ~seen ~frontier = if Module_name.Set.is_empty frontier @@ -202,7 +222,7 @@ let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kin in let must_glob_set = Lib.Set.of_list must_glob_libs in let* tight_set = - cross_lib_tight_set ~sandbox ~sctx ~lib_index ~initial_refs:referenced + cross_lib_tight_set ~sandbox ~sctx ~lib_index ~mode ~initial_refs:referenced in (* Classify each lib in [all_libs]: - lib has [None]-entry referenced (mixed-entry or wrapped) → glob (covers the None entries' [.cmi]s); - diff --git a/src/dune_rules/stanzas/library.ml b/src/dune_rules/stanzas/library.ml index 48404d80d5c..060556d9072 100644 --- a/src/dune_rules/stanzas/library.ml +++ b/src/dune_rules/stanzas/library.ml @@ -642,6 +642,7 @@ let to_lib_info ~jsoo_runtime ~wasmoo_runtime ~preprocess + ~stanza_flags:conf.buildable.flags ~enabled ~virtual_deps ~dune_version diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-open-flag-barrier.t b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-open-flag-barrier.t new file mode 100644 index 00000000000..dc34fcad80b --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-open-flag-barrier.t @@ -0,0 +1,61 @@ +Regression guard for cross-library transitive [.cmi] reads through +an intermediate library whose stanza injects [-open] on the leaf. +The intermediate's interface mentions a type from a leaf library +that the consumer never names syntactically; the intermediate's +own source also never names the leaf, because [-open] hides it. + +[prelude] defines [color = Red | Green | Blue]. [middle] depends +on [prelude] and is compiled with [(flags (:standard -open +Prelude))], exposing [val pick : unit -> color] (where [color] +resolves through the open to [Prelude.color]). [consumer] depends +on both libraries and pattern-matches on the result of +[Middle.pick] against the bare constructors [Green], [Red], [Blue]. + +The consumer's compile genuinely needs [prelude.cmi] so the OCaml +compiler can resolve the constructors back to [Prelude.color]. The +per-module BFS over [ocamldep -modules] cannot find this dep: +ocamldep on [middle.{ml,mli}] reports no token [Prelude] (the +source uses the open), and ocamldep on [consumer.ml] reports only +[Middle]. None of the three [must_glob_libs] recovery branches +catch [prelude]: it is not wrapped, not a ppx-runtime lib, not a +virtual-impl. + + $ make_dune_project 3.23 + + $ mkdir prelude middle consumer + + $ cat > prelude/dune < (library (name prelude) (wrapped false)) + > EOF + $ cat > prelude/prelude.ml < type color = Red | Green | Blue + > EOF + + $ cat > middle/dune < (library + > (name middle) + > (wrapped false) + > (libraries prelude) + > (flags (:standard -open Prelude))) + > EOF + $ cat > middle/middle.mli < val pick : unit -> color + > EOF + $ cat > middle/middle.ml < let pick () = Green + > EOF + + $ cat > consumer/dune < (executable (name consumer) (libraries middle prelude)) + > EOF + $ cat > consumer/consumer.ml < let () = match Middle.pick () with + > | Green -> print_endline "g" + > | Red | Blue -> print_endline "nb" + > EOF + +Sandbox-forced build of the consumer succeeds: [prelude]'s [.cmi] +is declared as a compile-rule dep through the per-module filter's +cross-lib walk and is therefore present in the sandbox. + + $ dune build --sandbox=copy consumer/consumer.exe From c41ecc5c1be4fad38d5e0aee0caa2a685e38486e Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Sun, 17 May 2026 16:42:05 -0700 Subject: [PATCH 12/12] doc: extend narrowing reference with stanza-open BFS edge Documents the fourth class of soundness recovery added in L5: the BFS's per-iteration step now extends the frontier with the modules named by each visited library's stanza [-open] flags, in addition to the entry's impl + intf ocamldep raw refs. The reachability rule the BFS computes becomes a three-way disjunction (consumer references it; reached module's ocamldep names it; reached module's owning lib stanza-opens it). Updates the [cross_lib_tight_set] code snippet's signature (now threads [~mode]), the per-iteration description, the "Soundness recovery and known edge cases" list (adds a fourth class), the L5 layer-summary line, and the cost-characteristics list (adds the per-visited-lib stanza-flags expansion). --- doc/dev/per-module-narrowing.md | 51 ++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/doc/dev/per-module-narrowing.md b/doc/dev/per-module-narrowing.md index 7ac3b2afc18..340453e90fe 100644 --- a/doc/dev/per-module-narrowing.md +++ b/doc/dev/per-module-narrowing.md @@ -312,7 +312,7 @@ classification. ```ocaml let* tight_set = cross_lib_tight_set - ~sandbox ~sctx ~lib_index ~initial_refs:referenced + ~sandbox ~sctx ~lib_index ~mode ~initial_refs:referenced in ``` @@ -342,12 +342,37 @@ Each frontier iteration: entries`. The lookup skips entries with `None`-module (wrapped locals, externals) — those are non-tight-eligible and terminate chains through them. -2. For each `(lib, entry)`, read the entry module's impl + intf - ocamldep raw refs. +2. For each `(lib, entry)`, the helper `read_entry_deps` returns the + union of three contributions: + - the entry module's impl ocamldep raw refs; + - the entry module's intf ocamldep raw refs; + - the modules named by `-open` flags injected through the + entry's owning library's stanza `(flags (...))`. The stanza + flags are expanded via `Ocaml_flags_db.ocaml_flags sctx ~dir` + against the consumer's `mode`, and the `-open M` tokens are + extracted with `Ocaml_flags.extract_open_module_names`. The + short-circuit `Dune_lang.Ocaml_flags.Spec.equal spec standard` + avoids the expansion for libraries that carry the default + (`Spec.standard`), which includes every external lib. 3. Union all the discovered names into `discovered`. 4. Add the current frontier to `seen`; the new frontier is `discovered \ seen`. +The stanza-opens contribution closes a soundness gap that the three +must-glob recoveries (step 5) and the syntactic ocamldep walk +together cannot cover. When a dependency library's stanza injects +`-open Foo`, its source can reference `Foo`'s identifiers without +ever naming `Foo` syntactically, so ocamldep on the dep lib's source +produces no token to drive the walker — and `Foo`'s defining +library may then be silently dropped from the consumer's compile +even though the consumer transitively needs its `.cmi`. Treating +the stanza opens as a third class of frontier edge ensures the BFS +reaches those libraries through the very same fixpoint that handles +ocamldep-visible references. The reachability rule the BFS computes +is: a module is reachable from the consumer iff the consumer +references it, OR some reached module's ocamldep names it, OR some +reached module's owning lib stanza-opens it. + The post-pp module map (built by `Compilation_context.build_lib_ index`) ensures that the entry module passed to ocamldep is the form the dep lib's own compile pipeline produces — so the walker reads @@ -526,6 +551,16 @@ Three categories of fallback to the glob: classification fold force-globs these libraries even when their tight classification would otherwise narrow them. +4. **Stanza-injected `-open` on dep libraries**: handled inside the + BFS itself (step 6). When the walker visits a module of library + `L`, it extends the frontier with the modules named by `L`'s + stanza `-open` flags in addition to the module's ocamldep raw + refs. Without this, a consumer that pattern-matches a type from + a library reached only via an intermediate's stanza `-open` (the + type being inherited into the intermediate's `.mli` via the + open) would silently drop the leaf library's `.cmi` from its + compile rule. + `Melange` paths bypass the narrowing entirely at the `can_filter` check; the cm_kind machinery there is different and L9 leaves it unchanged. @@ -542,6 +577,12 @@ Per consumer module, the narrowing does: - One BFS over the cross-library reference graph (state machine runs per module; the individual ocamldep reads inside it hit L8 across modules). +- Per visited entry, an `Ocaml_flags_db.ocaml_flags` expansion of the + owning lib's stanza `(flags ...)` followed by + `Ocaml_flags.extract_open_module_names`, when the lib's + `stanza_flags` differs from `Spec.standard`. External libs always + carry `Spec.standard`, so the expansion is skipped for them. Each + expansion is memoised through `Ocaml_flags_db`'s underlying Memo. - Two `filter_libs_with_modules` calls (step 3 and step 7). - One `filtered_include_flags` lookup (memoised by L7 — shared with siblings whose `kept_libs` match). @@ -571,7 +612,9 @@ For reference, the algorithm above is the cumulative result of: `lib_deps_for_module` body); the `can_filter` gate is added. - **L5** (#14517): soundness recovery — `has_virtual_impl` early-out, `pps_runtime_libs` fold-in, - `must_glob_set` from wrapped-referenced libs. + `must_glob_set` from wrapped-referenced libs, plus stanza-open + BFS expansion (each visited lib's stanza `-open` modules join + the frontier). - **L6** (#14518): `filtered_include_flags` per `kept_libs`. - **L7** (#14519): `Filtered_includes.t` per-cctx cache keyed by `(lib_mode, kept_libs)`.