From d4bfd089570427bdc3a779469a46e12c48879913 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Mon, 18 May 2026 10:46:58 -0700 Subject: [PATCH 1/5] test: mixed-pp lib soundness + precision baselines (pair) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two co-designed fixtures covering an unwrapped lib with per-module preprocessing where one module is default-pp (Some-entry in the per-lib index) and one is staged-pps (None-entry): - `mixed-per-module-preprocess.t` (soundness): consumer references the Some-entry module; build succeeds under `--sandbox=copy` because today's wide cmi glob over `mylib`'s objdir covers the staged-pps module's cmi. - `mixed-per-module-preprocess-precision.t` (precision): consumer references the Some-entry module while the staged-pps module's source contains an unresolvable identifier. Today, the wide glob pulls the bad module into consumer's deps and the build fails on the unbound identifier — pinning the current over-invalidation. The forthcoming per-module narrowing work (#14492) will flip the precision test's expected output from the compile error back to silent exit-0, demonstrating that the staged-pps None-entry module is no longer pulled in when the consumer doesn't reference it. Signed-off-by: Robin Bate Boerop --- .../mixed-per-module-preprocess-precision.t | 85 +++++++++++++++++++ .../mixed-per-module-preprocess.t | 83 ++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t new file mode 100644 index 00000000000..04b217edb46 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t @@ -0,0 +1,85 @@ +Reproduction: today, building only the consumer's `.cmo` of an +executable that depends on `mylib` (where `mylib` has two modules, +`a` default-pp and `b` `(staged_pps ...)`) fails on a compile error +in `mylib/b.ml` — even though the consumer references only `A`. +The cctx-wide `.cmi` glob over `mylib`'s objdir pulls `b.cmi` into +the consumer's compile rule, which forces dune to compile `b.ml`, +and `b.ml` contains an unresolvable identifier. + +Companion to `mixed-per-module-preprocess.t` (the soundness sibling). + + $ make_dune_project 3.24 + +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 preprocessing (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 + +The consumer's compile rule tracks the wide glob over `mylib`'s +byte objdir — which materialises `b.cmi` and so forces dune to +compile `b.ml`. + + $ dune rules --root . --format=json --deps '%{cmo:consumer/consumer}' > deps.json + $ jq -r 'include "dune"; .[] | depsGlobs + > | select(.dir | endswith("mylib/.mylib.objs/byte")) + > | .dir + " " + .predicate' < deps.json + _build/default/mylib/.mylib.objs/byte *.cmi + $ jq -r 'include "dune"; .[] | depsFilePaths + > | select(endswith("mylib/.mylib.objs/byte/a.cmi"))' < deps.json + $ jq -r 'include "dune"; .[] | depsFilePaths + > | select(endswith("mylib/.mylib.objs/byte/b.cmi"))' < deps.json + +Build only the consumer's `.cmo` (compile rule, not link). Today, +dune attempts to compile `b.ml` to produce `b.cmi` and fails on +the unresolvable identifier. + + $ dune build '%{cmo:consumer/consumer}' + File "mylib/b.ml", line 1, characters 10-23: + 1 | let bar = no_such_thing + ^^^^^^^^^^^^^ + Error: Unbound value no_such_thing + [1] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t new file mode 100644 index 00000000000..e1438b4ac09 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t @@ -0,0 +1,83 @@ +An unwrapped library has two modules with mixed preprocessing: `a` +uses default preprocessing, `b` uses `(staged_pps ...)`. The +consumer references `A.identity` whose type is `B.t -> B.t`. Pins +that the consumer's compile correctly tracks `b.cmi` as a sandbox- +required dep — even though the consumer never names `B` in source. + + $ make_dune_project 3.24 + +A no-op staged ppx, modelled on +`test/blackbox-tests/test-cases/staged-pps-relative-directory-gh8158.t`. +The driver copies its input verbatim. + + $ 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 `(wrapped false)` with `a` default-pp and `b` staged-pps. +`a`'s interface mentions `B.t`, so the consumer's call to +`A.identity` forces the compiler to load `b.cmi` to resolve the +type. + + $ 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 `A.identity` but never names `B`. The +`--sandbox=copy` build below is the discriminator: if a regression +dropped `b.cmi` from the consumer's compile-rule deps, the sandbox +would not stage it and the build would fail with "no such file" +deterministically rather than passing silently from a stale +`_build/`. + + $ mkdir consumer + $ cat > consumer/dune < (executable (name consumer) (libraries mylib)) + > EOF + $ cat > consumer/consumer.ml < let _ = A.identity 0 + > EOF + + $ dune build --sandbox=copy consumer/consumer.exe + +The consumer's compile rule for `consumer` tracks `mylib`'s byte +objdir — both `a.cmi` (referenced) and `b.cmi` (needed for the +type of `A.identity`). + + $ dune rules --root . --format=json --deps '%{cmo:consumer/consumer}' > deps.json + $ jq -r 'include "dune"; .[] | depsGlobs + > | select(.dir | endswith("mylib/.mylib.objs/byte")) + > | .dir + " " + .predicate' < deps.json + _build/default/mylib/.mylib.objs/byte *.cmi + $ jq -r 'include "dune"; .[] | depsFilePaths + > | select(endswith("mylib/.mylib.objs/byte/a.cmi"))' < deps.json + $ jq -r 'include "dune"; .[] | depsFilePaths + > | select(endswith("mylib/.mylib.objs/byte/b.cmi"))' < deps.json From a57bc393b2117ebd8168b5d80c19e09e5849104a Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 09:58:38 -0700 Subject: [PATCH 2/5] 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 953802c98a60836b338193eba9f8b826b17917c2 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Tue, 19 May 2026 11:36:05 -0700 Subject: [PATCH 3/5] scaffold: pass pps_runtime_libs from cinaps and toplevel stanzas The cinaps and toplevel `Compilation_context.create` call sites omit the new `?pps_runtime_libs` field, leaving it at the default `Resolve.Memo.return []`. Under the per-module narrowing introduced later in the stack, this silently drops ppx runtime libs from `must_glob_libs`, producing missing `.cmi` globs in the stanzas' compile rules. Compute and pass it via the same `Lib.Compile.pps` + `Lib.ppx_runtime_deps` pattern used in `exe_rules.ml` and `lib_rules.ml`. Melange bypasses narrowing (`can_filter = false`) so it keeps the optional default and is unaffected today. Signed-off-by: Robin Bate Boerop --- src/dune_rules/cinaps.ml | 6 ++++++ src/dune_rules/toplevel.ml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/dune_rules/cinaps.ml b/src/dune_rules/cinaps.ml index 99cc3f2e638..7a03ee3351b 100644 --- a/src/dune_rules/cinaps.ml +++ b/src/dune_rules/cinaps.ml @@ -207,6 +207,11 @@ let gen_rules sctx t ~dir ~scope = 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 obj_dir = Obj_dir.make_exe ~dir:cinaps_dir ~name in Compilation_context.create for_ @@ -217,6 +222,7 @@ let gen_rules sctx t ~dir ~scope = ~opaque:(Explicit false) ~requires_compile ~requires_link + ~pps_runtime_libs ~flags ~js_of_ocaml:(Js_of_ocaml.Mode.Pair.make None) ~melange_package_name:None diff --git a/src/dune_rules/toplevel.ml b/src/dune_rules/toplevel.ml index 9d87b090978..bb0392366dd 100644 --- a/src/dune_rules/toplevel.ml +++ b/src/dune_rules/toplevel.ml @@ -236,6 +236,11 @@ module Stanza = struct 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 obj_dir = Source.obj_dir source in Compilation_context.create for_ @@ -246,6 +251,7 @@ module Stanza = struct ~opaque:(Explicit false) ~requires_compile ~requires_link + ~pps_runtime_libs ~flags ~js_of_ocaml:(Js_of_ocaml.Mode.Pair.make None) ~melange_package_name:None From 42983353590641cd8877dbfced62aed37a1d1219 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Tue, 19 May 2026 12:07:01 -0700 Subject: [PATCH 4/5] scaffold: make pps_runtime_libs required on Compilation_context.create The optional argument with default [Resolve.Memo.return []] hid the silent-wrongness Alizter flagged on #14514: a new caller that forgets to pass [~pps_runtime_libs] would silently drop ppx runtime libs from the must-glob set under narrowing. Make the parameter required. The five sites without a meaningful pps_runtime_libs to compute (ppx_driver, utop, mdx, inline_tests, melange) now pass [Resolve.Memo.return []] explicitly, documenting the absence at the call site rather than at the API. Melange compile rules bypass narrowing today (`can_filter = false` in module_compilation.ml), so the explicit empty preserves prior behaviour. utop's pps surface (collected from in-scope libraries) could in principle carry runtime libs; treat this as a separate follow-up rather than papering over via API. Signed-off-by: Robin Bate Boerop --- src/dune_rules/compilation_context.ml | 2 +- src/dune_rules/compilation_context.mli | 2 +- src/dune_rules/inline_tests.ml | 1 + src/dune_rules/mdx.ml | 1 + src/dune_rules/melange/melange_rules.ml | 1 + src/dune_rules/ppx_driver.ml | 1 + src/dune_rules/utop.ml | 1 + 7 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index 85d6c16a43b..69f80538b2f 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -229,7 +229,7 @@ let create ~flags ~requires_compile ~requires_link - ?(pps_runtime_libs = Resolve.Memo.return []) + ~pps_runtime_libs ?(preprocessing = Pp_spec.dummy) ~opaque ~js_of_ocaml diff --git a/src/dune_rules/compilation_context.mli b/src/dune_rules/compilation_context.mli index c917486e0cf..c9fa136a793 100644 --- a/src/dune_rules/compilation_context.mli +++ b/src/dune_rules/compilation_context.mli @@ -27,7 +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 + -> 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 diff --git a/src/dune_rules/inline_tests.ml b/src/dune_rules/inline_tests.ml index aabcff5bf10..a98e72d9f43 100644 --- a/src/dune_rules/inline_tests.ml +++ b/src/dune_rules/inline_tests.ml @@ -313,6 +313,7 @@ include Sub_system.Register_end_point (struct ~opaque:(Explicit false) ~requires_compile:runner_libs ~requires_link:(Memo.lazy_ (fun () -> runner_libs)) + ~pps_runtime_libs:(Resolve.Memo.return []) ~flags ~js_of_ocaml:(Js_of_ocaml.Mode.Pair.map ~f:Option.some js_of_ocaml) ~melange_package_name:None diff --git a/src/dune_rules/mdx.ml b/src/dune_rules/mdx.ml index 2a077ee2b74..86b31c39ef0 100644 --- a/src/dune_rules/mdx.ml +++ b/src/dune_rules/mdx.ml @@ -496,6 +496,7 @@ let mdx_prog_gen t ~sctx ~dir ~scope ~mdx_prog = ~flags ~requires_compile ~requires_link + ~pps_runtime_libs:(Resolve.Memo.return []) ~opaque:(Explicit false) ~js_of_ocaml:(Js_of_ocaml.Mode.Pair.make None) ~melange_package_name:None diff --git a/src/dune_rules/melange/melange_rules.ml b/src/dune_rules/melange/melange_rules.ml index e8a8e14e2b0..16badf3e5e4 100644 --- a/src/dune_rules/melange/melange_rules.ml +++ b/src/dune_rules/melange/melange_rules.ml @@ -548,6 +548,7 @@ let setup_emit_cmj_rules ~flags ~requires_link ~requires_compile:direct_requires + ~pps_runtime_libs:(Resolve.Memo.return []) ~preprocessing:pp ~js_of_ocaml:(Js_of_ocaml.Mode.Pair.make None) ~opaque:Inherit_from_settings diff --git a/src/dune_rules/ppx_driver.ml b/src/dune_rules/ppx_driver.ml index 341ddfbbbdf..e649cded566 100644 --- a/src/dune_rules/ppx_driver.ml +++ b/src/dune_rules/ppx_driver.ml @@ -320,6 +320,7 @@ let build_ppx_driver = ~flags ~requires_compile:(Memo.return requires_compile) ~requires_link + ~pps_runtime_libs:(Resolve.Memo.return []) ~opaque ~js_of_ocaml:(Js_of_ocaml.Mode.Pair.make None) ~melange_package_name:None diff --git a/src/dune_rules/utop.ml b/src/dune_rules/utop.ml index 0ea31cbccc1..de51531ef05 100644 --- a/src/dune_rules/utop.ml +++ b/src/dune_rules/utop.ml @@ -247,6 +247,7 @@ let setup sctx ~dir = ~opaque:(Explicit false) ~requires_link ~requires_compile:requires + ~pps_runtime_libs:(Resolve.Memo.return []) ~flags ~js_of_ocaml:(Js_of_ocaml.Mode.Pair.make None) ~melange_package_name:None From 63c36512e3ad727000b2a0d27850fa7a40541c78 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:05:38 -0700 Subject: [PATCH 5/5] 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 69f80538b2f..e9bd6ae1a3e 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 fc5bcee741c..2559b1cf58a 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"