return ToolBuildResult to utilize them from callers

Signed-off-by: onur-ozkan <work@onurozkan.dev>

Author: onur-ozkan

src/bootstrap/src/core/build_steps/dist.rs

@@ -1180,7 +1180,7 @@ impl Step for Rls {
         let mut tarball = Tarball::new(builder, "rls", &target.triple);
         tarball.set_overlay(OverlayKind::Rls);
         tarball.is_preview(true);
-        tarball.add_file(rls, "bin", 0o755);
+        tarball.add_file(rls.tool_path, "bin", 0o755);
         tarball.add_legal_and_readme_to("share/doc/rls");
         Some(tarball.generate())
     }
@@ -1268,8 +1268,8 @@ impl Step for Clippy {
         let mut tarball = Tarball::new(builder, "clippy", &target.triple);
         tarball.set_overlay(OverlayKind::Clippy);
         tarball.is_preview(true);
-        tarball.add_file(clippy, "bin", 0o755);
-        tarball.add_file(cargoclippy, "bin", 0o755);
+        tarball.add_file(clippy.tool_path, "bin", 0o755);
+        tarball.add_file(cargoclippy.tool_path, "bin", 0o755);
         tarball.add_legal_and_readme_to("share/doc/clippy");
         Some(tarball.generate())
     }
@@ -1318,8 +1318,8 @@ impl Step for Miri {
         let mut tarball = Tarball::new(builder, "miri", &target.triple);
         tarball.set_overlay(OverlayKind::Miri);
         tarball.is_preview(true);
-        tarball.add_file(miri, "bin", 0o755);
-        tarball.add_file(cargomiri, "bin", 0o755);
+        tarball.add_file(miri.tool_path, "bin", 0o755);
+        tarball.add_file(cargomiri.tool_path, "bin", 0o755);
         tarball.add_legal_and_readme_to("share/doc/miri");
         Some(tarball.generate())
     }
@@ -1449,8 +1449,8 @@ impl Step for Rustfmt {
         let mut tarball = Tarball::new(builder, "rustfmt", &target.triple);
         tarball.set_overlay(OverlayKind::Rustfmt);
         tarball.is_preview(true);
-        tarball.add_file(rustfmt, "bin", 0o755);
-        tarball.add_file(cargofmt, "bin", 0o755);
+        tarball.add_file(rustfmt.tool_path, "bin", 0o755);
+        tarball.add_file(cargofmt.tool_path, "bin", 0o755);
         tarball.add_legal_and_readme_to("share/doc/rustfmt");
         Some(tarball.generate())
     }

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

@@ -427,7 +427,8 @@ impl Step for Rustfmt {
         let host = self.host;
         let compiler = builder.compiler(stage, host);
 
-        builder.ensure(tool::Rustfmt { compiler, target: self.host });
+        let tool_result = builder.ensure(tool::Rustfmt { compiler, target: self.host });
+        let compiler = tool_result.build_compiler;
 
         let mut cargo = tool::prepare_tool_cargo(
             builder,
@@ -571,7 +572,7 @@ impl Step for Miri {
         // miri tests need to know about the stage sysroot
         cargo.env("MIRI_SYSROOT", &miri_sysroot);
         cargo.env("MIRI_HOST_SYSROOT", &host_sysroot);
-        cargo.env("MIRI", &miri);
+        cargo.env("MIRI", &miri.tool_path);
 
         // Set the target.
         cargo.env("MIRI_TEST_TARGET", target.rustc_target_arg());
@@ -743,7 +744,8 @@ impl Step for Clippy {
         let host = self.host;
         let compiler = builder.compiler(stage, host);
 
-        builder.ensure(tool::Clippy { compiler, target: self.host });
+        let tool_result = builder.ensure(tool::Clippy { compiler, target: self.host });
+        let compiler = tool_result.build_compiler;
         let mut cargo = tool::prepare_tool_cargo(
             builder,
             compiler,

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

@@ -65,10 +65,10 @@ impl Builder<'_> {
 }
 
 #[derive(Clone)]
-struct ToolBuildResult {
-    tool_path: PathBuf,
-    build_compiler: Compiler,
-    target_compiler: Compiler,
+pub struct ToolBuildResult {
+    pub tool_path: PathBuf,
+    pub build_compiler: Compiler,
+    pub target_compiler: Compiler,
 }
 
 impl Step for ToolBuild {
@@ -114,10 +114,28 @@ impl Step for ToolBuild {
             self.source_type,
             &self.extra_features,
         );
+
+        if path.ends_with("/rustdoc") &&
+            // rustdoc is performance sensitive, so apply LTO to it.
+            is_lto_stage(&self.compiler)
+        {
+            let lto = match builder.config.rust_lto {
+                RustcLto::Off => Some("off"),
+                RustcLto::Thin => Some("thin"),
+                RustcLto::Fat => Some("fat"),
+                RustcLto::ThinLocal => None,
+            };
+            if let Some(lto) = lto {
+                cargo.env(cargo_profile_var("LTO", &builder.config), lto);
+            }
+        }
+
         if !self.allow_features.is_empty() {
             cargo.allow_features(self.allow_features);
         }
+
         cargo.args(self.cargo_args);
+
         let _guard = builder.msg_tool(
             Kind::Build,
             self.mode,
@@ -163,9 +181,6 @@ pub fn prepare_tool_cargo(
     source_type: SourceType,
     extra_features: &[String],
 ) -> CargoCommand {
-    let compiler =
-        if mode == Mode::ToolRustc { get_tool_rustc_compiler(builder, compiler) } else { compiler };
-
     let mut cargo = builder::Cargo::new(builder, compiler, mode, source_type, target, cmd_kind);
 
     let dir = builder.src.join(path);
@@ -667,80 +682,46 @@ impl Step for Rustdoc {
             }
         }
 
-        let build_compiler = get_tool_rustc_compiler(builder, target_compiler);
-
-        // When using `download-rustc` and a stage0 build_compiler, copying rustc doesn't actually
-        // build stage0 libstd (because the libstd in sysroot has the wrong ABI). Explicitly build
-        // it.
-        builder.ensure(compile::Std::new(build_compiler, target_compiler.host));
-        builder.ensure(compile::Rustc::new(build_compiler, target_compiler.host));
-
         // The presence of `target_compiler` ensures that the necessary libraries (codegen backends,
         // compiler libraries, ...) are built. Rustdoc does not require the presence of any
         // libraries within sysroot_libdir (i.e., rustlib), though doctests may want it (since
         // they'll be linked to those libraries). As such, don't explicitly `ensure` any additional
         // libraries here. The intuition here is that If we've built a compiler, we should be able
         // to build rustdoc.
         //
-        let mut features = Vec::new();
+        let mut extra_features = Vec::new();
         if builder.config.jemalloc(target) {
-            features.push("jemalloc".to_string());
+            extra_features.push("jemalloc".to_string());
         }
 
-        // NOTE: Never modify the rustflags here, it breaks the build cache for other tools!
-        let mut cargo = prepare_tool_cargo(
-            builder,
-            target_compiler,
-            Mode::ToolRustc,
-            target,
-            Kind::Build,
-            "src/tools/rustdoc",
-            SourceType::InTree,
-            features.as_slice(),
-        );
-
-        // rustdoc is performance sensitive, so apply LTO to it.
-        if is_lto_stage(&build_compiler) {
-            let lto = match builder.config.rust_lto {
-                RustcLto::Off => Some("off"),
-                RustcLto::Thin => Some("thin"),
-                RustcLto::Fat => Some("fat"),
-                RustcLto::ThinLocal => None,
-            };
-            if let Some(lto) = lto {
-                cargo.env(cargo_profile_var("LTO", &builder.config), lto);
-            }
-        }
-
-        let _guard = builder.msg_tool(
-            Kind::Build,
-            Mode::ToolRustc,
-            "rustdoc",
-            build_compiler.stage,
-            &self.compiler.host,
-            &target,
-        );
-        cargo.into_cmd().run(builder);
-
-        // Cargo adds a number of paths to the dylib search path on windows, which results in
-        // the wrong rustdoc being executed. To avoid the conflicting rustdocs, we name the "tool"
-        // rustdoc a different name.
-        let tool_rustdoc = builder
-            .cargo_out(build_compiler, Mode::ToolRustc, target)
-            .join(exe("rustdoc_tool_binary", target_compiler.host));
+        let ToolBuildResult { tool_path, build_compiler: _build_compiler, target_compiler } =
+            builder.ensure(ToolBuild {
+                compiler: target_compiler,
+                target,
+                // Cargo adds a number of paths to the dylib search path on windows, which results in
+                // the wrong rustdoc being executed. To avoid the conflicting rustdocs, we name the "tool"
+                // rustdoc a different name.
+                tool: "rustdoc_tool_binary",
+                mode: Mode::ToolRustc,
+                path: "src/tools/rustdoc",
+                source_type: SourceType::InTree,
+                extra_features,
+                allow_features: "",
+                cargo_args: Vec::new(),
+            });
 
         // don't create a stage0-sysroot/bin directory.
         if target_compiler.stage > 0 {
             if builder.config.rust_debuginfo_level_tools == DebuginfoLevel::None {
                 // Due to LTO a lot of debug info from C++ dependencies such as jemalloc can make it into
                 // our final binaries
-                compile::strip_debug(builder, target, &tool_rustdoc);
+                compile::strip_debug(builder, target, &tool_path);
             }
             let bin_rustdoc = bin_rustdoc();
-            builder.copy_link(&tool_rustdoc, &bin_rustdoc);
+            builder.copy_link(&tool_path, &bin_rustdoc);
             bin_rustdoc
         } else {
-            tool_rustdoc
+            tool_path
         }
     }
 }
@@ -1084,7 +1065,7 @@ macro_rules! tool_extended {
         }
 
         impl Step for $name {
-            type Output = PathBuf;
+            type Output = ToolBuildResult;
             const DEFAULT: bool = true; // Overridden by `should_run_tool_build_step`
             const ONLY_HOSTS: bool = true;
 
@@ -1104,7 +1085,7 @@ macro_rules! tool_extended {
                 });
             }
 
-            fn run(self, builder: &Builder<'_>) -> PathBuf {
+            fn run(self, builder: &Builder<'_>) -> ToolBuildResult {
                 let Self { compiler, target } = self;
                 run_tool_build_step(
                     builder,
@@ -1150,9 +1131,9 @@ fn run_tool_build_step(
     tool_name: &'static str,
     path: &'static str,
     add_bins_to_sysroot: Option<&[&str]>,
-) -> PathBuf {
-    let ToolBuildResult { tool_path, build_compiler: _build_compiler, target_compiler } = builder
-        .ensure(ToolBuild {
+) -> ToolBuildResult {
+    let ToolBuildResult { tool_path, build_compiler, target_compiler } =
+        builder.ensure(ToolBuild {
             compiler,
             target,
             tool: tool_name,
@@ -1177,9 +1158,10 @@ fn run_tool_build_step(
         }
 
         // Return a path into the bin dir.
-        bindir.join(exe(tool_name, target_compiler.host))
+        let path = bindir.join(exe(tool_name, target_compiler.host));
+        ToolBuildResult { tool_path: path, build_compiler, target_compiler }
     } else {
-        tool_path
+        ToolBuildResult { tool_path, build_compiler, target_compiler }
     }
 }
 

src/bootstrap/src/core/builder/mod.rs

@@ -1415,7 +1415,7 @@ impl<'a> Builder<'a> {
         let mut dylib_path = helpers::dylib_path();
         dylib_path.insert(0, self.sysroot(run_compiler).join("lib"));
 
-        let mut cmd = command(cargo_clippy);
+        let mut cmd = command(cargo_clippy.tool_path);
         cmd.env(helpers::dylib_path_var(), env::join_paths(&dylib_path).unwrap());
         cmd.env("CARGO", &self.initial_cargo);
         cmd
@@ -1430,8 +1430,8 @@ impl<'a> Builder<'a> {
         let cargo_miri =
             self.ensure(tool::CargoMiri { compiler: build_compiler, target: self.build.build });
         // Invoke cargo-miri, make sure it can find miri and cargo.
-        let mut cmd = command(cargo_miri);
-        cmd.env("MIRI", &miri);
+        let mut cmd = command(cargo_miri.tool_path);
+        cmd.env("MIRI", &miri.tool_path);
         cmd.env("CARGO", &self.initial_cargo);
         // Need to add the `run_compiler` libs. Those are the libs produces *by* `build_compiler`,
         // so they match the Miri we just built. However this means they are actually living one

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

@@ -797,10 +797,7 @@ mod dist {
         // stage minus 1 if --stage is not 0. Very confusing!
         assert_eq!(
             first(builder.cache.all::<tool::Rustdoc>()),
-            &[
-                tool::Rustdoc { compiler: Compiler { host: a, stage: 1 } },
-                tool::Rustdoc { compiler: Compiler { host: a, stage: 2 } },
-            ]
+            &[tool::Rustdoc { compiler: Compiler { host: a, stage: 2 } },]
         );
     }