Fix staging of `TestFloatParse`

The tool wasn't useful for anything, it was only built as a part of the test, but we can just use `cargo test` and `cargo run` in the test, no need to (pre-)build the tool itself.

3 files changed, 61 insertions, 74 deletions

diff --git a/src/bootstrap/src/core/build_steps/check.rs b/src/bootstrap/src/core/build_steps/check.rs
index bebae893ee7..a604e7c0585 100644
--- a/src/bootstrap/src/core/build_steps/check.rs
+++ b/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, ToolTargetBuildMode, get_tool_target_compiler,
-    prepare_tool_cargo,
+    COMPILETEST_ALLOW_FEATURES, 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,
@@ -791,7 +791,7 @@ tool_check_step!(MiroptTestTools {
 tool_check_step!(TestFloatParse {
     path: "src/tools/test-float-parse",
     mode: |_builder| Mode::ToolStd,
-    allow_features: tool::TestFloatParse::ALLOW_FEATURES
+    allow_features: TEST_FLOAT_PARSE_ALLOW_FEATURES
 });
 tool_check_step!(FeaturesStatusDump {
     path: "src/tools/features-status-dump",
diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs
index a8002cd293b..8d07874bbc5 100644
--- a/src/bootstrap/src/core/build_steps/test.rs
+++ b/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, Tool, ToolTargetBuildMode,
-    get_tool_target_compiler,
+    self, COMPILETEST_ALLOW_FEATURES, 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};
@@ -2886,6 +2886,7 @@ impl Step for Crate {
     }
 }
 
+/// Run cargo tests for the rustdoc crate.
 /// Rustdoc is special in various ways, which is why this step is different from `Crate`.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct CrateRustdoc {
@@ -2981,7 +2982,8 @@ impl Step for CrateRustdoc {
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct CrateRustdocJsonTypes {
-    host: TargetSelection,
+    build_compiler: Compiler,
+    target: TargetSelection,
 }
 
 impl Step for CrateRustdocJsonTypes {
@@ -2996,23 +2998,22 @@ impl Step for CrateRustdocJsonTypes {
     fn make_run(run: RunConfig<'_>) {
         let builder = run.builder;
 
-        builder.ensure(CrateRustdocJsonTypes { host: run.target });
+        builder.ensure(CrateRustdocJsonTypes {
+            build_compiler: get_tool_target_compiler(
+                builder,
+                ToolTargetBuildMode::Build(run.target),
+            ),
+            target: run.target,
+        });
     }
 
     fn run(self, builder: &Builder<'_>) {
-        let target = self.host;
-
-        // Use the previous stage compiler to reuse the artifacts that are
-        // created when running compiletest for tests/rustdoc. If this used
-        // `compiler`, then it would cause rustdoc to be built *again*, which
-        // isn't really necessary.
-        let compiler = builder.compiler_for(builder.top_stage, target, target);
-        builder.ensure(compile::Rustc::new(compiler, target));
+        let target = self.target;
 
         let cargo = tool::prepare_tool_cargo(
             builder,
-            compiler,
-            Mode::ToolRustc,
+            self.build_compiler,
+            Mode::ToolTarget,
             target,
             builder.kind,
             "src/rustdoc-json-types",
@@ -3021,7 +3022,7 @@ impl Step for CrateRustdocJsonTypes {
         );
 
         // FIXME: this looks very wrong, libtest doesn't accept `-C` arguments and the quotes are fishy.
-        let libtest_args = if self.host.contains("musl") {
+        let libtest_args = if target.contains("musl") {
             ["'-Ctarget-feature=-crt-static'"].as_slice()
         } else {
             &[]
@@ -3705,14 +3706,35 @@ impl Step for CodegenGCC {
     }
 }
 
+/// Get a build compiler that can be used to test the standard library (i.e. its stage will
+/// correspond to the stage that we want to test).
+fn get_test_build_compiler_for_std(builder: &Builder<'_>) -> Compiler {
+    if builder.top_stage == 0 {
+        eprintln!(
+            "ERROR: cannot run tests on stage 0. `build.compiletest-allow-stage0` only works for compiletest test suites."
+        );
+        exit!(1);
+    }
+    builder.compiler(builder.top_stage, builder.host_target)
+}
+
 /// Test step that does two things:
 /// - Runs `cargo test` for the `src/tools/test-float-parse` tool.
 /// - Invokes the `test-float-parse` tool to test the standard library's
 ///   float parsing routines.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct TestFloatParse {
-    path: PathBuf,
-    host: TargetSelection,
+    /// The build compiler which will build and run unit tests of `test-float-parse`, and which will
+    /// build the `test-float-parse` tool itself.
+    ///
+    /// Note that the staging is a bit funny here, because this step essentially tests std, but it
+    /// also needs to build the tool. So if we test stage1 std, we build:
+    /// 1) stage1 rustc
+    /// 2) Use that to build stage1 libstd
+    /// 3) Use that to build and run *stage2* test-float-parse
+    build_compiler: Compiler,
+    /// Target for which we build std and test that std.
+    target: TargetSelection,
 }
 
 impl Step for TestFloatParse {
@@ -3725,47 +3747,47 @@ impl Step for TestFloatParse {
     }
 
     fn make_run(run: RunConfig<'_>) {
-        for path in run.paths {
-            let path = path.assert_single_path().path.clone();
-            run.builder.ensure(Self { path, host: run.target });
-        }
+        run.builder.ensure(Self {
+            build_compiler: get_test_build_compiler_for_std(run.builder),
+            target: run.target,
+        });
     }
 
     fn run(self, builder: &Builder<'_>) {
-        let bootstrap_host = builder.config.host_target;
-        let compiler = builder.compiler(builder.top_stage, bootstrap_host);
-        let path = self.path.to_str().unwrap();
-        let crate_name = self.path.iter().next_back().unwrap().to_str().unwrap();
+        let build_compiler = self.build_compiler;
+        let target = self.target;
 
-        builder.ensure(tool::TestFloatParse { host: self.host });
+        // Build the standard library that will be tested, and a stdlib for host code
+        builder.std(build_compiler, target);
+        builder.std(build_compiler, builder.host_target);
 
         // Run any unit tests in the crate
         let mut cargo_test = tool::prepare_tool_cargo(
             builder,
-            compiler,
+            build_compiler,
             Mode::ToolStd,
-            bootstrap_host,
+            target,
             Kind::Test,
-            path,
+            "src/tools/test-float-parse",
             SourceType::InTree,
             &[],
         );
-        cargo_test.allow_features(tool::TestFloatParse::ALLOW_FEATURES);
+        cargo_test.allow_features(TEST_FLOAT_PARSE_ALLOW_FEATURES);
 
-        run_cargo_test(cargo_test, &[], &[], crate_name, bootstrap_host, builder);
+        run_cargo_test(cargo_test, &[], &[], "test-float-parse", target, builder);
 
         // Run the actual parse tests.
         let mut cargo_run = tool::prepare_tool_cargo(
             builder,
-            compiler,
+            build_compiler,
             Mode::ToolStd,
-            bootstrap_host,
+            target,
             Kind::Run,
-            path,
+            "src/tools/test-float-parse",
             SourceType::InTree,
             &[],
         );
-        cargo_run.allow_features(tool::TestFloatParse::ALLOW_FEATURES);
+        cargo_run.allow_features(TEST_FLOAT_PARSE_ALLOW_FEATURES);
 
         if !matches!(env::var("FLOAT_PARSE_TESTS_NO_SKIP_HUGE").as_deref(), Ok("1") | Ok("true")) {
             cargo_run.args(["--", "--skip-huge"]);
diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs
index b62c9a906b7..e15b570a5a7 100644
--- a/src/bootstrap/src/core/build_steps/tool.rs
+++ b/src/bootstrap/src/core/build_steps/tool.rs
@@ -1539,42 +1539,7 @@ tool_rustc_extended!(Rustfmt {
     add_bins_to_sysroot: ["rustfmt"]
 });
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub struct TestFloatParse {
-    pub host: TargetSelection,
-}
-
-impl TestFloatParse {
-    pub const ALLOW_FEATURES: &'static str = "f16,cfg_target_has_reliable_f16_f128";
-}
-
-impl Step for TestFloatParse {
-    type Output = ToolBuildResult;
-    const IS_HOST: bool = true;
-    const DEFAULT: bool = false;
-
-    fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("src/tools/test-float-parse")
-    }
-
-    fn run(self, builder: &Builder<'_>) -> ToolBuildResult {
-        let bootstrap_host = builder.config.host_target;
-        let compiler = builder.compiler(builder.top_stage, bootstrap_host);
-
-        builder.ensure(ToolBuild {
-            build_compiler: compiler,
-            target: bootstrap_host,
-            tool: "test-float-parse",
-            mode: Mode::ToolStd,
-            path: "src/tools/test-float-parse",
-            source_type: SourceType::InTree,
-            extra_features: Vec::new(),
-            allow_features: Self::ALLOW_FEATURES,
-            cargo_args: Vec::new(),
-            artifact_kind: ToolArtifactKind::Binary,
-        })
-    }
-}
+pub const TEST_FLOAT_PARSE_ALLOW_FEATURES: &'static str = "f16,cfg_target_has_reliable_f16_f128";
 
 impl Builder<'_> {
     /// Gets a `BootstrapCommand` which is ready to run `tool` in `stage` built for