Skip to content

test: record consumer rebuild count when shadowed dep lib changes#14324

Merged
Alizter merged 1 commit into
ocaml:mainfrom
robinbb:robinbb-test-lib-vs-lib-shadowing
Apr 29, 2026
Merged

test: record consumer rebuild count when shadowed dep lib changes#14324
Alizter merged 1 commit into
ocaml:mainfrom
robinbb:robinbb-test-lib-vs-lib-shadowing

Conversation

@robinbb
Copy link
Copy Markdown
Collaborator

@robinbb robinbb commented Apr 25, 2026

Summary

Adds a cram test for an under-tested corner: two unwrapped libraries export an entry module of the same name, and a consumer depends on both. OCaml's -I path resolution picks the library listed first in (libraries ...); the other lib's same-named module is shadowed and unreachable from consumer code.

The test is observational. It records the rebuild-target count for the consumer module after editing the losing (shadowed) lib's same-named module, asserting the count is positive. On current main, cross-library dep tracking is library-level: the consumer depends on a glob over each declared library's public cmi dir, so any change to the losing lib's public cmis invalidates the consumer.

Why add this now

Two reasons:

  1. Regression anchor. The corner is mechanically surprising (shadowing across library boundaries via -I order) and has no existing test.
  2. Bounds future work. Follow-on per-module dependency filtering (Finer dependency analysis between libraries #4572) cannot tighten this corner without qualified-path analysis — ocamldep -modules reports only the top-level reference, so the filter cannot know which lib's Shared the consumer resolves through. Any future filter must conservatively continue to track both. This test documents that expectation.

The assertion is > 0, so the test passes unchanged whether a future filter takes the library-level glob path or emits over-specified tight deps on both matching modules.

Test plan

  • dune runtest test/blackbox-tests/test-cases/per-module-lib-deps/lib-vs-lib-name-collision.t passes.

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 covering an OCaml module-name collision between two unwrapped libraries, ensuring the consumer still rebuilds when the shadowed library’s public interface changes—anchoring expectations for any future cross-library per-module dependency filtering work (issue #4572).

Changes:

  • Introduce a new per-module-lib-deps cram test for library-vs-library module name shadowing via -I path order.
  • Assert the consumer recompiles after a change to the losing library’s same-named module by inspecting dune trace output with jq.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/blackbox-tests/test-cases/per-module-lib-deps/lib-vs-lib-name-collision.t Outdated
@robinbb robinbb force-pushed the robinbb-test-lib-vs-lib-shadowing branch from 210879f to c8a0909 Compare April 25, 2026 00:34
@robinbb robinbb marked this pull request as ready for review April 25, 2026 01:06
Adds a cram test for the corner where two unwrapped libraries
export an entry module of the same name and a consumer depends
on both. OCaml's [-I] path resolution picks the first library
listed in [(libraries ...)]; the other lib's same-named module is
shadowed. Editing the losing lib's module still triggers a
rebuild of the consumer on current [main] — cross-library dep
tracking is library-level, so the consumer depends on a glob over
each lib's public cmi dir and any change invalidates.

The test is observational. It anchors current behavior for this
under-tested corner and documents the expectation for future
per-module filtering work on issue ocaml#4572: without qualified-path
analysis, the filter cannot disambiguate which lib's [Shared] the
consumer resolves through and must conservatively track both.

Signed-off-by: Robin Bate Boerop <me@robinbb.com>
@robinbb robinbb force-pushed the robinbb-test-lib-vs-lib-shadowing branch from c8a0909 to b0e81cc Compare April 27, 2026 15:40
robinbb added a commit to robinbb/dune that referenced this pull request Apr 27, 2026
Each of the dep PRs supplying a test in
[per-module-lib-deps/] was renamed per Leonidas's review on
ocaml#14309: generic [libA]/[modA1]-style names replaced by
role-bearing names like [dep_lib]/[spurious_rebuild]. This commit
applies the same renames to this PR's local copies of those tests
so they stay convergent with the dep PRs' canonical versions.

Touches:
- single-module-unreferenced-lib.t (ocaml#14308)
- sibling-unreferenced-lib.t (ocaml#14309)
- unwrapped-tight-deps.t (ocaml#14310)
- lib-vs-lib-name-collision.t (ocaml#14324)
- opaque-mli-change.t, opaque-cmx-deps-local.t (ocaml#14331)
- transitive-unreferenced-lib.t (ocaml#14332)
- wrapped-reexport-via-open-flag.t (ocaml#14346)
- auto-wrapped-child-reexport.t (ocaml#14347)

Pure rename + path/dir adjustments. Count assertions preserved
(this PR's promotions to the post-fix counts are unchanged).
Test suite passes.

Signed-off-by: Robin Bate Boerop <me@robinbb.com>
@robinbb robinbb requested a review from Copilot April 29, 2026 00:24
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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@Alizter Alizter self-assigned this Apr 29, 2026
@Alizter Alizter self-requested a review April 29, 2026 17:51
@Alizter Alizter merged commit aebbec9 into ocaml:main Apr 29, 2026
58 of 62 checks passed
@robinbb robinbb deleted the robinbb-test-lib-vs-lib-shadowing branch April 29, 2026 18:02
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