Skip to content

Fix path representations that are causing expect-like tests to fail in dune 3.24 - #482

Open
shonfeder wants to merge 3 commits into
realworldocaml:mainfrom
shonfeder:fix-path-represtation
Open

Fix path representations that are causing expect-like tests to fail in dune 3.24#482
shonfeder wants to merge 3 commits into
realworldocaml:mainfrom
shonfeder:fix-path-represtation

Conversation

@shonfeder

@shonfeder shonfeder commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

In ocaml/dune#15156 , release in dune 3.24, the representation of paths was change so that paths would always include their local directory prefix. Test fixtures that depend on the unnormalized representation of paths coming from dune start breaking as a result.

The two changes offered here fix the test breakage reported at #478 (comment) and should make #479 unnecessary for new release.

An alternative approach would be just promote the changes to the file representation into the test fixtures. However, normalizing and fully specifying path representations is the more robust choice if they get printed into text fixtures, IMO.

@shonfeder shonfeder changed the title Fix path representation causing change to exepect tests Fix path representations that are causing expect-like tests to fail Jul 22, 2026
@shonfeder shonfeder changed the title Fix path representations that are causing expect-like tests to fail Fix path representations that are causing expect-like tests to fail in dune 3.24 Jul 22, 2026
@shonfeder
shonfeder marked this pull request as draft July 22, 2026 16:24
Comment thread test/bin/misc-test-cases/mdx-pp/dune
@shonfeder
shonfeder force-pushed the fix-path-represtation branch from 2a9e585 to cc2579a Compare July 22, 2026 16:42
Strip off current directory prefix, if its present in a file name

This avoids use of String.starts_with to support ocaml =< 4.13 since
String.starts_with was only added in 4.13, but this project supports
ocaml >= 4.08.

The string prefix logic her duplicates code that can already be found in
lib/label.ml, but reusing it here would require a change to the public
API of the library, which would then be a breaking change rather than a
purely internal fix.
@shonfeder
shonfeder force-pushed the fix-path-represtation branch from cc2579a to 5aed4bd Compare July 22, 2026 16:43
This reverts part of commit bee52bd

Since the breaking changes to dune's file path representation have been
accounted for, there shouldn't be a need for this conflict any more.
@shonfeder
shonfeder marked this pull request as ready for review July 22, 2026 16:55
@shonfeder

Copy link
Copy Markdown
Contributor Author

Not sure if a changelog entry is actually wanted for this, but it could be user-visible so I've offered one.

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.

1 participant