Enable statx on supported musl versions#154981
Enable statx on supported musl versions#154981strophy wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
r? @jhpratt rustbot has assigned @jhpratt. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Failed to set assignee to
|
This comment has been minimized.
This comment has been minimized.
| println!("cargo:rustc-check-cfg=cfg(musl_v1_2_3)"); | ||
| println!("cargo:rerun-if-env-changed=RUST_LIBC_UNSTABLE_MUSL_V1_2_3"); | ||
| if target_env == "musl" { | ||
| let musl_v1_2_3 = env::var("RUST_LIBC_UNSTABLE_MUSL_V1_2_3").is_ok() |
There was a problem hiding this comment.
Where is this environment variable set?
There was a problem hiding this comment.
Sorry, this variable is from the rust libc repo ci - I was setting it in Alpine Linux where I developed this patch, since I think both packages get built there, so I used the same logic. Should we set it unconditionally here? I'm unclear how to handle the upgrade, since I think a simultaneous change is needed in the vendored rustix version to prevent broken builds.
@tgross35 @neuschaefer @heiher can probably help clarify the issue better than me
There was a problem hiding this comment.
Aren't we already using a recent musl in std? I don't think we should need this kind of cfg if we link a known version.
The env is going away in favor of a cfg in libc anyway.
| // musl 1.2.5+: statx is a proper libc symbol, no weak linking needed. | ||
| #[cfg(all(target_env = "musl", musl_v1_2_3))] | ||
| unsafe fn statx( |
There was a problem hiding this comment.
Could weak linking be kept to avoid conditional compilation on musl version?
There was a problem hiding this comment.
Yes, I think we could keep that to avoid this condition since the upgrade to musl 1.2.5 has already taken place
Rust has feature-gated fs statx calls to gnu libc only. This feature was added to musl libc in the 1.2.5 release, which was in turn included in Rust 1.93.0, see #142682
There is a stale feature gate that is still blocking access to statx calls, even though this is now available in musl libc through the libc crate v0.2.175+, see rust-lang/libc#3976
This PR enables the feature. Note that this will likely cause build breakages anywhere rustix is used, and the issue has already popped up in the rustix repo bytecodealliance/rustix#1496 for archs that never supported musl <1.2.5
This is my first PR here and I need some help coordinating between all these moving parts. @Gankra could you help maybe? Thanks!
More refs:
#67774
vectordotdev/vector#23813
https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/98199
r? @Gankra