test: guards for wrapped-lib soundness under non-trivial wrapping#14444
Open
robinbb wants to merge 1 commit into
Open
test: guards for wrapped-lib soundness under non-trivial wrapping#14444robinbb wants to merge 1 commit into
robinbb wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds forward-looking blackbox (cram) regression tests to guard dependency-soundness around wrapped libraries in the per-module-lib-deps suite, specifically ensuring downstream modules are invalidated when relevant inner-module interfaces change—even when type info is not carried by the wrapper .cmi.
Changes:
- Add a cram guard for
(wrapped (transition ...))libraries to ensure consumers rebuild when an inner module’s.cmichanges, even if referenced only through the wrapper alias. - Add a cram guard for virtual-library implementations that inherit
(wrapped ...)from the vlib, ensuring consumers rebuild when a concrete vlib module’s interface changes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-transition-soundness.t | Guards consumer rebuild soundness when accessing inner modules via a (wrapped (transition ...)) wrapper alias. |
| test/blackbox-tests/test-cases/per-module-lib-deps/wrapped-from-vlib-soundness.t | Guards consumer rebuild soundness when depending on a vlib implementation that inherits wrapped, ensuring concrete vlib module .cmi changes invalidate consumers. |
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 9, 2026
Address Copilot review feedback on ocaml#14444. Under [(wrapped ...)], the inner-module artifact is the mangled [wrapped_lib__Inner_a.cmi]/[vlib__Helper.cmi]; the un-prefixed [inner_a.cmi] is the transition compat shim (and no shim exists under plain [(wrapped true)]). Naming the mangled artifact in the prose matches the convention in wrapped-transition-incremental.t and removes ambiguity about which .cmi the soundness story rests on. Test logic unchanged. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Collaborator
Author
|
|
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 9, 2026
Address Copilot review feedback on ocaml#14444. Under [(wrapped ...)], the inner-module artifact is the mangled [wrapped_lib__Inner_a.cmi]/[vlib__Helper.cmi]; the un-prefixed [inner_a.cmi] is the transition compat shim (and no shim exists under plain [(wrapped true)]). Naming the mangled artifact in the prose matches the convention in wrapped-transition-incremental.t and removes ambiguity about which .cmi the soundness story rests on. Test logic unchanged. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 10, 2026
Address Copilot review feedback on #14444. Under [(wrapped ...)], the inner-module artifact is the mangled [wrapped_lib__Inner_a.cmi]/[vlib__Helper.cmi]; the un-prefixed [inner_a.cmi] is the transition compat shim (and no shim exists under plain [(wrapped true)]). Naming the mangled artifact in the prose matches the convention in wrapped-transition-incremental.t and removes ambiguity about which .cmi the soundness story rests on. Test logic unchanged. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 10, 2026
Address Copilot review feedback on #14444. Under [(wrapped ...)], the inner-module artifact is the mangled [wrapped_lib__Inner_a.cmi]/[vlib__Helper.cmi]; the un-prefixed [inner_a.cmi] is the transition compat shim (and no shim exists under plain [(wrapped true)]). Naming the mangled artifact in the prose matches the convention in wrapped-transition-incremental.t and removes ambiguity about which .cmi the soundness story rests on. Test logic unchanged. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-test-wrapped-lib-soundness-guards
branch
from
30ae1b6 to
0b48a07
Compare
Collaborator
Author
|
|
robinbb
force-pushed
the
robinbb-test-wrapped-lib-soundness-guards
branch
from
0b48a07 to
324abf6
Compare
robinbb
force-pushed
the
robinbb-test-wrapped-lib-soundness-guards
branch
from
324abf6 to
cf2f7f2
Compare
Two forward-looking guard files in `test/blackbox-tests/test-cases/ per-module-lib-deps/`. Each file mixes soundness cases (must keep rebuilding) with a forward-looking pin on current per-library filter behaviour (currently rebuilds the consumer when an unreferenced sibling module changes; will need promotion once per-module narrowing within a library lands): - `wrapped-transition-soundness.t`: a consumer reaches an inner module of a `(wrapped (transition ...))` library through the wrapper alias `Wrapped_lib.Inner_a.x`. Case 1 (soundness) pins that the consumer rebuilds when the referenced inner module's interface changes — its compile-rule deps must cover `wrapped_lib__Inner_a.cmi` (the mangled inner-module artifact, not the un-prefixed transition compat shim), not only the wrapper's `.cmi`. Case 2 (narrowing pin) covers an unreferenced sibling (`inner_b`). - `wrapped-from-vlib-soundness.t`: a consumer depends on a virtual-library implementation that inherits its `(wrapped ...)` setting from the vlib (the impl does not redeclare `wrapped`). Cases 1–2 (soundness) pin that the consumer rebuilds when a concrete vlib module (`helper`) or a virtual module (`virt_iface`) has its interface change — its compile-rule deps must cover the impl's `.cmi` directory. Case 3 (narrowing pin) covers an unreferenced sibling (`unused`). The soundness cases hold trivially today via the cctx-wide compile-rule glob over each dep library's `.cmi` directory. Future changes that narrow compile-rule deps per-module must keep that coverage for the referenced-module / inherited-wrapped-library edge cases (the soundness cases); the narrowing pins will flip when the narrowing lands and should be promoted then. Test structure: jq regexes are anchored to the objdir (`\.consumer\.objs/byte/consumer\.cm` and `consumer/\.main\.eobjs/byte/`) rather than relying on dune's internal mangling. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-test-wrapped-lib-soundness-guards
branch
from
cf2f7f2 to
e4625ea
Compare
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
Two forward-looking cram guards under
test/blackbox-tests/test-cases/per-module-lib-deps/:wrapped-transition-soundness.t— soundness for(wrapped (transition ...))consumers reaching an inner module through the wrapper alias.wrapped-from-vlib-soundness.t— soundness for consumers of a vlib implementation that inherits(wrapped ...)from the vlib.Both pass trivially on
main; they fail only when future per-module narrowing of compile-rule deps drops the relevant.cmicoverage. See each test's prose header for the full soundness argument.