Fix directory iterator treating all files subsequent to a symlink as symlink on Windows #162
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix directory iterator treating all files subsequent to a symlink as symlink on Windows
|
Thank you for the fix! |
xTVaser
pushed a commit
to open-goal/jak-project
that referenced
this pull request
Jul 31, 2025
The context for this is that i was symlinking files from a mod installation that were identical to the unmodded game, for space considerations, because i remembered having success with that in the past, before the project made the switch to ghc's filesystem library, it seems. <sub>(I was also much more pressed for space back then, but well, i thought writing a small program to check for matches and link them for me would be a good, not-so-difficult exercise...)</sub> However i quickly started getting weird errors that it couldn't find GAME.CGO or KERNEL.CGO even when i hadn't touched them. Initially since i found that it occurred only when symlinking any files that were earlier alphabetically, i assumed it was some kind of index mismatch, but it was actually caused by a bug in an older version of filesystem.hpp causing the `directory_iterator` to read everything after a symlink as also a symlink, making them fail to be read properly: gulrak/filesystem#162 Adding `f.is_symlink()` is not as much of a change to the loop's condition from the previous behavior as it might first appear. It seems that even without that condition the iterator *was* reading symlinks before, just incorrectly (and before ghc::filesystem, they were being read just fine, iirc). With the new version, though, symlinks do have to be accounted for explicitly. Given that the library's major and minor version are both the same, i don't expect there should be any breaking changes to the API, and i didn't find any when i was testing but there might need to be more investigation into that before merging. Also, without the added check for `f.exists()`, the behavior is exactly the same as if, for some reason, a fakeiso file the game needs just doesn't exist in the first place (which has no real reason to happen in a normal installation of the game): there's a decent chance it will either crash at some point later during runtime, or something will happen like a level failing to load causing Jak to trip infinitely. I thought it might be helpful to have it fail right away in the case that, say, someone moved their vanilla installation and staled their symlinks, but i'll defer to maintainers' judgement on that point.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to microsoft documentation on the
dwReserved0member of_WIN32_FIND_DATAW,reparse_tag_from_INFOusesdwReserved0without checking theFILE_ATTRIBUTE_REPARSE_POINTattribute, which causesdirectory_iteratorto use a staledwReserved0value for all files subsequent to a symlink, causing all of these files to be treated as symlink incorrectly.This patch checks
FILE_ATTRIBUTE_REPARSE_POINTbefore checkingdwReserved0to correctly get the symlink status of the files. Also replaced a hack that uses structure size to determine structure type with proper templates.This patch is tested on CleverRaven/Cataclysm-DDA#63612 and cherrypicked from that PR.