-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add support for the unstable check-cfg
feature behind an environment variable
#3037
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
Conversation
r? @JohnTitor (rustbot has picked a reviewer for you, use r? to override) |
0934237
to
84373f1
Compare
Thanks for the detailed explanation, that sounds sensible! Could you fix Cirrus CI's failures? |
I'll get to that in the new year (this is work-related and I'm on vacation). |
84373f1
to
59ef5e2
Compare
Rebased on top of master. I have no clue why FreeBSD is failing though, my PR only changes how |
7058cf1
to
6874f57
Compare
a5b23ed
to
921e11c
Compare
Turns out the problem was enabling |
Sorry for the delay, I'll review in a few days! |
☔ The latest upstream changes (presumably #3127) made this pull request unmergeable. Please resolve the merge conflicts. |
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!
Could you resolve the merge conflict (I made it to remove the semver checks, sorry)? |
921e11c
to
b9a49d4
Compare
Rebased, and added |
Great, thank you! @bors r+ |
Add support for the unstable `check-cfg` feature behind an environment variable `check-cfg` ([Rust](https://doc.rust-lang.org/stable/unstable-book/compiler-flags/check-cfg.html), [Cargo](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg)) is an unstable features that warns when you write an unknown `#[cfg]` (likely due to a typo). The feature works out of the box for default cfgs and features provided by Cargo, but requires providing the list of extra cfgs when custom ones are used. This PR adds the `LIBC_CHECK_CFG` environment variable. When enabled, the build script will use the `cargo:rustc-check-cfg` println to instruct the compiler of all the possible cfgs set by libc. The build script was also refactored to ensure all cfgs are accounted for, and a CI job using `-Z check-cfg` was added. This PR is best reviewed commit-by-commit. ## Why is this needed? The main motivation for this PR is that `rust-lang/rust` enforces `check-cfg` across the whole codebase. Normally this is not a problem for dependencies like `libc`, as Cargo caps the lints and thus doesn't show the generated warnings. When developing support for new targets though, it's helpful to use a custom libc fork to develop the libc port and the std port together. Unfortunately doing that today results in a bunch of compilation errors, since lints are not capped with `path` dependencies. My goal with this PR is to address that shortcoming, as we'd then be able to set the `LIBC_CHECK_CFG=1` environment variable in the Rust build system and remove the compilation errors. This PR might also be helpful for libc maintainers, as the CI check might spot typos in `#[cfg]`s.
💔 Test failed - checks-actions |
@bors delegate+ |
✌️ @pietroalbini can now approve this pull request |
@bors r=JohnTitor |
☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14 |
1 similar comment
☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14 |
👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request. |
…k-Simulacrum Set `LIBC_CHECK_CFG=1` when building Rust code in bootstrap Downstream forks of the Rust compiler might want to use a custom `libc` to add support for targets that are not yet available upstream. Adding a patch to replace `libc` with a custom one would cause compilation errors though, because Cargo would interpret the custom `libc` as part of the workspace, and apply the check-cfg lints on it. Since rust-lang/libc#3037, the `libc` build script emits check-cfg flags only when the `LIBC_CHECK_CFG` environment variable is set, so this PR allows the use of custom `libc`s.
check-cfg
(Rust, Cargo) is an unstable features that warns when you write an unknown#[cfg]
(likely due to a typo). The feature works out of the box for default cfgs and features provided by Cargo, but requires providing the list of extra cfgs when custom ones are used.This PR adds the
LIBC_CHECK_CFG
environment variable. When enabled, the build script will use thecargo:rustc-check-cfg
println to instruct the compiler of all the possible cfgs set by libc. The build script was also refactored to ensure all cfgs are accounted for, and a CI job using-Z check-cfg
was added.This PR is best reviewed commit-by-commit.
Why is this needed?
The main motivation for this PR is that
rust-lang/rust
enforcescheck-cfg
across the whole codebase. Normally this is not a problem for dependencies likelibc
, as Cargo caps the lints and thus doesn't show the generated warnings.When developing support for new targets though, it's helpful to use a custom libc fork to develop the libc port and the std port together. Unfortunately doing that today results in a bunch of compilation errors, since lints are not capped with
path
dependencies. My goal with this PR is to address that shortcoming, as we'd then be able to set theLIBC_CHECK_CFG=1
environment variable in the Rust build system and remove the compilation errors.This PR might also be helpful for libc maintainers, as the CI check might spot typos in
#[cfg]
s.