From d8f36603d16b7e6a67bb5bee9883b121f70802a5 Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 29 Apr 2026 17:16:08 -0700 Subject: [PATCH 1/2] test: observational baseline for per-module include flags (#4572) Adds [test-cases/per-module-lib-deps/per-module-include-flags.t]. 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. The test sets up a stanza with two declared libraries where the only consumer module references just one of them, then inspects the [.cmo] compile rule via [dune rules --format=json] and asserts the [-I] paths. Today the asserted array contains both [dep_lib]'s and [unrelated_lib]'s objdirs. A future per-module filter (#14186) narrows each module's compile command to only the libraries its source reaches, at which point this assertion flips to a single entry. Companion to the cache-key-stability angle in [add-unreferenced-sibling-lib.t]: that test records the *consequence* (consumer doesn't rebuild on stanza change), this test records the *cause* (the compile command's actual [-I] content). Related: #4572, #14186 Signed-off-by: Robin Bate Boerop --- .../per-module-include-flags.t | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t 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 new file mode 100644 index 00000000000..bb17d140556 --- /dev/null +++ b/test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t @@ -0,0 +1,58 @@ +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. + +[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. + + $ cat > dune-project < (lang dune 3.0) + > EOF + + $ cat > dune < (library (name dep_lib) (wrapped false) (modules dep_module)) + > (library (name unrelated_lib) (wrapped false) (modules unrelated_module)) + > (library + > (name consumer_lib) + > (wrapped false) + > (modules consumer_module) + > (libraries dep_lib unrelated_lib)) + > EOF + + $ cat > dep_module.ml < let v = 1 + > EOF + $ cat > unrelated_module.ml < let _ = () + > EOF + $ cat > consumer_module.ml < let _ = Dep_module.v + > EOF + + $ dune build @check + +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: + + $ dune rules --root . --format=json _build/default/.consumer_lib.objs/byte/consumer_module.cmo \ + > | jq -r ' + > .[] + > | .action + > | .. | arrays? + > | select((.[0]? // "") == "run") + > | [.[] | select(test("\\.dep_lib\\.objs|\\.unrelated_lib\\.objs"))] + > | select(length > 0) + > ' + [ + ".dep_lib.objs/byte", + ".unrelated_lib.objs/byte" + ] From 626029609fcc2001c7081f36d1f234c8e9696e1b Mon Sep 17 00:00:00 2001 From: Robin Bate Boerop Date: Wed, 29 Apr 2026 17:35:34 -0700 Subject: [PATCH 2/2] test: use [ruleActionFlagValues("-I")] from [dune.jq] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hand-rolled walk through [.action | .. | arrays? | select((.[0]? // "") == "run") | ...] with the shared [ruleActionFlagValues("-I")] helper from [test/blackbox-tests/dune.jq]. Same observed output. Two wins: - Idiomatic — the helper exists for exactly this case. - Future-proof — if the action argv shape evolves (e.g. a wrapping action node added above [run]), [ruleActionFlagValues] is the place such evolution gets absorbed; consumers don't break. Addresses Copilot's review feedback: https://github.com/ocaml/dune/pull/14375#discussion_r3164966640 Signed-off-by: Robin Bate Boerop --- .../per-module-lib-deps/per-module-include-flags.t | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) 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 bb17d140556..bb474822490 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 @@ -44,14 +44,7 @@ not what this test is about. Today both [dep_lib]'s objdir and [unrelated_lib]'s objdir appear: $ dune rules --root . --format=json _build/default/.consumer_lib.objs/byte/consumer_module.cmo \ - > | jq -r ' - > .[] - > | .action - > | .. | arrays? - > | select((.[0]? // "") == "run") - > | [.[] | select(test("\\.dep_lib\\.objs|\\.unrelated_lib\\.objs"))] - > | select(length > 0) - > ' + > | jq 'include "dune"; .[] | [ruleActionFlagValues("-I") | select(test("\\.dep_lib\\.objs|\\.unrelated_lib\\.objs"))]' [ ".dep_lib.objs/byte", ".unrelated_lib.objs/byte"