Try statx for all linux-gnu target. #67774
Conversation
|
r? @rkruppe (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
This looks vaguely plausible to me, but I am not familiar with this code or the platforms this change affects, so I'd like to pass the buck to someone who (presumably) is more familiar: r? @alexcrichton |
src/libstd/sys/unix/fs.rs
Outdated
| // We need to port these structs here, since libc does not always contain | ||
| // them, eg. on musl. | ||
| #[repr(C)] | ||
| struct statx { |
There was a problem hiding this comment.
This is scary. libc has CI in place to verify that we don’t end up with wrong struct definitions, but that’s not true for std… I think it makes more sense overall to just not use statx on the environments that do not want to expose this type.
There was a problem hiding this comment.
Yes this should be using the definitions in libc rather than duplicating them here.
There was a problem hiding this comment.
But musl does not export these structs currently... So we should only enable this when using glibc on Linux?
There was a problem hiding this comment.
Musl does support statx since 1.1.24: https://gitlab.com/gsauthof/musl/commit/dfc81828f7ab41da08f744c44117a1bb20a05749
There was a problem hiding this comment.
@mati865 No yet. It never exposes statx function wrapper and related structs. It just use it internally.
There's no way to get file birth time from public API, except invoking syscall with manual defined struct statx.
There was a problem hiding this comment.
To land, this must be using the statx structure from a location that is verified to be correct. The libc crate is one source which is verified correct, but it doesn't have to be the only one. Linux structures are notorious for changing across architectures and across libraries, so having an unverified copy here won't be maintainable over time.
|
☔ The latest upstream changes (presumably #66899) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@alexcrichton Rebased. Now it is limited to linux-gnu targets. |
oxalica
changed the title
|
@bors: r+ FWIW I think it's worthwhile to get these structures into |
|
📌 Commit f5baa03 has been approved by |
bors
added
S-waiting-on-bors
Try statx for all linux-gnu target. After rust-lang/libc#1577, which is contained in `libc` 0.2.66, provides `SYS_statx` for all Linux platform, so we can try to use `statx` for ~all Linux target~ all linux-gnu targets. Unfortunately, `struct statx` and `fn statx` is not a part of public interface of musl (currently), ~we still need to invoke it through `syscall`~ we does **not** support statx for musl or other libc impls currently. Previous PR: rust-lang#65094 cc @alexcrichton
Rollup of 10 pull requests Successful merges: - #67774 (Try statx for all linux-gnu target.) - #67781 (Move `is_min_const_fn` query to librustc_mir.) - #67798 (Remove wrong advice about spin locks from `spin_loop_hint` docs) - #67849 (Add a check for swapped words when we can't find an identifier) - #67875 (Distinguish between private items and hidden items in rustdoc) - #67887 (`Option::{expect,unwrap}` and `Result::{expect, expect_err, unwrap, unwrap_err}` have `#[track_caller]`) - #67955 (rustdoc: Remove more `#[doc(cfg(..))]` duplicates) - #67977 (Updates for VxWorks) - #67985 (Remove insignificant notes from CStr documentation) - #68003 (ci: fix wrong shared.sh import for publish_toolstate) Failed merges: - #67820 (Parse the syntax described in RFC 2632) - #67979 (Move `intravisit` => `rustc_hir` + misc cleanup) r? @ghost
Rollup of 10 pull requests Successful merges: - #67774 (Try statx for all linux-gnu target.) - #67781 (Move `is_min_const_fn` query to librustc_mir.) - #67798 (Remove wrong advice about spin locks from `spin_loop_hint` docs) - #67849 (Add a check for swapped words when we can't find an identifier) - #67875 (Distinguish between private items and hidden items in rustdoc) - #67887 (`Option::{expect,unwrap}` and `Result::{expect, expect_err, unwrap, unwrap_err}` have `#[track_caller]`) - #67955 (rustdoc: Remove more `#[doc(cfg(..))]` duplicates) - #67977 (Updates for VxWorks) - #67985 (Remove insignificant notes from CStr documentation) - #68003 (ci: fix wrong shared.sh import for publish_toolstate) Failed merges: - #67820 (Parse the syntax described in RFC 2632) - #67979 (Move `intravisit` => `rustc_hir` + misc cleanup) r? @ghost
After rust-lang/libc#1577, which is contained in
libc0.2.66, providesSYS_statxfor all Linux platform, so we can try to usestatxforall Linux targetall linux-gnu targets.Unfortunately,
struct statxandfn statxis not a part of public interface of musl (currently),we still need to invoke it throughwe does not support statx for musl or other libc impls currently.syscallPrevious PR: #65094
cc @alexcrichton