-
Notifications
You must be signed in to change notification settings - Fork 1.6k
RUSTC_ALLOW_UNSTABLE_<feature>: a RUSTC_BOOTSTRAP alternative
#3882
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
base: master
Are you sure you want to change the base?
Conversation
This RFC was in large part drafted in May 2025, shortly after my Rust All Hands talk, "Futile Feature Gates". There were about 25 people in that room; I want to thank all of them for their feedback during that session, and to everyone who reviewed early drafts of this RFC.
|
tagging this as t-lang per rust-lang/leadership-council#180 (comment) |
|
|
||
| Our motivation for 2 (disabling feature gates on stable) is three-fold: | ||
| 1. Prevent people from relying on features that may change in the future while on the stable toolchain, upholding our "stability without stagnation" motto. | ||
| 2. Disallow library authors from "silently" opt-ing in to unstable features, such that the person running the top-level build doesn't know they're using unstable features that may break when the toolchain is updated. This rationale doesn't apply to nightly, where the party running the top-level build is assumed to know that nightly comes with no stability guarantees. |
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 think Cargo already blocks build scripts from exporting RUSTC_BOOTSTRAP.
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.
If we allowed feature gates on stable without what is proposed in this RFC, then Cargo can't block dependencies from using unstable features as RUSTC_BOOTSTRAP wouldn't be necessary to set.
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.
There are two different things here. First, the current behavior. @celinval is correct that Cargo blocks build scripts from setting RUSTC_BOOTSTRAP (I wrote that PR).
Secondly, the new behavior. I think we should continue to disallow build scripts to silently opt-in, which means that Cargo should check if the env variable it's trying to set starts with RUSTC_ALLOW_UNSTABLE_, and if so, disallow it. That's equivalent to the current situation today, there's no change there.
| - Enabling RUSTC_BOOTSTRAP for one part of the toolchain enables it for *all* parts of the toolchain; in particular: | ||
| - `proc-macro2` uses `cargo:rerun-on-env-changed=RUSTC_BOOTSTRAP`, causing cache thrashing whenever this env var changes. <!--Using different env variables makes it so that we only rebuild when the feature the build script actually cares about changes.--> | ||
| - rust-analyzer wants to enable RUSTC_BOOTSTRAP only for cargo and libtest, but the variable enables features for rustc as well. `RUSTFLAGS="-Z allow-features="` fixes this for lang features, but at the price of thrashing the cache; and there is no equivalent way to disable unstable CLI features. |
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.
How should build tools like Cargo track build cache freshness, when people can pass arbitrary RUSTC_ALLOW_UNSTABLE_<feature> environment variable between different invocation?
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.
RUSTC_ALLOW_UNSTABLE_<feature> can contain the name of a crate, in which case you did only need to rebuild that crate.
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.
Again there are two things here. Currently, Cargo does not track RUSTC_BOOTSTRAP at all. The cache thrashing I'm talking about comes specifically from build scripts setting rerun-on-env-changed and from RUSTFLAGS, which are the only related things that Cargo tracks. When I say this helps with caching, I mean that proc_macro2 can start rerunning on RUSTC_ALLOW_FEATURE_PROC_MACRO_SPAN, which will almost never get set, rather than RUSTC_BOOTSTRAP, which currently gets set frequently.
Secondly, @weihanglo I think you are saying you want to add first-class support for cache invalidations once this RFC is merged. I am not sure that's necessary.
- Say that a build failed because the env variable was unset; then Cargo will try the build again regardless, so no action is necessary.
- Now, say the build succeeded because it was set, and someone runs the build again with it unset. In this case Cargo will successfully run the build, when it would have failed given a clean cache. I think this is ok. Again, these variables are super unstable, and the right answer IMO is "don't do that".
The only case where this could possibly affect behavior, rather than just whether the crate compiles or not, is if build scripts are trying to do feature detection. In that case, setting rerun-on-env-changed is the responsibility of the build script, not of Cargo.
This RFC was in large part drafted in May 2025, shortly after my Rust All Hands talk, "Futile Feature Gates". There were about 25 people in that room; I want to thank all of them for their feedback during that session, and everyone who reviewed early drafts of this RFC.
Rendered
Important
When responding to RFCs, try to use inline review comments (it is possible to leave an inline review comment for the entire file at the top) instead of direct comments for normal comments and keep normal comments for procedural matters like starting FCPs.
This keeps the discussion more organized.