Skip to content

Simplify the reading of RUST_LIBC_UNSTABLE_GNU_*_BITS variables #4652

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

snogge
Copy link
Contributor

@snogge snogge commented Aug 14, 2025

Description

Use (almost) the same code in build.rs and libc-test/build.rs. Make the decision chain clearer.
Have a single default value that can be overriden in various ways, which should make switching the default much easier.

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot rustbot added S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Aug 14, 2025
@snogge snogge force-pushed the simplify-gnu-bits branch from 3db7606 to 808d0ba Compare August 14, 2025 12:41
@tgross35
Copy link
Contributor

Appreciate the cleanup, this looks better.

Would it make sense to require that OFFSET_BITS is unset if TIME_BITS is set? I think that may be more straightforward for users, so they only ever set one config at a time.

Another thing we talked about is switching to a rust cfg rather than env, so users can pass RUSTFLAGS='--cfg libc_unstable_gnu_time_bits="64"'. The benefit is this only affects the target, not anything built on the host system. The implementation should be the same if you are up for changing it here, just check CARGO_CFG_LIBC_UNSTABLE_GNU_TIME_BITS and CARGO_CFG_LIBC_UNSTABLE_GNU_FILE_OFFSET_BITS rather than the RUST_-prefixed env.

Use (almost) the same code in build.rs and libc-test/build.rs.
Make the decision chain clearer.
Have a single default value that can be overriden in various ways,
which should make switching the default much easier.
@snogge snogge force-pushed the simplify-gnu-bits branch from 808d0ba to 49b857d Compare August 18, 2025 13:33
@snogge
Copy link
Contributor Author

snogge commented Aug 18, 2025

I did a rewrite to require at most one variable to be set. The logic was also wrong and did not support the easy switch of default that I wanted.
This version supports just changing defaultbits to 64 to switch to 64 bit time_t. I hope this difference is small enough to handle on master.

The change to rust cfg requires CI-changes as well. I'll do that separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants