test: single-module consumer unreferenced-dep regression guard#14308
Merged
robinbb merged 2 commits intoApr 29, 2026
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new blackbox cram regression test in the per-module-lib-deps suite to ensure that when a single-module consumer library does not reference any module from a declared dependency library, editing the dependency’s implementation (without a signature change) does not trigger recompilation of the consumer.
Changes:
- Add a new cram test case
single-module-unreferenced-lib.tthat builds two unwrapped libraries where the consumer does not reference the dependency. - Verify via
dune trace+jqthat no targets related to the consumer module are rebuilt after the dependency implementation changes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
Author
|
Looking for a volunteer to review. |
Add a blackbox test in which a single-module consumer library has a dependency library whose module it does not reference. When the dependency's module is edited (no signature change), the consumer module must not be recompiled. On current main, the cmi is hash-stable under this edit, so the observed rebuild target count is zero; the test guards against regressions of that behaviour. The test also anchors the per-module-lib-deps surface for future tightening work — when tighter per-module filtering lands (ocaml#4572), this scenario remains at zero rebuilds but for a different reason (consumer has no declared dep on the module). See: ocaml#14116 (comment) Signed-off-by: Robin Bate Boerop <me@robinbb.com>
The previous form used an implementation-only edit to [modA.ml] with no [.mli], leaving the cmi hash-stable and producing a trivially-true rebuild-count assertion of 0 that did not exercise any aspect of per-module library filtering. Rewrite with [modA.mli] so the edit forces a cmi hash change. The consumer [modB] still rebuilds on current main (count = 1) because [libB] is a single-module stanza: dune skips ocamldep for it and the consumer falls back to a glob over [libA]'s object directory. This form tests a distinct corner from [single-module-lib.t] (which covers single-module consumers that reference some but not all modules of a multi-module dep): the zero-reference case could be tightened to zero rebuilds by a future fix that drops the lib dep when ocamldep yields no references, without needing to solve the broader single-module-consumer skip-ocamldep limitation. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-test-single-module-unreferenced-lib
branch
from
33c4fd9 to
fa0984e
Compare
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
Apr 27, 2026
Each of the dep PRs supplying a test in [per-module-lib-deps/] was renamed per Leonidas's review on ocaml#14309: generic [libA]/[modA1]-style names replaced by role-bearing names like [dep_lib]/[spurious_rebuild]. This commit applies the same renames to this PR's local copies of those tests so they stay convergent with the dep PRs' canonical versions. Touches: - single-module-unreferenced-lib.t (ocaml#14308) - sibling-unreferenced-lib.t (ocaml#14309) - unwrapped-tight-deps.t (ocaml#14310) - lib-vs-lib-name-collision.t (ocaml#14324) - opaque-mli-change.t, opaque-cmx-deps-local.t (ocaml#14331) - transitive-unreferenced-lib.t (ocaml#14332) - wrapped-reexport-via-open-flag.t (ocaml#14346) - auto-wrapped-child-reexport.t (ocaml#14347) Pure rename + path/dir adjustments. Count assertions preserved (this PR's promotions to the post-fix counts are unchanged). Test suite passes. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
Apr 29, 2026
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>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
Apr 29, 2026
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
added a commit
to robinbb/dune
that referenced
this pull request
Apr 29, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a blackbox cram test recording the rebuild-target count for a
single-module consumer library whose only module references no module
of its declared dependency.
On current main the count is
1:consumer_libis a single-modulestanza, so dune skips ocamldep for it and falls back to a glob over
dep_lib's object directory, which is invalidated by the cmi change.This corner is distinct from the one documented by
test-cases/per-module-lib-deps/single-module-lib.t(single-moduleconsumer that references some modules of a multi-module dep): a
future fix that drops a lib dep when ocamldep yields zero references
to it could tighten this corner to
0without needing to solve thebroader single-module-consumer skip-ocamldep limitation.
Related: #4572, #14116