diff --git a/doc/changes/added/14492.md b/doc/changes/added/14492.md new file mode 100644 index 00000000000..58fa028907e --- /dev/null +++ b/doc/changes/added/14492.md @@ -0,0 +1,7 @@ +- Filter inter-library dependencies per-module using `ocamldep` output, and + match the `-I`/`-H` include flags to that filter. When a dependency + library's cmi changes, only consumer modules that reference it get rebuilt + — eliminating spurious recompilations of unrelated sibling modules. + Applies to unwrapped dependency libraries; wrapped libraries continue to + use a glob over the whole library's objdir. (#14492, fixes #4572, + @robinbb) diff --git a/src/dune_rules/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/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/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/module_compilation.ml b/src/dune_rules/module_compilation.ml index e9ac37bd300..a6a6e95aa13 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -26,18 +26,38 @@ let module_kind_is_filterable m = [lookup_tight_entries], terminating chains through them. The [Module.t] supplied here is the post-pp form (constructed in [build_lib_index]), so ocamldep runs on the dep lib's [.pp.ml] (action / non-staged Pps) or directly - on the source (no preprocessing / future-syntax). *) -let cross_lib_tight_set ~sandbox ~sctx ~lib_index ~initial_refs = + on the source (no preprocessing / future-syntax). + + Each visited lib's stanza [(flags (... -open Foo))] also extends the + frontier: a dep lib whose stanza opens [Foo] can use [Foo]'s identifiers + without naming [Foo] in its source, so ocamldep on the dep lib's source + produces no token to walk through. The stanza-open names are the missing + edges. *) +let cross_lib_tight_set ~sandbox ~sctx ~lib_index ~mode ~initial_refs = let open Action_builder.O in + let read_stanza_opens lib = + let info = Lib.info lib in + let spec = Lib_info.stanza_flags info in + if Dune_lang.Ocaml_flags.Spec.equal spec Dune_lang.Ocaml_flags.Spec.standard + then Action_builder.return Module_name.Set.empty + else ( + let dir = Lib_info.src_dir info |> Path.as_in_build_dir_exn in + let* ocaml_flags = + Action_builder.of_memo (Ocaml_flags_db.ocaml_flags sctx ~dir spec) + in + let+ flag_strings = Ocaml_flags.get ocaml_flags mode in + Ocaml_flags.extract_open_module_names flag_strings) + in let read_entry_deps (lib, m) = let obj_dir = Lib.info lib |> Lib_info.obj_dir |> Obj_dir.as_local_exn in let* impl_deps = Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Impl m in - let+ intf_deps = + let* intf_deps = Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf m in - Module_name.Set.union impl_deps intf_deps + let+ stanza_opens = read_stanza_opens lib in + Module_name.Set.union impl_deps (Module_name.Set.union intf_deps stanza_opens) in let rec loop ~seen ~frontier = if Module_name.Set.is_empty frontier @@ -85,103 +105,149 @@ let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kin Action_builder.return (cctx_includes_for_cm_kind (), Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs) else - let* lib_index = Resolve.Memo.read (Compilation_context.lib_index cctx) in - let sandbox = Compilation_context.sandbox cctx in - let sctx = Compilation_context.super_context cctx in - let* trans_deps = Dep_graph.deps_of dep_graph m in - (* Read [dep_m]'s [.ml]-side ocamldep only when its references can - propagate to the consumer: - - | [dep_m] is | [cm_kind] | [opaque] | read [.ml]? | - | ----------------------- | ----------- | -------- | ------------ | - | consumer ([m] itself) | any | any | iff [Impl] | - | trans_dep, no [.mli] | any | any | yes | - | trans_dep, has [.mli] | [Cmx] | false | yes (inline) | - | trans_dep, has [.mli] | [Cmx] | true | no | - | trans_dep, has [.mli] | [Cmi]/[Cmo] | any | no | *) - let need_impl_deps_of dep_m ~is_consumer = - if is_consumer - then ( - match ml_kind with - | Ml_kind.Impl -> true - | Intf -> false) - else - (not (Module.has dep_m ~ml_kind:Intf)) - || - match cm_kind with - | Ocaml Cmx -> not opaque - | Ocaml (Cmi | Cmo) | Melange _ -> false + let* has_virtual_impl = + Resolve.Memo.read (Compilation_context.has_virtual_impl cctx) in - let read_dep_m_raw dep_m ~is_consumer = - let* impl_deps = - if need_impl_deps_of dep_m ~is_consumer - then - Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Impl dep_m - else Action_builder.return Module_name.Set.empty + if has_virtual_impl + then + Action_builder.return + (cctx_includes_for_cm_kind (), Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs) + else + let* lib_index = Resolve.Memo.read (Compilation_context.lib_index cctx) in + let sandbox = Compilation_context.sandbox cctx in + let sctx = Compilation_context.super_context cctx in + let* trans_deps = Dep_graph.deps_of dep_graph m in + (* Read [dep_m]'s [.ml]-side ocamldep only when its references can + propagate to the consumer: + + | [dep_m] is | [cm_kind] | [opaque] | read [.ml]? | + | ----------------------- | ----------- | -------- | ------------ | + | consumer ([m] itself) | any | any | iff [Impl] | + | trans_dep, no [.mli] | any | any | yes | + | trans_dep, has [.mli] | [Cmx] | false | yes (inline) | + | trans_dep, has [.mli] | [Cmx] | true | no | + | trans_dep, has [.mli] | [Cmi]/[Cmo] | any | no | *) + let need_impl_deps_of dep_m ~is_consumer = + if is_consumer + then ( + match ml_kind with + | Ml_kind.Impl -> true + | Intf -> false) + else + (not (Module.has dep_m ~ml_kind:Intf)) + || + match cm_kind with + | Ocaml Cmx -> not opaque + | Ocaml (Cmi | Cmo) | Melange _ -> false in - let+ intf_deps = - Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf dep_m + let read_dep_m_raw dep_m ~is_consumer = + let* impl_deps = + if need_impl_deps_of dep_m ~is_consumer + then + Ocamldep.read_immediate_deps_raw_of + ~sandbox + ~sctx + ~obj_dir + ~ml_kind:Impl + dep_m + else Action_builder.return Module_name.Set.empty + in + let+ intf_deps = + Ocamldep.read_immediate_deps_raw_of ~sandbox ~sctx ~obj_dir ~ml_kind:Intf dep_m + in + Module_name.Set.union impl_deps intf_deps in - Module_name.Set.union impl_deps intf_deps - in - let* m_raw = read_dep_m_raw m ~is_consumer:true in - let* trans_raw = - union_module_name_sets_mapped trans_deps ~f:(read_dep_m_raw ~is_consumer:false) - in - let all_raw = Module_name.Set.union m_raw trans_raw in - let* flags = Ocaml_flags.get (Compilation_context.flags cctx) mode in - let open_modules = Ocaml_flags.extract_open_module_names flags in - let referenced = Module_name.Set.union all_raw open_modules in - let { Lib_file_deps.Lib_index.tight; non_tight } = - Lib_file_deps.Lib_index.filter_libs_with_modules - lib_index - ~referenced_modules:referenced - in - let direct_libs = - List.sort_uniq ~compare:Lib.compare (Lib.Map.keys tight @ Lib.Set.to_list non_tight) - in - (* Close transitively over transparent aliases that ocamldep doesn't - report. *) - let* all_libs = Resolve.Memo.read (Lib.closure direct_libs ~linking:false ~for_) in - let* tight_set = - cross_lib_tight_set ~sandbox ~sctx ~lib_index ~initial_refs:referenced - in - (* Classify each lib in [all_libs]: - lib has [None]-entry referenced - (mixed-entry or wrapped) → glob (covers the None entries' [.cmi]s); - - lib has only [Some] entries referenced → per-module deps; - lib - unreached but tight-eligible → drop (link rule pulls it in via - [requires_link]); - lib unreached and not tight-eligible → glob. - [kept_libs] gets every lib that contributes a tight or glob dep — used - by [Compilation_context.filtered_include_flags] to scope the consumer's - [-I]/[-H] flags. *) - let { Lib_file_deps.Lib_index.tight = tight_modules; non_tight = non_tight_set } = - Lib_file_deps.Lib_index.filter_libs_with_modules - lib_index - ~referenced_modules:tight_set - in - let tight_deps, glob_libs, _kept_libs = - List.fold_left - all_libs - ~init:(Dep.Set.empty, [], Lib.Set.empty) - ~f:(fun (td, gl, kl) lib -> - if Lib.Set.mem non_tight_set lib - then td, lib :: gl, Lib.Set.add kl lib - else ( - match Lib.Map.find tight_modules lib with - | Some modules -> - ( Dep.Set.union - td - (Lib_file_deps.deps_of_entry_modules ~opaque ~cm_kind lib modules) - , gl - , Lib.Set.add kl lib ) - | None -> - if Lib_file_deps.Lib_index.is_tight_eligible lib_index lib - then td, gl, kl - else td, lib :: gl, Lib.Set.add kl lib)) - in - let glob_deps = Lib_file_deps.deps_of_entries ~opaque ~cm_kind glob_libs in - Action_builder.return - (cctx_includes_for_cm_kind (), Dep.Set.union tight_deps glob_deps) + let* m_raw = read_dep_m_raw m ~is_consumer:true in + let* trans_raw = + union_module_name_sets_mapped trans_deps ~f:(read_dep_m_raw ~is_consumer:false) + in + let all_raw = Module_name.Set.union m_raw trans_raw in + let* flags = Ocaml_flags.get (Compilation_context.flags cctx) mode in + let open_modules = Ocaml_flags.extract_open_module_names flags in + let referenced = Module_name.Set.union all_raw open_modules in + let { Lib_file_deps.Lib_index.tight; non_tight } = + Lib_file_deps.Lib_index.filter_libs_with_modules + lib_index + ~referenced_modules:referenced + in + (* [ppx_runtime_libraries] introduce module references through post-pp + source that ocamldep cannot see; carry them through to [all_libs] so + the classification fold sees them, and force them onto the glob path + via [must_glob_set]. *) + let* pps_runtime_libs = + Resolve.Memo.read (Compilation_context.pps_runtime_libs cctx) + in + (* [Lib.closure]'s memo key is order- and multiplicity-sensitive on the + input list. [pps_runtime_libs] can both contain duplicates (multiple + pps sharing a runtime dep) and overlap with [tight]/[non_tight] (a lib + referenced both syntactically and via [add_pp_runtime_deps]). + [sort_uniq] keeps the input canonical for memoization. *) + let direct_libs = + List.sort_uniq + ~compare:Lib.compare + (Lib.Map.keys tight @ Lib.Set.to_list non_tight @ pps_runtime_libs) + in + (* Close transitively over transparent aliases that ocamldep doesn't + report. *) + let* all_libs = Resolve.Memo.read (Lib.closure direct_libs ~linking:false ~for_) in + (* Wrapped-lib soundness recovery: when [referenced] names a wrapped local + lib's wrapper, the consumer may reach the lib's transitive closure + through aliases the cross-library walk cannot see; glob that closure + unconditionally. *) + let wrapped_referenced = + Lib_file_deps.Lib_index.wrapped_libs_referenced + lib_index + ~referenced_modules:referenced + in + let* must_glob_libs = + Resolve.Memo.read + (Lib.closure + (List.sort_uniq + ~compare:Lib.compare + (Lib.Set.to_list wrapped_referenced @ pps_runtime_libs)) + ~linking:false + ~for_) + in + let must_glob_set = Lib.Set.of_list must_glob_libs in + let* tight_set = + cross_lib_tight_set ~sandbox ~sctx ~lib_index ~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 + Action_builder.return + (cctx_includes_for_cm_kind (), Dep.Set.union tight_deps glob_deps) ;; let lib_cm_deps ~cctx ~cm_kind ~ml_kind ~mode m = diff --git a/src/dune_rules/modules.ml b/src/dune_rules/modules.ml index eed14fdbd33..8b5a6e07914 100644 --- a/src/dune_rules/modules.ml +++ b/src/dune_rules/modules.ml @@ -670,6 +670,12 @@ module Wrapped = struct let lib_interface t = Group.lib_interface t.group + (* Entry modules visible to consumers of a wrapped library: the wrapper + itself, plus any [wrapped_compat] shims (present for + [(wrapped (transition ...))] libraries, which expose bare module names in + addition to qualified ones). *) + let entry_modules t = lib_interface t :: Module_name.Map.values t.wrapped_compat + let fold_user_available { group; toplevel_module; _ } ~init ~f = let init = match toplevel_module with @@ -1011,7 +1017,7 @@ let entry_modules t = | Unwrapped m -> Unwrapped.entry_modules m | Wrapped m -> (* we assume this is never called for implementations *) - [ Wrapped.lib_interface m ]) + Wrapped.entry_modules m) ;; module With_vlib = struct diff --git a/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/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 index 4c5eb7bfaf6..d82dacbc90e 100644 --- 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 @@ -1,7 +1,9 @@ -Regression baseline for an unwrapped library declaring +Per-module narrowing on an unwrapped library declaring `(instrumentation (backend X))` without `--instrument-with` at -build time. Today, the consumer's compile rule depends on the -lib's full `.cmi` glob over its objdir. +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 @@ -41,18 +43,16 @@ no `.pp.ml` files are produced. > let _ = Middle.identity 0 > EOF -Build without `--instrument-with`. Today this succeeds with the -cctx-wide `.cmi` glob covering `leaf.cmi` through `middle`'s objdir -chain. +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 today carries a wide `.cmi` glob over -`middle`'s objdir. +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 - _build/default/middle/.middle.objs/byte *.cmi 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 index eb141584cdf..67c1008fccc 100644 --- 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 @@ -1,10 +1,10 @@ -Reproduction: today, building only the consumer's `.cmo` of an -executable that depends on `mylib` (where `mylib` has two modules, -`a` default-pp and `b` `(staged_pps ...)`) fails on a compile error -in `mylib/b.ml` — even though the consumer references only `A`. -The cctx-wide `.cmi` glob over `mylib`'s objdir pulls `b.cmi` into -the consumer's compile rule, which forces dune to compile `b.ml`, -and `b.ml` contains an unresolvable identifier. +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). @@ -59,9 +59,9 @@ it will fail. > let () = print_int A.answer > EOF -The consumer's compile rule tracks the wide glob over `mylib`'s -byte objdir — which materialises `b.cmi` and so forces dune to -compile `b.ml`. +The consumer's compile rule tracks only `a.cmi` from `mylib`'s +byte objdir — narrowing dropped the wide glob, so `b.cmi` is not +materialised and `b.ml` is not compiled. $ dune rules --root . --format=json --deps '%{cmo:consumer/consumer}' > deps.json $ jq -r 'include "dune"; .[] | depsGlobs @@ -73,8 +73,7 @@ compile `b.ml`. $ jq -r 'include "dune"; .[] | depsFilePaths > | select(endswith("mylib/.mylib.objs/byte/b.cmi"))' < deps.json -Build only the consumer's `.cmo` (compile rule, not link). Today, -dune attempts to compile `b.ml` to produce `b.cmi` and fails on -the unresolvable identifier. +Build only the consumer's `.cmo` (compile rule, not link). The +narrowed dep set means `b.ml` is not compiled. $ dune build '%{cmo:consumer/consumer}'