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/doc/dev/per-module-narrowing.md b/doc/dev/per-module-narrowing.md new file mode 100644 index 00000000000..340453e90fe --- /dev/null +++ b/doc/dev/per-module-narrowing.md @@ -0,0 +1,626 @@ +# 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 ~mode ~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)`, 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 +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. + +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. + +## 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). +- 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). + +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, 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)`. +- **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. 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/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/compilation_context.ml b/src/dune_rules/compilation_context.ml index de6a2e4cfc1..5ac5e1d9ab4 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -4,61 +4,138 @@ 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 }) } ;; 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 + [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 @@ -91,7 +168,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 @@ -104,6 +184,8 @@ type t = ; loc : Loc.t option ; ocaml : Ocaml_toolchain.t ; for_ : Compilation_mode.t + ; filtered_includes : Filtered_includes.t + ; raw_refs : Raw_refs.t } let loc t = t.loc @@ -118,7 +200,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 +232,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 +308,7 @@ let create ~flags ~requires_compile ~requires_link + ~pps_runtime_libs ?(preprocessing = Pp_spec.dummy) ~opaque ~js_of_ocaml @@ -210,6 +364,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) @@ -219,6 +386,7 @@ let create ~impl:implements ~modules ~for_ + ~has_library_deps and+ bin_annot = match bin_annot with | Some b -> Memo.return b @@ -244,9 +412,21 @@ 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 + 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 @@ -260,11 +440,54 @@ let create ; ocaml ; 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 + 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 = let extra = [ "-w"; "-49" ] in fun base -> Ocaml_flags.append_common base extra |> Ocaml_flags.append_nostdlib @@ -337,7 +560,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 @@ -347,7 +569,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..ac1b8bb00cf 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,58 @@ 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 + +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 + 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 + 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/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/dune_package.ml b/src/dune_rules/dune_package.ml index f7cc7ed5788..d656e9c1e64 100644 --- a/src/dune_rules/dune_package.ml +++ b/src/dune_rules/dune_package.ml @@ -353,6 +353,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/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/findlib.ml b/src/dune_rules/findlib.ml index 6f5ad839281..fa9ed157421 100644 --- a/src/dune_rules/findlib.ml +++ b/src/dune_rules/findlib.ml @@ -273,6 +273,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/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/lib.ml b/src/dune_rules/lib.ml index a0b2cdd0cd2..ccd1ecb9893 100644 --- a/src/dune_rules/lib.ml +++ b/src/dune_rules/lib.ml @@ -2201,11 +2201,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 diff --git a/src/dune_rules/lib_file_deps.ml b/src/dune_rules/lib_file_deps.ml index 13048f499e4..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 = @@ -118,8 +117,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..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 @@ -42,9 +40,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_info.ml b/src/dune_rules/lib_info.ml index 7dd42bf21f5..7cbe29ea3ca 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/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/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/module_compilation.ml b/src/dune_rules/module_compilation.ml index fc5bcee741c..b9b33055363 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -1,6 +1,287 @@ 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 +;; + +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). + + 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 = + Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf m + in + 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 + 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 + 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* 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 + 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 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: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 + (* [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 ~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); - + 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 + 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 = + 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 +562,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 +702,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 +818,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 +838,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" diff --git a/src/dune_rules/modules.ml b/src/dune_rules/modules.ml index 6a42fed4e9a..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 @@ -918,6 +924,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 @@ -1005,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/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 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/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/stanzas/library.ml b/src/dune_rules/stanzas/library.ml index 99e25dc6343..ad21a03e3f2 100644 --- a/src/dune_rules/stanzas/library.ml +++ b/src/dune_rules/stanzas/library.ml @@ -644,6 +644,7 @@ let to_lib_info ~jsoo_runtime ~wasmoo_runtime ~preprocess + ~stanza_flags:conf.buildable.flags ~enabled ~virtual_deps ~dune_version 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 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 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/melange/melange-conditional-modules.t b/test/blackbox-tests/test-cases/melange/melange-conditional-modules.t index 277e3bd43bd..98bae15313a 100644 --- a/test/blackbox-tests/test-cases/melange/melange-conditional-modules.t +++ b/test/blackbox-tests/test-cases/melange/melange-conditional-modules.t @@ -33,8 +33,9 @@ Show `(melange.modules ..)` subset is preferred when compiling in Melange mode ocamldep (internal) melc lib/.a.objs/melange/a.{cmi,cmj,cmt} ocamldep (internal) - ocamlc lib/.a.objs/byte/a__Helper.{cmi,cmo,cmt} + ocamldep (internal) melc lib/.a.objs/melange/a__Foo.{cmi,cmj,cmt} + ocamlc lib/.a.objs/byte/a__Helper.{cmi,cmo,cmt} ocamlc lib/.a.objs/byte/a__Foo.{cmi,cmo,cmt} ocamlc lib/a.cma Leaving directory 'a' 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/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/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..6d6cae41e94 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cmx-native-tight-deps.t @@ -0,0 +1,63 @@ +A `consumer` library that references a module of an unwrapped +local dep, compiled under `--profile release`, builds correctly +including the native-only `.cmx` artefacts. Pins the cmx-deps +code path as a regression guard — the default `dev` profile has +`opaque = true` and the native compile of consumer modules skips +the `.cmx` branch entirely; `--profile release` flips `opaque` off +so the cmx branch fires and the consumer's `.cmxa` builds. + + $ make_dune_project 3.24 + +`dep_lib`: an unwrapped library with two modules, one of which +has both an `.ml` and a `.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`: 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 + +The consumer's native compile rule depends on specific `.cmi` and +`.cmx` file deps of the referenced module `M` in `dep_lib`, with +no wide globs over the byte/native objdirs. The presence of the +`m.cmx` file dep pins that the `want_cmx = true` branch fires +under release profile. + + $ dune rules --root . --format=json --profile release --deps '%{cmx:consumer/c}' > deps.json + $ jq -r 'include "dune"; .[] | depsGlobs + > | select(.dir | endswith("dep_lib/.dep_lib.objs/byte")) + > | .dir + " " + .predicate' < deps.json + $ jq -r 'include "dune"; .[] | depsGlobs + > | select(.dir | endswith("dep_lib/.dep_lib.objs/native")) + > | .dir + " " + .predicate' < deps.json + $ jq -r 'include "dune"; .[] | depsFilePaths + > | select(endswith("dep_lib/.dep_lib.objs/byte/m.cmi"))' < deps.json + _build/default/dep_lib/.dep_lib.objs/byte/m.cmi + $ jq -r 'include "dune"; .[] | depsFilePaths + > | select(endswith("dep_lib/.dep_lib.objs/native/m.cmx"))' < deps.json + _build/default/dep_lib/.dep_lib.objs/native/m.cmx 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-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..fcd12b3a8a1 --- /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 3.24) + +(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..d82dacbc90e --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-instrumentation-barrier.t/run.t @@ -0,0 +1,58 @@ +Per-module narrowing on an unwrapped library declaring +`(instrumentation (backend X))` without `--instrument-with` at +build time. The narrowing must (a) not demand non-existent +`.pp.ml` files from the instrumentation-disabled lib, and (b) +emit per-module deps instead of the cctx-wide `.cmi` glob over +`middle`'s objdir. + + $ make_dune_project 3.24 + +`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)) + > 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 narrowing produces per- +module narrow deps on the consumer's compile rule. + + $ dune build consumer/consumer.exe + +The consumer's compile rule has no glob entry over `middle`'s +objdir. + + $ dune rules --root . --format=json --deps '%{cmo:consumer/consumer}' > 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-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..741a60dec8e --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-open-flag-barrier.t @@ -0,0 +1,54 @@ +A consumer references a constructor of a leaf library's type +through an intermediate library compiled with +`(flags (:standard -open Prelude))`. The intermediate's source +never names `Prelude` (the open hides it), and the consumer never +names `Prelude` either. Pins that the consumer's compile correctly +tracks `prelude`'s `.cmi` as a sandbox-required dep. + + $ make_dune_project 3.24 + +`prelude` exposes a sum type: + + $ mkdir prelude + $ cat > prelude/dune < (library (name prelude)) + > EOF + $ cat > prelude/prelude.ml < type color = Red | Green | Blue + > EOF + +`middle` depends on `prelude` and is compiled with +`(flags (:standard -open Prelude))`, exposing +`val pick : unit -> color` whose `color` resolves through the open +to `Prelude.color`: + + $ mkdir middle + $ cat > middle/dune < (library + > (name middle) + > (libraries prelude) + > (flags (:standard -open Prelude))) + > EOF + $ cat > middle/middle.mli < val pick : unit -> color + > EOF + $ cat > middle/middle.ml < let pick () = Green + > EOF + +`consumer` depends on `middle` and `prelude` and pattern-matches on +the result of `Middle.pick` against the bare constructors `Green`, +`Red`, `Blue`. `ocamldep` on `middle.{ml,mli}` and `consumer.ml` +reports no `Prelude` token in either case. + + $ mkdir consumer + $ 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 + + $ dune build --sandbox=copy consumer/consumer.exe diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-pps-runtime-no-ocamldep-barrier.t b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-pps-runtime-no-ocamldep-barrier.t new file mode 100644 index 00000000000..70e9245c3ba --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-pps-runtime-no-ocamldep-barrier.t @@ -0,0 +1,66 @@ +A single-module local library with no `(libraries ...)` but with +`(preprocess (pps X))` has non-empty *resolved* requires (X's +runtime libs added via `add_pp_runtime_deps`). Pins that the +consumer's compile correctly tracks the ppx runtime lib's `.cmi` +as a sandbox-required dep, even though the consumer never names +the runtime lib syntactically. + + $ make_dune_project 3.24 + +`hello` is the ppx runtime lib (single-module unwrapped, no library +deps of its own). It exposes `Hello.t`: + + $ mkdir hello + $ cat > hello/dune < (library (name hello)) + > 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) + > (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 + + $ 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..d9abc5028e5 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-preprocess-barrier.t @@ -0,0 +1,50 @@ +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. Pins that the consumer's +compile correctly tracks `leaf`'s `.cmi` as a sandbox-required dep. + + $ make_dune_project 3.24 + +`leaf` exposes `Leaf.t`: + + $ mkdir leaf + $ cat > leaf/dune < (library (name leaf)) + > 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) + > (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. 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 reports +only `Middle`, since `Leaf` is not named. + + $ mkdir consumer + $ cat > consumer/dune < (executable (name consumer) (libraries middle)) + > EOF + $ cat > consumer/consumer.ml < let _ = Middle.identity 0 + > EOF + + $ dune build --sandbox=copy consumer/consumer.exe 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/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..95cebf92fa5 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess-precision.t @@ -0,0 +1,66 @@ +Precision regression guard for per-pair tight-eligibility on an +unwrapped library with mixed per-module preprocessing. When a +consumer references only the default-pp module of a mixed-pp lib, +the per-module narrowing excludes the staged-pps module from the +consumer's compile-rule deps. Asserted by giving the staged-pps +module an unresolvable identifier; if the narrowing regressed, +dune would compile that module and fail. + +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 + +Build only the consumer's `.cmo` (compile rule, not link). The +narrowing excludes `b.cmi` from the deps, so dune does not attempt +to compile `b.ml`. + + $ dune build '%{cmo:consumer/consumer}' 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..72cab9a4bee --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/mixed-per-module-preprocess.t @@ -0,0 +1,69 @@ +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 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/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" ] 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..b313668f5ab --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/singleton-with-requires.t @@ -0,0 +1,57 @@ +A multi-module consumer depends on a `(wrapped false)` singleton +library `bridge` that itself has `(libraries leaf)`. The singleton's +module exposes a value typed `Mod_leaf.t`; the consumer reads it +through `Mod_bridge.v` without naming `Mod_leaf` in source. Pins +that this transitive dependency shape builds. + +Sister to `no-ocamldep-leaf-lib.t`, which covers the +singleton-with-no-requires case. + + $ make_dune_project 3.24 + +`leaf`: transitive dep reached only through `bridge`. 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. Only `uses_bridge` names `Mod_bridge`; +`leaf` reaches the consumer's compile scope only via `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 index 1994c06f0a2..0514de0dace 100644 --- 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 @@ -1,11 +1,13 @@ -Regression baseline for the cctx-wide `.cmi` glob over an unwrapped -library whose module set is declared explicitly via `(modules ...)`. -Today, editing one sibling module's `.mli` invalidates every -sibling's compile rule — even siblings that don't reference it. +Per-module narrowing on a `(wrapped false)` library whose module +set is declared explicitly via `(modules ...)`. Under the per- +module narrowing, editing one sibling module's `.mli` invalidates +only modules that reference it — siblings that don't are +unaffected. Matches the implicit-discovery behaviour covered by +`lib-to-lib-unwrapped.t`, but the explicit `(modules ...)` clause +routes through a different parse path; this test pins the +equivalence observationally. -Matches the shape raised in #14492's review feedback. The explicit -`(modules ...)` clause routes through a different parse path from -the implicit form covered by `lib-to-lib-unwrapped.t`. +Matches the shape raised in #14492's review feedback. $ make_dune_project 3.24 @@ -54,29 +56,28 @@ references `A` only; `uses_b` references `B` only. $ dune build @check -Each consumer compile rule carries a wide `.cmi` glob over `foo`'s -objdir; no specific cmi from `foo` appears as a file dep. The wide -glob is the structural cause of the sibling over-rebuilds asserted -below. +Each consumer compile rule depends on only the specific cmi it +references — `uses_a` on `a.cmi`, `uses_b` on `b.cmi` — with no +wide glob over `foo`'s objdir. $ dune rules --root . --format=json --deps '%{cmo:consumer/uses_a}' > deps_a.json $ dune rules --root . --format=json --deps '%{cmo:consumer/uses_b}' > deps_b.json $ jq -r 'include "dune"; .[] | depsGlobs > | select(.dir | endswith("foo/.foo.objs/byte")) > | .dir + " " + .predicate' < deps_a.json - _build/default/foo/.foo.objs/byte *.cmi $ jq -r 'include "dune"; .[] | depsGlobs > | select(.dir | endswith("foo/.foo.objs/byte")) > | .dir + " " + .predicate' < deps_b.json - _build/default/foo/.foo.objs/byte *.cmi $ jq -r 'include "dune"; .[] | depsFilePaths > | select(endswith("foo/.foo.objs/byte/a.cmi"))' < deps_a.json + _build/default/foo/.foo.objs/byte/a.cmi $ jq -r 'include "dune"; .[] | depsFilePaths > | select(endswith("foo/.foo.objs/byte/b.cmi"))' < deps_b.json + _build/default/foo/.foo.objs/byte/b.cmi Case 1: edit `A`'s interface to expose `extra`. `uses_b` references -only `B`, but the cctx-wide `.cmi` glob over `foo`'s objdir includes -`A.cmi`, so today `uses_b.cm*` rebuilds anyway. +only `B`, so the per-module narrowing must drop `A.cmi` from +`uses_b`'s dep set — `uses_b.cm*` must not rebuild. $ cat > foo/a.mli < val v : int @@ -84,18 +85,10 @@ only `B`, but the cctx-wide `.cmi` glob over `foo`'s objdir includes > EOF $ dune build @check $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatching("uses_b.cm")]' - [ - { - "target_files": [ - "_build/default/consumer/.consumer.objs/byte/uses_b.cmi", - "_build/default/consumer/.consumer.objs/byte/uses_b.cmo", - "_build/default/consumer/.consumer.objs/byte/uses_b.cmt" - ] - } - ] + [] -Case 2: edit `B`'s interface to expose `extra`. Symmetric — today -`uses_a.cm*` rebuilds anyway. +Case 2: edit `B`'s interface to expose `extra`. Symmetric — +`uses_a.cm*` must not rebuild. $ cat > foo/b.mli < val v : int @@ -103,12 +96,4 @@ Case 2: edit `B`'s interface to expose `extra`. Symmetric — today > EOF $ dune build @check $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatching("uses_a.cm")]' - [ - { - "target_files": [ - "_build/default/consumer/.consumer.objs/byte/uses_a.cmi", - "_build/default/consumer/.consumer.objs/byte/uses_a.cmo", - "_build/default/consumer/.consumer.objs/byte/uses_a.cmt" - ] - } - ] + [] 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-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-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/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" + ] + } + ] 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...