-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix missing SO_* values #4242
base: main
Are you sure you want to change the base?
Fix missing SO_* values #4242
Conversation
ci/verify-build.sh
Outdated
@@ -73,7 +72,7 @@ test_target() { | |||
|
|||
if [ "$os" = "linux" ]; then | |||
# Test with the equivalent of __USE_TIME_BITS64 | |||
RUST_LIBC_UNSTABLE_LINUX_TIME_BITS64=1 $cmd | |||
RUST_LIBC_UNSTABLE_LINUX_TIME_BITS64=1 $cmd --target-dir "target/$target-time64" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of the changes to this file, could you add println!("cargo:rerun-if-env-changed=RUST_LIBC_UNSTABLE_LINUX_TIME_BITS64")
to build.rs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pushed an attempt. It seems to trigger rebuilds, but I'm not sure it's in the right place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you just mean the right place within build.rs
? If so, I would put it immediately before/after the env is read at let linux_time_bits64 = env::var("RUST_LIBC_UNSTABLE_LINUX_TIME_BITS64").is_ok();
so it's easy to see they go together.
LGTM otherwise, note that CI will fail until a rust-lang/rust fix gets into nightly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my question. I moved things around a bit to collect the linux_time_bits64 stuff in one place as much as possible.
a5e53d7
to
45eefd1
Compare
Collect the linux_time_bits64 in one place.
The _NEW defines are not available in musl and ohos.
45eefd1
to
57ffa6c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for following up, this looks good to merge as soon as CI gets fixed.
Some identifiers for #4148 was missing, and this was not detected because
verify-build
does not rebuild whenRUST_LIBC_UNSTABLE_LINUX_TIME_BITS64=1 is set.
Fix the
verify-build
task by using a separatetarget-dir
and add the missing identifiersThis is probably not the best way of structuring the CI steps, it could also be set up using a matrix in the
verify-build
task ofci.yml
.Sources
https://github.com/torvalds/linux/blob/master/arch/mips/include/uapi/asm/socket.h#L125
https://github.com/torvalds/linux/blob/master/arch/powerpc/include/uapi/asm/socket.h
https://github.com/torvalds/linux/blob/master/tools/include/uapi/asm-generic/socket.h
Checklist
libc-test/semver
have been updated*LAST
or*MAX
areincluded (see #3131)
cd libc-test && cargo test --target mytarget
);especially relevant for platforms that may not be checked in CI