Skip to content
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

Cargo misleads users when RUSTC_BOOTSTRAP set by build script #9362

Closed
Mark-Simulacrum opened this issue Apr 15, 2021 · 5 comments · Fixed by #9365
Closed

Cargo misleads users when RUSTC_BOOTSTRAP set by build script #9362

Mark-Simulacrum opened this issue Apr 15, 2021 · 5 comments · Fixed by #9365
Labels
C-bug Category: bug P-high Priority: High regression-from-stable-to-beta Regression in beta that previously worked in stable.

Comments

@Mark-Simulacrum
Copy link
Member

Problem

Starting with 1.52 (currently in beta), Cargo will prevent crates from setting RUSTC_BOOTSTRAP via build scripts. However, that prevention can be by-passed with an explicit RUSTC_BOOTSTRAP=1 in the environment. Cargo misleadingly tells users that RUSTC_BOOTSTRAP=<crate-name> will be sufficient to bypass the error.

Steps

Cargo.toml:

[package]
name = "foo"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
obfstr = "=0.2.1" # pin to a version that sets this variable

Running with the bootstrap variable setup as Cargo asks:

$ RUSTC_BOOTSTRAP=obfstr cargo +beta build
   Compiling obfstr v0.2.1
error: Cannot set `RUSTC_BOOTSTRAP=1` from build script of `obfstr v0.2.1`.
note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.
help: If you're sure you want to do this in your project, set the environment variable `RUSTC_BOOTSTRAP=obfstr` before running cargo instead.

Possible Solution(s)

Likely we should either:

  • enable nightly features in Cargo if any value of rustc_bootstrap is defined
  • adjust the message to indicate =1 (but @joshtriplett IIRC was against this approach)
@Mark-Simulacrum Mark-Simulacrum added C-bug Category: bug regression-from-stable-to-beta Regression in beta that previously worked in stable. labels Apr 15, 2021
@ehuss
Copy link
Contributor

ehuss commented Apr 16, 2021

Link PR #9181 cc @jyn514

Sorry, I'm a bit confused about what exactly is misleading here (there are definitely some confusing bits, I just want to clarify what is misleading to you).

enable nightly features in Cargo if any value of rustc_bootstrap is defined

I think that is exactly what the change is trying to prevent? I'm uncertain what this means.

adjust the message to indicate =1

I think the message is saying, remove RUSTC_BOOTSTRAP from the build script, and use RUSTC_BOOTSTRAP=obfstr outside of cargo to make it work in a targeted fashion. I think the intent here is to limit the scope of what gets nightly features enabled, and that must be an external opt-in. Using =1 is kinda against the intent of this exercise I think.

I just noticed that externally setting RUSTC_BOOTSTRAP=obfstr without changing the build script still fails. That seems like that should be changed to be allowed. That might make the error message make more sense?

@ehuss ehuss added the P-high Priority: High label Apr 16, 2021
@jyn514
Copy link
Member

jyn514 commented Apr 16, 2021

Ugh, I really really wish there were a way to test stable things from nightly channels. Yes, this is a bug and I intended RUSTC_BOOTSTRAP=crate_name to turn the error into a warning.

@jyn514
Copy link
Member

jyn514 commented Apr 16, 2021

I think the message is saying, remove RUSTC_BOOTSTRAP from the build script, and use RUSTC_BOOTSTRAP=obfstr outside of cargo to make it work in a targeted fashion. I think the intent here is to limit the scope of what gets nightly features enabled, and that must be an external opt-in. Using =1 is kinda against the intent of this exercise I think.

The whole point of rust-lang/rust#77802 was so that you don't have to modify the original build script.

@jyn514
Copy link
Member

jyn514 commented Apr 16, 2021

Ok I think I see the issue - cargo treats RUSTC_BOOTSTRAP different than rustc used to: it only allows nightly features when RUSTC_BOOTSTRAP=1, not on any other value:

if let Ok(staging) = env::var("RUSTC_BOOTSTRAP") {
if staging == "1" {
return "dev".to_string();
}
}

@ehuss do you want cargo to keep that behavior? If so the fix is simple enough, I can just check if RUSTC_BOOTSTRAP is set to any value in custom_build.rs.

@jyn514
Copy link
Member

jyn514 commented Apr 16, 2021

Err wait no, any value is incorrect, it should only allow it if RUSTC_BOOTSTRAP contains the crate name. Otherwise RUSTC_BOOTSTRAP=crate is no different from RUSTC_BOOTSTRAP=1.

@bors bors closed this as completed in fb0130c Apr 21, 2021
bors added a commit that referenced this issue Apr 21, 2021
Backport "Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name"

Backports #9365, fixing #9362.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug P-high Priority: High regression-from-stable-to-beta Regression in beta that previously worked in stable.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants