test: document broad-include leak of wrapped-library internal names#14317
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a blackbox (cram) test case documenting the current ability for a consumer to compile against a wrapped library’s mangled internal module name via broad -I include paths, establishing a baseline that can be inverted once per-module -I filtering lands.
Changes:
- Add a new cram test that builds an executable referencing
Mylib__Internaland records that it currently compiles successfully. - Document the expected behavior change under per-module
-Ifiltering (future failure withUnbound module Mylib__Internal).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Just a note that although this behaviour is documented, it is not officially supported. It is not OK for users to rely on this. |
@rgrinberg Does that mean that I can break this behaviour if it is convenient for me to do so? (I am documenting this behaviour because someone, somewhere is relying on this, and so a test failed when I attempted to make changes in #14116). |
|
In theory, yes we can break it. In practice, we'll often need to provide the fixes ourselves to the packages that depend on this. So there's a trade off to be made |
A consumer that writes [Mylib__Internal.x] currently compiles successfully, even though [Mylib__Internal] is an implementation detail of [mylib]'s wrapping, not a declared public entry. The library's object directory contains the mangled [Mylib__Internal.cmi] as part of the wrapping convention, and broad per-compile [-I] flags make that directory reachable from any consumer of the library. This behaviour is not officially supported — consumers should reach a wrapped library's internals only through the wrapper API, not through the mangled form. In practice some packages currently rely on it, which is why this baseline is recorded: when a downstream change flips the behaviour the diff surfaces visibly in CI rather than silently breaking those consumers. Per-module [-I] filtering (ocaml#4572 / ocaml#14186) is expected to break the leak deliberately. [Mylib__Internal] is not an entry in the lib index, so the filter excludes [mylib] for a consumer that names only the mangled form, the objdir is dropped from [-I], and the compile fails with "Unbound module Mylib__Internal". The flip is acceptable because the leak was never supported; downstream consumers depending on the mangled form should migrate to the wrapper API. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
01c8088 to
fb38c03
Compare
|
@rgrinberg This will be very useful information when I implement #14186 , which might flip the logic in this test. May I bother you for a review? I have altered the text in the test to reflect this new understanding that you explained. |
Summary
Adds an observational cram test recording that a consumer currently
compiles successfully when it writes the mangled internal name of a
wrapped library's private module (e.g.
Mylib__Internal.x). Broadper-compile
-Iflags make the library's object directory reachable,and the mangled
Mylib__Internal.cmiis physically present there aspart of dune's wrapping convention, so the access succeeds and
silently side-steps the library's declared wrapper API.
Status of the documented behaviour
This behaviour is not officially supported. As @rgrinberg
clarified in review, users should reach a wrapped library's internals
only through the wrapper API, not through the mangled form, and we
are free to break the mangled access if convenient. In practice some
packages currently depend on it (a test in #14116 surfaced this when
a code change happened to flip the behaviour), so the baseline is
recorded here as a tripwire: any future change that flips it shows
up as a visible cram diff rather than silently breaking those
consumers, giving us the chance to provide migration patches
upstream.
Future direction
Per-module
-Ifiltering (#4572 / #14186) is expected to break thisleak deliberately by scoping each consumer's
-Ipaths to thelibraries its source references.
Mylib__Internalis not an entryin the lib index, so the filter excludes
mylib, the objdir isdropped from
-I, and the compile fails with "Unbound moduleMylib__Internal". The flip is acceptable because the leak was never
supported; downstream consumers depending on the mangled form should
migrate to the wrapper API.
Related: #4572, #14186