test: observational baseline for over-rebuilds under implicit_transitive_deps=false by robinbb · Pull Request #14350 · ocaml/dune

robinbb · 2026-04-27T03:31:27Z

Summary

Observational baseline. Under (implicit_transitive_deps false), a
transitive link-only library should not trigger a consumer rebuild
when its source changes — the consumer cannot see it through -I
or -H, so the compile rule should not depend on its objdir.

Trunk does not yet satisfy this property: the cctx-wide glob
over link_only_lib's objdir invalidates Main on any .cmi
content change in the link-only library. This test records the
current rebuild targets for Main as a baseline that a future
per-module dep filter (#14116) can promote to [].

The earlier draft of this PR asserted count = 0 and was reported
to pass on main; commit 9948ed896 ("strengthen the .cmi
perturbation") replaced the weak edit (let x = 42 → let x = 43,
which produces an identical .cmi) with echo > link_only_module.ml
(which empties the binding). Under the strengthened perturbation
the property fails on trunk, and the test was reframed as an
observational baseline rather than a regression guard.

Reported by @nojb in
#14116 (comment),
where an early version of #14116 violated this property and was
later fixed.

Copilot

Pull request overview

Adds a blackbox regression test to ensure that with (implicit_transitive_deps false), a transitive link-only library change does not trigger recompilation of a consumer that cannot see that transitive library via -I/-H.

Changes:

Introduces a new per-module dependency blackbox test case covering the “link-only transitive dep” scenario under implicit_transitive_deps=false.
Asserts that editing the transitive library does not cause rebuilds of the consumer module.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@nojb

…ps=false Records that under [(implicit_transitive_deps false)], a transitive link-only library does NOT trigger a consumer rebuild when its source changes — the consumer cannot see it through [-I] or [-H], so the compile rule must not depend on its objdir. Trunk satisfies this property today: the compile-rule deps are restricted to libs in [direct_requires + hidden_requires], so a purely-link-only transitive lib doesn't enter the deps. The test asserts count = 0 and passes on [main]. Pre-emptive guard against changes that would expand a consumer's compile-rule deps via [Lib.closure] (or a similar transitive walk) without re-restricting to the cctx's compile scope. Reported by @nojb in ocaml#14116 (comment) where an early version of ocaml#14116 violated this property. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

…-false] Caught while diagnosing ocaml#14116 (comment): the prior version of this test edited [link_only_module.ml] from [let x = 42] to [let x = 43]. Both produce the same [.cmi] (the exported signature [val x : int] is unchanged), so dune's content-hash invalidation didn't fire any glob over the lib's objdir — even when the consumer's compile-rule deps incorrectly included such a glob. The test passed on a binary that was actually broken, masking the bug. Switched to [echo > link_only_module.ml] which empties the file, removing the [val x] binding from the exported signature so the [.cmi] content actually changes. Combined with reframing the assertion as an observational baseline (count = 2 on trunk, the two [Main.cmi]/[Main.cmo] rebuilds the cctx-wide glob causes), the test now empirically reflects trunk's current behaviour and gives ocaml#14116 something to flip when its per-module filter narrows the deps. Audit of the other [robinbb-test-*] branches confirmed no other test relies on the same weak-edit pattern: every other [count = 0] / [[]] assertion either edits a [.mli] (signature- changing) or a [.ml] of a module without [.mli] while adding a new top-level binding. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

…itive-deps-false] [| length] hides which targets actually re-ran — a future change that swaps one expected target for a different one (e.g. the [.cmo]/[.cmt] rule skipped, replaced by [.cmx]/[.o]) would still pass [length = 2]. Inspecting the array of [{ target_files }] records each rule's exact outputs, so any divergence from the documented post-build state shows up as a cram diff. This pattern is consistent with other tests in this cluster that already match against the full target list (e.g. [alias-reexport.t] asserting [[]] for the no-rebuild case rather than [length 0]). Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

@check

@check's deps are gated on [Context.merlin] (see all three [add_*] functions in [src/dune_rules/check_rules.ml]). Under any workspace configuration that disables merlin, [dune build @check] is a no-op and the post-edit trace assertion would pass vacuously — the over-rebuild this test exists to record would go silent. Switching to [./main.exe] forces compilation regardless of the merlin gating. The expected [target_files] arrays change from [.cmo]/[.cmt] (bytecode, fired via @check) to [.cmx]/[.o] (native, fired via the explicit native exe target). Reported in review. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

The [-H]-glob path this test exercises is reached only when [(implicit_transitive_deps false)] resolves to mode [Disabled_with_hidden_includes] — which requires dune lang [3.17+] plus OCaml [5.2+] hidden-includes support (gated in [src/dune_lang/dune_project.ml]'s [Implicit_transitive_deps.mode]). Older dune lang versions fall back to mode [Disabled] without [-H] and don't exhibit the over-rebuild recorded here. Spell that out in the prose so a reader doesn't wonder whether the test should also run under the older behaviour. Addresses Alizter's review feedback: ocaml#14350 (comment) Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Three cluster tests landed on main as observational baselines (via PRs ocaml#14308, ocaml#14331, ocaml#14350) recording the trunk-today rebuild list. With this PR's per-module filter applied, the [.cmi]/[.cmti] byte rule no longer globs over a dep lib's objdir for unreferenced sibling modules, so: - [single-module-unreferenced-lib.t]: [spurious_rebuild]'s rebuild count drops from 1 to 0. - [opaque-mli-change.t]: only the [.cmx]/[.o] native rule re-runs (release and dev profile both); the byte rule's deps no longer include the changed dep lib's [.cmi]. - [implicit-transitive-deps-false.t]: same — the [.cmx]/[.o] native rule re-runs, the byte rule does not. Promote the asserted [target_files] arrays accordingly. No behavioural surprise here — all three drops match the design of the per-module filter; the upstream baselines exist specifically to be flipped by this PR. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Cluster tests landed on main as observational baselines (via PRs ocaml#14308, ocaml#14331, ocaml#14350, and the transitive-unreferenced-module follow-up) recording the trunk rebuild list. With this PR's per-module filter applied, the [.cmi]/[.cmti] byte rule no longer globs over a dep lib's objdir for unreferenced sibling modules, so: - [single-module-unreferenced-lib.t]: [spurious_rebuild]'s rebuild count drops from 1 to 0. - [opaque-mli-change.t]: only the [.cmx]/[.o] native rule re-runs (release and dev profile both); the byte rule's deps no longer include the changed dep lib's [.cmi]. - [implicit-transitive-deps-false.t]: same — the [.cmx]/[.o] native rule re-runs, the byte rule does not. - [transitive-unreferenced-module.t]: the consumer doesn't even directly reference the transitive lib, so the filter drops it entirely; both rules are skipped, [target_files] becomes [[]]. Promote the asserted [target_files] arrays accordingly. No behavioural surprise here — all four drops match the design of the per-module filter; the upstream baselines exist specifically to be flipped by this PR. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

robinbb mentioned this pull request Apr 27, 2026

feat: per-module inter-library dependency filtering (#4572) #14116

Closed

robinbb self-assigned this Apr 27, 2026

robinbb force-pushed the robinbb-test-implicit-transitive-deps-false branch from 5c1c3c2 to 6d37749 Compare April 27, 2026 03:36

robinbb requested a review from Copilot April 27, 2026 03:36

Copilot started reviewing on behalf of robinbb April 27, 2026 03:36 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t Outdated

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t Outdated

robinbb force-pushed the robinbb-test-implicit-transitive-deps-false branch from 6d37749 to 0cd3bb1 Compare April 27, 2026 03:53

robinbb requested a review from Copilot April 27, 2026 04:56

Copilot started reviewing on behalf of robinbb April 27, 2026 04:57 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t Outdated

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t Outdated

robinbb force-pushed the robinbb-test-implicit-transitive-deps-false branch 2 times, most recently from ba90ae7 to 08b85f5 Compare April 27, 2026 05:23

robinbb requested a review from Copilot April 27, 2026 05:24

Copilot started reviewing on behalf of robinbb April 27, 2026 05:24 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

robinbb marked this pull request as ready for review April 27, 2026 06:25

robinbb force-pushed the robinbb-test-implicit-transitive-deps-false branch from 08b85f5 to 49d37ca Compare April 27, 2026 15:41

robinbb added 3 commits April 27, 2026 17:56

robinbb force-pushed the robinbb-test-implicit-transitive-deps-false branch from 49d37ca to e800109 Compare April 28, 2026 01:02

robinbb requested a review from Copilot April 28, 2026 14:40

Copilot started reviewing on behalf of robinbb April 28, 2026 14:41 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

robinbb changed the title ~~test: regression guard for over-rebuilds under implicit_transitive_deps=false~~ test: observational baseline for over-rebuilds under implicit_transitive_deps=false Apr 28, 2026

robinbb requested a review from Copilot April 28, 2026 15:18

Copilot started reviewing on behalf of robinbb April 28, 2026 15:18 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

Alizter self-assigned this Apr 29, 2026

Alizter reviewed Apr 29, 2026

View reviewed changes

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/implicit-transitive-deps-false.t

Alizter approved these changes Apr 29, 2026

View reviewed changes

Alizter merged commit b320c3d into ocaml:main Apr 29, 2026
22 of 23 checks passed

robinbb deleted the robinbb-test-implicit-transitive-deps-false branch April 29, 2026 18:09

robinbb mentioned this pull request May 10, 2026

feat: per-module inter-library dependency filtering (#4572) #14472

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: observational baseline for over-rebuilds under implicit_transitive_deps=false#14350

test: observational baseline for over-rebuilds under implicit_transitive_deps=false#14350
Alizter merged 5 commits into
ocaml:mainfrom
robinbb:robinbb-test-implicit-transitive-deps-false

robinbb commented Apr 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

robinbb commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robinbb commented Apr 27, 2026 •

edited

Loading