Cargo: Forbid setting `RUSTC_BOOTSTRAP` from a build script

[matklad]

Currently, crates can set environmental variables, including RUSTC_BOOTSTRAP, from a build.rs file, thus subverting current stability guarantees. Cargo and, if that's feasible, crates.io additionally should forbid doing so.

This was originally discussed in https://github.com/rust-lang/cargo/pull/6608 and https://github.com/rust-lang/cargo/issues/6627, both of which are closed, so I thought it would be helpful to create this issue!

[est31]

I think one should go even further and let cargo clear any RUSTC_BOOTSTRAP env var passed to it, making RUSTC_BOOTSTRAP=1 cargo build not work any more, to make it as restricted as possible.

This doesn't prevent Firefox from using nightly features, they can just use a rustc wrapper using .cargo/config, similar to how rust is doing it themselves. In that wrapper, they could specify precisely which crate to give a RUSTC_BOOTSTRAP=1 env var and which not. It would be at a central place, and most importantly, people who use the crate as a library would know, e.g. Debian. It's basically what @hsivonen wanted in #6627.

Of course this can still be circumvented by build.rs calling rustc itself manually but it makes it considerably harder than it is now.

I guess a permanent and more restricting solution is to lock down RUSTC_BOOTSTRAP usage from the rust side, e.g. by not requiring #![feature(thing] to be set if a crate has #[unstable(thing] which greatly reduces the features that std has to have enabled, and then adding a whitelist of (crate_name, feature_name) tuples for all crates used by rustc. At the start, the unstable features used by Firefox and other production users which show up could even be on that whitelist, and as features stabilize they could be removed from the whitelist one by one, hopefully getting the whitelist into a state that it only contains rustc crates at a certain point in the future. Most features that RUSTC_BOOTSTRAP allows actually aren't needed for bootstrapping rustc.

[skade]

@est31 I would not go as far as that. Activating RUSTC_BOOTSTRAP is a user action and it's fine. If they opt into instability, they at least know what they are doing.

[est31]

@skade the user action is not fine because RUSTC_BOOTSTRAP is meant for bootstrapping rust, nothing more. It's no official feature of rust and never has been. It can be removed, changed, etc at any time. And in order for a right to exist, it has to be executed otherwise you'll soon notice you can't any more.

Firefox devs have their reasons to use RUSTC_BOOTSTRAP and they should be allowed to keep their usage, and ideally simd and this allocator stuff they depend on should be stabilized. But the restrictions I propose don't stop them from doing it, they just make casual/accidental use harder. The more hoops you have to jump through, the better.

[hsivonen]

Currently, crates can set environmental variables, including RUSTC_BOOTSTRAP, from a build.rs file, thus subverting current stability guarantees. Cargo and, if that's feasible, crates.io additionally should forbid doing so.

I find it disappointing that people react to the RUSTC_BOOTSTRAP hack by thinking "How do we forbid this?" instead of "How has Rust failed to stabilize useful features in a timely manner to necessitate such a hack, and how can we do better on getting useful features to stable?"

Having the hack in encoding_rs on crates.io has been harmless since February, because the build fails unless you've replaced packed_simd with a non-crates.io fork. (See the comment in its build.rs, which you can verify by trying to build the crate with the simd-accel feature using the stable compiler.) The simd crate has been rendered obsolete, so hindsight about that crate isn't particularly productive at this point.

So there isn't really an acute crates.io problem in need of solving. (Debian independently discovered this now, because they still had a copy of the code from January.)

This was originally discussed in #6608 and #6627, both of which are closed, so I thought it would be helpful to create this issue!

As far as subversion goes, it seems to me that filing a new issue subverts the issue closure. (I can see the temptation, though. It irks me that I can't reply on those issues.)

Before proposing changes (including possibly forbidding something), I invite people to re-read the original description of #6627 as well as my other comments there, especially this one.

[est31]

@hsivonen as I've pointed out above, one has already now fine grained control from the top level about which crate to give RUST_BOOTSTRAP and which not, which is precisely what you requested in #6627 (modulo how it's exposed to users). In fact, making the build.rs hack impossible would give more control to the top level crates (as enabling RUSTC_BOOSTRAP without blessing from the top level becomes harder) which seems to be what you want as well.

[matklad]

I suggest that we don't argue about "what stability policy Rust should have" in this, or any other issue. This is definitely an RFC-worthy material. #6627 in particular contains a treasure trove of useful information about real-world use-cases, and is already written in the form of an RFC, but is filed in the wrong venue. It seems to be a process failure that we haven't redirected this important discussion to a more appropriate venue in five months, despite the fact that a lot of prominent members participated in it!

I also would like to emphasize that we have an existing agreed upon stability policy, articulated in https://blog.rust-lang.org/2014/10/30/Stability.html, and this issue is about fixing a bug that allows to subvert that existing policy.

In particular,

Why not allow opting in to instability in the stable release?
...
Finally, we simply cannot deliver stability for Rust unless we enforce it. Our promise is that, if you are using the stable release of Rust, you will never dread upgrading to the next release. If libraries could opt in to instability, then we could only keep this promise if all library authors guaranteed the same thing by supporting all three release channels simultaneously.

packed_simd is only a "bug reproduction" for this issue, packed_simd being fixed doesn't change the fact that Cargo still has this bug.

[hsivonen]

@matklad

this issue is about fixing a bug that allows to subvert that existing policy.

It would be good to fix things in the right order. Specifically, to introduce the replacement solution before forbidding the current one.

Our promise is that, if you are using the stable release of Rust, you will never dread upgrading to the next release.

Dreading that a Rust upgrade breaks the build for enforcement purposes isn't that great, either.

packed_simd is only a "bug reproduction" for this issue, packed_simd being fixed doesn't change the fact that Cargo still has this bug.

packed_simd is not at fault here. It has a rust-toolchain override, so even if you say rustup default stable, it picks your nightly compiler. However, if you try to build it as a dependency of another crate using the stable compiler, the build fails.

@est31

The more hoops you have to jump through, the better.

I'm not a Firefox build system developer. CC @glandium to evaluate your suggestion.

(AFAICT, we already optionally use sccache as a wrapper and a wrapperception probably isn't practical. Putting a feature that's logically more in Cargo's domain than in sccache's domain into sccache seems like an unfortunate outcome.)

In fact, making the build.rs hack impossible would give more control to the top level crates (as enabling RUSTC_BOOSTRAP without blessing from the top level becomes harder) which seems to be what you want as well.

Once a top-level thing is actually in place and working, I agree.

[nox]

It would be good to fix things in the right order. Specifically, to introduce the replacement solution before forbidding the current one.

The replacement solution has always existed: use nightly. You and others forcing the use of RUSTC_BOOTSTRAP is sending a very bad message, making more RUSTC_BOOTSTRAP usage spread across the community.

Dreading that a Rust upgrade breaks the build for enforcement purposes isn't that great, either.

Well, we have been telling you since the very beginning that you shouldn't be using RUSTC_BOOTSTRAP given you are not, you know, bootstrapping the compiler.

Once a top-level thing is actually in place and working, I agree.

It is already in place, it's called a nightly compiler.

[skade]

I think a couple of problems need to be appreciated:

• There's no nightly available that matches a released compiler. Currently, if you want to run a nightly compile matching the feature set of the released compiler, you have to use stable in RUSTC_BOOTSTRAP
• Insisting on nightly would force distributions to ship a specific nightly per package as a build dependency.
• Nightly has no guarantee on stability or anything, which makes the previous step annoying.

I don't agree with the path taken (strongly), but I do agree with @hsivonen that our story there is not straight-forward and should be better thought out.

[nox]

Those 3 points would be fixed doing what @eddyb has been suggesting for years: two separate executables and no moreRUSTC_BOOTSTRAP.

https://twitter.com/eddyb_r/status/1146760915179716608.

[eddyb]

@nox I think I described it better in the previous issue on this repo: https://github.com/rust-lang/cargo/issues/6627#issuecomment-473182066

[nox]

Couldn't find that comment anymore, thanks for linking it.

[joshtriplett]

Following up on this with a concrete plan:

• https://github.com/rust-lang/rust/pull/77802 is in FCP, which will make it so that you can set RUSTC_BOOTSTRAP=cratename to allow nightly in only that one crate, rather than a blanket RUSTC_BOOTSTRAP=1.
• Once that goes in, Cargo should start detecting if a build.rs script attempts to set RUSTC_BOOTSTRAP=value.
  
  
  
  • If the value is anything other than "1", reject it with an error message (since no crates currently do this, and we can keep it that way).
  
  
  • If the value is "1", and cargo doesn't already know that rustc is accepting nightly features (which Cargo already detects), and cargo RUSTC_BOOTSTRAP is not set in Cargo's own environment, reject it with an error message. The error message should suggest that if the user understands the stability implications of allowing nightly features and accepts that their project may not compile with newer versions of Rust, they can set RUSTC_BOOTSTRAP=thecratename.
  
  
  • If the value is "1", and either cargo already knows that rustc is accepting nightly features or RUSTC_BOOTSTRAP in Cargo's own environment is either set to 1 or includes the crate name as one of the comma-separated tokens, warn and ignore it (since it's a no-op and Cargo will already pass through its own value of RUSTC_BOOTSTRAP).
  
  

[est31]

@joshtriplett I guess your last point is made to facilitate a transition plan for projects with MSRV requirements like Firefox?

[joshtriplett]

@est31 It's to ensure that people (including downstream builders of Firefox and other projects, such as distros) don't have to immediately drop everything and fix any crates that do this in order to keep building; they just have to set RUSTC_BOOTSTRAP themselves from the top level. And for that matter, that point allows people to compile older versions of crates that still do this.

[jyn514]

Now that https://github.com/rust-lang/rust/pull/77802 is implemented, cargo can both deny setting RUSTC_BOOTSTRAP in build scripts and have an alternative for the user that doesn't require getting a new version of the code.