Remove several usages of the as_command_mut method

Kobzol · Kobzol · commit 06b1e04bdc5f · 2024-07-13T14:06:08.000+02:00
Passes `&amp;Builder&lt;'_&gt;` to additional places, so that they could use the `BootstrapCommand` APIs directly, rather than going through `as_command_mut`.
diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/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(),
         )
diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs
@@ -474,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() {
diff --git a/src/bootstrap/src/core/build_steps/toolstate.rs b/src/bootstrap/src/core/build_steps/toolstate.rs
@@ -99,24 +99,16 @@ fn print_error(tool: &str, submodule: &str) {
     crate::exit!(3);
 }
 
-fn check_changed_files(toolstates: &HashMap<Box<str>, ToolState>) {
+fn check_changed_files(builder: &Builder<'_>, toolstates: &HashMap<Box<str>, ToolState>) {
     // Changed files
     let output = helpers::git(None)
+        .capture()
         .arg("diff")
         .arg("--name-status")
         .arg("HEAD")
         .arg("HEAD^")
-        .as_command_mut()
-        .output();
-    let output = match output {
-        Ok(o) => o,
-        Err(e) => {
-            eprintln!("Failed to get changed files: {e:?}");
-            crate::exit!(1);
-        }
-    };
-
-    let output = t!(String::from_utf8(output.stdout));
+        .run(builder)
+        .stdout();
 
     for (tool, submodule) in STABLE_TOOLS.iter().chain(NIGHTLY_TOOLS.iter()) {
         let changed = output.lines().any(|l| l.starts_with('M') && l.ends_with(submodule));
@@ -186,8 +178,8 @@ impl Step for ToolStateCheck {
             crate::exit!(1);
         }
 
-        check_changed_files(&toolstates);
-        checkout_toolstate_repo();
+        check_changed_files(builder, &toolstates);
+        checkout_toolstate_repo(builder);
         let old_toolstate = read_old_toolstate();
 
         for (tool, _) in STABLE_TOOLS.iter() {
@@ -231,7 +223,7 @@ impl Step for ToolStateCheck {
         }
 
         if builder.config.channel == "nightly" && env::var_os("TOOLSTATE_PUBLISH").is_some() {
-            commit_toolstate_change(&toolstates);
+            commit_toolstate_change(builder, &toolstates);
         }
     }
 
@@ -315,55 +307,34 @@ fn toolstate_repo() -> String {
 const TOOLSTATE_DIR: &str = "rust-toolstate";
 
 /// Checks out the toolstate repo into `TOOLSTATE_DIR`.
-fn checkout_toolstate_repo() {
+fn checkout_toolstate_repo(builder: &Builder<'_>) {
     if let Ok(token) = env::var("TOOLSTATE_REPO_ACCESS_TOKEN") {
-        prepare_toolstate_config(&token);
+        prepare_toolstate_config(builder, &token);
     }
     if Path::new(TOOLSTATE_DIR).exists() {
         eprintln!("Cleaning old toolstate directory...");
         t!(fs::remove_dir_all(TOOLSTATE_DIR));
     }
 
-    let status = helpers::git(None)
+    helpers::git(None)
         .arg("clone")
         .arg("--depth=1")
         .arg(toolstate_repo())
         .arg(TOOLSTATE_DIR)
-        .as_command_mut()
-        .status();
-    let success = match status {
-        Ok(s) => s.success(),
-        Err(_) => false,
-    };
-    if !success {
-        panic!("git clone unsuccessful (status: {status:?})");
-    }
+        .run(builder);
 }
 
 /// Sets up config and authentication for modifying the toolstate repo.
-fn prepare_toolstate_config(token: &str) {
-    fn git_config(key: &str, value: &str) {
-        let status = helpers::git(None)
-            .arg("config")
-            .arg("--global")
-            .arg(key)
-            .arg(value)
-            .as_command_mut()
-            .status();
-        let success = match status {
-            Ok(s) => s.success(),
-            Err(_) => false,
-        };
-        if !success {
-            panic!("git config key={key} value={value} failed (status: {status:?})");
-        }
+fn prepare_toolstate_config(builder: &Builder<'_>, token: &str) {
+    fn git_config(builder: &Builder<'_>, key: &str, value: &str) {
+        helpers::git(None).arg("config").arg("--global").arg(key).arg(value).run(builder);
     }
 
     // If changing anything here, then please check that `src/ci/publish_toolstate.sh` is up to date
     // as well.
-    git_config("user.email", "7378925+rust-toolstate-update@users.noreply.github.com");
-    git_config("user.name", "Rust Toolstate Update");
-    git_config("credential.helper", "store");
+    git_config(builder, "user.email", "7378925+rust-toolstate-update@users.noreply.github.com");
+    git_config(builder, "user.name", "Rust Toolstate Update");
+    git_config(builder, "credential.helper", "store");
 
     let credential = format!("https://{token}:x-oauth-basic@github.com\n",);
     let git_credential_path = PathBuf::from(t!(env::var("HOME"))).join(".git-credentials");
@@ -403,55 +374,51 @@ fn read_old_toolstate() -> Vec<RepoState> {
 ///
 ///       * See <https://help.github.com/articles/about-commit-email-addresses/>
 ///           if a private email by GitHub is wanted.
-fn commit_toolstate_change(current_toolstate: &ToolstateData) {
+fn commit_toolstate_change(builder: &Builder<'_>, current_toolstate: &ToolstateData) {
     let message = format!("({} CI update)", OS.expect("linux/windows only"));
     let mut success = false;
     for _ in 1..=5 {
         // Upload the test results (the new commit-to-toolstate mapping) to the toolstate repo.
         // This does *not* change the "current toolstate"; that only happens post-landing
         // via `src/ci/docker/publish_toolstate.sh`.
-        publish_test_results(current_toolstate);
+        publish_test_results(builder, current_toolstate);
 
         // `git commit` failing means nothing to commit.
-        let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
+        let status = helpers::git(Some(Path::new(TOOLSTATE_DIR)))
+            .allow_failure()
             .arg("commit")
             .arg("-a")
             .arg("-m")
             .arg(&message)
-            .as_command_mut()
-            .status());
-        if !status.success() {
+            .run(builder);
+        if !status.is_success() {
             success = true;
             break;
         }
 
-        let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
+        let status = helpers::git(Some(Path::new(TOOLSTATE_DIR)))
+            .allow_failure()
             .arg("push")
             .arg("origin")
             .arg("master")
-            .as_command_mut()
-            .status());
+            .run(builder);
         // If we successfully push, exit.
-        if status.success() {
+        if status.is_success() {
             success = true;
             break;
         }
         eprintln!("Sleeping for 3 seconds before retrying push");
         std::thread::sleep(std::time::Duration::from_secs(3));
-        let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
+        helpers::git(Some(Path::new(TOOLSTATE_DIR)))
             .arg("fetch")
             .arg("origin")
             .arg("master")
-            .as_command_mut()
-            .status());
-        assert!(status.success());
-        let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
+            .run(builder);
+        helpers::git(Some(Path::new(TOOLSTATE_DIR)))
             .arg("reset")
             .arg("--hard")
             .arg("origin/master")
-            .as_command_mut()
-            .status());
-        assert!(status.success());
+            .run(builder);
     }
 
     if !success {
@@ -464,9 +431,8 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
 /// These results will later be promoted to `latest.json` by the
 /// `publish_toolstate.py` script if the PR passes all tests and is merged to
 /// master.
-fn publish_test_results(current_toolstate: &ToolstateData) {
-    let commit = t!(helpers::git(None).arg("rev-parse").arg("HEAD").as_command_mut().output());
-    let commit = t!(String::from_utf8(commit.stdout));
+fn publish_test_results(builder: &Builder<'_>, current_toolstate: &ToolstateData) {
+    let commit = helpers::git(None).capture().arg("rev-parse").arg("HEAD").run(builder).stdout();
 
     let toolstate_serialized = t!(serde_json::to_string(&current_toolstate));
 
diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs
@@ -23,7 +23,7 @@ use std::fmt::Display;
 use std::fs::{self, File};
 use std::io;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
+use std::process::Command;
 use std::str;
 use std::sync::OnceLock;
 use std::time::SystemTime;
@@ -37,7 +37,7 @@ use utils::channel::GitInfo;
 use utils::helpers::hex_encode;
 
 use crate::core::builder;
-use crate::core::builder::Kind;
+use crate::core::builder::{Builder, Kind};
 use crate::core::config::{flags, LldMode};
 use crate::core::config::{DryRun, Target};
 use crate::core::config::{LlvmLibunwind, TargetSelection};
@@ -490,18 +490,18 @@ impl Build {
             return;
         }
 
-        let submodule_git = || helpers::git(Some(&absolute_path));
+        let submodule_git = || helpers::git(Some(&absolute_path)).capture_stdout();
 
         // Determine commit checked out in submodule.
-        let checked_out_hash = output(submodule_git().args(["rev-parse", "HEAD"]).as_command_mut());
+        let checked_out_hash = submodule_git().args(["rev-parse", "HEAD"]).run(self).stdout();
         let checked_out_hash = checked_out_hash.trim_end();
         // Determine commit that the submodule *should* have.
-        let recorded = output(
-            helpers::git(Some(&self.src))
-                .args(["ls-tree", "HEAD"])
-                .arg(relative_path)
-                .as_command_mut(),
-        );
+        let recorded = helpers::git(Some(&self.src))
+            .capture_stdout()
+            .args(["ls-tree", "HEAD"])
+            .arg(relative_path)
+            .run(self)
+            .stdout();
         let actual_hash = recorded
             .split_whitespace()
             .nth(2)
@@ -522,21 +522,14 @@ impl Build {
         let update = |progress: bool| {
             // Git is buggy and will try to fetch submodules from the tracking branch for *this* repository,
             // even though that has no relation to the upstream for the submodule.
-            let current_branch = {
-                let output = helpers::git(Some(&self.src))
-                    .args(["symbolic-ref", "--short", "HEAD"])
-                    .as_command_mut()
-                    .stderr(Stdio::inherit())
-                    .output();
-                let output = t!(output);
-                if output.status.success() {
-                    Some(String::from_utf8(output.stdout).unwrap().trim().to_owned())
-                } else {
-                    None
-                }
-            };
+            let current_branch = helpers::git(Some(&self.src))
+                .capture_stdout()
+                .args(["symbolic-ref", "--short", "HEAD"])
+                .run(self)
+                .stdout_if_ok()
+                .map(|s| s.trim().to_owned());
 
-            let mut git = helpers::git(Some(&self.src));
+            let mut git = helpers::git(Some(&self.src)).allow_failure();
             if let Some(branch) = current_branch {
                 // If there is a tag named after the current branch, git will try to disambiguate by prepending `heads/` to the branch name.
                 // This syntax isn't accepted by `branch.{branch}`. Strip it.
@@ -550,8 +543,7 @@ impl Build {
             git.arg(relative_path);
             git
         };
-        // NOTE: doesn't use `try_run` because this shouldn't print an error if it fails.
-        if !update(true).as_command_mut().status().map_or(false, |status| status.success()) {
+        if !update(true).run(self).is_success() {
             update(false).run(self);
         }
 
@@ -1943,22 +1935,28 @@ fn envify(s: &str) -> String {
 ///
 /// In case of errors during `git` command execution (e.g., in tarball sources), default values
 /// are used to prevent panics.
-pub fn generate_smart_stamp_hash(dir: &Path, additional_input: &str) -> String {
+pub fn generate_smart_stamp_hash(
+    builder: &Builder<'_>,
+    dir: &Path,
+    additional_input: &str,
+) -> String {
     let diff = helpers::git(Some(dir))
+        .capture_stdout()
+        .allow_failure()
         .arg("diff")
-        .as_command_mut()
-        .output()
-        .map(|o| String::from_utf8(o.stdout).unwrap_or_default())
+        .run(builder)
+        .stdout_if_ok()
         .unwrap_or_default();
 
     let status = helpers::git(Some(dir))
+        .capture_stdout()
+        .allow_failure()
         .arg("status")
         .arg("--porcelain")
         .arg("-z")
         .arg("--untracked-files=normal")
-        .as_command_mut()
-        .output()
-        .map(|o| String::from_utf8(o.stdout).unwrap_or_default())
+        .run(builder)
+        .stdout_if_ok()
         .unwrap_or_default();
 
     let mut hasher = sha2::Sha256::new();
diff --git a/src/bootstrap/src/utils/tarball.rs b/src/bootstrap/src/utils/tarball.rs
@@ -369,13 +369,13 @@ impl<'a> Tarball<'a> {
         // gets the same timestamp.
         if self.builder.rust_info().is_managed_git_subrepository() {
             // %ct means committer date
-            let timestamp = helpers::output(
-                helpers::git(Some(&self.builder.src))
-                    .arg("log")
-                    .arg("-1")
-                    .arg("--format=%ct")
-                    .as_command_mut(),
-            );
+            let timestamp = helpers::git(Some(&self.builder.src))
+                .capture_stdout()
+                .arg("log")
+                .arg("-1")
+                .arg("--format=%ct")
+                .run(self.builder)
+                .stdout();
             cmd.args(["--override-file-mtime", timestamp.trim()]);
         }
 

Original file line number	Diff line number	Diff line change
`@@ -125,6 +125,7 @@ pub fn prebuilt_llvm_config(builder: &Builder<'_>, target: TargetSelection) -> L`
`125`	`125`	`static STAMP_HASH_MEMO: OnceLock<String> = OnceLock::new();`
`126`	`126`	`let smart_stamp_hash = STAMP_HASH_MEMO.get_or_init(\|\| {`
`127`	`127`	`generate_smart_stamp_hash(`
	`128`	`+ builder,`
`128`	`129`	`&builder.config.src.join("src/llvm-project"),`
`129`	`130`	`builder.in_tree_llvm_info.sha().unwrap_or_default(),`
`130`	`131`	`)`