Unrolled build for #146929

Rollup merge of #146929 - Zalathar:capture, r=Kobzol,jieyouxu

compiletest: Remove old-output-capture and become a stage0 bootstrap tool

The new compiletest output-capture system became the default in #146574, and no problems have been reported since.

This PR therefore removes the old output-capture implementation from compiletest, and adjusts bootstrap to always build and test compiletest as a stage0 bootstrap tool.

In other words, compiletest no longer relies on any unstable features (such as `libtest` or `internal_output_capture`), and is now written entirely in stable Rust!

The compiletest self-tests still need access to an in-tree rustc, in order to obtain information via `rustc --print`, so we interpret `--stage` as indicating the stage of that secondary compiler, but always use the stage0 compiler to build compiletest itself.

r? Kobzol

Author: rust-timer

src/bootstrap/src/core/build_steps/check.rs

@@ -8,8 +8,8 @@ use crate::core::build_steps::compile::{
 };
 use crate::core::build_steps::tool;
 use crate::core::build_steps::tool::{
-    COMPILETEST_ALLOW_FEATURES, SourceType, TEST_FLOAT_PARSE_ALLOW_FEATURES, ToolTargetBuildMode,
-    get_tool_target_compiler, prepare_tool_cargo,
+    SourceType, TEST_FLOAT_PARSE_ALLOW_FEATURES, ToolTargetBuildMode, get_tool_target_compiler,
+    prepare_tool_cargo,
 };
 use crate::core::builder::{
     self, Alias, Builder, Cargo, Kind, RunConfig, ShouldRun, Step, StepMetadata, crate_description,
@@ -654,7 +654,7 @@ macro_rules! tool_check_step {
             // The part of this path after the final '/' is also used as a display name.
             path: $path:literal
             $(, alt_path: $alt_path:literal )*
-            // Closure that returns `Mode` based on the passed `&Builder<'_>`
+            // `Mode` to use when checking this tool
             , mode: $mode:expr
             // Subset of nightly features that are allowed to be used when checking
             $(, allow_features: $allow_features:expr )?
@@ -682,8 +682,7 @@ macro_rules! tool_check_step {
 
             fn make_run(run: RunConfig<'_>) {
                 let target = run.target;
-                let builder = run.builder;
-                let mode = $mode(builder);
+                let mode: Mode = $mode;
 
                 let compiler = prepare_compiler_for_check(run.builder, target, mode);
 
@@ -704,7 +703,7 @@ macro_rules! tool_check_step {
                     _value
                 };
                 let extra_features: &[&str] = &[$($($enable_features),*)?];
-                let mode = $mode(builder);
+                let mode: Mode = $mode;
                 run_tool_check_step(builder, compiler, target, $path, mode, allow_features, extra_features);
             }
 
@@ -767,81 +766,69 @@ fn run_tool_check_step(
 tool_check_step!(Rustdoc {
     path: "src/tools/rustdoc",
     alt_path: "src/librustdoc",
-    mode: |_builder| Mode::ToolRustcPrivate
+    mode: Mode::ToolRustcPrivate
 });
 // Clippy, miri and Rustfmt are hybrids. They are external tools, but use a git subtree instead
 // of a submodule. Since the SourceType only drives the deny-warnings
 // behavior, treat it as in-tree so that any new warnings in clippy will be
 // rejected.
-tool_check_step!(Clippy { path: "src/tools/clippy", mode: |_builder| Mode::ToolRustcPrivate });
-tool_check_step!(Miri { path: "src/tools/miri", mode: |_builder| Mode::ToolRustcPrivate });
-tool_check_step!(CargoMiri {
-    path: "src/tools/miri/cargo-miri",
-    mode: |_builder| Mode::ToolRustcPrivate
-});
-tool_check_step!(Rustfmt { path: "src/tools/rustfmt", mode: |_builder| Mode::ToolRustcPrivate });
+tool_check_step!(Clippy { path: "src/tools/clippy", mode: Mode::ToolRustcPrivate });
+tool_check_step!(Miri { path: "src/tools/miri", mode: Mode::ToolRustcPrivate });
+tool_check_step!(CargoMiri { path: "src/tools/miri/cargo-miri", mode: Mode::ToolRustcPrivate });
+tool_check_step!(Rustfmt { path: "src/tools/rustfmt", mode: Mode::ToolRustcPrivate });
 tool_check_step!(RustAnalyzer {
     path: "src/tools/rust-analyzer",
-    mode: |_builder| Mode::ToolRustcPrivate,
+    mode: Mode::ToolRustcPrivate,
     allow_features: tool::RustAnalyzer::ALLOW_FEATURES,
     enable_features: ["in-rust-tree"],
 });
 tool_check_step!(MiroptTestTools {
     path: "src/tools/miropt-test-tools",
-    mode: |_builder| Mode::ToolBootstrap
+    mode: Mode::ToolBootstrap
 });
 // We want to test the local std
 tool_check_step!(TestFloatParse {
     path: "src/tools/test-float-parse",
-    mode: |_builder| Mode::ToolStd,
+    mode: Mode::ToolStd,
     allow_features: TEST_FLOAT_PARSE_ALLOW_FEATURES
 });
 tool_check_step!(FeaturesStatusDump {
     path: "src/tools/features-status-dump",
-    mode: |_builder| Mode::ToolBootstrap
+    mode: Mode::ToolBootstrap
 });
 
-tool_check_step!(Bootstrap {
-    path: "src/bootstrap",
-    mode: |_builder| Mode::ToolBootstrap,
-    default: false
-});
+tool_check_step!(Bootstrap { path: "src/bootstrap", mode: Mode::ToolBootstrap, default: false });
 
 // `run-make-support` will be built as part of suitable run-make compiletest test steps, but support
 // check to make it easier to work on.
 tool_check_step!(RunMakeSupport {
     path: "src/tools/run-make-support",
-    mode: |_builder| Mode::ToolBootstrap,
+    mode: Mode::ToolBootstrap,
     default: false
 });
 
 tool_check_step!(CoverageDump {
     path: "src/tools/coverage-dump",
-    mode: |_builder| Mode::ToolBootstrap,
+    mode: Mode::ToolBootstrap,
     default: false
 });
 
 // Compiletest is implicitly "checked" when it gets built in order to run tests,
 // so this is mainly for people working on compiletest to run locally.
 tool_check_step!(Compiletest {
     path: "src/tools/compiletest",
-    mode: |builder: &Builder<'_>| if builder.config.compiletest_use_stage0_libtest {
-        Mode::ToolBootstrap
-    } else {
-        Mode::ToolStd
-    },
-    allow_features: COMPILETEST_ALLOW_FEATURES,
+    mode: Mode::ToolBootstrap,
     default: false,
 });
 
 tool_check_step!(Linkchecker {
     path: "src/tools/linkchecker",
-    mode: |_builder| Mode::ToolBootstrap,
+    mode: Mode::ToolBootstrap,
     default: false
 });
 
 tool_check_step!(BumpStage0 {
     path: "src/tools/bump-stage0",
-    mode: |_builder| Mode::ToolBootstrap,
+    mode: Mode::ToolBootstrap,
     default: false
 });

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

@@ -18,8 +18,8 @@ use crate::core::build_steps::llvm::get_llvm_version;
 use crate::core::build_steps::run::get_completion_paths;
 use crate::core::build_steps::synthetic_targets::MirOptPanicAbortSyntheticTarget;
 use crate::core::build_steps::tool::{
-    self, COMPILETEST_ALLOW_FEATURES, RustcPrivateCompilers, SourceType,
-    TEST_FLOAT_PARSE_ALLOW_FEATURES, Tool, ToolTargetBuildMode, get_tool_target_compiler,
+    self, RustcPrivateCompilers, SourceType, TEST_FLOAT_PARSE_ALLOW_FEATURES, Tool,
+    ToolTargetBuildMode, get_tool_target_compiler,
 };
 use crate::core::build_steps::toolstate::ToolState;
 use crate::core::build_steps::{compile, dist, llvm};
@@ -36,7 +36,7 @@ use crate::utils::helpers::{
     linker_args, 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, CodegenBackendKind, DocTests, GitRepo, Mode, PathSet, debug, envify};
+use crate::{CLang, CodegenBackendKind, DocTests, GitRepo, Mode, PathSet, envify};
 
 const ADB_TEST_DIR: &str = "/data/local/tmp/work";
 
@@ -786,26 +786,26 @@ impl Step for CompiletestTest {
     fn run(self, builder: &Builder<'_>) {
         let host = self.host;
 
+        // Now that compiletest uses only stable Rust, building it always uses
+        // the stage 0 compiler. However, some of its unit tests need to be able
+        // to query information from an in-tree compiler, so we treat `--stage`
+        // as selecting the stage of that secondary compiler.
+
         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
+ERROR: `--stage 0` causes compiletest to query information from the stage0 (precompiled) compiler, instead of the in-tree compiler, which can cause some tests to fail inappropriately
 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);
+        let bootstrap_compiler = builder.compiler(0, host);
+        let staged_compiler = builder.compiler(builder.top_stage, host);
 
-        // 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.
-            Mode::ToolStd,
+            bootstrap_compiler,
+            Mode::ToolBootstrap,
             host,
             Kind::Test,
             "src/tools/compiletest",
@@ -816,9 +816,8 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
         // Used for `compiletest` self-tests to have the path to the *staged* compiler. Getting this
         // right is important, as `compiletest` is intended to only support one target spec JSON
         // format, namely that of the staged compiler.
-        cargo.env("TEST_RUSTC", builder.rustc(compiler));
+        cargo.env("TEST_RUSTC", builder.rustc(staged_compiler));
 
-        cargo.allow_features(COMPILETEST_ALLOW_FEATURES);
         run_cargo_test(cargo, &[], &[], "compiletest self test", host, builder);
     }
 }

src/bootstrap/src/core/build_steps/tool.rs

@@ -380,7 +380,6 @@ macro_rules! bootstrap_tool {
     ($(
         $name:ident, $path:expr, $tool_name:expr
         $(,is_external_tool = $external:expr)*
-        $(,is_unstable_tool = $unstable:expr)*
         $(,allow_features = $allow_features:expr)?
         $(,submodules = $submodules:expr)?
         $(,artifact_kind = $artifact_kind:expr)?
@@ -438,19 +437,11 @@ macro_rules! bootstrap_tool {
                     }
                 )*
 
-                let is_unstable = false $(|| $unstable)*;
-                let compiletest_wants_stage0 = $tool_name == "compiletest" && builder.config.compiletest_use_stage0_libtest;
-
                 builder.ensure(ToolBuild {
                     build_compiler: self.compiler,
                     target: self.target,
                     tool: $tool_name,
-                    mode: if is_unstable && !compiletest_wants_stage0 {
-                        // use in-tree libraries for unstable features
-                        Mode::ToolStd
-                    } else {
-                        Mode::ToolBootstrap
-                    },
+                    mode: Mode::ToolBootstrap,
                     path: $path,
                     source_type: if false $(|| $external)* {
                         SourceType::Submodule
@@ -483,8 +474,6 @@ macro_rules! bootstrap_tool {
     }
 }
 
-pub(crate) const COMPILETEST_ALLOW_FEATURES: &str = "internal_output_capture";
-
 bootstrap_tool!(
     // This is marked as an external tool because it includes dependencies
     // from submodules. Trying to keep the lints in sync between all the repos
@@ -495,7 +484,7 @@ bootstrap_tool!(
     Tidy, "src/tools/tidy", "tidy";
     Linkchecker, "src/tools/linkchecker", "linkchecker";
     CargoTest, "src/tools/cargotest", "cargotest";
-    Compiletest, "src/tools/compiletest", "compiletest", is_unstable_tool = true, allow_features = COMPILETEST_ALLOW_FEATURES;
+    Compiletest, "src/tools/compiletest", "compiletest";
     BuildManifest, "src/tools/build-manifest", "build-manifest";
     RemoteTestClient, "src/tools/remote-test-client", "remote-test-client";
     RustInstaller, "src/tools/rust-installer", "rust-installer";
@@ -509,8 +498,7 @@ bootstrap_tool!(
     CollectLicenseMetadata, "src/tools/collect-license-metadata", "collect-license-metadata";
     GenerateCopyright, "src/tools/generate-copyright", "generate-copyright";
     GenerateWindowsSys, "src/tools/generate-windows-sys", "generate-windows-sys";
-    // rustdoc-gui-test has a crate dependency on compiletest, so it needs the same unstable features.
-    RustdocGUITest, "src/tools/rustdoc-gui-test", "rustdoc-gui-test", is_unstable_tool = true, allow_features = COMPILETEST_ALLOW_FEATURES;
+    RustdocGUITest, "src/tools/rustdoc-gui-test", "rustdoc-gui-test";
     CoverageDump, "src/tools/coverage-dump", "coverage-dump";
     UnicodeTableGenerator, "src/tools/unicode-table-generator", "unicode-table-generator";
     FeaturesStatusDump, "src/tools/features-status-dump", "features-status-dump";

src/bootstrap/src/core/builder/tests.rs

@@ -2004,21 +2004,6 @@ mod snapshot {
                 .render_steps(), @"[check] rustc 0 <host> -> Compiletest 1 <host>");
     }
 
-    #[test]
-    fn check_compiletest_stage1_libtest() {
-        let ctx = TestCtx::new();
-        insta::assert_snapshot!(
-            ctx.config("check")
-                .path("compiletest")
-                .args(&["--set", "build.compiletest-use-stage0-libtest=false"])
-                .render_steps(), @r"
-        [build] llvm <host>
-        [build] rustc 0 <host> -> rustc 1 <host>
-        [build] rustc 1 <host> -> std 1 <host>
-        [check] rustc 1 <host> -> Compiletest 2 <host>
-        ");
-    }
-
     #[test]
     fn check_codegen() {
         let ctx = TestCtx::new();
@@ -2145,6 +2130,17 @@ mod snapshot {
         ");
     }
 
+    #[test]
+    fn test_compiletest_self_test() {
+        let ctx = TestCtx::new();
+        let steps = ctx.config("test").arg("compiletest").render_steps();
+        insta::assert_snapshot!(steps, @r"
+        [build] llvm <host>
+        [build] rustc 0 <host> -> rustc 1 <host>
+        [build] rustdoc 0 <host>
+        ");
+    }
+
     #[test]
     fn test_compiletest_suites_stage1() {
         let ctx = TestCtx::new();

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

@@ -310,9 +310,6 @@ pub struct Config {
     /// 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,
@@ -497,7 +494,8 @@ impl Config {
             optimized_compiler_builtins: build_optimized_compiler_builtins,
             jobs: build_jobs,
             compiletest_diff_tool: build_compiletest_diff_tool,
-            compiletest_use_stage0_libtest: build_compiletest_use_stage0_libtest,
+            // No longer has any effect; kept (for now) to avoid breaking people's configs.
+            compiletest_use_stage0_libtest: _,
             tidy_extra_checks: build_tidy_extra_checks,
             ccache: build_ccache,
             exclude: build_exclude,
@@ -1197,7 +1195,6 @@ impl Config {
             compiler_docs: build_compiler_docs.unwrap_or(false),
             compiletest_allow_stage0: build_compiletest_allow_stage0.unwrap_or(false),
             compiletest_diff_tool: build_compiletest_diff_tool,
-            compiletest_use_stage0_libtest: build_compiletest_use_stage0_libtest.unwrap_or(true),
             config: toml_path,
             configure_args: build_configure_args.unwrap_or_default(),
             control_flow_guard: rust_control_flow_guard.unwrap_or(false),

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

@@ -47,11 +47,17 @@ use crate::str::FromStr;
 #[macro_export]
 macro_rules! define_config {
     ($(#[$attr:meta])* struct $name:ident {
-        $($field:ident: Option<$field_ty:ty> = $field_key:literal,)*
+        $(
+            $(#[$field_attr:meta])*
+            $field:ident: Option<$field_ty:ty> = $field_key:literal,
+        )*
     }) => {
         $(#[$attr])*
         pub struct $name {
-            $(pub $field: Option<$field_ty>,)*
+            $(
+                $(#[$field_attr])*
+                pub $field: Option<$field_ty>,
+            )*
         }
 
         impl Merge for $name {

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

@@ -70,6 +70,8 @@ define_config! {
         jobs: Option<u32> = "jobs",
         compiletest_diff_tool: Option<String> = "compiletest-diff-tool",
         compiletest_allow_stage0: Option<bool> = "compiletest-allow-stage0",
+        /// No longer has any effect; kept (for now) to avoid breaking people's configs.
+        /// FIXME(#146929): Remove this in 2026.
         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/lib.rs

@@ -327,8 +327,8 @@ pub enum Mode {
     ToolTarget,
 
     /// Build a tool which uses the locally built std, placing output in the
-    /// "stageN-tools" directory. Its usage is quite rare, mainly used by
-    /// compiletest which needs libtest.
+    /// "stageN-tools" directory. Its usage is quite rare; historically it was
+    /// needed by compiletest, but now it is mainly used by `test-float-parse`.
     ToolStd,
 
     /// Build a tool which uses the `rustc_private` mechanism, and thus

src/bootstrap/src/utils/change_tracker.rs

@@ -557,8 +557,13 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
         summary: "New option `build.windows-rc` that will override which resource compiler on Windows will be used to compile Rust.",
     },
     ChangeInfo {
-        change_id: 99999,
+        change_id: 147046,
         severity: ChangeSeverity::Warning,
         summary: "The `rust.use-lld` option has been renamed to `rust.bootstrap-override-lld`. Note that it only serves for overriding the linker used when building Rust code in bootstrap to be LLD.",
     },
+    ChangeInfo {
+        change_id: 146929,
+        severity: ChangeSeverity::Info,
+        summary: "`compiletest` is now always built with the stage 0 compiler, so `build.compiletest-use-stage0-libtest` has no effect.",
+    },
 ];