search

alexcrichton / filetime

Accessing file timestamps in a platform-agnostic fashion in Rust
Apache License 2.0
122 stars 56 forks source link

Enable utimensat for macos target OS #43

Closed kubkon closed 5 years ago

kubkon commented 5 years ago

This PR enables utimensat for MacOS target. It's probably not done in the most ergonomic way possible but I'm happy to work on it until it meets the expectations :-) Anyhow, it fixes problems with MacOS target in CraneStation/wasi-common#54.

cc @marmistrz @alexcrichton

kubkon commented 5 years ago

Thanks for the PR! Looking at the API it looks like this was a relatively recent addition to OSX, only in 10.13 was the utimensat API added.

Yep, it looks like it.

If that's the case I think we'll want this to retain compatibility with older OSX releases, so could this use dlsym and such to detect when utimensat is available on newer systems and use that, but otherwise fallback to the current implementation?

SGTM! I'll make the necessary adjustments and report back here :-)

kubkon commented 5 years ago

So here's a weird conundrum: the changes pass the tests for MacOS pre 10.13, and for MacOS 10.14 with XCode 11.0. For anything in-between, however, the tests fail at setting the file access time while leaving modified time alone only! I have absolutely no idea what could be the cause of this. I'll try to get my hands on an older version of MacOS and test it out in the VM (I've got only the latest MacOS at hand and everything passes at my end so I cannot reproduce the Travis CI problem). Anyhow, if you've got any ideas/suggestions, please do let me know! This problem's been eating away at my brain for the entire night last night!

alexcrichton commented 5 years ago

This may just be a bug in OSX from reports like https://github.com/haskell/directory/issues/88, although I can't find much other information on the problem.

I pushed a few commits with small refactorings, but otherwise I think it's fine to remove the other versions of OSX from testing and just test on the latest. This crate doesn't receive all that many changes to it tends to not be worth it to have a comprehensive CI matrix

kubkon commented 5 years ago

This may just be a bug in OSX from reports like haskell/directory#88, although I can't find much other information on the problem.

Yeah, it seems like it is! So, I've managed to get my hands on HighSierra running in a VM (via QEMU/KVM on Arch, don't ask me why this crazy setup), and I've managed to replicate the problem 100% even using pure C syscalls. From what I can see, the bug seems to be buried deep with setattrlistat and fsetattrlist syscalls, but no idea where as I can't find the source for either of those. If you have any suggestions here, lemme know and I'll be happy to explore them!

I pushed a few commits with small refactorings, but otherwise I think it's fine to remove the other versions of OSX from testing and just test on the latest. This crate doesn't receive all that many changes to it tends to not be worth it to have a comprehensive CI matrix

Saw them, thanks! Okay dokay then, I'll remove other versions of OSXs and I guess we can go from there.

alexcrichton commented 5 years ago

👍