Rollup merge of rust-lang#127680 - Kobzol:bootstrap-cmd-refactor-6, r=onur-ozkan

Bootstrap command refactoring: port remaining commands with access to `Build` (step 6)

Continuation of rust-lang#127450.

This PR ports commands in bootstrap that can easily get access to `Build(er)` to `BootstrapCommand`. After this PR, everything that can access `Build(er)` should be using the new API.

Statistics of `bootstrap` code (ignoring `src/bin/<shims>`) after this PR:
```
7 usages of `Command::new`
69 usages of `command()` (new API)
 - out of that: 16 usages of `as_command_mut()` (new API, but accesses the inner command)
```

Tracking issue: rust-lang#126819

r? `@onur-ozkan`

Author: tgross35

src/bootstrap/src/core/build_steps/compile.rs

@@ -14,7 +14,7 @@ use std::fs;
 use std::io::prelude::*;
 use std::io::BufReader;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
+use std::process::Stdio;
 use std::str;
 
 use serde_derive::Deserialize;
@@ -695,10 +695,10 @@ fn copy_sanitizers(
             || target == "x86_64-apple-ios"
         {
             // Update the library’s install name to reflect that it has been renamed.
-            apple_darwin_update_library_name(&dst, &format!("@rpath/{}", &runtime.name));
+            apple_darwin_update_library_name(builder, &dst, &format!("@rpath/{}", &runtime.name));
             // Upon renaming the install name, the code signature of the file will invalidate,
             // so we will sign it again.
-            apple_darwin_sign_file(&dst);
+            apple_darwin_sign_file(builder, &dst);
         }
 
         target_deps.push(dst);
@@ -707,25 +707,17 @@ fn copy_sanitizers(
     target_deps
 }
 
-fn apple_darwin_update_library_name(library_path: &Path, new_name: &str) {
-    let status = Command::new("install_name_tool")
-        .arg("-id")
-        .arg(new_name)
-        .arg(library_path)
-        .status()
-        .expect("failed to execute `install_name_tool`");
-    assert!(status.success());
+fn apple_darwin_update_library_name(builder: &Builder<'_>, library_path: &Path, new_name: &str) {
+    command("install_name_tool").arg("-id").arg(new_name).arg(library_path).run(builder);
 }
 
-fn apple_darwin_sign_file(file_path: &Path) {
-    let status = Command::new("codesign")
+fn apple_darwin_sign_file(builder: &Builder<'_>, file_path: &Path) {
+    command("codesign")
         .arg("-f") // Force to rewrite the existing signature
         .arg("-s")
         .arg("-")
         .arg(file_path)
-        .status()
-        .expect("failed to execute `codesign`");
-    assert!(status.success());
+        .run(builder);
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -1171,7 +1163,7 @@ fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelect
     if builder.config.llvm_profile_generate && target.is_msvc() {
         if let Some(ref clang_cl_path) = builder.config.llvm_clang_cl {
             // Add clang's runtime library directory to the search path
-            let clang_rt_dir = get_clang_cl_resource_dir(clang_cl_path);
+            let clang_rt_dir = get_clang_cl_resource_dir(builder, clang_cl_path);
             llvm_linker_flags.push_str(&format!("-L{}", clang_rt_dir.display()));
         }
     }

src/bootstrap/src/core/build_steps/llvm.rs

@@ -125,6 +125,7 @@ pub fn prebuilt_llvm_config(builder: &Builder<'_>, target: TargetSelection) -> L
     static STAMP_HASH_MEMO: OnceLock<String> = OnceLock::new();
     let smart_stamp_hash = STAMP_HASH_MEMO.get_or_init(|| {
         generate_smart_stamp_hash(
+            builder,
             &builder.config.src.join("src/llvm-project"),
             builder.in_tree_llvm_info.sha().unwrap_or_default(),
         )
@@ -912,7 +913,7 @@ impl Step for Lld {
             if let Some(clang_cl_path) = builder.config.llvm_clang_cl.as_ref() {
                 // Find clang's runtime library directory and push that as a search path to the
                 // cmake linker flags.
-                let clang_rt_dir = get_clang_cl_resource_dir(clang_cl_path);
+                let clang_rt_dir = get_clang_cl_resource_dir(builder, clang_cl_path);
                 ldflags.push_all(format!("/libpath:{}", clang_rt_dir.display()));
             }
         }

src/bootstrap/src/core/build_steps/setup.rs

@@ -8,6 +8,7 @@
 use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
 use crate::t;
 use crate::utils::change_tracker::CONFIG_CHANGE_HISTORY;
+use crate::utils::exec::command;
 use crate::utils::helpers::{self, hex_encode};
 use crate::Config;
 use sha2::Digest;
@@ -16,7 +17,6 @@ use std::fmt::Write as _;
 use std::fs::File;
 use std::io::Write;
 use std::path::{Path, PathBuf, MAIN_SEPARATOR_STR};
-use std::process::Command;
 use std::str::FromStr;
 use std::{fmt, fs, io};
 
@@ -266,20 +266,16 @@ impl Step for Link {
         }
         let stage_path =
             ["build", config.build.rustc_target_arg(), "stage1"].join(MAIN_SEPARATOR_STR);
-        if !rustup_installed() {
+        if !rustup_installed(builder) {
             eprintln!("`rustup` is not installed; cannot link `stage1` toolchain");
         } else if stage_dir_exists(&stage_path[..]) && !config.dry_run() {
-            attempt_toolchain_link(&stage_path[..]);
+            attempt_toolchain_link(builder, &stage_path[..]);
         }
     }
 }
 
-fn rustup_installed() -> bool {
-    Command::new("rustup")
-        .arg("--version")
-        .stdout(std::process::Stdio::null())
-        .output()
-        .map_or(false, |output| output.status.success())
+fn rustup_installed(builder: &Builder<'_>) -> bool {
+    command("rustup").capture_stdout().arg("--version").run(builder).is_success()
 }
 
 fn stage_dir_exists(stage_path: &str) -> bool {
@@ -289,8 +285,8 @@ fn stage_dir_exists(stage_path: &str) -> bool {
     }
 }
 
-fn attempt_toolchain_link(stage_path: &str) {
-    if toolchain_is_linked() {
+fn attempt_toolchain_link(builder: &Builder<'_>, stage_path: &str) {
+    if toolchain_is_linked(builder) {
         return;
     }
 
@@ -301,7 +297,7 @@ fn attempt_toolchain_link(stage_path: &str) {
         return;
     }
 
-    if try_link_toolchain(stage_path) {
+    if try_link_toolchain(builder, stage_path) {
         println!(
             "Added `stage1` rustup toolchain; try `cargo +stage1 build` on a separate rust project to run a newly-built toolchain"
         );
@@ -315,22 +311,24 @@ fn attempt_toolchain_link(stage_path: &str) {
     }
 }
 
-fn toolchain_is_linked() -> bool {
-    match Command::new("rustup")
+fn toolchain_is_linked(builder: &Builder<'_>) -> bool {
+    match command("rustup")
+        .capture_stdout()
+        .allow_failure()
         .args(["toolchain", "list"])
-        .stdout(std::process::Stdio::piped())
-        .output()
+        .run(builder)
+        .stdout_if_ok()
     {
-        Ok(toolchain_list) => {
-            if !String::from_utf8_lossy(&toolchain_list.stdout).contains("stage1") {
+        Some(toolchain_list) => {
+            if !toolchain_list.contains("stage1") {
                 return false;
             }
             // The toolchain has already been linked.
             println!(
                 "`stage1` toolchain already linked; not attempting to link `stage1` toolchain"
             );
         }
-        Err(_) => {
+        None => {
             // In this case, we don't know if the `stage1` toolchain has been linked;
             // but `rustup` failed, so let's not go any further.
             println!(
@@ -341,12 +339,12 @@ fn toolchain_is_linked() -> bool {
     true
 }
 
-fn try_link_toolchain(stage_path: &str) -> bool {
-    Command::new("rustup")
-        .stdout(std::process::Stdio::null())
+fn try_link_toolchain(builder: &Builder<'_>, stage_path: &str) -> bool {
+    command("rustup")
+        .capture_stdout()
         .args(["toolchain", "link", "stage1", stage_path])
-        .output()
-        .map_or(false, |output| output.status.success())
+        .run(builder)
+        .is_success()
 }
 
 fn ensure_stage1_toolchain_placeholder_exists(stage_path: &str) -> bool {
@@ -476,20 +474,18 @@ impl Step for Hook {
         if config.dry_run() {
             return;
         }
-        t!(install_git_hook_maybe(config));
+        t!(install_git_hook_maybe(builder, config));
     }
 }
 
 // install a git hook to automatically run tidy, if they want
-fn install_git_hook_maybe(config: &Config) -> io::Result<()> {
+fn install_git_hook_maybe(builder: &Builder<'_>, config: &Config) -> io::Result<()> {
     let git = helpers::git(Some(&config.src))
+        .capture()
         .args(["rev-parse", "--git-common-dir"])
-        .as_command_mut()
-        .output()
-        .map(|output| {
-            assert!(output.status.success(), "failed to run `git`");
-            PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
-        })?;
+        .run(builder)
+        .stdout();
+    let git = PathBuf::from(git.trim());
     let hooks_dir = git.join("hooks");
     let dst = hooks_dir.join("pre-push");
     if dst.exists() {

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

@@ -9,7 +9,6 @@ use std::ffi::OsString;
 use std::fs;
 use std::iter;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
 
 use clap_complete::shells;
 
@@ -169,12 +168,8 @@ You can skip linkcheck with --skip src/tools/linkchecker"
     }
 }
 
-fn check_if_tidy_is_installed() -> bool {
-    Command::new("tidy")
-        .arg("--version")
-        .stdout(Stdio::null())
-        .status()
-        .map_or(false, |status| status.success())
+fn check_if_tidy_is_installed(builder: &Builder<'_>) -> bool {
+    command("tidy").capture_stdout().allow_failure().arg("--version").run(builder).is_success()
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -188,16 +183,17 @@ impl Step for HtmlCheck {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
+        let builder = run.builder;
         let run = run.path("src/tools/html-checker");
-        run.lazy_default_condition(Box::new(check_if_tidy_is_installed))
+        run.lazy_default_condition(Box::new(|| check_if_tidy_is_installed(builder)))
     }
 
     fn make_run(run: RunConfig<'_>) {
         run.builder.ensure(HtmlCheck { target: run.target });
     }
 
     fn run(self, builder: &Builder<'_>) {
-        if !check_if_tidy_is_installed() {
+        if !check_if_tidy_is_installed(builder) {
             eprintln!("not running HTML-check tool because `tidy` is missing");
             eprintln!(
                 "You need the HTML tidy tool https://www.html-tidy.org/, this tool is *not* part of the rust project and needs to be installed separately, for example via your package manager."
@@ -2099,9 +2095,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
         let git_config = builder.config.git_config();
         cmd.arg("--git-repository").arg(git_config.git_repository);
         cmd.arg("--nightly-branch").arg(git_config.nightly_branch);
-
-        // FIXME: Move CiEnv back to bootstrap, it is only used here anyway
-        builder.ci_env.force_coloring_in_ci(cmd.as_command_mut());
+        cmd.force_coloring_in_ci(builder.ci_env);
 
         #[cfg(feature = "build-metrics")]
         builder.metrics.begin_test_suite(