From 8634a3050f923cb25b5aa618f19651e539fdffaa Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 13 May 2026 10:22:53 -0700 Subject: [PATCH] feat: per-module [-I]/[-H] include flags via [kept_libs] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Compilation_context.filtered_include_flags]: new function returning the [-I]/[-H] flags restricted to [kept_libs]. The cctx's [requires_compile] and [requires_hidden] are each filtered by [Lib.Set.mem kept_libs]; the result is built as a single [Command.Args.t] under [Action_builder]. No caching yet — each call recomputes; a follow-up adds the cache. [Module_compilation.lib_deps_for_module]: the tight branch was already threading [kept_libs] through the classification fold (it had been unused at L4-L5). Now wired into [filtered_include_flags]; the returned pair is [(filtered_include_flags, tight_deps + glob_deps)] instead of [(cctx_includes_for_cm_kind (), …)]. Behavioural effect: a consumer module's compile command sees [-I] / [-H] only for libraries its ocamldep reference set actually reaches. Adding an unreferenced sibling to the cctx's [(libraries ...)] no longer changes the consumer module's compile command, so the rule does not re-execute. Tests: - [per-module-include-flags.t]: promoted — [-I] for the unreferenced [unrelated_lib] no longer appears in the consumer's compile rule. - [add-unreferenced-sibling-lib.t]: promoted — adding an unreferenced sibling lib produces no rebuild for consumer modules. Signed-off-by: Robin Bate Boerop --- src/dune_rules/compilation_context.ml | 18 +++++++++ src/dune_rules/compilation_context.mli | 11 ++++++ src/dune_rules/module_compilation.ml | 8 ++-- .../add-unreferenced-sibling-lib.t | 39 +++++-------------- .../per-module-include-flags.t | 21 ++++------ 5 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index 6de2be794e4..defe044df9d 100644 --- a/src/dune_rules/compilation_context.ml +++ b/src/dune_rules/compilation_context.ml @@ -340,6 +340,24 @@ let create let for_ t = t.for_ +let filtered_include_flags t ~cm_kind ~kept_libs = + let lib_mode = Lib_mode.of_cm_kind cm_kind in + let open Action_builder.O in + let* direct_requires = Resolve.Memo.read t.requires_compile in + let+ hidden_requires = Resolve.Memo.read t.requires_hidden in + let direct_filtered = List.filter direct_requires ~f:(Lib.Set.mem kept_libs) in + let hidden_filtered = List.filter hidden_requires ~f:(Lib.Set.mem kept_libs) in + let project = Scope.project t.scope in + let lib_config = t.ocaml.lib_config in + Lib_flags.L.include_flags + ~project + ~direct_libs:direct_filtered + ~hidden_libs:hidden_filtered + lib_mode + lib_config + |> Command.Args.memo +;; + let alias_and_root_module_flags = let extra = [ "-w"; "-49" ] in fun base -> Ocaml_flags.append_common base extra |> Ocaml_flags.append_nostdlib diff --git a/src/dune_rules/compilation_context.mli b/src/dune_rules/compilation_context.mli index c917486e0cf..1075762651d 100644 --- a/src/dune_rules/compilation_context.mli +++ b/src/dune_rules/compilation_context.mli @@ -63,6 +63,17 @@ val requires_hidden : t -> Lib.t list Resolve.Memo.t val requires_compile : t -> Lib.t list Resolve.Memo.t val parameters : t -> Module_name.t list Resolve.Memo.t val includes : t -> Command.Args.without_targets Command.Args.t Lib_mode.Cm_kind.Map.t + +(** Include flags ([-I]/[-H]) for compiling a module against [kept_libs]. The + cctx's [requires_compile] and [requires_hidden] are each restricted to + libraries in [kept_libs]; the kept direct entries become [-I], the kept + hidden entries become [-H]. *) +val filtered_include_flags + : t + -> cm_kind:Lib_mode.Cm_kind.t + -> kept_libs:Lib.Set.t + -> Command.Args.without_targets Command.Args.t Action_builder.t + val lib_index : t -> Lib_file_deps.Lib_index.t Resolve.Memo.t (** [true] iff any library in the compilation context's direct or hidden diff --git a/src/dune_rules/module_compilation.ml b/src/dune_rules/module_compilation.ml index 501cd31d955..109b586ca0a 100644 --- a/src/dune_rules/module_compilation.ml +++ b/src/dune_rules/module_compilation.ml @@ -205,7 +205,7 @@ let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kin lib_index ~referenced_modules:tight_set in - let tight_deps, glob_libs, _kept_libs = + let tight_deps, glob_libs, kept_libs = List.fold_left all_libs ~init:(Dep.Set.empty, [], Lib.Set.empty) @@ -226,8 +226,10 @@ let lib_deps_for_module ~cctx ~obj_dir ~for_ ~dep_graph ~opaque ~cm_kind ~ml_kin else td, lib :: gl, Lib.Set.add kl lib)) in let glob_deps = Lib_file_deps.deps_of_entries ~opaque ~cm_kind glob_libs in - Action_builder.return - (cctx_includes_for_cm_kind (), Dep.Set.union tight_deps glob_deps) + let+ include_flags = + Compilation_context.filtered_include_flags cctx ~cm_kind ~kept_libs + in + include_flags, Dep.Set.union tight_deps glob_deps ;; let lib_cm_deps ~cctx ~cm_kind ~ml_kind ~mode m = diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t b/test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t index 7ad07e032cb..d0bcc28a18a 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t @@ -1,20 +1,17 @@ -Observational baseline: adding a library to a stanza's -[(libraries ...)] list currently rebuilds every module in the -stanza, including modules that reference nothing from the new -library. The [-I]/[-H] flags on each module's compiler invocation -are derived from the cctx-wide library list rather than from the -module's own ocamldep reference set, so adding an unreferenced -library still changes every consumer's compile-action hash and -invalidates the rule cache. +Adding a library to a stanza's [(libraries ...)] list does not +rebuild stanza modules that reference nothing from the new +library. The per-module [-I]/[-H] include filter narrows each +module's compile-action hash to the libraries its ocamldep +reference set actually reaches, so an unreferenced added lib +leaves every other consumer's cache key unchanged. [consumer_lib] starts with [(libraries existing_dep)] and two modules: [consumes_dep] references [Existing_dep_module]; [unrelated_module] references nothing from [existing_dep] or any other library. After the initial build, [added_lib] is added to the stanza's [(libraries ...)] list. Neither [consumes_dep] nor -[unrelated_module] uses anything from [added_lib], yet both -rebuild. Records the rebuild counts on each so a future change -can flip them to 0. +[unrelated_module] uses anything from [added_lib], so neither +rebuilds — the trace shows empty target lists for both. $ cat > dune-project < (lang dune 3.23) @@ -72,22 +69,6 @@ Add [added_lib] to [consumer_lib]'s [(libraries ...)]. Neither $ dune build @check $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("consumes_dep"))]' - [ - { - "target_files": [ - "_build/default/.consumer_lib.objs/byte/consumes_dep.cmi", - "_build/default/.consumer_lib.objs/byte/consumes_dep.cmo", - "_build/default/.consumer_lib.objs/byte/consumes_dep.cmt" - ] - } - ] + [] $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("unrelated_module"))]' - [ - { - "target_files": [ - "_build/default/.consumer_lib.objs/byte/unrelated_module.cmi", - "_build/default/.consumer_lib.objs/byte/unrelated_module.cmo", - "_build/default/.consumer_lib.objs/byte/unrelated_module.cmt" - ] - } - ] + [] diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t b/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t index bb474822490..51eea2c23d1 100644 --- a/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t @@ -1,15 +1,12 @@ -Observational baseline: a consumer module's compile command -currently carries [-I] flags for every library in the cctx-wide -[(libraries ...)] closure, regardless of which libraries the -module's source actually references. +A consumer module's compile command carries [-I] flags only for +the libraries its ocamldep reference set actually reaches, not +for every library in the cctx-wide [(libraries ...)] closure. [consumer_lib] declares two library dependencies, [dep_lib] and [unrelated_lib], but its only module references just [Dep_module]. -A future per-module filter could observe via ocamldep that the -module references nothing from [unrelated_lib] and drop that -library's objdir from the [-I] path. This test records today's -[-I] contents so that a future filter improvement can flip the -asserted array to a single entry. +The per-module filter observes via ocamldep that the module +references nothing from [unrelated_lib] and drops that library's +objdir from the [-I] path; only [dep_lib]'s objdir survives. $ cat > dune-project < (lang dune 3.0) @@ -40,12 +37,10 @@ asserted array to a single entry. Inspect the [-I] flags on [consumer_module]'s [.cmo] compile rule. Filter to objdir-shaped paths so we don't print the consumer's own [.consumer_lib.objs/byte] entry — that's always present and -not what this test is about. Today both [dep_lib]'s objdir and -[unrelated_lib]'s objdir appear: +not what this test is about. Only [dep_lib]'s objdir appears: $ dune rules --root . --format=json _build/default/.consumer_lib.objs/byte/consumer_module.cmo \ > | jq 'include "dune"; .[] | [ruleActionFlagValues("-I") | select(test("\\.dep_lib\\.objs|\\.unrelated_lib\\.objs"))]' [ - ".dep_lib.objs/byte", - ".unrelated_lib.objs/byte" + ".dep_lib.objs/byte" ]