test: regression guard for ppx_runtime_libraries cmi-dep coverage#14440
Merged
robinbb merged 2 commits intoMay 11, 2026
Conversation
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 6, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 6, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 7, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 7, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in #14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 7, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in #14440 so it can land on [main] independently. 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 rule-time dependency tracking for ppx_runtime_libraries remains conservative: consumers using (preprocess (pps ...)) must record dependencies that cover all .cmi files in the runtime library’s objdir, preventing stale incremental builds when any runtime .cmi changes.
Changes:
- Introduces a new cram-style test case exercising a
ppx_rewriter(bar) with(ppx_runtime_libraries baz). - Asserts the consumer’s compile rule deps include an objdir glob over
baz’s.cmifiles.
Adds [test-cases/per-module-lib-deps/ppx-runtime-libraries.t]. Asserts that 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. Forward-looking guard. The current build path makes this test pass via the broad-glob over each [requires_compile] library's objdir; the test would surface a regression in any future inter-library dep handling that narrows the recorded deps for ppx_runtime_libraries-induced libraries. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-test-ppx-runtime-libraries-guard
branch
from
f5e5efa to
23bd960
Compare
robinbb
added a commit
that referenced
this pull request
May 7, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in #14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 7, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 7, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 8, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 8, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Leonidas-from-XIV
requested changes
May 8, 2026
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 8, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 8, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Rename metasyntactic library names to descriptive role names: [baz] -> [runtime_dep], [bar] -> [preprocessor], [foo] -> [user]. Drop [(wrapped false)] from both [runtime_dep] and [user] (no functional reason; the test pins a glob over the runtime library's objdir which works regardless of wrapping). Rewrite the line-77 paragraph: split the convoluted single paragraph into two — one stating the dep coverage requirement and what staleness looks like, the other contrasting the [-I] flag (initial build) against the recorded deps (rebuilds). Replace the jq string interpolation [.dir] [.predicate] with explicit concatenation, and drop [sort -u] — only one matching glob is expected, so the test now fails loudly if a future change emits more than one rather than hiding the change. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
May 9, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <ocaml#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in ocaml#14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 9, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in #14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 10, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in #14440 so it can land on [main] independently. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
added a commit
that referenced
this pull request
May 10, 2026
A consumer that uses [(preprocess (pps bar))] gets [bar]'s [ppx_runtime_libraries] (e.g. [baz]) folded into its [requires_compile] via [add_pp_runtime_deps] in [lib.ml]. Those runtime libraries' modules are visible only to the post-pp source the ppx-rewriter emits — the next ppx invocation may reference any of them — so the per-module dependency filter cannot reason about which are actually needed. Before this commit, the filter routed [baz] through the same classification fold as user-declared [(libraries ...)]. When [baz] is unwrapped (so [tight_eligible(baz) = true]) and no ocamldep reference of [baz]'s modules appeared in the consumer, the fold's "tight-eligible but unreached" branch silently dropped [baz] from the compile rule's recorded deps. The [-I baz/...] flag still made it onto the compile command line via [requires_compile], so initial builds succeeded; but a change to any cmi in [baz] would not invalidate the consumer's [.cmi] rule, leaving incremental rebuilds silently stale. Thread the closure of every [pps]'s [ppx_runtime_libraries] through to [Compilation_context.t] (new field [pps_runtime_libs]), and in [lib_deps_for_module]: - include [pps_runtime_libs] in [direct_libs] so the [Lib.closure] step puts them in [all_libs] for the classification fold to see; - include them in [must_glob_set] so the fold routes them onto the glob path, recording a glob predicate over each runtime lib's objdir. The wrapped-lib soundness recovery already used [must_glob_set] for an analogous reason; this commit extends the same pattern to the pps-runtime case. Reported by @rgrinberg via @art-w in <#14116 (comment)>. The regression guard for this case lives at [test-cases/per-module-lib-deps/ppx-runtime-libraries.t] and is spun out as a standalone forward-looking guard in #14440 so it can land on [main] independently. 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/ppx-runtime-libraries.t, a forward-looking guard that asserts a consumer using(preprocess (pps bar))— wherebardeclares(ppx_runtime_libraries baz)— has a compile-rule dep that covers every cmi inbaz's objdir. A glob overbaz/.baz.objs/byte/*.cmiis 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 ofbaz's cmis, a change to any cmi inbazwould not invalidate the consumer's.cmirule 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).