Make git helper return BootstrapCmd

src/bootstrap/src/core/build_steps/format.rs

@@ -7,7 +7,7 @@ use build_helper::git::get_git_modified_files;
 use ignore::WalkBuilder;
 use std::collections::VecDeque;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
+use std::process::Command;
 use std::sync::mpsc::SyncSender;
 use std::sync::Mutex;
 
@@ -160,35 +160,29 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
             override_builder.add(&format!("!{ignore}")).expect(&ignore);
         }
     }
-    let git_available = match helpers::git(None)
+    let git_available = build
-        .arg("--version")
+        .run(helpers::git(None).print_on_failure().allow_failure().arg("--version"))
-        .stdout(Stdio::null())
+        .is_success();
-        .stderr(Stdio::null())
-        .status()
-    {
-        Ok(status) => status.success(),
-        Err(_) => false,
-    };
 
     let mut adjective = None;
     if git_available {
-        let in_working_tree = match helpers::git(Some(&build.src))
+        let in_working_tree = build
-            .arg("rev-parse")
+            .run(
-            .arg("--is-inside-work-tree")
+                helpers::git(Some(&build.src))
-            .stdout(Stdio::null())
+                    .print_on_failure()
-            .stderr(Stdio::null())
+                    .allow_failure()
-            .status()
+                    .arg("rev-parse")
-        {
+                    .arg("--is-inside-work-tree"),
-            Ok(status) => status.success(),
+            )
-            Err(_) => false,
+            .is_success();
-        };
         if in_working_tree {
             let untracked_paths_output = output(
-                helpers::git(Some(&build.src))
+                &mut helpers::git(Some(&build.src))
                     .arg("status")
                     .arg("--porcelain")
                     .arg("-z")
-                    .arg("--untracked-files=normal"),
+                    .arg("--untracked-files=normal")
+                    .command,
             );
             let untracked_paths: Vec<_> = untracked_paths_output
                 .split_terminator('\0')

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

@@ -172,7 +172,7 @@ pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
             // the LLVM shared object file is named `LLVM-12-rust-{version}-nightly`
             config.src.join("src/version"),
         ]);
-        output(&mut rev_list).trim().to_owned()
+        output(&mut rev_list.command).trim().to_owned()
     } else if let Some(info) = channel::read_commit_info_file(&config.src) {
         info.sha.trim().to_owned()
     } else {
@@ -253,7 +253,8 @@ pub(crate) fn is_ci_llvm_modified(config: &Config) -> bool {
         // We assume we have access to git, so it's okay to unconditionally pass
         // `true` here.
         let llvm_sha = detect_llvm_sha(config, true);
-        let head_sha = output(helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD"));
+        let head_sha =
+            output(&mut helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD").command);
         let head_sha = head_sha.trim();
         llvm_sha == head_sha
     }

src/bootstrap/src/core/build_steps/perf.rs

@@ -2,6 +2,7 @@ use crate::core::build_steps::compile::{Std, Sysroot};
 use crate::core::build_steps::tool::{RustcPerf, Tool};
 use crate::core::builder::Builder;
 use crate::core::config::DebuginfoLevel;
+use crate::utils::exec::BootstrapCommand;
 
 /// Performs profiling using `rustc-perf` on a built version of the compiler.
 pub fn perf(builder: &Builder<'_>) {

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

@@ -484,6 +484,7 @@ impl Step for Hook {
 fn install_git_hook_maybe(config: &Config) -> io::Result<()> {
     let git = helpers::git(Some(&config.src))
         .args(["rev-parse", "--git-common-dir"])
+        .command
         .output()
         .map(|output| {
             assert!(output.status.success(), "failed to run `git`");

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

@@ -2349,7 +2349,7 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) ->
 
     cmd = cmd.delay_failure();
     if !builder.config.verbose_tests {
-        cmd = cmd.quiet();
+        cmd = cmd.print_on_failure();
     }
     builder.run(cmd).is_success()
 }

src/bootstrap/src/core/build_steps/toolstate.rs

@@ -101,8 +101,13 @@ fn print_error(tool: &str, submodule: &str) {
 
 fn check_changed_files(toolstates: &HashMap<Box<str>, ToolState>) {
     // Changed files
-    let output =
+    let output = helpers::git(None)
-        helpers::git(None).arg("diff").arg("--name-status").arg("HEAD").arg("HEAD^").output();
+        .arg("diff")
+        .arg("--name-status")
+        .arg("HEAD")
+        .arg("HEAD^")
+        .command
+        .output();
     let output = match output {
         Ok(o) => o,
         Err(e) => {
@@ -324,6 +329,7 @@ fn checkout_toolstate_repo() {
         .arg("--depth=1")
         .arg(toolstate_repo())
         .arg(TOOLSTATE_DIR)
+        .command
         .status();
     let success = match status {
         Ok(s) => s.success(),
@@ -337,7 +343,8 @@ fn checkout_toolstate_repo() {
 /// 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).status();
+        let status =
+            helpers::git(None).arg("config").arg("--global").arg(key).arg(value).command.status();
         let success = match status {
             Ok(s) => s.success(),
             Err(_) => false,
@@ -406,6 +413,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
             .arg("-a")
             .arg("-m")
             .arg(&message)
+            .command
             .status());
         if !status.success() {
             success = true;
@@ -416,6 +424,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
             .arg("push")
             .arg("origin")
             .arg("master")
+            .command
             .status());
         // If we successfully push, exit.
         if status.success() {
@@ -428,12 +437,14 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
             .arg("fetch")
             .arg("origin")
             .arg("master")
+            .command
             .status());
         assert!(status.success());
         let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
             .arg("reset")
             .arg("--hard")
             .arg("origin/master")
+            .command
             .status());
         assert!(status.success());
     }
@@ -449,7 +460,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
 /// `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").output());
+    let commit = t!(helpers::git(None).arg("rev-parse").arg("HEAD").command.output());
     let commit = t!(String::from_utf8(commit.stdout));
 
     let toolstate_serialized = t!(serde_json::to_string(&current_toolstate));

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

@@ -1259,6 +1259,7 @@ impl Config {
         cmd.arg("rev-parse").arg("--show-cdup");
         // Discard stderr because we expect this to fail when building from a tarball.
         let output = cmd
+            .command
             .stderr(std::process::Stdio::null())
             .output()
             .ok()
@@ -2141,7 +2142,7 @@ impl Config {
 
         let mut git = helpers::git(Some(&self.src));
         git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap()));
-        output(&mut git)
+        output(&mut git.command)
     }
 
     /// Bootstrap embeds a version number into the name of shared libraries it uploads in CI.
@@ -2445,8 +2446,9 @@ impl Config {
         };
 
         // Handle running from a directory other than the top level
-        let top_level =
+        let top_level = output(
-            output(helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]));
+            &mut helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]).command,
+        );
         let top_level = top_level.trim_end();
         let compiler = format!("{top_level}/compiler/");
         let library = format!("{top_level}/library/");
@@ -2454,10 +2456,11 @@ impl Config {
         // Look for a version to compare to based on the current commit.
         // Only commits merged by bors will have CI artifacts.
         let merge_base = output(
-            helpers::git(Some(&self.src))
+            &mut helpers::git(Some(&self.src))
                 .arg("rev-list")
                 .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email))
-                .args(["-n1", "--first-parent", "HEAD"]),
+                .args(["-n1", "--first-parent", "HEAD"])
+                .command,
         );
         let commit = merge_base.trim_end();
         if commit.is_empty() {
@@ -2471,6 +2474,7 @@ impl Config {
         // Warn if there were changes to the compiler or standard library since the ancestor commit.
         let has_changes = !t!(helpers::git(Some(&self.src))
             .args(["diff-index", "--quiet", commit, "--", &compiler, &library])
+            .command
             .status())
         .success();
         if has_changes {
@@ -2542,17 +2546,19 @@ impl Config {
         if_unchanged: bool,
     ) -> Option<String> {
         // Handle running from a directory other than the top level
-        let top_level =
+        let top_level = output(
-            output(helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]));
+            &mut helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]).command,
+        );
         let top_level = top_level.trim_end();
 
         // Look for a version to compare to based on the current commit.
         // Only commits merged by bors will have CI artifacts.
         let merge_base = output(
-            helpers::git(Some(&self.src))
+            &mut helpers::git(Some(&self.src))
                 .arg("rev-list")
                 .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email))
-                .args(["-n1", "--first-parent", "HEAD"]),
+                .args(["-n1", "--first-parent", "HEAD"])
+                .command,
         );
         let commit = merge_base.trim_end();
         if commit.is_empty() {
@@ -2571,7 +2577,7 @@ impl Config {
             git.arg(format!("{top_level}/{path}"));
         }
 
-        let has_changes = !t!(git.status()).success();
+        let has_changes = !t!(git.command.status()).success();
         if has_changes {
             if if_unchanged {
                 if self.verbose > 0 {

src/bootstrap/src/lib.rs

@@ -494,11 +494,12 @@ impl Build {
         let submodule_git = || helpers::git(Some(&absolute_path));
 
         // Determine commit checked out in submodule.
-        let checked_out_hash = output(submodule_git().args(["rev-parse", "HEAD"]));
+        let checked_out_hash = output(&mut submodule_git().args(["rev-parse", "HEAD"]).command);
         let checked_out_hash = checked_out_hash.trim_end();
         // Determine commit that the submodule *should* have.
-        let recorded =
+        let recorded = output(
-            output(helpers::git(Some(&self.src)).args(["ls-tree", "HEAD"]).arg(relative_path));
+            &mut helpers::git(Some(&self.src)).args(["ls-tree", "HEAD"]).arg(relative_path).command,
+        );
         let actual_hash = recorded
             .split_whitespace()
             .nth(2)
@@ -521,6 +522,7 @@ impl Build {
             let current_branch = {
                 let output = helpers::git(Some(&self.src))
                     .args(["symbolic-ref", "--short", "HEAD"])
+                    .command
                     .stderr(Stdio::inherit())
                     .output();
                 let output = t!(output);
@@ -546,7 +548,7 @@ impl Build {
             git
         };
         // NOTE: doesn't use `try_run` because this shouldn't print an error if it fails.
-        if !update(true).status().map_or(false, |status| status.success()) {
+        if !update(true).command.status().map_or(false, |status| status.success()) {
             self.run(update(false));
         }
 
@@ -577,12 +579,15 @@ impl Build {
         if !self.config.submodules(self.rust_info()) {
             return;
         }
-        let output = output(
+        let output = self
-            helpers::git(Some(&self.src))
+            .run(
-                .args(["config", "--file"])
+                helpers::git(Some(&self.src))
-                .arg(self.config.src.join(".gitmodules"))
+                    .quiet()
-                .args(["--get-regexp", "path"]),
+                    .args(["config", "--file"])
-        );
+                    .arg(self.config.src.join(".gitmodules"))
+                    .args(["--get-regexp", "path"]),
+            )
+            .stdout();
         for line in output.lines() {
             // Look for `submodule.$name.path = $path`
             // Sample output: `submodule.src/rust-installer.path src/tools/rust-installer`
@@ -950,7 +955,10 @@ impl Build {
                 command.command.status().map(|status| status.into()),
                 matches!(mode, OutputMode::All),
             ),
-            OutputMode::OnlyOnFailure => (command.command.output().map(|o| o.into()), true),
+            mode @ (OutputMode::OnlyOnFailure | OutputMode::Quiet) => (
+                command.command.output().map(|o| o.into()),
+                matches!(mode, OutputMode::OnlyOnFailure),
+            ),
         };
 
         let output = match output {
@@ -1480,14 +1488,18 @@ impl Build {
             // Figure out how many merge commits happened since we branched off master.
             // That's our beta number!
             // (Note that we use a `..` range, not the `...` symmetric difference.)
-            output(
+            self.run(
-                helpers::git(Some(&self.src)).arg("rev-list").arg("--count").arg("--merges").arg(
+                helpers::git(Some(&self.src))
-                    format!(
+                    .quiet()
+                    .arg("rev-list")
+                    .arg("--count")
+                    .arg("--merges")
+                    .arg(format!(
                         "refs/remotes/origin/{}..HEAD",
                         self.config.stage0_metadata.config.nightly_branch
-                    ),
+                    )),
-                ),
             )
+            .stdout()
         });
         let n = count.trim().parse().unwrap();
         self.prerelease_version.set(Some(n));
@@ -1914,6 +1926,7 @@ fn envify(s: &str) -> String {
 pub fn generate_smart_stamp_hash(dir: &Path, additional_input: &str) -> String {
     let diff = helpers::git(Some(dir))
         .arg("diff")
+        .command
         .output()
         .map(|o| String::from_utf8(o.stdout).unwrap_or_default())
         .unwrap_or_default();
@@ -1923,6 +1936,7 @@ pub fn generate_smart_stamp_hash(dir: &Path, additional_input: &str) -> String {
         .arg("--porcelain")
         .arg("-z")
         .arg("--untracked-files=normal")
+        .command
         .output()
         .map(|o| String::from_utf8(o.stdout).unwrap_or_default())
         .unwrap_or_default();

src/bootstrap/src/utils/channel.rs

@@ -45,7 +45,7 @@ impl GitInfo {
         }
 
         // Make sure git commands work
-        match helpers::git(Some(dir)).arg("rev-parse").output() {
+        match helpers::git(Some(dir)).arg("rev-parse").command.output() {
             Ok(ref out) if out.status.success() => {}
             _ => return GitInfo::Absent,
         }
@@ -58,15 +58,17 @@ impl GitInfo {
 
         // Ok, let's scrape some info
         let ver_date = output(
-            helpers::git(Some(dir))
+            &mut helpers::git(Some(dir))
                 .arg("log")
                 .arg("-1")
                 .arg("--date=short")
-                .arg("--pretty=format:%cd"),
+                .arg("--pretty=format:%cd")
+                .command,
+        );
+        let ver_hash = output(&mut helpers::git(Some(dir)).arg("rev-parse").arg("HEAD").command);
+        let short_ver_hash = output(
+            &mut helpers::git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD").command,
         );
-        let ver_hash = output(helpers::git(Some(dir)).arg("rev-parse").arg("HEAD"));
-        let short_ver_hash =
-            output(helpers::git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD"));
         GitInfo::Present(Some(Info {
             commit_date: ver_date.trim().to_string(),
             sha: ver_hash.trim().to_string(),

src/bootstrap/src/utils/exec.rs

@@ -23,6 +23,8 @@ pub enum OutputMode {
     OnlyOutput,
     /// Suppress the output if the command succeeds, otherwise print the output.
     OnlyOnFailure,
+    /// Suppress the output of the command.
+    Quiet,
 }
 
 /// Wrapper around `std::process::Command`.
@@ -105,10 +107,15 @@ impl BootstrapCommand {
     }
 
     /// Do not print the output of the command, unless it fails.
-    pub fn quiet(self) -> Self {
+    pub fn print_on_failure(self) -> Self {
         self.output_mode(OutputMode::OnlyOnFailure)
     }
 
+    /// Do not print the output of the command.
+    pub fn quiet(self) -> Self {
+        self.output_mode(OutputMode::Quiet)
+    }
+
     pub fn output_mode(self, output_mode: OutputMode) -> Self {
         Self { output_mode: Some(output_mode), ..self }
     }
@@ -116,15 +123,15 @@ impl BootstrapCommand {
 
 /// FIXME: This implementation is temporary, until all `Command` invocations are migrated to
 /// `BootstrapCommand`.
-impl<'a> From<&'a mut Command> for BootstrapCommand {
+impl<'a> From<&'a mut BootstrapCommand> for BootstrapCommand {
-    fn from(command: &'a mut Command) -> Self {
+    fn from(command: &'a mut BootstrapCommand) -> Self {
         // This is essentially a manual `Command::clone`
-        let mut cmd = Command::new(command.get_program());
+        let mut cmd = Command::new(command.command.get_program());
-        if let Some(dir) = command.get_current_dir() {
+        if let Some(dir) = command.command.get_current_dir() {
             cmd.current_dir(dir);
         }
-        cmd.args(command.get_args());
+        cmd.args(command.command.get_args());
-        for (key, value) in command.get_envs() {
+        for (key, value) in command.command.get_envs() {
             match value {
                 Some(value) => {
                     cmd.env(key, value);
@@ -134,16 +141,11 @@ impl<'a> From<&'a mut Command> for BootstrapCommand {
                 }
             }
         }
-
+        Self {
-        cmd.into()
+            command: cmd,
-    }
+            output_mode: command.output_mode,
-}
+            failure_behavior: command.failure_behavior,
-
+        }
-/// FIXME: This implementation is temporary, until all `Command` invocations are migrated to
-/// `BootstrapCommand`.
-impl<'a> From<&'a mut BootstrapCommand> for BootstrapCommand {
-    fn from(command: &'a mut BootstrapCommand) -> Self {
-        BootstrapCommand::from(&mut command.command)
     }
 }
 

src/bootstrap/src/utils/helpers.rs

@@ -498,8 +498,8 @@ pub fn check_cfg_arg(name: &str, values: Option<&[&str]>) -> String {
 /// manually building a git `Command`. This approach allows us to manage bootstrap-specific
 /// needs/hacks from a single source, rather than applying them on next to every `Command::new("git")`,
 /// which is painful to ensure that the required change is applied on each one of them correctly.
-pub fn git(source_dir: Option<&Path>) -> Command {
+pub fn git(source_dir: Option<&Path>) -> BootstrapCommand {
-    let mut git = Command::new("git");
+    let mut git = BootstrapCommand::new("git");
 
     if let Some(source_dir) = source_dir {
         git.current_dir(source_dir);