test: regression guard for (root_module ...) + (libraries ...)#14439
Open
robinbb wants to merge 3 commits into
Open
test: regression guard for (root_module ...) + (libraries ...)#14439robinbb wants to merge 3 commits into
robinbb wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new blackbox regression test to guard Dune’s build correctness when a library uses (root_module ...) alongside (libraries ...), specifically when consumer modules reference a dependency through the synthesized Root alias (covering the synthesized-.d dependency-rule path for root modules).
Changes:
- Add a new cram/blackbox test case
root-module-tight-deps.tunderper-module-lib-deps/to assert the scenario builds successfully.
robinbb
force-pushed
the
robinbb-test-root-module-tight-deps
branch
from
b0d33fe to
960145f
Compare
Collaborator
|
I think since #14385 we no longer explicitly produce |
robinbb
force-pushed
the
robinbb-test-root-module-tight-deps
branch
from
960145f to
2b018f9
Compare
Collaborator
Author
robinbb
added a commit
that referenced
this pull request
May 11, 2026
…4440) ## Summary Adds `test/blackbox-tests/test-cases/per-module-lib-deps/ppx-runtime-libraries.t`, a forward-looking guard that asserts a consumer using `(preprocess (pps bar))` — where `bar` declares `(ppx_runtime_libraries baz)` — has a compile-rule dep that covers every cmi in `baz`'s objdir. A glob over `baz/.baz.objs/byte/*.cmi` is the natural shape and what this test pins. The runtime libraries' modules are referenced by ppx-rewritten output, so ocamldep on the consumer's source cannot reason about which of them are referenced — the next ppx invocation may use any module of `baz`. Without a recorded dep that covers all of `baz`'s cmis, a change to any cmi in `baz` would not invalidate the consumer's `.cmi` rule and an incremental rebuild could silently use a stale artefact. Same shape as #14400, #14403, and #14439 — small standalone tests landed ahead of larger work that would touch the same code paths. Spun out of #14116 so its chain doesn't carry the test. Reported by @rgrinberg via @art-w in #14116 (comment). --------- Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Alizter
reviewed
May 11, 2026
Collaborator
Author
|
@Alizter Thanks for your comments. I have made adjustments. Back to you. |
robinbb
force-pushed
the
robinbb-test-root-module-tight-deps
branch
from
4eb7451 to
7d99bfa
Compare
Adds [test-cases/per-module-lib-deps/root-module-tight-deps.t]. Asserts that a library declaring [(root_module ...)] alongside [(libraries ...)] builds correctly when its modules reference the dep library through the Root alias. Root modules carry no ocamldep dep-rule: the kind matches [Root | Alias _] in [Dep_rules.read_transitive_deps_of_module] and [read_immediate_deps_of], both of which short-circuit to empty deps (avoiding the cycle that would otherwise arise from a Root that references itself transitively). Any future change to inter-library dep handling that misses this short-circuit would regress this scenario. Forward-looking guard. The build path on current main uses a broad-glob over the dep library's objdir, so the test passes trivially today. The [Root | Alias _] short-circuit is still exercised by rule generation, and that is what the test pins. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Per @Alizter's review: build the root module's cmi directly (instead of @check) and snapshot the [dune rules --deps] output so the test pins the specific dep on dep_lib's cmi glob. A future regression that omits the dep fails the snapshot; a refinement narrower than the glob flips it and requires deliberate promotion. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-test-root-module-tight-deps
branch
from
7d99bfa to
aa9e908
Compare
Alizter
reviewed
May 11, 2026
Alizter
reviewed
May 11, 2026
Per Alizter's review of ocaml#14439: - r3221123272: replace the explicit `_build/...consumer_lib__Root.cmi` path (and its `ROOT_CMI=...` shell variable) with the `%{cmi:consumer_lib/root}` artefact variable. The synthetic root module resolves through this expansion correctly; verified by running the cram test. - r3221068391: replace `dune rules --deps ... | dune_cmd print-from | dune_cmd print-until` (sexp-fragment matching) with `dune rules --root . --format=json --deps ... > deps.json` plus two jq queries via the `depsFilePaths` / `depsGlobs` helpers in `test/blackbox-tests/dune.jq`. Each assertion now names exactly what it checks (File-dep on `root.ml-gen`; glob on `dep_lib/.dep_lib.objs/byte` with predicate `*.cmi`). The compiler-binary-filtering paragraph becomes dead text and is deleted — jq selects by name, no External entry to navigate. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Alizter
approved these changes
May 12, 2026
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
test/blackbox-tests/test-cases/per-module-lib-deps/root-module-tight-deps.t, a forward-looking guard that asserts a library declaring(root_module ...)alongside(libraries ...)builds correctly when its modules reference the dep library through the Root alias.Root modules carry no ocamldep dep-rule: the kind matches
Root | Alias _inDep_rules.read_transitive_deps_of_moduleandread_immediate_deps_of, both of which short-circuit to empty deps, avoiding the cycle that would otherwise arise from a Root that references itself transitively. Any future change to inter-library dep handling that misses this short-circuit would regress this scenario.Same shape as #14400 (cross-lib pre-pp guard) and #14403 (invalid
-openguard) — small standalone tests landed ahead of larger work that would touch the same code paths. Spun out of #14116 so #14116's chain doesn't carry the test.