Skip to content
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

Implement File.readable? and .writable? in Win32 #14420

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Apr 2, 2024

Note that File.writable returns true for read-only directories, which is inconsistent with File.info; this has always been the case for the original LibC._waccess_s, this PR merely exposes this fact in Crystal code.

@straight-shoota straight-shoota added this to the 1.13.0 milestone Apr 2, 2024
@HertzDevil
Copy link
Contributor Author

Could anyone reproduce the interpreter failures locally?

@straight-shoota
Copy link
Member

straight-shoota commented Apr 3, 2024

Nope. This seems very odd. Especially that both, readable and writable are failing. Maybe a file system issue? But it would be weird to have that only in the interpreter and not in compiled mode...
I added a commit to inspect the file permissions.

@straight-shoota straight-shoota marked this pull request as draft April 3, 2024 20:36
@straight-shoota
Copy link
Member

So the FS permissions definitely seem to be correct, yet the specs are still failing:

File::Permissions[OtherWrite, GroupWrite, OwnerWrite]F
File::Permissions[OtherRead, GroupRead, OwnerRead]F

I pushed another commit to see what happens for 0o000 permissions.

@straight-shoota
Copy link
Member

Interestingly, in the latest run there's only one failure: https://github.com/crystal-lang/crystal/actions/runs/8551832454/job/23432079701?pr=14420

@straight-shoota straight-shoota removed this from the 1.13.0 milestone Apr 17, 2024
@HertzDevil HertzDevil marked this pull request as ready for review April 17, 2024 18:23
@HertzDevil
Copy link
Contributor Author

Turns out this was just #13217 and compiled code would have had the same issue, CI didn't notice because we pass -u 1001 to Docker

@straight-shoota
Copy link
Member

Doh 🤦 Yeah that makes a lot of sense. Great find!

@straight-shoota straight-shoota added this to the 1.13.0 milestone Apr 17, 2024
@straight-shoota straight-shoota merged commit 8d2d480 into crystal-lang:master Apr 18, 2024
60 checks passed
@HertzDevil HertzDevil deleted the refactor/windows-file-accessible branch April 19, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants