Avoid accidentally enabling unstable features in compilers

[jyn514]

This allows rustbuild to control whether crates can use nightly features or not.
In practice, it has no effect because builder.rs already sets RUSTC_BOOTSTRAP unconditionally.

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

This seems correct, but I think it's an incorrect description of the patch?

RUSTC_BOOTSTRAP in this PR's diff is affecting the invocation of bootstrap (i.e., running target/debug/bootstrap), as far as I can tell, not the cargo build that builds it. It's still a good idea; we don't want to blanket set it for all child rustc's since rustbuild should be managing that. But the PR description + title should get updated.

If you want to expand this patch, it would be good to adjust RUSTC_BOOTSTRAP being set in src/bootstrap/builder.rs to not do so for ToolBootstrap which shouldn't be using unstable features. Though that I think will currently fail due to feature(test) in compiletest, so maybe just a FIXME on that line is a good idea to start.

[jyn514]

If you want to expand this patch, it would be good to adjust RUSTC_BOOTSTRAP being set in src/bootstrap/builder.rs to not do so for ToolBootstrap which shouldn't be using unstable features. Though that I think will currently fail due to feature(test) in compiletest, so maybe just a FIXME on that line is a good idea to start.

No, we set compiletest as a ToolStd to avoid exactly this issue:

rust/src/bootstrap/tool.rs

Line 365 in f624427

Compiletest, "src/tools/compiletest", "compiletest", is_unstable_tool = true;

rust/src/bootstrap/tool.rs

Lines 338 to 342 in f624427

	mode: if false $(|| $unstable)* {
	// use in-tree libraries for unstable features
	Mode::ToolStd
	} else {
	Mode::ToolBootstrap

Instead it fails because bootstrap unconditionally passes -Z flags to cargo:

Building stage0 tool build-manifest (x86_64-unknown-linux-gnu(x86_64-unknown-linux-gnu))
running: "/home/jnelson/rust-lang/rust/target/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "8" "-v" "--release" "--manifest-path" "/home/jnelson/rust-lang/rust/src/tools/build-manifest/Cargo.toml" "--message-format" "json-render-diagnostics"
error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `beta` channel

I don't know why binary-dep-depinfo is used; there's a comment but I haven't bothered to follow up on the PR links.

rust/src/bootstrap/builder.rs

Lines 1124 to 1135 in f624427

	// This tells Cargo (and in turn, rustc) to output more complete
	// dependency information. Most importantly for rustbuild, this
	// includes sysroot artifacts, like libstd, which means that we don't
	// need to track those in rustbuild (an error prone process!). This
	// feature is currently unstable as there may be some bugs and such, but
	// it represents a big improvement in rustbuild's reliability on
	// rebuilds, so we're using it here.
	//
	// For some additional context, see #63470 (the PR originally adding
	// this), as well as #63012 which is the tracking issue for this
	// feature on the rustc side.
	cargo.arg("-Zbinary-dep-depinfo");

[jyn514]

anyway, if you want to follow up on that, you can reproduce the error with

diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs
index 4f88b5854b6..74419605ec6 100644
--- a/src/bootstrap/builder.rs
+++ b/src/bootstrap/builder.rs
@@ -1378,7 +1378,12 @@ pub fn cargo(
         }
 
         // Enable usage of unstable features
-        cargo.env("RUSTC_BOOTSTRAP", "1");
+        match mode {
+            Mode::ToolBootstrap => {}
+            Mode::Std | Mode::Rustc | Mode::ToolStd | Mode::Codegen | Mode::ToolRustc => {
+                cargo.env("RUSTC_BOOTSTRAP", "1");
+            }
+        }
         self.add_rust_test_threads(&mut cargo);
 
         // Almost all of the crates that we compile as part of the bootstrap may

I'm not planning to spend much time on it.

[Mark-Simulacrum]

Yeah, OK. So it seems like this patch probably doesn't really change much (since we internally unconditionally set bootstrap for cargo invocations anyway). Regardless, it feels like it is the right thing to do, and if there's a long-tail of RUSTC_BOOTSTRAP that we need to add, seems like it's ultimately good to know where that's getting passed down.

@bors r+

[bors]

📌 Commit 4ea1df8 has been approved by Mark-Simulacrum

[jyn514]

also, setting __CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS=nightly doesn't help because bootstrap sets other -Z flags for rustc itself (in particular, -Ztls-model=initial-exec and -Zmacro-backtrace):

Building stage0 tool build-manifest (x86_64-unknown-linux-gnu(x86_64-unknown-linux-gnu))
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `/home/jnelson/rust-lang/rust/target/bootstrap/debug/rustc - --crate-name ___ --print=file-names --cfg=bootstrap -Csymbol-mangling-version=v0 -Zmacro-backtrace -Clink-args=-Wl,-z,origin '-Clink-args=-Wl,-rpath,$ORIGIN/../lib' -Ztls-model=initial-exec -Cllvm-args=-import-instr-limit=10 --target x86_64-unknown-linux-gnu --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=cfg` (exit status: 1)
  --- stderr
  error: the option `Z` is only accepted on the nightly compiler

[matthiaskrgr]

@bors r-
Looks like this breaks things while generating lint docs
#93768 (comment)

[jyn514]

Wasn't able to reproduce this locally. Don't know why it's failing.