POSIX: change struct stat/stat64 times to timespec#5069
Conversation
|
Hmm, since freebsd11 no longer depends on c_long, the prelude is no longer necessary - but it feels wrong to remove. |
Source: https://github.com/illumos/illumos-gate/blob/5882dd1367bb5bcc67b3d8719c715fe3feb0de15/usr/src/uts/common/sys/stat.h#L72 Note: timestruc_t is an alias for struct timespec: https://github.com/illumos/illumos-gate/blob/5882dd1367bb5bcc67b3d8719c715fe3feb0de15/usr/src/uts/common/sys/time_impl.h#L81
They have all been fixed.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
There was a problem hiding this comment.
All of this looks pretty good to me, but I'm going to hold off on merging for a bit. We have a handful of changes coming down the pipeline that are doing 64-bit time conversions, which this will conflict with, and it will be easier to apply this after all of that rather than before. (Not on its own, but taking stable backports into consideration).
Would you mind adding the reasoning (posix spec alignment) to the PR description? For any drive by readers.
|
For some time to get time64 changes in, |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Come on alarm clock... |
|
(https://snoozeth.is/WTvxKr50tBM) I will wait until Wed, 19 Aug 2026 07:45:28 UTC and then add label S-waiting-on-review and remove label S-blocked. @rustbot claim. |
Done - I made a nice and prominent "Why?" section at the top :) |
|
☔ The latest upstream changes (possibly #5237) made this pull request unmergeable. Please resolve the merge conflicts. |
Why?
The primary reason for this change is that this is the correct POSIX specification, and using _nsec etc. may be considered deprecated - meaning this better follows the C struct definition more closely as well as C man pages / docs. Additionally, this makes the struct easier to read, and removes the need for many test skips, and brings all established platforms in parity with Hurd etc. which already use the new declaration in this crate. More discussion in #4965
Description
Since this was evidently a problem when doing musl changes and v1 being in alpha, it's time to rip the band aid off and do it.
Of course, there are a couple unsolved questions:
libc/src/windows/mod.rs
Lines 52 to 54 in 17adf2d
libc/src/windows/mod.rs
Line 43 in 17adf2d
libc/src/unix/newlib/generic.rs
Lines 22 to 27 in 0dddfc8
xtimespecinstead ofxtim: https://github.com/apple-oss-distributions/xnu/blob/f6217f891ac0bb64f3d375211650a4c1ff8ca1ea/bsd/sys/stat.h#L202-L206 - but interestingly, Swift seems to rename this to the POSIX correct form, despite the underlying C headers not having it: https://github.com/apple/swift-system/blob/main/Sources/System/FileSystem/Stat.swift#L428. We need to pick one.Also, solid and qurt do not have nsec at all. NTO and VxWorks have already migrated.
Sources
In each relevant commit.
Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see #3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI
As discussed in / Fixes: #4965