Skip to content

test: regression guard for cross-lib reads of action-preprocessed deps#14383

Merged
robinbb merged 1 commit into
ocaml:mainfrom
robinbb:robinbb-test-cross-lib-ppx
May 1, 2026
Merged

test: regression guard for cross-lib reads of action-preprocessed deps#14383
robinbb merged 1 commit into
ocaml:mainfrom
robinbb:robinbb-test-cross-lib-ppx

Conversation

@robinbb
Copy link
Copy Markdown
Collaborator

@robinbb robinbb commented Apr 30, 2026

Summary

Adds test-cases/per-module-lib-deps/cross-lib-action-preprocess.t, an observational baseline asserting that a consumer of a library that uses an action preprocessor builds without error.

Why

When the dep library uses (preprocess (action ...)), dune feeds the .pp.ml-form of dep's source to ocamldep, so dep's .d file's left-hand basename is foo.pp.ml rather than foo.ml. Any future inter-library dependency tracking that reads dep's .d files from outside the producing rule must tolerate this mismatch with the raw Module.t's .ml source path.

This pitfall surfaced during #14116 development: the dev-tool build of utop (which is action-preprocessed) broke under the per-module filter's cross-library walk because the parser's basename check fired. The test guards against the same shape of regression in any future inter-library dep-tracking change.

Related: #14116

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new blackbox regression test to ensure that consumers can build successfully when depending on a library whose modules are preprocessed (and thus whose ocamldep .d outputs may refer to .pp.ml basenames).

Changes:

  • Introduces a new blackbox test case that sets up a trivial “passthrough” preprocessor.
  • Builds a small dep library with preprocessing enabled and a consumer library that references it, asserting the build succeeds.

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-ppx.t Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new blackbox regression test case to guard against failures when a consumer library depends on another library whose modules are preprocessed (producing .pp.ml sources that can affect ocamldep .d basenames).

Changes:

  • Add a new cram/blackbox test that sets up a tiny OCaml preprocessor executable.
  • Create a preprocessed, unwrapped multi-module dep library and a consumer library that references one dep module.
  • Assert that dune build @check succeeds in this scenario.

@robinbb robinbb marked this pull request as ready for review April 30, 2026 22:59
@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Apr 30, 2026

This test seems to be about action preprocessing rather than ppx drivers which work slightly differently. They build and link an executable that gets passed to ocamldep and ocamlc with -pp (no .pp.ml) is generated.

I think its fine to test preprocessing, but I would make it more precise rather than claiming it covers all of ppx.

@robinbb
Copy link
Copy Markdown
Collaborator Author

robinbb commented Apr 30, 2026

This test seems to be about action preprocessing rather than ppx drivers which work slightly differently. They build and link an executable that gets passed to ocamldep and ocamlc with -pp (no .pp.ml) is generated.

I think its fine to test preprocessing, but I would make it more precise rather than claiming it covers all of ppx.

Agreed. Changing...

@robinbb robinbb changed the title test: regression guard for cross-lib reads of ppx-transformed deps test: regression guard for cross-lib reads of action-preprocessed deps Apr 30, 2026
@robinbb robinbb force-pushed the robinbb-test-cross-lib-ppx branch from 919ce44 to b83c527 Compare April 30, 2026 23:26
@robinbb robinbb requested a review from Copilot April 30, 2026 23:27
@robinbb robinbb requested a review from Alizter April 30, 2026 23:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a blackbox regression test to ensure a consumer library can build against a dependency library that uses an action-based preprocessor, guarding against regressions in cross-library dependency tracking when .pp.ml-based ocamldep artifacts are involved.

Changes:

  • Add a new blackbox test case cross-lib-ppx.t under per-module-lib-deps.
  • Introduce a trivial “passthrough” preprocessor executable used via (preprocess (action ...)).
  • Build a small dep/consumer library setup and assert dune build @check succeeds.

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-ppx.t Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new blackbox regression test to guard against failures when a library that is action-preprocessed (producing .pp.ml) is consumed by another library—covering a corner case relevant to future cross-library .d/ocamldep output consumption.

Changes:

  • Add cross-lib-ppx.t blackbox test case that sets up an action-preprocessed dependency library and a consumer library.
  • Assert the build succeeds (dune build @check) for this cross-library scenario.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a blackbox regression test to ensure Dune’s per-module, cross-library dependency walk can consume ocamldep output produced from action-preprocessed sources (where the .d left-hand side uses a *.pp.ml basename), preventing build failures like the one encountered during #14116 work.

Changes:

  • Add a new per-module-lib-deps blackbox test case for cross-library reads of ocamldep output from (preprocess (action ...)) libraries.
  • Construct a minimal action preprocessor and two unwrapped libraries (dep and consumer) to assert dune build @check succeeds.

Adds [test-cases/per-module-lib-deps/cross-lib-action-preprocess.t]. Documents that a consumer of a library that uses an action preprocessor builds without error today.

When the dep library uses [(preprocess (action ...))], dune feeds the [.pp.ml]-form of dep's source to ocamldep, so dep's [.d] file's left-hand basename is [foo.pp.ml] rather than [foo.ml]. Any future inter-library dependency tracking that reads dep's [.d] files from outside the producing rule must tolerate this mismatch with the raw [Module.t]'s [.ml] source path. This pitfall surfaced during ocaml#14116 development (the dev-tool build of utop, which is action-preprocessed, broke under the per-module filter's cross-library walk because the parser's basename check fired). This test guards against the same shape of regression in any future inter-library dep-tracking change.

Companion to the other observational baselines under [test-cases/per-module-lib-deps/]: each pins a structural shape that an inter-library filter must respect.

Signed-off-by: Robin Bate Boerop <me@robinbb.com>
@robinbb robinbb force-pushed the robinbb-test-cross-lib-ppx branch from be043f4 to a2f93a1 Compare May 1, 2026 01:05
@robinbb robinbb merged commit dd11f68 into ocaml:main May 1, 2026
31 checks passed
@robinbb robinbb deleted the robinbb-test-cross-lib-ppx branch May 1, 2026 16:22
robinbb added a commit to robinbb/dune that referenced this pull request May 3, 2026
The neighbour [cross-lib-action-preprocess.t] (regression guard for ocaml#14383, ocaml#14383) covers the same scenario — a consumer of a library that uses [(preprocess (action ...))] — but with a pass-through preprocessor and plain-OCaml sources. Any future code path that resolves the consumer's cross-library dependencies by running ocamldep on the dep's pre-[Module.pped] source would still pass that test, because ocamldep is happy with plain OCaml.

This test plugs the gap: [pp.exe] strips [#]-prefixed lines, and [dep/foo.ml] starts with a [#if]/[#endif] block that is syntactically invalid OCaml. Ocamldep on the post-pp output is fine; ocamldep on the pre-pp input is a syntax error. So if a future change runs ocamldep against the [Module.t] from [Dir_contents.modules_of_local_lib] (which is the raw, pre-[Module.pped] form, with [Module.source] pointing at the unprocessed [.ml] and [Module.pp_flags = None]), the consumer's build will fail with that syntax error and this test will catch it.

[dep] is multi-module so a hypothetical short-circuit for single-module stanzas can't mask the regression.

Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb added a commit to robinbb/dune that referenced this pull request May 3, 2026
The neighbour [cross-lib-action-preprocess.t] (regression guard for ocaml#14383, ocaml#14383) covers the same scenario — a consumer of a library that uses [(preprocess (action ...))] — but with a pass-through preprocessor and plain-OCaml sources. Any future code path that resolves the consumer's cross-library dependencies by running ocamldep on the dep's pre-[Module.pped] source would still pass that test, because ocamldep is happy with plain OCaml.

This test plugs the gap: [pp.exe] strips [#]-prefixed lines, and [dep/foo.ml] starts with a [#if]/[#endif] block that is syntactically invalid OCaml. Ocamldep on the post-pp output is fine; ocamldep on the pre-pp input is a syntax error. So if a future change runs ocamldep against the [Module.t] from [Dir_contents.modules_of_local_lib] (which is the raw, pre-[Module.pped] form, with [Module.source] pointing at the unprocessed [.ml] and [Module.pp_flags = None]), the consumer's build will fail with that syntax error and this test will catch it.

[dep] is multi-module so a hypothetical short-circuit for single-module stanzas can't mask the regression.

Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb added a commit to robinbb/dune that referenced this pull request May 3, 2026
The neighbour [cross-lib-action-preprocess.t] (regression guard for ocaml#14383, ocaml#14383) covers the same scenario — a consumer of a library that uses [(preprocess (action ...))] — but with a pass-through preprocessor and plain-OCaml sources. Any future code path that resolves the consumer's cross-library dependencies by running ocamldep on the dep's pre-[Module.pped] source would still pass that test, because ocamldep is happy with plain OCaml.

This test plugs the gap: [pp.exe] strips [#]-prefixed lines, and [dep/foo.ml] starts with a [#if]/[#endif] block that is syntactically invalid OCaml. Ocamldep on the post-pp output is fine; ocamldep on the pre-pp input is a syntax error. So if a future change runs ocamldep against the [Module.t] from [Dir_contents.modules_of_local_lib] (which is the raw, pre-[Module.pped] form, with [Module.source] pointing at the unprocessed [.ml] and [Module.pp_flags = None]), the consumer's build will fail with that syntax error and this test will catch it.

[dep] is multi-module so a hypothetical short-circuit for single-module stanzas can't mask the regression.

Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb added a commit that referenced this pull request May 5, 2026
## Summary

Adds a cram test that catches the failure mode where any inter-library
dependency-tracking code path runs ocamldep against the raw,
pre-`Module.pped` form of a foreign-lib module's source. For libs whose
preprocessor strips OCaml-invalid prefixes (like cppo's `#if`/`#endif`),
ocamldep on the pre-pp source rejects the file with a syntax error and
the consumer's build fails.

The neighbour test
`test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-action-preprocess.t`
(regression guard for #14383) covers the same scenario but uses a
pass-through preprocessor and plain-OCaml sources, so a code path that
runs ocamldep on the pre-pp form goes undetected — ocamldep is happy
with plain OCaml whether or not the preprocessor would have changed it.
This test plugs that gap by using a stripping preprocessor and a source
that is invalid OCaml on its own.

---------

Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants