alexcrichton / filetime

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

Bug #28: Add set_file_handle_times #40

Closed rbtcollins closed 5 years ago

rbtcollins commented 5 years ago

I have matched the new API present in #36 for the new function so that when that is merged the API will be nice and regular.

Large delta due to cargo fmt - let me know if you want me to disable that for this project or something...

alexcrichton commented 5 years ago

Thanks! I like the new API but I felt the internal implementation was going too much in the wrong direction (no one's fault, I think it was just long overdue from some love) so I've pushed up a number of commits to update the internals.

alexcrichton commented 5 years ago

Hm and I didn't mean to merge already but I can fix any errors that come up in CI

alexcrichton commented 5 years ago

Also FWIW if this is in relation to performance of rustup on Windows, shouldn't the mtime/atime configuration be disabled when extracting tarballs on Windows? (there's no point in preservation for Rust's use case)

rbtcollins commented 5 years ago

Also FWIW if this is in relation to performance of rustup on Windows, shouldn't the mtime/atime configuration be disabled when extracting tarballs on Windows? (there's no point in preservation for Rust's use case)

This is a yak on that path in two ways. 1) To return the file handle from tar, tar needs to perform all its operations [ on windows] on the open file; otherwise readonly files will cause errors, as shown by the test suite. So this patch when released will let me add another patch towards (perhaps completing even) bug 189 in tar-rs; that in turn will let the File object drop occur after return from unpack without causing mode setting to be done while the file is open. And 2) to do the optimisation of setting the file readonly at creation time, we must do time setting on the returned handle, as handles opened for write later will get access-denied.