Skip to content

Use std::filesystem to fix TestFileSys unittests on MSVC - #17395

Closed
Desour wants to merge 6 commits into
luanti-org:masterfrom
Desour:filesys_relpath_unittest_fix_stdfilesystem_smol
Closed

Use std::filesystem to fix TestFileSys unittests on MSVC#17395
Desour wants to merge 6 commits into
luanti-org:masterfrom
Desour:filesys_relpath_unittest_fix_stdfilesystem_smol

Conversation

@Desour

@Desour Desour commented Jul 30, 2026

Copy link
Copy Markdown
Member
  • Alternative to Fix AbsolutePath returning non identical strings on semantically identical paths #17355. But I'm no longer sure if it's better.
  • Fixes parts of MSVC Build has Unit Test Errors #17354.
  • testAbsolutePath: new requirement that doesn't hold (ends with no /) is removed
    (std::filesystem doesn't consider trailing / for normalization. maybe we should just make all our code robust against this. or implement our own even-more-normal-ize function)
  • testMakePathRelativeTo: MakePathRelativeTo is implemented with std::filesystem
    Note: only MakePathRelativeTo, which is new, so this PR is very unlikely to break anything
  • MakePathRelativeTo now constructs paths starting with .. if child is not in parent. => it's more general

To do

This PR is Ready for Review.

How to test

  • ./bin/luanti --run-unittests --test-module TestFileSys
  • Start the game and look for builtin sha warnings. There should be none.
  • Look at the CI output of MSVC

Desour added 6 commits July 30, 2026 18:15
@Desour
Fix MSVC TestFileSys unittests
67bf373 
* testAbsolutePath: new requirement that doesn't hold (ends with no /) removed
* testMakePathRelativeTo: MakePathRelativeTo implemented with std::filesystem
@Desour
native() doesnt return std::string on windows
18eca3f
@Desour
fix oopsie
696a0a4 
TODOs btw:
* check if relative() guarantees that it doesn't end with /
* check if weakly_canonical() actually works
@Desour
found out: operator/= doesn't add / if only a component is added (whi…
dc8e69e 
…ch is what lexically_relative does)

but I saw that lexically_relative can return default constred paths (probably
doesn't happen after weakly_canonical, I hope)
@Desour
tmp
d12c02a 
can't easily use weakly_canonical for AbsolutePathPartial, fyi, because
it doesn't guarantee removed trailing / (std::filesystem doesn't seem to care about trailing /
when it comes to normalization)
@Desour
cleanup
f03e18b
@Desour Desour added Bugfix 🐛 PRs that fix a bug @ Engine Core What happens inside the very engine labels Jul 30, 2026
@Desour

Desour commented Jul 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Look at the CI output of MSVC

Did now. And the trailing / doesn't work anymore. (not sure why, it used to work in previous iterations)

@Desour Desour closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Bugfix 🐛 PRs that fix a bug @ Engine Core What happens inside the very engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Desour