Rollup merge of #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 https://github.com/rust-lang/rust/pull/144563 that tries to do a way more invasive change which would cause the rebuild problem.

Best reviewed commit-by-commit.

---

r? `@Kobzol`

7 files changed, 45 insertions, 10 deletions

diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs
index 0f9268097d7..951ca73fcc4 100644
--- a/src/bootstrap/src/core/build_steps/test.rs
+++ b/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,9 +716,23 @@ 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.
@@ -723,8 +740,8 @@ impl Step for CompiletestTest {
         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);
         }
diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs
index 9644ade00b3..78abdd7f9b8 100644
--- a/src/bootstrap/src/core/config/config.rs
+++ b/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();
diff --git a/src/bootstrap/src/core/config/toml/build.rs b/src/bootstrap/src/core/config/toml/build.rs
index 4d29691f38b..728367b3972 100644
--- a/src/bootstrap/src/core/config/toml/build.rs
+++ b/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",
diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs
index 4b0c4821364..d3331b81587 100644
--- a/src/bootstrap/src/utils/change_tracker.rs
+++ b/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.",
+    },
 ];
diff --git a/src/ci/docker/host-x86_64/pr-check-1/Dockerfile b/src/ci/docker/host-x86_64/pr-check-1/Dockerfile
index f7d51fba861..04ac0f33daf 100644
--- a/src/ci/docker/host-x86_64/pr-check-1/Dockerfile
+++ b/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 && \
diff --git a/src/ci/docker/host-x86_64/pr-check-2/Dockerfile b/src/ci/docker/host-x86_64/pr-check-2/Dockerfile
index ce18a181d31..f82e19bcbb4 100644
--- a/src/ci/docker/host-x86_64/pr-check-2/Dockerfile
+++ b/src/ci/docker/host-x86_64/pr-check-2/Dockerfile
@@ -30,6 +30,7 @@ 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 && \
@@ -37,6 +38,6 @@ ENV SCRIPT \
         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
diff --git a/src/tools/opt-dist/src/tests.rs b/src/tools/opt-dist/src/tests.rs
index c9a21fc6fb2..d5121b8c786 100644
--- a/src/tools/opt-dist/src/tests.rs
+++ b/src/tools/opt-dist/src/tests.rs
@@ -79,6 +79,7 @@ lld = false
 rustc = "{rustc}"
 cargo = "{cargo}"
 local-rebuild = true
+compiletest-allow-stage0=true
 
 [target.{host_triple}]
 llvm-config = "{llvm_config}"
@@ -117,7 +118,6 @@ llvm-config = "{llvm_config}"
             args.extend(["--skip", test_path]);
         }
         cmd(&args)
-            .env("COMPILETEST_FORCE_STAGE0", "1")
             // Also run dist-only tests
             .env("COMPILETEST_ENABLE_DIST_TESTS", "1")
             .run()