Enable statx on all Linux targets and short-circuit it for musl#159540
Enable statx on all Linux targets and short-circuit it for musl#159540Gelbpunkt wants to merge 3 commits into
Conversation
rustbot
added
S-waiting-on-review
|
|
This comment has been minimized.
This comment has been minimized.
Gelbpunkt
changed the title
Since statx is currently invoked through the syscall directly anyways, it makes no sense to gate it to glibc targets, since none of this is glibc specific at the moment. As long as the kernel is recent enough, it supports statx.
This is a requirement for using libc::statx in std and the musl baseline in the Rust compiler is currently 1.2.5.
On musl targets, we can assume that musl has support for statx and falls back to fstatat internally, so there's no need to do detection and caching of statx in std.
|
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. |
|
The job Click to see the possible cause of the failure (guessed by this bot)Important For more information how to resolve CI failures of this job, visit this link. |
| // default unless opted into via an environment variable. The version baseline of the Rust | ||
| // compiler targets is currently 1.2.5, so we can set the variable to get access to statx in | ||
| // std, amongst other things. | ||
| cargo.env("RUST_LIBC_UNSTABLE_MUSL_V1_2_3", "1"); |
There was a problem hiding this comment.
This would break with build-std, right? Cargo wouldn't set this env var.
There was a problem hiding this comment.
That's a good point, right. Not sure how to go about that. I presume we'd want to modify cargo to set the variable for rustc processes while compiling std? That sounds fairly complicated
There was a problem hiding this comment.
Can we make this a regular cargo feature prefixed with unstable or internal-do-not-use or something like that and then enable it here?
7 participants
This supersedes #154981. cc @strophy and sorry for taking a while to get around to this.
The first patch is the original PR with the commit message slightly adjusted, which makes use of the statx runtime detection on all Linux targets.
Since we want to unconditionally use statx on musl targets (because we know statx is available on our supported musl versions), we can call it through
libc::statx. Making use of that requires telling the libc crate that we are on a musl version recent enough for it, which I decided to do inbootstrap. If there's a better place to set the environment variable, let me know, but this is the most straightforward way I could come up with.With that in place, we can then wire up a short circuit to
libc::statxfor musl targets and skip the detection logic that all other Linux targets use.On my
aarch64-unknown-linux-muslmachine,./x.py test library/stdstill passes with these changes.r? @tgross35 since I presume you have opinions on how I'm holding libc here