Rollup merge of rust-lang#144675 - jieyouxu:compiletest-staging, r=Kobzol

Reject running `compiletest` self-tests against stage 0 rustc unless explicitly allowed

Currently, in `pr-check-1`, we run `python3 ../x.py test --stage 0 src/tools/compiletest`, which is `compiletest` self-tests against stage 0 rustc. This makes it very annoying for PRs wanting to change target spec JSON format, which `compiletest` depends on for target information, as otherwise `compiletest` would have to know how to handle 2 different target spec JSON formats and know when to pick which.

Instead of doing that, we change `compiletest` self-tests to reject running against stage 0 `rustc` *unless* explicitly allowed with `build.compiletest-allow-stage0=true`. `build.compiletest-allow-stage0` is a proper bootstrap config option in favor of the ad-hoc `COMPILETEST_FORCE_STAGE0` env var. This means that:

- `./x test src/tools/compiletest --stage=0` is not allowed, unless `build.compiletest-allow-stage0=true` is set. In this scenario, `compiletest` self-tests should be expected to fail unless the stage 0 `rustc` as provided is like codegen_cranelift where it's *actually* built from in-tree `rustc` sources.
- In CI, we change `./x test src/tools/compiletest --stage=0` to `./x test src/tools/compiletest --stage=1`, and move it to `pr-check-2`. Yes, this involves building the stage 1 compiler, but `pr-check-2` already has to build stage 1 compiler to test stage 1 library crates.
- Crucially, this means that **`compiletest` is only intended to support one target spec JSON format**, namely the one corresponding to the in-tree `rustc`.
- This should preserve the `compiletest-use-stage0-libtest` UX optimization where changing `compiler/` tree should still not require rebuilding `compiletest` as long as `build.compiletest-use-stage0-libtest=true`, as that should remain orthogonal. This is completely unlike my previous attempt at rust-lang#144563 that tries to do a way more invasive change which would cause the rebuild problem.

Best reviewed commit-by-commit.

---

r? `@Kobzol`

Author: samueltardieu

bootstrap.example.toml

@@ -465,6 +465,11 @@
 # What custom diff tool to use for displaying compiletest tests.
 #build.compiletest-diff-tool = <none>
 
+# Whether to allow `compiletest` self-tests and `compiletest`-managed test
+# suites to be run against the stage 0 rustc. This is only intended to be used
+# when the stage 0 compiler is actually built from in-tree sources.
+#build.compiletest-allow-stage0 = false
+
 # Whether to use the precompiled stage0 libtest with compiletest.
 #build.compiletest-use-stage0-libtest = true
 

compiler/rustc_codegen_cranelift/scripts/setup_rust_fork.sh

@@ -33,6 +33,7 @@ rustc = "$(pwd)/../dist/bin/rustc-clif"
 cargo = "$(rustup which cargo)"
 full-bootstrap = true
 local-rebuild = true
+compiletest-allow-stage0 = true
 
 [rust]
 download-rustc = false

compiler/rustc_codegen_cranelift/scripts/test_rustc_tests.sh

@@ -166,5 +166,5 @@ index 073116933bd..c3e4578204d 100644
 EOF
 
 echo "[TEST] rustc test suite"
-COMPILETEST_FORCE_STAGE0=1 ./x.py test --stage 0 --test-args=--no-capture tests/{codegen-units,run-make,ui,incremental}
+./x.py test --stage 0 --test-args=--no-capture tests/{codegen-units,run-make,ui,incremental}
 popd

compiler/rustc_codegen_gcc/build_system/src/test.rs

@@ -561,8 +561,6 @@ fn asm_tests(env: &Env, args: &TestArg) -> Result<(), String> {
     // FIXME: create a function "display_if_not_quiet" or something along the line.
     println!("[TEST] rustc asm test suite");
 
-    env.insert("COMPILETEST_FORCE_STAGE0".to_string(), "1".to_string());
-
     let codegen_backend_path = format!(
         "{pwd}/target/{channel}/librustc_codegen_gcc.{dylib_ext}",
         pwd = std::env::current_dir()
@@ -588,6 +586,8 @@ fn asm_tests(env: &Env, args: &TestArg) -> Result<(), String> {
             &"always",
             &"--stage",
             &"0",
+            &"--set",
+            &"build.compiletest-allow-stage0=true",
             &"tests/assembly-llvm/asm",
             &"--compiletest-rustc-args",
             &rustc_args,
@@ -1047,7 +1047,6 @@ where
 
     // FIXME: create a function "display_if_not_quiet" or something along the line.
     println!("[TEST] rustc {test_type} test suite");
-    env.insert("COMPILETEST_FORCE_STAGE0".to_string(), "1".to_string());
 
     let extra =
         if args.is_using_gcc_master_branch() { "" } else { " -Csymbol-mangling-version=v0" };
@@ -1070,6 +1069,8 @@ where
             &"always",
             &"--stage",
             &"0",
+            &"--set",
+            &"build.compiletest-allow-stage0=true",
             &format!("tests/{test_type}"),
             &"--compiletest-rustc-args",
             &rustc_args,

src/bootstrap/src/core/build_steps/test.rs

@@ -8,6 +8,9 @@ use std::ffi::{OsStr, OsString};
 use std::path::{Path, PathBuf};
 use std::{env, fs, iter};
 
+#[cfg(feature = "tracing")]
+use tracing::instrument;
+
 use crate::core::build_steps::compile::{Std, run_cargo};
 use crate::core::build_steps::doc::DocumentationFormat;
 use crate::core::build_steps::gcc::{Gcc, add_cg_gcc_cargo_flags};
@@ -30,7 +33,7 @@ use crate::utils::helpers::{
     linker_flags, t, target_supports_cranelift_backend, up_to_date,
 };
 use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests};
-use crate::{CLang, DocTests, GitRepo, Mode, PathSet, envify};
+use crate::{CLang, DocTests, GitRepo, Mode, PathSet, debug, envify};
 
 const ADB_TEST_DIR: &str = "/data/local/tmp/work";
 
@@ -713,18 +716,32 @@ impl Step for CompiletestTest {
     }
 
     /// Runs `cargo test` for compiletest.
+    #[cfg_attr(
+        feature = "tracing",
+        instrument(level = "debug", name = "CompiletestTest::run", skip_all)
+    )]
     fn run(self, builder: &Builder<'_>) {
         let host = self.host;
+
+        if builder.top_stage == 0 && !builder.config.compiletest_allow_stage0 {
+            eprintln!("\
+ERROR: `--stage 0` runs compiletest self-tests against the stage0 (precompiled) compiler, not the in-tree compiler, and will almost always cause tests to fail
+NOTE: if you're sure you want to do this, please open an issue as to why. In the meantime, you can override this with `--set build.compiletest-allow-stage0=true`."
+            );
+            crate::exit!(1);
+        }
+
         let compiler = builder.compiler(builder.top_stage, host);
+        debug!(?compiler);
 
         // We need `ToolStd` for the locally-built sysroot because
         // compiletest uses unstable features of the `test` crate.
         builder.std(compiler, host);
         let mut cargo = tool::prepare_tool_cargo(
             builder,
             compiler,
-            // compiletest uses libtest internals; make it use the in-tree std to make sure it never breaks
-            // when std sources change.
+            // compiletest uses libtest internals; make it use the in-tree std to make sure it never
+            // breaks when std sources change.
             Mode::ToolStd,
             host,
             Kind::Test,
@@ -1612,12 +1629,11 @@ impl Step for Compiletest {
             return;
         }
 
-        if builder.top_stage == 0 && env::var("COMPILETEST_FORCE_STAGE0").is_err() {
+        if builder.top_stage == 0 && !builder.config.compiletest_allow_stage0 {
             eprintln!("\
 ERROR: `--stage 0` runs compiletest on the stage0 (precompiled) compiler, not your local changes, and will almost always cause tests to fail
-HELP: to test the compiler, use `--stage 1` instead
-HELP: to test the standard library, use `--stage 0 library/std` instead
-NOTE: if you're sure you want to do this, please open an issue as to why. In the meantime, you can override this with `COMPILETEST_FORCE_STAGE0=1`."
+HELP: to test the compiler or standard library, omit the stage or explicitly use `--stage 1` instead
+NOTE: if you're sure you want to do this, please open an issue as to why. In the meantime, you can override this with `--set build.compiletest-allow-stage0=true`."
             );
             crate::exit!(1);
         }

src/bootstrap/src/core/config/config.rs

@@ -298,8 +298,16 @@ pub struct Config {
     /// Command for visual diff display, e.g. `diff-tool --color=always`.
     pub compiletest_diff_tool: Option<String>,
 
+    /// Whether to allow running both `compiletest` self-tests and `compiletest`-managed test suites
+    /// against the stage 0 (rustc, std).
+    ///
+    /// This is only intended to be used when the stage 0 compiler is actually built from in-tree
+    /// sources.
+    pub compiletest_allow_stage0: bool,
+
     /// Whether to use the precompiled stage0 libtest with compiletest.
     pub compiletest_use_stage0_libtest: bool,
+
     /// Default value for `--extra-checks`
     pub tidy_extra_checks: Option<String>,
     pub is_running_on_ci: bool,
@@ -749,6 +757,7 @@ impl Config {
             optimized_compiler_builtins,
             jobs,
             compiletest_diff_tool,
+            compiletest_allow_stage0,
             compiletest_use_stage0_libtest,
             tidy_extra_checks,
             ccache,
@@ -1020,8 +1029,12 @@ impl Config {
 
         config.optimized_compiler_builtins =
             optimized_compiler_builtins.unwrap_or(config.channel != "dev");
+
         config.compiletest_diff_tool = compiletest_diff_tool;
+
+        config.compiletest_allow_stage0 = compiletest_allow_stage0.unwrap_or(false);
         config.compiletest_use_stage0_libtest = compiletest_use_stage0_libtest.unwrap_or(true);
+
         config.tidy_extra_checks = tidy_extra_checks;
 
         let download_rustc = config.download_rustc_commit.is_some();

src/bootstrap/src/core/config/toml/build.rs

@@ -68,6 +68,7 @@ define_config! {
         optimized_compiler_builtins: Option<bool> = "optimized-compiler-builtins",
         jobs: Option<u32> = "jobs",
         compiletest_diff_tool: Option<String> = "compiletest-diff-tool",
+        compiletest_allow_stage0: Option<bool> = "compiletest-allow-stage0",
         compiletest_use_stage0_libtest: Option<bool> = "compiletest-use-stage0-libtest",
         tidy_extra_checks: Option<String> = "tidy-extra-checks",
         ccache: Option<StringOrBool> = "ccache",

src/bootstrap/src/utils/change_tracker.rs

@@ -486,4 +486,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
         severity: ChangeSeverity::Warning,
         summary: "Removed `rust.description` and `llvm.ccache` as it was deprecated in #137723 and #136941 long time ago.",
     },
+    ChangeInfo {
+        change_id: 144675,
+        severity: ChangeSeverity::Warning,
+        summary: "Added `build.compiletest-allow-stage0` flag instead of `COMPILETEST_FORCE_STAGE0` env var, and reject running `compiletest` self tests against stage 0 rustc unless explicitly allowed.",
+    },
 ];

src/ci/docker/host-x86_64/pr-check-1/Dockerfile

@@ -43,7 +43,6 @@ ENV SCRIPT \
   python3 ../x.py check bootstrap && \
   /scripts/check-default-config-profiles.sh && \
   python3 ../x.py build src/tools/build-manifest && \
-  python3 ../x.py test --stage 0 src/tools/compiletest && \
   python3 ../x.py check compiletest --set build.compiletest-use-stage0-libtest=true && \
   python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu && \
   python3 ../x.py check --set build.optimized-compiler-builtins=false core alloc std --target=aarch64-unknown-linux-gnu,i686-pc-windows-msvc,i686-unknown-linux-gnu,x86_64-apple-darwin,x86_64-pc-windows-gnu,x86_64-pc-windows-msvc && \

src/ci/docker/host-x86_64/pr-check-2/Dockerfile

@@ -30,13 +30,14 @@ ENV SCRIPT \
         python3 ../x.py check && \
         python3 ../x.py clippy ci && \
         python3 ../x.py test --stage 1 core alloc std test proc_macro && \
+        python3 ../x.py test --stage 1 src/tools/compiletest && \
         python3 ../x.py doc --stage 0 bootstrap && \
         # Build both public and internal documentation.
         RUSTDOCFLAGS=\"--document-private-items --document-hidden-items\" python3 ../x.py doc --stage 0 compiler && \
         RUSTDOCFLAGS=\"--document-private-items --document-hidden-items\" python3 ../x.py doc --stage 1 library && \
         mkdir -p /checkout/obj/staging/doc && \
         cp -r build/x86_64-unknown-linux-gnu/doc /checkout/obj/staging && \
         RUSTDOCFLAGS=\"--document-private-items --document-hidden-items\" python3 ../x.py doc --stage 1 library/test && \
-        # The BOOTSTRAP_TRACING flag is added to verify whether the 
+        # The BOOTSTRAP_TRACING flag is added to verify whether the
         # bootstrap process compiles successfully with this flag enabled.
         BOOTSTRAP_TRACING=1 python3 ../x.py --help