test: regression guard for menhir mock-compile + sibling-module incremental rebuild on lib-cmi change#14479
Open
robinbb wants to merge 2 commits into
Open
Conversation
…mental rebuild on lib-cmi change When a library declares a menhir grammar whose semantic action references types or values from a [(libraries ...)] dep, dune emits two compile rules whose deps cover that library's cmis: menhir's [--infer] mock-compile (which runs [ocamlc -i] on a generated mock module to type-check the grammar's semantic actions) and the regular sibling-module compile that also references the dep. Both rules must invalidate when the dep's interface changes. The menhir mock-compile path ([menhir_rules.ml]) derives a sandboxed cctx via [Compilation_context.set_sandbox] for the [ocamlc -i] action. The test asserts the two invariants independently — the mock-compile rebuilds and the helper [.cmo] rebuilds — without pinning the exact set of rebuilt artefacts, so it is robust to whether the surrounding code uses cctx-wide-glob or per-module-filtered cmi-deps. Surfaced during review of ocaml#14474. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
There was a problem hiding this comment.
Pull request overview
Adds a new blackbox regression test to ensure Dune’s incremental rebuild logic correctly invalidates both Menhir’s --infer mock-compile and a regular sibling module compilation when a dependent library’s .cmi changes (covering the sandboxed cctx path used by the mock-compile).
Changes:
- Add a new blackbox test case that builds a Menhir + sibling-module library depending on a sibling
deplibrary. - Assert the Menhir
ocamlc -imock-compile runs and includesdep’s objdir on its-Ipath. - After changing
dep’s interface, assert both the mock-compile inferred.mliand the sibling module’s.cmoare rebuilt.
The previous single composite query
([{target: .target_files[0], dep_in_args: any(. == "dep/.dep.objs/byte")}])
relied on (1) [target_files] being a singleton (indexing [[0]]
silently masks regressions if a future dune change adds extra
targets) and (2) exact-string match against the dep objdir
spelling (brittle to path-rendering changes).
Replace with two separate queries, mirroring the pattern used by
[test-cases/menhir/with-library-deps.t]:
- one query asserts the mock-compile's full [target_files] array;
- the other filters [process_args] for entries [startswith("dep/")].
Robust to multiple [target_files] entries and to dune path-rendering
changes.
Addresses Copilot review at
ocaml#14479 (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.
Summary
Adds
test/blackbox-tests/test-cases/per-module-lib-deps/menhir-incremental-lib-cmi.t, a regression guard for two incremental-rebuild invariants on a library that combines a menhir grammar with a sibling module, both referencing types/values from a(libraries ...)dep:--infermock-compile (grammar__mock.mli.inferred) rebuilds when the dep's.cmichanges..cmorebuilds when the dep's.cmichanges.The mock-compile path (
menhir_rules.ml) derives a sandboxed cctx viaCompilation_context.set_sandboxfor theocamlc -iaction. The test asserts the two invariants independently and does not pin the full set of rebuilt artefacts, so it stays robust under cctx-wide-vs-per-module dep filtering.