test: observational baseline for auto-wrapped child re-export (#4572)#14347
Merged
Conversation
robinbb
force-pushed
the
robinbb-test-wrapped-child-reexport
branch
from
804a8d2 to
62651aa
Compare
There was a problem hiding this comment.
Pull request overview
Adds new blackbox “observational baseline” tests in per-module-lib-deps/ to record rebuild counts for consumers that reach a dependency through re-export chains involving -open and wrapping, to guard future work on per-module dependency filtering (#4572).
Changes:
- Add a baseline test for a wrapped sibling library that re-exports a dependency and is brought into scope via
-open. - Add a baseline test for the auto-wrapped case where the cross-library reference is inside a child module of the wrapped library.
- Assert (via
dune trace+jq) that the consumer rebuild count is1after a dependency interface change on trunk behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-reexport-via-open-flag.t |
New observational baseline covering re-export via a hand-written wrapper + -open. |
test/blackbox-tests/test-cases/per-module-lib-deps/auto-wrapped-child-reexport.t |
New observational baseline for cross-library reference via a child of an auto-wrapped library opened with -open. |
robinbb
force-pushed
the
robinbb-test-wrapped-child-reexport
branch
from
d122359 to
0b80afc
Compare
robinbb
force-pushed
the
robinbb-test-wrapped-child-reexport
branch
from
0b80afc to
d4e2039
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>
Records the rebuild count for a consumer module that reaches a dep library's modules through a child of an auto-wrapped sibling library, where the sibling is opened via the [-open] compiler flag and the child's source includes the dep library's module via [include]. On trunk the consumer correctly rebuilds when the dep library's interface changes (count = 1) because the cctx-wide compile-rule deps cover every library in the stanza's [(libraries ...)] closure. This is the structural shape of menhir's [base]/[middle] arrangement: [base] is auto-wrapped (no [base.ml] in source — dune generates the wrapper) with a child [PPrint.ml] containing [include Vendored_pprint]; [middle] uses [-open Base] in flags and references [PPrint.foo]. The reference chain crosses library boundaries through the auto-wrapped sibling's child, not through a hand-written wrapper. Pairs with future work that narrows compile-rule deps per module. Distinct from [wrapped-reexport-via-open-flag.t] (where the alias lives in a hand-written wrapper) — the auto-wrapped case has the cross-library reference inside a child module whose ocamldep is not directly read by walks rooted at the wrapper, so a per-module filter must descend into wrapped libs' children to remain sound. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
The test was originally drafted around [include Vendored_pprint] to mirror menhir's [base]/[middle] arrangement, but was switched to [module Re = Original_name] because a transparent alias is what exposes the per-module-filter precision gap. The prose was not updated alongside. Three Copilot review comments flagged the residual mismatch. This commit: - Updates the intro to describe the actual mechanism (transparent module alias), not [include]. - Reframes the menhir analogy as a structural mirror, with an explicit note that menhir uses [include] and this test deliberately uses an alias for sharper precision; defers the full "why alias" explanation to the paragraph below the Structure block, where it already lives. - Fixes the Structure paragraph: [consumer.ml] does name [Pprint] (the child of [lib_re_export]); the prose previously claimed it named no children. Reworded to "names [Pprint] but not [lib_a] or the wrapper [Lib_re_export]". No code change. Test still passes. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-test-wrapped-child-reexport
branch
from
dd5e348 to
1115c08
Compare
Alizter
approved these changes
Apr 29, 2026
Alizter
added a commit
that referenced
this pull request
Apr 29, 2026
…4572) (#14365) ## Summary Adds a cram test recording the rebuild count when a consumer references a wrapped sibling library whose `Lib.closure` includes a transitively-reached unwrapped library, and an unread module of that transitive library is edited. ## Setup - `dep_lib` is unwrapped with two entry modules: `reached_module` (transitively reached via an alias chain) and `unreached_module` (which no source in the test reaches). - `lib_re_export` is wrapped (dune auto-generates the wrapper) with a single child `pprint` that aliases only `reached_module` of `dep_lib`. - `consumer_lib` writes `Lib_re_export.Pprint.Re.x`, naming `Lib_re_export` in source. ## What the test records Editing `unreached_module.mli` rebuilds the consumer on trunk today: the cctx-wide `-I`/`-H` glob covers `dep_lib`'s objdir, so any `.cmi` content change in `dep_lib` invalidates the consumer regardless of whether the change is to a module the consumer's alias chain actually reaches. A precision-perfect filter could observe that `pprint`'s ocamldep output names only `reached_module` and emit a per-module dep on `reached_module.cmi` alone — leaving the consumer untouched on `unreached_module.mli` changes. The test asserts the current behaviour (consumer rebuilds) so a future filter improvement that closes the precision gap will flip the assertion to `[]`. ## Relationship to #14116 #14116's per-module filter does not close this specific gap. Its conservative wrapped-lib soundness fix globs the entire `Lib.closure` of any wrapped sibling that appears in the consumer's reference set, which covers `dep_lib` here — so `unreached_module` changes still invalidate the consumer through the glob. The deeper filter that walks ocamldep through wrapped libs' children to extract exactly which transitive entry modules are reached is described as follow-on work in `lib_file_deps.ml`. This PR is purely observational; no code change. The test companions the soundness regression guards in [`auto-wrapped-child-reexport.t`](#14347) and [`wrapped-reexport-via-open-flag.t`](#14346): those assert the consumer DOES rebuild on a *reached* transitive module's change (soundness preservation); this one asserts the consumer rebuilds on an *unreached* transitive module's change (precision gap). Related: #4572, #14116
Alizter
reviewed
Apr 29, 2026
Alizter
approved these changes
Apr 29, 2026
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
Apr 29, 2026
The imperative lines that introduce a perturbation in the
[per-module-lib-deps] cluster say things like "Edit [X]'s
interface" or "Change X's interface", but the cat-blocks that
follow always rewrite both X.mli AND X.ml — adding a [val] to the
[.mli] requires a paired [let] in the [.ml] for the test to
actually build through. The prose was misleading.
Fix the imperative across nine cluster tests to spell out that
both files are edited:
- auto-wrapped-child-reexport.t
- basic-wrapped.t
- multiple-libraries.t (two occurrences)
- single-module-unreferenced-lib.t
- transitive.t
- transitive-unreferenced-module.t
- transparent-alias.t
- unwrapped-tight-deps.t (two occurrences)
- wrapped-reexport-via-open-flag.t
[sibling-unreferenced-lib.t] already has the explicit form
("Edit X's interface (add a binding to both [.mli] and [.ml] so
the [.cmi] content actually changes)") and is left as-is.
No assertion or build-step changes; this is prose-only.
Addresses Alizter's review feedback:
ocaml#14347 (comment)
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.
Records the rebuild targets for a consumer module that reaches a
dep library's modules through a child of an auto-wrapped sibling
library. The sibling is opened via the
-opencompiler flag andthe child's source exposes the dep library through a transparent
module alias. On trunk the consumer correctly rebuilds when the
dep library's interface changes (exactly one
consumer-namedrule fires, producing
consumer.cmi/.cmo/.cmt) becausethe cctx-wide compile-rule deps cover every library in the
stanza's
(libraries ...)closure.The structural shape mirrors menhir's
base/middlearrangement:baseis auto-wrapped (nobase.mlin source — dune generatesthe wrapper) with a child module that re-exports a third-party
dep;
middleuses-open Baseand reaches the dep through thatchild. Menhir uses
include Vendored_pprint; this test uses atransparent alias instead, because the alias shape exposes the
precision gap a future per-module dep filter (#14116) must
handle. The reference chain still crosses library boundaries
through the auto-wrapped sibling's child, not through a
hand-written wrapper.
This is an observational test only; no code change. Pairs with
future work on the per-module dep filter (#14116). Distinct from
#14346 (where the alias lives in a hand-written wrapper) — the
auto-wrapped case has the cross-library reference inside a child
module whose ocamldep is not directly read by walks rooted at the
wrapper, so a per-module filter must descend into wrapped libs'
children to remain sound.
See: #14116
See: #14346