Allow making `RUSTC_BOOTSTRAP` conditional on the crate name #77802

jyn514 · 2020-10-10T19:01:16Z

Motivation: This came up in the Zulip stream for rust-lang/compiler-team#350.
See also rust-lang/cargo#6608 (comment); this implements rust-lang/cargo#6627.
The goal is for this to eventually allow prohibiting setting RUSTC_BOOTSTRAP in build.rs (rust-lang/cargo#7088).

User-facing changes

RUSTC_BOOTSTRAP=1 still works; there is no current plan to remove this.
Things like RUSTC_BOOTSTRAP=0 no longer activate nightly features. In practice this shouldn't be a big deal, since RUSTC_BOOTSTRAP is the opposite of stable and everyone uses RUSTC_BOOTSTRAP=1 anyway.
RUSTC_BOOTSTRAP=x will enable nightly features only for crate x.
RUSTC_BOOTSTRAP=x,y will enable nightly features only for crates x and y.

Implementation changes

The main change is that UnstableOptions::from_environment now requires
an (optional) crate name. If the crate name is unknown (None), then the new feature is not available and you still have to use RUSTC_BOOTSTRAP=1. In practice this means the feature is only available for --crate-name, not for #![crate_name]; I'm interested in supporting the second but I'm not sure how.

Other major changes:

Added Session::is_nightly_build(), which uses the crate_name of
the session
Added nightly_options::match_is_nightly_build, a convenience method
for looking up --crate-name from CLI arguments.
Session::is_nightly_build()should be preferred where possible, since
it will take into account #![crate_name] (I think).
Added unstable_features to rustdoc::RenderOptions

I'm not sure whether this counts as T-compiler or T-lang; technically RUSTC_BOOTSTRAP is an implementation detail, but it's been used so much it seems like this counts as a language change too.

r? @joshtriplett
cc @Mark-Simulacrum @hsivonen

jyn514 · 2020-10-10T21:06:21Z

For ease of review: the important changes are https://github.com/rust-lang/rust/pull/77802/files#diff-1b4d2b405b087e9d70d2bc2b0c24d2fe, the rest of the changes are just passing in the crate name.

joshtriplett · 2020-10-10T21:14:21Z

This looks great to me. Once this change is in place, cargo could start catching attempts to set RUSTC_BOOTSTRAP from build.rs and suggest using RUSTC_BOOTSTRAP=cratename at the top level instead.

compiler/rustc_incremental/src/persist/file_format.rs

compiler/rustc_incremental/src/persist/load.rs

joshtriplett · 2020-10-11T17:24:14Z

I'm not sure whether this counts as T-compiler or T-lang; technically RUSTC_BOOTSTRAP is an implementation detail, but it's been used so much it seems like this counts as a language change too.

Reasonable question. I would generally suggest that compiler is the primary team that needs to approve, and then lang just needs to not object.

jyn514 · 2020-10-11T18:37:02Z

What is the path forward here? It sounds like @joshtriplett and @petrochenkov are in favor of the change - does it need an FCP or something? Should I nominate it for the weekly meeting?

hsivonen · 2020-10-12T06:57:27Z

Thanks for the heads-up. I filed a corresponding Firefox build system bug.

petrochenkov · 2020-10-16T22:55:15Z

@rfcbot fcp merge
With this PR RUSTC_BOOTSTRAP=foo,bar will enable nightly features only for crates named foo and bar.
Other opinions on addressing this problem space exist, but this is at least a strict improvement on status quo allowing to make the use of RUSTC_BOOTSTRAP more targeted.

rfcbot · 2020-10-16T22:55:16Z

nikomatsakis · 2020-10-20T12:33:43Z

@rfcbot reviewed

eddyb · 2020-10-20T16:39:39Z

@rfcbot reviewed

Doing it through the bot so I can say this: I disagree with doing anything here other than forcing distros' hand to adapt to something like rust-lang/compiler-team#350 (comment) which would even allow them to compile some problematic (for now? hopefully there's a transition path...) packages with "unstable rustc for version 1.45" as opposed to "stable rustc 1.45 RUSTC_BOOTSTRAP".

The important thing is that they would have control, and have to choose to exert it, over what gets to even use "unstable rustc", instead of anyone and anything being able to take any random stable rustc and make it unstable.

rfcbot · 2020-10-20T16:39:45Z

🔔 This is now entering its final comment period, as per the review above. 🔔

joshtriplett · 2020-10-20T21:06:07Z

@eddyb That seems like an orthogonal notion to this issue. Right now, the primary goal is to stop letting build.rs scripts set RUSTC_BOOTSTRAP, so that it can only be set at the top-level; this change will provide a good alternative that cargo can suggest.

Changing the RUSTC_BOOTSTRAP mechanism so that even end-users can't necessarily access it within a given rustc binary would be much stricter. I'm not at all opposed to that change, and I can imagine shipping a separate binary package for that frontend, only for use in rustc's own bootstrapping. But I don't want to make this change and the build.rs change block on that additional step.

rfcbot · 2020-10-30T16:41:41Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

jyn514

The FCP finished, so I think this is just waiting on review.

compiler/rustc_incremental/src/persist/file_format.rs

The main change is that `UnstableOptions::from_environment` now requires an (optional) crate name. If the crate name is unknown (`None`), then the new feature is not available and you still have to use `RUSTC_BOOTSTRAP=1`. In practice this means the feature is only available for `--crate-name`, not for `#![crate_name]`; I'm interested in supporting the second but I'm not sure how. Other major changes: - Added `Session::is_nightly_build()`, which uses the `crate_name` of the session - Added `nightly_options::match_is_nightly_build`, a convenience method for looking up `--crate-name` from CLI arguments. `Session::is_nightly_build()`should be preferred where possible, since it will take into account `#![crate_name]` (I think). - Added `unstable_features` to `rustdoc::RenderOptions` There is a user-facing change here: things like `RUSTC_BOOTSTRAP=0` no longer active nightly features. In practice this shouldn't be a big deal, since `RUSTC_BOOTSTRAP` is the opposite of stable and everyone uses `RUSTC_BOOTSTRAP=1` anyway. - Add tests Check against `Cheat`, not whether nightly features are allowed. Nightly features are always allowed on the nightly channel. - Only call `is_nightly_build()` once within a function - Use booleans consistently for rustc_incremental Sessions can't be passed through threads, so `read_file` couldn't take a session. To be consistent, also take a boolean in `write_file_header`.

nikomatsakis · 2020-11-15T12:00:57Z

@bors r

bors · 2020-11-15T12:00:59Z

📌 Commit 622c48e has been approved by nikomatsakis

…as-schievink Rollup of 13 pull requests Successful merges: - rust-lang#77802 (Allow making `RUSTC_BOOTSTRAP` conditional on the crate name) - rust-lang#79004 (Add `--color` support to bootstrap) - rust-lang#79005 (cleanup: Remove `ParseSess::injected_crate_name`) - rust-lang#79016 (Make `_` an expression, to discard values in destructuring assignments) - rust-lang#79019 (astconv: extract closures into a separate trait) - rust-lang#79026 (Implement BTreeMap::retain and BTreeSet::retain) - rust-lang#79031 (Validate that locals have a corresponding `LocalDecl`) - rust-lang#79034 (rustc_resolve: Make `macro_rules` scope chain compression lazy) - rust-lang#79036 (Move Steal to rustc_data_structures.) - rust-lang#79041 (Rename clean::{ItemEnum -> ItemKind}, clean::Item::{inner -> kind}) - rust-lang#79058 (Move likely/unlikely argument outside of invisible unsafe block) - rust-lang#79059 (Print 'checking cranelift artifacts' to easily separate it from other artifacts) - rust-lang#79063 (Update rustfmt to v1.4.26) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

glandium · 2021-02-17T08:04:51Z

Is it expected that crates with a dash in their name as per the crate's Cargo.toml need to be listed in RUSTC_BOOTSTRAP with an underscore instead?

jyn514 · 2021-02-17T15:33:51Z

@glandium I think so, yes - that's the --crate-name that cargo passes to rustc:

$ cargo build -v
   Compiling test-dhat v0.1.0 (/home/joshua/test-rustdoc/test-dhat)
     Running `rustc --crate-name test_dhat --edition=2018 src/main.rs ...

This was the whole point of rust-lang/rust#77802. - Pass `pkg.name()` to `parse()`. This can't pass the `Package` directly because `PackageInner` is an `Rc` and therefore not thread-safe. Note that `pkg_name` was previously a *description* of the package, not the name passed with `--crate-name`.

Forbid setting `RUSTC_BOOTSTRAP` from a build script on stable Instead, recommend `RUSTC_BOOTSTRAP=crate_name`. If cargo is using a nightly toolchain, or if RUSTC_BOOTSTRAP was set in *cargo*'s build environment, the error is downgraded to a warning, since the variable won't affect the build. This is mostly the same as suggested in #7088 (comment), except that `RUSTC_BOOTSTRAP=` values other than 1 are treated the same as `RUSTC_BOOTSTRAP=1`. My reasoning was that rust-lang/rust#77802 is now on 1.50 stable, so some crates may have started using it, and I would still prefer not to give hard errors when there's no workaround. Closes #7088. r? `@joshtriplett`

Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name Fixes #9362. The whole point of rust-lang/rust#77802 was to allow specifying this granularly, giving a hard error defeats the point. I didn't know how to check what targets were reverse-dependencies of build.rs, so I just unconditionally use the library name (and give a hard error for anything else, even if it's the name of one of the binaries). End-users can still opt-in with RUSTC_BOOTSTRAP=1, and no public binaries use RUSTC_BOOTSTRAP=1, so I don't think this a big deal in practice. <details><summary>Script to verify all crates using RUSTC_BOOTSTRAP=1 have a library</summary> ```sh curl https://pastebin.com/raw/fGQ97xP6 | cut -d / -f1 | grep -v shnatsel | grep -v cargo- | sed 's#-\([0-9]\)#/\1#' | xargs -i curl -s -I -L "https://docs.rs/{}/" -w "%{http_code}\n" -o/dev/null ``` It should output 20 200s in a row. </details> r? `@ehuss` cc `@mark-simulacrum` I don't know what cargo's policy is for backports, but this should be backported to 1.52.

jyn514 added A-stability Area: issues related to #[stable] and #[unstable] attributes themselves. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 10, 2020

rust-highfive assigned joshtriplett Oct 10, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2020

petrochenkov self-assigned this Oct 10, 2020

jyn514 force-pushed the bootstrap-specific branch from f05b859 to 1e21d44 Compare October 10, 2020 19:17

petrochenkov reviewed Oct 11, 2020

View reviewed changes

compiler/rustc_incremental/src/persist/file_format.rs Outdated Show resolved Hide resolved

compiler/rustc_incremental/src/persist/load.rs Outdated Show resolved Hide resolved

petrochenkov removed their assignment Oct 11, 2020

This comment has been minimized.

Sign in to view

jyn514 force-pushed the bootstrap-specific branch from 8246bc6 to 2cb2420 Compare October 13, 2020 13:19

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 16, 2020

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 20, 2020

joshtriplett mentioned this pull request Oct 21, 2020

Forbid setting RUSTC_BOOTSTRAP from a build script rust-lang/cargo#7088

Closed

jyn514 commented Nov 7, 2020

View reviewed changes

compiler/rustc_incremental/src/persist/file_format.rs Outdated Show resolved Hide resolved

jyn514 force-pushed the bootstrap-specific branch from cf7add5 to 70bfc50 Compare November 7, 2020 18:39

jyn514 force-pushed the bootstrap-specific branch from 70bfc50 to 622c48e Compare November 7, 2020 18:45

oli-obk approved these changes Nov 9, 2020

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2020

jonas-schievink mentioned this pull request Nov 15, 2020

Rollup of 13 pull requests #79070

Merged

bors merged commit 8828632 into rust-lang:master Nov 15, 2020

rustbot added this to the 1.50.0 milestone Nov 15, 2020

jyn514 deleted the bootstrap-specific branch November 15, 2020 15:49

jyn514 mentioned this pull request Dec 24, 2020

Require users to confirm they know RUSTC_BOOTSTRAP is unsupported before using it rust-lang/compiler-team#350

Closed

3 tasks

jyn514 mentioned this pull request Feb 17, 2021

Forbid setting RUSTC_BOOTSTRAP from a build script on stable rust-lang/cargo#9181

Merged

cuviper mentioned this pull request Mar 24, 2021

Probe rustc channel cuviper/autocfg#33

Closed

This was referenced Apr 16, 2021

Cargo misleads users when RUSTC_BOOTSTRAP set by build script rust-lang/cargo#9362

Closed

Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name rust-lang/cargo#9365

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow making `RUSTC_BOOTSTRAP` conditional on the crate name #77802

Allow making `RUSTC_BOOTSTRAP` conditional on the crate name #77802

jyn514 commented Oct 10, 2020

jyn514 commented Oct 10, 2020

joshtriplett commented Oct 10, 2020

joshtriplett commented Oct 11, 2020

jyn514 commented Oct 11, 2020

hsivonen commented Oct 12, 2020

This comment has been minimized.

petrochenkov commented Oct 16, 2020

rfcbot commented Oct 16, 2020 •

edited by wesleywiser

Loading

nikomatsakis commented Oct 20, 2020

eddyb commented Oct 20, 2020 •

edited

Loading

rfcbot commented Oct 20, 2020

joshtriplett commented Oct 20, 2020

rfcbot commented Oct 30, 2020

jyn514 left a comment •

edited

Loading

nikomatsakis commented Nov 15, 2020

bors commented Nov 15, 2020

glandium commented Feb 17, 2021

jyn514 commented Feb 17, 2021

Allow making RUSTC_BOOTSTRAP conditional on the crate name #77802

Allow making RUSTC_BOOTSTRAP conditional on the crate name #77802

Conversation

jyn514 commented Oct 10, 2020

User-facing changes

Implementation changes

jyn514 commented Oct 10, 2020

joshtriplett commented Oct 10, 2020

joshtriplett commented Oct 11, 2020

jyn514 commented Oct 11, 2020

hsivonen commented Oct 12, 2020

This comment has been minimized.

petrochenkov commented Oct 16, 2020

rfcbot commented Oct 16, 2020 • edited by wesleywiser Loading

nikomatsakis commented Oct 20, 2020

eddyb commented Oct 20, 2020 • edited Loading

rfcbot commented Oct 20, 2020

joshtriplett commented Oct 20, 2020

rfcbot commented Oct 30, 2020

jyn514 left a comment • edited Loading

Choose a reason for hiding this comment

nikomatsakis commented Nov 15, 2020

bors commented Nov 15, 2020

glandium commented Feb 17, 2021

jyn514 commented Feb 17, 2021

Allow making `RUSTC_BOOTSTRAP` conditional on the crate name #77802

Allow making `RUSTC_BOOTSTRAP` conditional on the crate name #77802

rfcbot commented Oct 16, 2020 •

edited by wesleywiser

Loading

eddyb commented Oct 20, 2020 •

edited

Loading

jyn514 left a comment •

edited

Loading