std: Expose SystemTime accessors on fs::Metadata #30865
Conversation
|
cc @rust-lang/libs, we'll likely want to discuss this a bit before landing, especially the choice about returning r? @aturon |
|
Could you elaborate on the "not all platforms support this" aspect? It seems from the PR that this is known to succeed or fail at compile time, which seems to imply that it fits in with our platform-specific extension traits. Are we expecting some platforms to have "truly conditional" support, or is the extension trait too annoying? |
|
A creation time (birth time) is in the works for a few filesystems on Linux (like btrfs, ext4 and jfs), but most filesystems and older kernels will not support it. So an option looks right. |
|
@gankro currently it's a statically made decision based on the OS, but @pitdicker brings up an interesting point I had not thought of! |
| /// Returns the last access time of this metadata. | ||
| /// | ||
| /// The returned value corresponds to the `atime` field of `stat` on Unix | ||
| /// platforms and the `ftLastAccessTime` field on Windows platforms. |
There was a problem hiding this comment.
It may also be good to point out that even though all platforms do provide this value, not all platforms will actually keep it updated. I know at least on Windows there is an option to disable updating of the last access time.
|
File access time is such a misfeature I wonder if this might not be very futureproof. Maybe places we might want to run std won't support that feature. I don't have concrete examples though, and there are probably lots of places were we've wed std to the union of unix + windows. |
|
Maybe make all of the file times be |
alexcrichton
added
the
T-libs-api
Not a bad idea (especially for the case of filesystem variance). Though I wonder if |
alexcrichton
force-pushed
the
mtime-system-time
branch
from
698a49e to
421d57f
Compare
|
Updated with some tweaked docs and all methods returning |
|
r=me implementation-wise. Given that these are all unstable, I'm OK going ahead and landing, but if you'd rather wait till the next libs meeting, that's fine too. |
|
Cool, I'm fine waiting for the libs meeting, wanna be sure we're on the same page. |
alexcrichton
force-pushed
the
mtime-system-time
branch
from
421d57f to
cf74cd9
Compare
|
@bors: r=aturon cf74cd917feee816b2b7c5babe6aa6c99c87e40c (carrying over from the libs triage meeting) |
|
Oh, and discussed during libs triage today, we concluded that it's fine to add these functions as unstable for now. We may want to revisit signatures and/or naming, but the functionality seems good. |
alexcrichton
removed
the
T-libs-api
|
⌛ Testing commit cf74cd9 with merge 7ded903... |
|
💔 Test failed - auto-mac-64-nopt-t |
alexcrichton
force-pushed
the
mtime-system-time
branch
from
cf74cd9 to
208cd38
Compare
|
@bors: r=aturon |
|
📌 Commit 208cd38 has been approved by |
|
⌛ Testing commit 208cd38 with merge 13cb16a... |
|
💔 Test failed - auto-mac-64-opt |
alexcrichton
force-pushed
the
mtime-system-time
branch
from
208cd38 to
32cf074
Compare
|
@bors: r=aturon 32cf074 |
|
☔ The latest upstream changes (presumably #31360) made this pull request unmergeable. Please resolve the merge conflicts. |
These accessors are used to get at the last modification, last access, and creation time of the underlying file. Currently not all platforms provide the creation time, so that currently returns `Option`.
alexcrichton
force-pushed
the
mtime-system-time
branch
from
32cf074 to
d1681bb
Compare
These accessors are used to get at the last modification, last access, and creation time of the underlying file. Currently not all platforms provide the creation time, so that currently returns `Option`.
These accessors are used to get at the last modification, last access, and
creation time of the underlying file. Currently not all platforms provide the
creation time, so that currently returns
Option.