Skip to content

test: regression guard for cross-lib reads of single-module leaf libs#14389

Merged
robinbb merged 1 commit into
ocaml:mainfrom
robinbb:robinbb-test-no-ocamldep-leaf-lib
May 1, 2026
Merged

test: regression guard for cross-lib reads of single-module leaf libs#14389
robinbb merged 1 commit into
ocaml:mainfrom
robinbb:robinbb-test-no-ocamldep-leaf-lib

Conversation

@robinbb
Copy link
Copy Markdown
Collaborator

@robinbb robinbb commented Apr 30, 2026

Summary

Adds test-cases/per-module-lib-deps/no-ocamldep-leaf-lib.t, an observational baseline asserting that a multi-module consumer of a single-module leaf library builds without error.

Why

The dep is a "leaf": one .ml module, no library dependencies of its own. Single-module stanzas trigger a fast path in dune that skips ocamldep for the stanza, so no .d rule is produced for the leaf's module. Any future inter-library dependency tracking that walks consumer ocamldep into dep libraries must recognise this shape and not demand a nonexistent .d file — otherwise the consumer's compile fails with "No rule found" during rule evaluation.

This pitfall surfaced during #14116 development. The test guards against the same shape of regression in any future inter-library dep-tracking change.

Test plan

  • On main today: dune runtest test/blackbox-tests/test-cases/per-module-lib-deps/no-ocamldep-leaf-lib.t passes.
  • Sensitivity-checked by deliberately breaking consumer/c.ml to reference an unbound module — the test diffs against the resulting compile error, confirming the assertion catches build failures.

The shape pinned here (single-module unwrapped leaf consumed by a multi-module consumer with a real cross-library reference) is not exercised by any existing test on main. Companion to the other observational baselines under test-cases/per-module-lib-deps/ (cross-lib-ppx.t, private-modules.t, etc.): each pins a structural shape that an inter-library filter must respect.

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 guard against a failure mode in per-module inter-library dependency tracking when a dependency library is a single-module “leaf” that doesn’t produce ocamldep .d output.

Changes:

  • Add a new cram test case that builds a multi-module consumer library depending on a single-module unwrapped leaf library.
  • Document (in-test) the expected shape and the specific “No rule found” regression this is meant to prevent.

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/no-ocamldep-leaf-lib.t Outdated
@robinbb robinbb force-pushed the robinbb-test-no-ocamldep-leaf-lib branch from 4906d19 to 8d54e20 Compare April 30, 2026 23:51
@robinbb robinbb requested a review from Copilot April 30, 2026 23:52
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 cram test that guards against a specific regression in inter-library dependency tracking: consumers of a single-module “leaf” library (which doesn’t produce ocamldep .d files due to Dune’s single-module fast path) must still build successfully.

Changes:

  • Add a new per-module-lib-deps test case that builds a multi-module consumer library depending on a single-module leaf library.
  • Document within the test why the leaf shape is sensitive to cross-library ocamldep-walking implementations.

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/no-ocamldep-leaf-lib.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 ensure a multi-module consumer can build against a single-module “leaf” library where Dune’s singleton-module fast path produces no ocamldep .d artifacts—guarding against future inter-library dep tracking changes that might incorrectly require those .d files.

Changes:

  • Add no-ocamldep-leaf-lib.t blackbox test constructing a single-module leaf lib and a multi-module consumer that references it.
  • Assert dune build @check succeeds for that shape to catch “No rule found” regressions tied to missing .d rules.

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/no-ocamldep-leaf-lib.t Outdated
Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/no-ocamldep-leaf-lib.t Outdated
@robinbb robinbb force-pushed the robinbb-test-no-ocamldep-leaf-lib branch 2 times, most recently from 7fec8a0 to 657ca03 Compare May 1, 2026 01:07
@robinbb robinbb requested a review from Copilot May 1, 2026 01:07
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 cram test case to guard against regressions where inter-library dependency tracking attempts to read ocamldep .d artifacts for a single-module “leaf” library stanza (where Dune may skip ocamldep and therefore not produce .d rules), ensuring a multi-module consumer can still build successfully.

Changes:

  • Add a new observational baseline cram test for a multi-module consumer depending on a single-module, dependency-free leaf library.
  • Exercise both .ml and .mli presence in the leaf library to cover impl/intf-side dependency-walk behavior.
  • Assert successful build via dune build @check for this project shape.

@robinbb robinbb marked this pull request as ready for review May 1, 2026 01:11
Copy link
Copy Markdown
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

Looks good however I am a bit wary of writing so much about the internal workings of Dune in the description of the test. In case that changes for whatever reason whoever changes it must be aware of this test and change the text to reflect the implementation details or will forget in which case this test will contain misleading information that doesn't match the implementation.

Adds [test-cases/per-module-lib-deps/no-ocamldep-leaf-lib.t]. Documents that a multi-module consumer of a single-module leaf library builds without error today.

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-no-ocamldep-leaf-lib branch from 657ca03 to 1ed3ac9 Compare May 1, 2026 14:22
@robinbb robinbb requested a review from Copilot May 1, 2026 14:23
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 a specific per-module inter-library dependency tracking pitfall: consumers must not require ocamldep-generated .d files from single-module “leaf” libraries where Dune intentionally skips ocamldep.

Changes:

  • Add a new per-module-lib-deps cram test that builds a multi-module consumer library depending on an unwrapped, single-module leaf library.
  • Assert the build succeeds to prevent regressions where rule evaluation demands a nonexistent leaf .d file.

@robinbb
Copy link
Copy Markdown
Collaborator Author

robinbb commented May 1, 2026

I have made the change that you implicitly suggest, @Leonidas-from-XIV .

@robinbb robinbb merged commit 66d01ca into ocaml:main May 1, 2026
35 checks passed
@robinbb robinbb deleted the robinbb-test-no-ocamldep-leaf-lib branch May 1, 2026 15:17
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