-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #same_file? by providing access to 64 bit inode numbers. #8355
Conversation
Without this |
Could you elaborate on what this change does? Why is it necessary? Is it only necessary on macOS or do we need to port this to other platforms as well? |
MacOS has a 32 bit legacy and 64 bit API. The c include files I think Linux and *BSD went through the same process long ago. The Linux stat64 man page states:
I assume whatever tool generated the bindings didn't take the MacOS is my main test platform. I'm not sure about the others but they appear to use the 32 bit API's. Platforms using 32 bit inode numbers:
Platforms correctly using 64 bit inodes.
|
Nope. ULong is target dependent, thus UInt64 on 64-bit targets, so the only targets that don't use UInt64 are arm-linux-gnueabihf, i386-linux-gnu, and windows, and it's not necessary for them to have stat64. What I see, however, is that this PR actually broke stats on macOS :( |
You must also update |
If ULong's are 64 bit then I guess most platforms don't need changes. I don't have them available for testing. MacOS and FreeBSD still need changes. Proof:
The 32 bit and 64 stat structures have different layouts on MacOS. The current stat structure uses the 32 bit layout. This PR corrects it and changes the 32 bit Printing the inode number from crystal show a truncated value when compared to This PR show values that match the other programs.
The specs work. Eyeballing the output compared to command line programs looks correct. What broke? |
No, specs don't pass.
This can't be merged until this is fixed. |
07c7e8d
to
d782539
Compare
d782539
to
373134b
Compare
@ysbaddaden The specs pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you 👍
Note that x86_64-freebsd
target is fine —InoT is ULong since FreeBSD 12+ only, it's handled properly.
No description provided.