Skip to content

test(libc): check case-ambiguous names only on a case-sensitive host directory - #11797

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
jotitadev:test/libc-path-case-insensitive-host
Oct 29, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
jotitadev:test/libc-path-case-insensitive-host

Conversation

@jotitadev

@jotitadev jotitadev commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What

guest_path_case aborts on macOS. Its non-Windows block creates Data/SETTINGS.INI next to Data/Settings.ini to check that two names differing only in case are read separately, and that an ambiguous lookup throws. APFS volumes are case-insensitive by default, so on macOS the second file overwrites the first (Data/Settings.ini then reads z, not x) and the test aborts in CheckRead.

The case-ambiguity checks now run only when the host directory is case-sensitive: after Data/Settings.ini is written, they run if Data/SETTINGS.INI does not exist. On a case-sensitive file system (Linux ext4, a case-sensitive APFS volume) that is always true, so the test runs exactly as before. On a case-insensitive one the ambiguity cannot happen, and the symlink, rewrite and dangling-link checks of the block still run. This is the same reason the block is already off Windows.

No library change: on a case-insensitive host the path resolver never sees two candidates.

Tested

Apple M2 Max, macOS 27.2 (APFS, case-insensitive), Apple clang 21, x86-64 Release under Rosetta 2. main alone does not build libc's tests on macOS, so the tree is main 81267b2 plus the open macOS build PRs (#7631, #11249, #11358, #10669, #10552, #7804, #11447, #11437, #11554).

  • Without this change: guest_path_case aborts in CheckRead called from GuestPathCase.cpp:64 (CheckRead("Data/Settings.ini", 'x')), symbolized from the crash report of a -g build.
  • With it: passes 3 runs out of 3.
  • Linux (Ubuntu, GCC 15.2): GuestPathCase.cpp passes g++ -std=gnu++20 -fsyntax-only, and on ext4 Data/SETTINGS.INI does not exist after Data/Settings.ini is written, so the original checks run. I did not build and run the test on Linux or Windows.

check_conventions.py --base origin/main passes.

Checklist

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 5021 core/libs/tests/GuestPathCase.cpp

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The macOS case-sensitivity guard itself matches the described behavior, but there is an unresolved overlap required by CONTRIBUTING. I manually compared the exact head with open PR #5021: both rewrite the same #ifndef _WIN32 block in GuestPathCase.cpp, and git merge-tree reports a content conflict in that file. Please coordinate the landing order with #5021 author and declare the agreed dependency before this is merge-ready. I am not suggesting either PR be closed.

@jotitadev

Copy link
Copy Markdown
Contributor Author

@MarcZX10 thanks. @yabanana, coordinating the overlap in core/libs/tests/GuestPathCase.cpp:

Proposed landing order: this PR first, and #5021 takes main's version of that hunk when it rebases (or keeps its own guard; either works). If you would rather land #5021's version, say so and I will close this one instead. I have added the overlap to the description.

Written with AI assistance (Claude Code).

@adriadam10 adriadam10 self-assigned this Oct 28, 2026

@THEMANnt THEMANnt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed 365e00176a838ff7c11eb3fa501b1420c036bf04 against ea048ce2c35e1fb65ea3331ed73bba35cb27e584. The non-Windows ambiguity fixture now checks whether the alternate-case name already resolves before creating it, avoiding overwrite on a case-insensitive host directory. The Windows fixture passes, but the changed non-Windows/symlink branch was source-reviewed rather than natively executed on Linux/macOS. No filesystem portability result beyond that scope is claimed. No actionable finding in this scoped test review.

Windows/GCC 15.3 Release: configured and built the actual pinned target; registered focused CTest: 1 passed, 0 skipped. Production is unchanged from the author base; baseline was source-compared, not rebuilt/rerun here. Matching cached dependency source pins and the matching existing FFmpeg package are used. No game, physical-console or GPU validation is claimed.

Formal COMMENT records the successful Windows regression check and source assessment while leaving the changed platform-specific path unverified locally.

@MarcZX10

Copy link
Copy Markdown
Contributor

Thanks, I see the proposed order recorded in this PR. I checked exact head 365e00176a838ff7c11eb3fa501b1420c036bf04 against live main 10bec5c322eb581b3e959e6c7e18bcf09e068e96; the current-main merge preview is clean, the one-file diff check is clean, and conventions pass.

I do not see confirmation from #5021’s author or a matching order recorded on that PR yet. Please get that confirmation and have both checklists record the agreed order. The guard itself is not the blocker; my existing changes-requested review remains limited to the unresolved overlap requirement.

@MarcZX10

Copy link
Copy Markdown
Contributor

For context, #5021's author has now proposed #11797 first, with #5021 retaining the equivalent guard when it rebases, and asked you to confirm whether that order works. I agree the two guards appear equivalent from the hunks described; I still need your explicit agreement before treating the landing order as settled. Once you confirm, please record the same order in both PR checklists and update the dependent branch as needed.

@jotitadev

Copy link
Copy Markdown
Contributor Author

@MarcZX10 @yabanana confirmed: I agree with that order. This PR (#11797) lands first, and #5021 keeps its equivalent GuestPathCase.cpp guard when it rebases. I have recorded it in this PR's checklist ("Depends on" line). Nothing on this branch needs updating for it: the current-main merge preview is clean.

Written with AI assistance (Claude Code).

@adriadam10
adriadam10 merged commit e2722b2 into boykopovar:main Oct 29, 2026
14 checks passed
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.

4 participants