Skip to content

test: show wrapped compat incremental builds broken#14088

Merged
Alizter merged 1 commit intoocaml:mainfrom
Alizter:push-vrkvxqtqwmnu
Apr 8, 2026
Merged

test: show wrapped compat incremental builds broken#14088
Alizter merged 1 commit intoocaml:mainfrom
Alizter:push-vrkvxqtqwmnu

Conversation

@Alizter
Copy link
Copy Markdown
Collaborator

@Alizter Alizter commented Apr 8, 2026

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.

I didn't know this functionality existed but I agree that it should be working. Thanks for the repro.

Copy link
Copy Markdown
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

Print the dependencies inside the test.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter force-pushed the push-vrkvxqtqwmnu branch from 8a76d34 to 3a2e0fe Compare April 8, 2026 13:10
@Alizter Alizter requested a review from rgrinberg April 8, 2026 13:10
@rgrinberg
Copy link
Copy Markdown
Member

By the way, we should start discouraging people from using this feature. It is basically pointless now that we have (wrapped false) along with (include_subdirs qualified). People are welcome to create these aliases themselves.

The compat shim foo.cmi only depends on the wrapper mylib.cmi, not on the
inner module mylib__Foo.cmi that it re-exports:

$ dune rules --deps mylib/.mylib.objs/byte/foo.cmi 2>&1 | grep In_build_dir
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't we have percent form that can replace these ugly paths?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We can write %{cmi:mylib/foo} but not for the compat shim here unfortunately.

@Alizter
Copy link
Copy Markdown
Collaborator Author

Alizter commented Apr 8, 2026

By the way, we should start discouraging people from using this feature. It is basically pointless now that we have (wrapped false) along with (include_subdirs qualified). People are welcome to create these aliases themselves.

Sure, this seems sensible to deprecate. I only found this issue when testing out #14021.

@Alizter Alizter merged commit 23cf938 into ocaml:main Apr 8, 2026
30 checks passed
@Alizter Alizter deleted the push-vrkvxqtqwmnu branch April 8, 2026 19:59
Alizter added a commit that referenced this pull request Apr 8, 2026
robinbb added a commit to robinbb/dune that referenced this pull request Apr 9, 2026
…ing (ocaml#4572)

Add safety test ensuring per-module dependency filtering does not break
incremental builds when consumers access a library through wrapped compat
shims (bare module name via (wrapped (transition ...))).

Related: PR ocaml#14088, ocaml#14090 (wrapped compat incremental build fix).
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