From f42e73bfece8390dd176353aaabd8511c3420b9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= <jakub.beranek@vsb.cz>
Date: Sun, 7 Jul 2024 18:59:01 +0200
Subject: [PATCH 1/4] Make command output capturing more explicit

Now there are separate functions for running a command without capturing, running while capturing stdout
and running while capturing everything. This should help avoid situations where stdout/stderr is accessed
when it was not captured.
---
 src/bootstrap/src/core/build_steps/compile.rs |  6 +-
 src/bootstrap/src/core/build_steps/dist.rs    |  6 +-
 src/bootstrap/src/core/build_steps/format.rs  | 10 +-
 src/bootstrap/src/core/build_steps/llvm.rs    |  6 +-
 src/bootstrap/src/core/build_steps/run.rs     |  2 +-
 src/bootstrap/src/core/build_steps/setup.rs   | 11 +--
 src/bootstrap/src/core/build_steps/suggest.rs |  3 +-
 .../src/core/build_steps/synthetic_targets.rs |  2 +-
 src/bootstrap/src/core/build_steps/test.rs    | 96 +++++++++----------
 src/bootstrap/src/core/build_steps/tool.rs    |  2 +-
 .../src/core/build_steps/toolstate.rs         |  9 +-
 src/bootstrap/src/core/builder.rs             |  2 +-
 src/bootstrap/src/core/metadata.rs            |  2 +-
 src/bootstrap/src/core/sanity.rs              |  2 +-
 src/bootstrap/src/lib.rs                      | 58 ++++++-----
 src/bootstrap/src/utils/cc_detect.rs          |  6 +-
 src/bootstrap/src/utils/exec.rs               | 33 +++----
 src/bootstrap/src/utils/helpers.rs            |  6 +-
 src/bootstrap/src/utils/tarball.rs            |  3 +-
 19 files changed, 120 insertions(+), 145 deletions(-)

diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs
index 3e79acad1c4b2..9b3a3300244ba 100644
--- a/src/bootstrap/src/core/build_steps/compile.rs
+++ b/src/bootstrap/src/core/build_steps/compile.rs
@@ -1481,7 +1481,7 @@ pub fn compiler_file(
     let mut cmd = command(compiler);
     cmd.args(builder.cflags(target, GitRepo::Rustc, c));
     cmd.arg(format!("-print-file-name={file}"));
-    let out = cmd.capture_stdout().run(builder).stdout();
+    let out = cmd.run_capture_stdout(builder).stdout();
     PathBuf::from(out.trim())
 }
 
@@ -1844,7 +1844,7 @@ impl Step for Assemble {
                 builder.ensure(llvm::Llvm { target: target_compiler.host });
             if !builder.config.dry_run() && builder.config.llvm_tools_enabled {
                 let llvm_bin_dir =
-                    command(llvm_config).capture_stdout().arg("--bindir").run(builder).stdout();
+                    command(llvm_config).arg("--bindir").run_capture_stdout(builder).stdout();
                 let llvm_bin_dir = Path::new(llvm_bin_dir.trim());
 
                 // Since we've already built the LLVM tools, install them to the sysroot.
@@ -2170,7 +2170,7 @@ pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path)
     }
 
     let previous_mtime = t!(t!(path.metadata()).modified());
-    command("strip").capture().arg("--strip-debug").arg(path).run(builder);
+    command("strip").arg("--strip-debug").arg(path).run_capture(builder);
 
     let file = t!(fs::File::open(path));
 
diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs
index 7d67cc3b36e22..ee89d2727a220 100644
--- a/src/bootstrap/src/core/build_steps/dist.rs
+++ b/src/bootstrap/src/core/build_steps/dist.rs
@@ -182,7 +182,7 @@ fn make_win_dist(
     //Ask gcc where it keeps its stuff
     let mut cmd = command(builder.cc(target));
     cmd.arg("-print-search-dirs");
-    let gcc_out = cmd.capture_stdout().run(builder).stdout();
+    let gcc_out = cmd.run_capture_stdout(builder).stdout();
 
     let mut bin_path: Vec<_> = env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect();
     let mut lib_path = Vec::new();
@@ -1062,7 +1062,7 @@ impl Step for PlainSourceTarball {
                 cmd.arg("--sync").arg(manifest_path);
             }
 
-            let config = cmd.capture().run(builder).stdout();
+            let config = cmd.run_capture(builder).stdout();
 
             let cargo_config_dir = plain_dst_src.join(".cargo");
             builder.create_dir(&cargo_config_dir);
@@ -2070,7 +2070,7 @@ fn maybe_install_llvm(
         let mut cmd = command(llvm_config);
         cmd.arg("--libfiles");
         builder.verbose(|| println!("running {cmd:?}"));
-        let files = cmd.capture_stdout().run(builder).stdout();
+        let files = cmd.run_capture_stdout(builder).stdout();
         let build_llvm_out = &builder.llvm_out(builder.config.build);
         let target_llvm_out = &builder.llvm_out(target);
         for file in files.trim_end().split(' ') {
diff --git a/src/bootstrap/src/core/build_steps/format.rs b/src/bootstrap/src/core/build_steps/format.rs
index d3ac2bae1e875..f254c058b129a 100644
--- a/src/bootstrap/src/core/build_steps/format.rs
+++ b/src/bootstrap/src/core/build_steps/format.rs
@@ -60,7 +60,7 @@ fn get_rustfmt_version(build: &Builder<'_>) -> Option<(String, PathBuf)> {
     });
     cmd.arg("--version");
 
-    let output = cmd.capture().allow_failure().run(build);
+    let output = cmd.allow_failure().run_capture(build);
     if output.is_failure() {
         return None;
     }
@@ -160,25 +160,23 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
         }
     }
     let git_available =
-        helpers::git(None).capture().allow_failure().arg("--version").run(build).is_success();
+        helpers::git(None).allow_failure().arg("--version").run_capture(build).is_success();
 
     let mut adjective = None;
     if git_available {
         let in_working_tree = helpers::git(Some(&build.src))
-            .capture()
             .allow_failure()
             .arg("rev-parse")
             .arg("--is-inside-work-tree")
-            .run(build)
+            .run_capture(build)
             .is_success();
         if in_working_tree {
             let untracked_paths_output = helpers::git(Some(&build.src))
-                .capture_stdout()
                 .arg("status")
                 .arg("--porcelain")
                 .arg("-z")
                 .arg("--untracked-files=normal")
-                .run(build)
+                .run_capture_stdout(build)
                 .stdout();
             let untracked_paths: Vec<_> = untracked_paths_output
                 .split_terminator('\0')
diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs
index f234b08f5e2ca..af987c59cb926 100644
--- a/src/bootstrap/src/core/build_steps/llvm.rs
+++ b/src/bootstrap/src/core/build_steps/llvm.rs
@@ -471,7 +471,7 @@ impl Step for Llvm {
                 builder.ensure(Llvm { target: builder.config.build });
             if !builder.config.dry_run() {
                 let llvm_bindir =
-                    command(&llvm_config).capture_stdout().arg("--bindir").run(builder).stdout();
+                    command(&llvm_config).arg("--bindir").run_capture_stdout(builder).stdout();
                 let host_bin = Path::new(llvm_bindir.trim());
                 cfg.define(
                     "LLVM_TABLEGEN",
@@ -522,7 +522,7 @@ impl Step for Llvm {
         // Helper to find the name of LLVM's shared library on darwin and linux.
         let find_llvm_lib_name = |extension| {
             let version =
-                command(&res.llvm_config).capture_stdout().arg("--version").run(builder).stdout();
+                command(&res.llvm_config).arg("--version").run_capture_stdout(builder).stdout();
             let major = version.split('.').next().unwrap();
 
             match &llvm_version_suffix {
@@ -578,7 +578,7 @@ fn check_llvm_version(builder: &Builder<'_>, llvm_config: &Path) {
         return;
     }
 
-    let version = command(llvm_config).capture_stdout().arg("--version").run(builder).stdout();
+    let version = command(llvm_config).arg("--version").run_capture_stdout(builder).stdout();
     let mut parts = version.split('.').take(2).filter_map(|s| s.parse::<u32>().ok());
     if let (Some(major), Some(_minor)) = (parts.next(), parts.next()) {
         if major >= 17 {
diff --git a/src/bootstrap/src/core/build_steps/run.rs b/src/bootstrap/src/core/build_steps/run.rs
index 3a2d3f675228d..b3ddb02801563 100644
--- a/src/bootstrap/src/core/build_steps/run.rs
+++ b/src/bootstrap/src/core/build_steps/run.rs
@@ -40,7 +40,7 @@ impl Step for BuildManifest {
             panic!("\n\nfailed to specify `dist.upload-addr` in `config.toml`\n\n")
         });
 
-        let today = command("date").capture_stdout().arg("+%Y-%m-%d").run(builder).stdout();
+        let today = command("date").arg("+%Y-%m-%d").run_capture_stdout(builder).stdout();
 
         cmd.arg(sign);
         cmd.arg(distdir(builder));
diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs
index 226b3729c10ce..7da91b83895b7 100644
--- a/src/bootstrap/src/core/build_steps/setup.rs
+++ b/src/bootstrap/src/core/build_steps/setup.rs
@@ -275,7 +275,7 @@ impl Step for Link {
 }
 
 fn rustup_installed(builder: &Builder<'_>) -> bool {
-    command("rustup").capture_stdout().arg("--version").run(builder).is_success()
+    command("rustup").arg("--version").run_capture_stdout(builder).is_success()
 }
 
 fn stage_dir_exists(stage_path: &str) -> bool {
@@ -313,10 +313,9 @@ fn attempt_toolchain_link(builder: &Builder<'_>, stage_path: &str) {
 
 fn toolchain_is_linked(builder: &Builder<'_>) -> bool {
     match command("rustup")
-        .capture_stdout()
         .allow_failure()
         .args(["toolchain", "list"])
-        .run(builder)
+        .run_capture_stdout(builder)
         .stdout_if_ok()
     {
         Some(toolchain_list) => {
@@ -341,9 +340,8 @@ fn toolchain_is_linked(builder: &Builder<'_>) -> bool {
 
 fn try_link_toolchain(builder: &Builder<'_>, stage_path: &str) -> bool {
     command("rustup")
-        .capture_stdout()
         .args(["toolchain", "link", "stage1", stage_path])
-        .run(builder)
+        .run_capture_stdout(builder)
         .is_success()
 }
 
@@ -481,9 +479,8 @@ impl Step for Hook {
 // install a git hook to automatically run tidy, if they want
 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"])
-        .run(builder)
+        .run_capture(builder)
         .stdout();
     let git = PathBuf::from(git.trim());
     let hooks_dir = git.join("hooks");
diff --git a/src/bootstrap/src/core/build_steps/suggest.rs b/src/bootstrap/src/core/build_steps/suggest.rs
index 2d4409d27c65f..0f4765fc12120 100644
--- a/src/bootstrap/src/core/build_steps/suggest.rs
+++ b/src/bootstrap/src/core/build_steps/suggest.rs
@@ -14,10 +14,9 @@ pub fn suggest(builder: &Builder<'_>, run: bool) {
     let git_config = builder.config.git_config();
     let suggestions = builder
         .tool_cmd(Tool::SuggestTests)
-        .capture_stdout()
         .env("SUGGEST_TESTS_GIT_REPOSITORY", git_config.git_repository)
         .env("SUGGEST_TESTS_NIGHTLY_BRANCH", git_config.nightly_branch)
-        .run(builder)
+        .run_capture_stdout(builder)
         .stdout();
 
     let suggestions = suggestions
diff --git a/src/bootstrap/src/core/build_steps/synthetic_targets.rs b/src/bootstrap/src/core/build_steps/synthetic_targets.rs
index 021680302cde6..04297c01d10aa 100644
--- a/src/bootstrap/src/core/build_steps/synthetic_targets.rs
+++ b/src/bootstrap/src/core/build_steps/synthetic_targets.rs
@@ -64,7 +64,7 @@ fn create_synthetic_target(
     // we cannot use nightly features. So `RUSTC_BOOTSTRAP` is needed here.
     cmd.env("RUSTC_BOOTSTRAP", "1");
 
-    let output = cmd.capture().run(builder).stdout();
+    let output = cmd.run_capture(builder).stdout();
     let mut spec: serde_json::Value = serde_json::from_slice(output.as_bytes()).unwrap();
     let spec_map = spec.as_object_mut().unwrap();
 
diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs
index cc5931c68db1f..9de92d41496be 100644
--- a/src/bootstrap/src/core/build_steps/test.rs
+++ b/src/bootstrap/src/core/build_steps/test.rs
@@ -169,7 +169,7 @@ You can skip linkcheck with --skip src/tools/linkchecker"
 }
 
 fn check_if_tidy_is_installed(builder: &Builder<'_>) -> bool {
-    command("tidy").capture_stdout().allow_failure().arg("--version").run(builder).is_success()
+    command("tidy").allow_failure().arg("--version").run_capture_stdout(builder).is_success()
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -468,7 +468,7 @@ impl Miri {
         cargo.arg("--print-sysroot");
 
         builder.verbose(|| println!("running: {cargo:?}"));
-        let stdout = cargo.capture_stdout().run(builder).stdout();
+        let stdout = cargo.run_capture_stdout(builder).stdout();
         // Output is "<sysroot>\n".
         let sysroot = stdout.trim_end();
         builder.verbose(|| println!("`cargo miri setup --print-sysroot` said: {sysroot:?}"));
@@ -749,7 +749,7 @@ impl Step for Clippy {
         let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host);
 
         // Clippy reports errors if it blessed the outputs
-        if cargo.allow_failure().run(builder).is_success() {
+        if cargo.allow_failure().run(builder) {
             // The tests succeeded; nothing to do.
             return;
         }
@@ -904,12 +904,12 @@ fn get_browser_ui_test_version_inner(
     npm: &Path,
     global: bool,
 ) -> Option<String> {
-    let mut command = command(npm).capture();
+    let mut command = command(npm);
     command.arg("list").arg("--parseable").arg("--long").arg("--depth=0");
     if global {
         command.arg("--global");
     }
-    let lines = command.allow_failure().run(builder).stdout();
+    let lines = command.allow_failure().run_capture(builder).stdout();
     lines
         .lines()
         .find_map(|l| l.split(':').nth(1)?.strip_prefix("browser-ui-test@"))
@@ -1846,19 +1846,17 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
 
         let lldb_exe = builder.config.lldb.clone().unwrap_or_else(|| PathBuf::from("lldb"));
         let lldb_version = command(&lldb_exe)
-            .capture()
             .allow_failure()
             .arg("--version")
-            .run(builder)
+            .run_capture(builder)
             .stdout_if_ok()
             .and_then(|v| if v.trim().is_empty() { None } else { Some(v) });
         if let Some(ref vers) = lldb_version {
             cmd.arg("--lldb-version").arg(vers);
             let lldb_python_dir = command(&lldb_exe)
                 .allow_failure()
-                .capture_stdout()
                 .arg("-P")
-                .run(builder)
+                .run_capture_stdout(builder)
                 .stdout_if_ok()
                 .map(|p| p.lines().next().expect("lldb Python dir not found").to_string());
             if let Some(ref dir) = lldb_python_dir {
@@ -1917,10 +1915,9 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
                 builder.ensure(llvm::Llvm { target: builder.config.build });
             if !builder.config.dry_run() {
                 let llvm_version =
-                    builder.run(command(&llvm_config).capture_stdout().arg("--version")).stdout();
-                let llvm_components = builder
-                    .run(command(&llvm_config).capture_stdout().arg("--components"))
-                    .stdout();
+                    command(&llvm_config).arg("--version").run_capture_stdout(builder).stdout();
+                let llvm_components =
+                    command(&llvm_config).arg("--components").run_capture_stdout(builder).stdout();
                 // Remove trailing newline from llvm-config output.
                 cmd.arg("--llvm-version")
                     .arg(llvm_version.trim())
@@ -1940,7 +1937,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
             // platform-specific environment variable as a workaround.
             if !builder.config.dry_run() && suite.ends_with("fulldeps") {
                 let llvm_libdir =
-                    builder.run(command(&llvm_config).capture_stdout().arg("--libdir")).stdout();
+                    command(&llvm_config).arg("--libdir").run_capture_stdout(builder).stdout();
                 add_link_lib_path(vec![llvm_libdir.trim().into()], &mut cmd);
             }
 
@@ -2212,7 +2209,7 @@ impl BookTest {
             compiler.host,
         );
         let _time = helpers::timeit(builder);
-        let toolstate = if rustbook_cmd.delay_failure().run(builder).is_success() {
+        let toolstate = if rustbook_cmd.delay_failure().run(builder) {
             ToolState::TestPass
         } else {
             ToolState::TestFail
@@ -2345,7 +2342,7 @@ impl Step for ErrorIndex {
         let guard =
             builder.msg(Kind::Test, compiler.stage, "error-index", compiler.host, compiler.host);
         let _time = helpers::timeit(builder);
-        tool.capture().run(builder);
+        tool.run_capture(builder);
         drop(guard);
         // The tests themselves need to link to std, so make sure it is
         // available.
@@ -2376,9 +2373,10 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) ->
 
     cmd = cmd.delay_failure();
     if !builder.config.verbose_tests {
-        cmd = cmd.capture();
+        cmd.run_capture(builder).is_success()
+    } else {
+        cmd.run(builder)
     }
-    cmd.run(builder).is_success()
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -2404,11 +2402,8 @@ impl Step for RustcGuide {
         let src = builder.src.join(relative_path);
         let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook).delay_failure();
         rustbook_cmd.arg("linkcheck").arg(&src);
-        let toolstate = if rustbook_cmd.run(builder).is_success() {
-            ToolState::TestPass
-        } else {
-            ToolState::TestFail
-        };
+        let toolstate =
+            if rustbook_cmd.run(builder) { ToolState::TestPass } else { ToolState::TestFail };
         builder.save_toolstate("rustc-dev-guide", toolstate);
     }
 }
@@ -2920,7 +2915,7 @@ impl Step for RemoteCopyLibs {
             let f = t!(f);
             let name = f.file_name().into_string().unwrap();
             if helpers::is_dylib(&name) {
-                builder.run(command(&tool).arg("push").arg(f.path()));
+                command(&tool).arg("push").arg(f.path()).run(builder);
             }
         }
     }
@@ -2951,21 +2946,21 @@ impl Step for Distcheck {
         builder.ensure(dist::PlainSourceTarball);
         builder.ensure(dist::Src);
 
-        let mut cmd = command("tar");
-        cmd.arg("-xf")
+        command("tar")
+            .arg("-xf")
             .arg(builder.ensure(dist::PlainSourceTarball).tarball())
             .arg("--strip-components=1")
-            .current_dir(&dir);
-        cmd.run(builder);
-        builder.run(
-            command("./configure")
-                .args(&builder.config.configure_args)
-                .arg("--enable-vendor")
-                .current_dir(&dir),
-        );
-        builder.run(
-            command(helpers::make(&builder.config.build.triple)).arg("check").current_dir(&dir),
-        );
+            .current_dir(&dir)
+            .run(builder);
+        command("./configure")
+            .args(&builder.config.configure_args)
+            .arg("--enable-vendor")
+            .current_dir(&dir)
+            .run(builder);
+        command(helpers::make(&builder.config.build.triple))
+            .arg("check")
+            .current_dir(&dir)
+            .run(builder);
 
         // Now make sure that rust-src has all of libstd's dependencies
         builder.info("Distcheck rust-src");
@@ -2973,24 +2968,23 @@ impl Step for Distcheck {
         let _ = fs::remove_dir_all(&dir);
         t!(fs::create_dir_all(&dir));
 
-        let mut cmd = command("tar");
-        cmd.arg("-xf")
+        command("tar")
+            .arg("-xf")
             .arg(builder.ensure(dist::Src).tarball())
             .arg("--strip-components=1")
-            .current_dir(&dir);
-        cmd.run(builder);
+            .current_dir(&dir)
+            .run(builder);
 
         let toml = dir.join("rust-src/lib/rustlib/src/rust/library/std/Cargo.toml");
-        builder.run(
-            command(&builder.initial_cargo)
-                // Will read the libstd Cargo.toml
-                // which uses the unstable `public-dependency` feature.
-                .env("RUSTC_BOOTSTRAP", "1")
-                .arg("generate-lockfile")
-                .arg("--manifest-path")
-                .arg(&toml)
-                .current_dir(&dir),
-        );
+        command(&builder.initial_cargo)
+            // Will read the libstd Cargo.toml
+            // which uses the unstable `public-dependency` feature.
+            .env("RUSTC_BOOTSTRAP", "1")
+            .arg("generate-lockfile")
+            .arg("--manifest-path")
+            .arg(&toml)
+            .current_dir(&dir)
+            .run(builder);
     }
 }
 
diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs
index 087df2f8a88e2..7f2eb9b7a8ca4 100644
--- a/src/bootstrap/src/core/build_steps/tool.rs
+++ b/src/bootstrap/src/core/build_steps/tool.rs
@@ -982,7 +982,7 @@ impl Step for LibcxxVersionTool {
             }
         }
 
-        let version_output = command(executable).capture_stdout().run(builder).stdout();
+        let version_output = command(executable).run_capture_stdout(builder).stdout();
 
         let version_str = version_output.split_once("version:").unwrap().1;
         let version = version_str.trim().parse::<usize>().unwrap();
diff --git a/src/bootstrap/src/core/build_steps/toolstate.rs b/src/bootstrap/src/core/build_steps/toolstate.rs
index 2ab0ce7454b17..912cd4b8676f6 100644
--- a/src/bootstrap/src/core/build_steps/toolstate.rs
+++ b/src/bootstrap/src/core/build_steps/toolstate.rs
@@ -102,12 +102,11 @@ fn print_error(tool: &str, submodule: &str) {
 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^")
-        .run(builder)
+        .run_capture(builder)
         .stdout();
 
     for (tool, submodule) in STABLE_TOOLS.iter().chain(NIGHTLY_TOOLS.iter()) {
@@ -391,7 +390,7 @@ fn commit_toolstate_change(builder: &Builder<'_>, current_toolstate: &ToolstateD
             .arg("-m")
             .arg(&message)
             .run(builder);
-        if !status.is_success() {
+        if !status {
             success = true;
             break;
         }
@@ -403,7 +402,7 @@ fn commit_toolstate_change(builder: &Builder<'_>, current_toolstate: &ToolstateD
             .arg("master")
             .run(builder);
         // If we successfully push, exit.
-        if status.is_success() {
+        if status {
             success = true;
             break;
         }
@@ -432,7 +431,7 @@ fn commit_toolstate_change(builder: &Builder<'_>, current_toolstate: &ToolstateD
 /// `publish_toolstate.py` script if the PR passes all tests and is merged to
 /// master.
 fn publish_test_results(builder: &Builder<'_>, current_toolstate: &ToolstateData) {
-    let commit = helpers::git(None).capture().arg("rev-parse").arg("HEAD").run(builder).stdout();
+    let commit = helpers::git(None).arg("rev-parse").arg("HEAD").run_capture(builder).stdout();
 
     let toolstate_serialized = t!(serde_json::to_string(&current_toolstate));
 
diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs
index 78fbea2e8107c..d21ddc5672bdf 100644
--- a/src/bootstrap/src/core/builder.rs
+++ b/src/bootstrap/src/core/builder.rs
@@ -1954,7 +1954,7 @@ impl<'a> Builder<'a> {
         if mode == Mode::ToolRustc || mode == Mode::Codegen {
             if let Some(llvm_config) = self.llvm_config(target) {
                 let llvm_libdir =
-                    command(llvm_config).capture_stdout().arg("--libdir").run(self).stdout();
+                    command(llvm_config).arg("--libdir").run_capture_stdout(self).stdout();
                 add_link_lib_path(vec![llvm_libdir.trim().into()], &mut cargo);
             }
         }
diff --git a/src/bootstrap/src/core/metadata.rs b/src/bootstrap/src/core/metadata.rs
index b18da844014be..9b4c85e6d34a4 100644
--- a/src/bootstrap/src/core/metadata.rs
+++ b/src/bootstrap/src/core/metadata.rs
@@ -81,7 +81,7 @@ fn workspace_members(build: &Build) -> Vec<Package> {
             .arg("--no-deps")
             .arg("--manifest-path")
             .arg(build.src.join(manifest_path));
-        let metadata_output = cargo.capture_stdout().run_always().run(build).stdout();
+        let metadata_output = cargo.run_always().run_capture_stdout(build).stdout();
         let Output { packages, .. } = t!(serde_json::from_str(&metadata_output));
         packages
     };
diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs
index 2be819d52ea1a..8aa0c43ad6e38 100644
--- a/src/bootstrap/src/core/sanity.rs
+++ b/src/bootstrap/src/core/sanity.rs
@@ -352,7 +352,7 @@ than building it.
             // There are three builds of cmake on windows: MSVC, MinGW, and
             // Cygwin. The Cygwin build does not have generators for Visual
             // Studio, so detect that here and error.
-            let out = command("cmake").capture_stdout().arg("--help").run(build).stdout();
+            let out = command("cmake").arg("--help").run_capture_stdout(build).stdout();
             if !out.contains("Visual Studio") {
                 panic!(
                     "
diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs
index 79bea50c626b2..1d388767d7e37 100644
--- a/src/bootstrap/src/lib.rs
+++ b/src/bootstrap/src/lib.rs
@@ -40,7 +40,7 @@ use crate::core::builder::{Builder, Kind};
 use crate::core::config::{flags, LldMode};
 use crate::core::config::{DryRun, Target};
 use crate::core::config::{LlvmLibunwind, TargetSelection};
-use crate::utils::exec::{command, BehaviorOnFailure, BootstrapCommand, CommandOutput};
+use crate::utils::exec::{command, BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode};
 use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir};
 
 mod core;
@@ -496,21 +496,21 @@ impl Build {
         // Therefore, all commands below are marked with `run_always()`, so that they also run in
         // dry run mode.
         let submodule_git = || {
-            let mut cmd = helpers::git(Some(&absolute_path)).capture_stdout();
+            let mut cmd = helpers::git(Some(&absolute_path));
             cmd.run_always();
             cmd
         };
 
         // Determine commit checked out in submodule.
-        let checked_out_hash = submodule_git().args(["rev-parse", "HEAD"]).run(self).stdout();
+        let checked_out_hash =
+            submodule_git().args(["rev-parse", "HEAD"]).run_capture_stdout(self).stdout();
         let checked_out_hash = checked_out_hash.trim_end();
         // Determine commit that the submodule *should* have.
         let recorded = helpers::git(Some(&self.src))
-            .capture_stdout()
             .run_always()
             .args(["ls-tree", "HEAD"])
             .arg(relative_path)
-            .run(self)
+            .run_capture_stdout(self)
             .stdout();
         let actual_hash = recorded
             .split_whitespace()
@@ -534,11 +534,10 @@ impl Build {
             // 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 = helpers::git(Some(&self.src))
-                .capture_stdout()
                 .allow_failure()
                 .run_always()
                 .args(["symbolic-ref", "--short", "HEAD"])
-                .run(self)
+                .run_capture_stdout(self)
                 .stdout_if_ok()
                 .map(|s| s.trim().to_owned());
 
@@ -557,17 +556,14 @@ impl Build {
             git.arg(relative_path);
             git
         };
-        if !update(true).run(self).is_success() {
+        if !update(true).run(self) {
             update(false).run(self);
         }
 
         // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error).
         // diff-index reports the modifications through the exit status
-        let has_local_modifications = submodule_git()
-            .allow_failure()
-            .args(["diff-index", "--quiet", "HEAD"])
-            .run(self)
-            .is_failure();
+        let has_local_modifications =
+            !submodule_git().allow_failure().args(["diff-index", "--quiet", "HEAD"]).run(self);
         if has_local_modifications {
             submodule_git().args(["stash", "push"]).run(self);
         }
@@ -588,11 +584,10 @@ impl Build {
             return;
         }
         let output = helpers::git(Some(&self.src))
-            .capture()
             .args(["config", "--file"])
             .arg(self.config.src.join(".gitmodules"))
             .args(["--get-regexp", "path"])
-            .run(self)
+            .run_capture(self)
             .stdout();
         for line in output.lines() {
             // Look for `submodule.$name.path = $path`
@@ -870,14 +865,14 @@ impl Build {
         if let Some(s) = target_config.and_then(|c| c.llvm_filecheck.as_ref()) {
             s.to_path_buf()
         } else if let Some(s) = target_config.and_then(|c| c.llvm_config.as_ref()) {
-            let llvm_bindir = command(s).capture_stdout().arg("--bindir").run(self).stdout();
+            let llvm_bindir = command(s).arg("--bindir").run_capture_stdout(self).stdout();
             let filecheck = Path::new(llvm_bindir.trim()).join(exe("FileCheck", target));
             if filecheck.exists() {
                 filecheck
             } else {
                 // On Fedora the system LLVM installs FileCheck in the
                 // llvm subdirectory of the libdir.
-                let llvm_libdir = command(s).capture_stdout().arg("--libdir").run(self).stdout();
+                let llvm_libdir = command(s).arg("--libdir").run_capture_stdout(self).stdout();
                 let lib_filecheck =
                     Path::new(llvm_libdir.trim()).join("llvm").join(exe("FileCheck", target));
                 if lib_filecheck.exists() {
@@ -944,7 +939,12 @@ impl Build {
     /// Execute a command and return its output.
     /// This method should be used for all command executions in bootstrap.
     #[track_caller]
-    fn run(&self, command: &mut BootstrapCommand) -> CommandOutput {
+    fn run(
+        &self,
+        command: &mut BootstrapCommand,
+        stdout: OutputMode,
+        stderr: OutputMode,
+    ) -> CommandOutput {
         command.mark_as_executed();
         if self.config.dry_run() && !command.run_always {
             return CommandOutput::default();
@@ -957,12 +957,11 @@ impl Build {
             println!("running: {command:?} (created at {created_at}, executed at {executed_at})")
         });
 
-        let stdout = command.stdout.stdio();
-        command.as_command_mut().stdout(stdout);
-        let stderr = command.stderr.stdio();
-        command.as_command_mut().stderr(stderr);
+        let cmd = command.as_command_mut();
+        cmd.stdout(stdout.stdio());
+        cmd.stderr(stderr.stdio());
 
-        let output = command.as_command_mut().output();
+        let output = cmd.output();
 
         use std::fmt::Write;
 
@@ -988,10 +987,10 @@ Executed at: {executed_at}"#,
                 // If the output mode is OutputMode::Capture, we can now print the output.
                 // If it is OutputMode::Print, then the output has already been printed to
                 // stdout/stderr, and we thus don't have anything captured to print anyway.
-                if command.stdout.captures() {
+                if stdout.captures() {
                     writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap();
                 }
-                if command.stderr.captures() {
+                if stderr.captures() {
                     writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap();
                 }
                 output
@@ -1529,7 +1528,6 @@ Executed at: {executed_at}"#,
             // That's our beta number!
             // (Note that we use a `..` range, not the `...` symmetric difference.)
             helpers::git(Some(&self.src))
-                .capture()
                 .arg("rev-list")
                 .arg("--count")
                 .arg("--merges")
@@ -1537,7 +1535,7 @@ Executed at: {executed_at}"#,
                     "refs/remotes/origin/{}..HEAD",
                     self.config.stage0_metadata.config.nightly_branch
                 ))
-                .run(self)
+                .run_capture(self)
                 .stdout()
         });
         let n = count.trim().parse().unwrap();
@@ -1972,21 +1970,19 @@ pub fn generate_smart_stamp_hash(
     additional_input: &str,
 ) -> String {
     let diff = helpers::git(Some(dir))
-        .capture_stdout()
         .allow_failure()
         .arg("diff")
-        .run(builder)
+        .run_capture_stdout(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")
-        .run(builder)
+        .run_capture_stdout(builder)
         .stdout_if_ok()
         .unwrap_or_default();
 
diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs
index 20d79e490ec48..da3d4e9701353 100644
--- a/src/bootstrap/src/utils/cc_detect.rs
+++ b/src/bootstrap/src/utils/cc_detect.rs
@@ -182,15 +182,15 @@ fn default_compiler(
                 return None;
             }
 
-            let cmd = BootstrapCommand::from(c.to_command());
-            let output = cmd.capture_stdout().arg("--version").run(build).stdout();
+            let mut cmd = BootstrapCommand::from(c.to_command());
+            let output = cmd.arg("--version").run_capture_stdout(build).stdout();
             let i = output.find(" 4.")?;
             match output[i + 3..].chars().next().unwrap() {
                 '0'..='6' => {}
                 _ => return None,
             }
             let alternative = format!("e{gnu_compiler}");
-            if command(&alternative).capture().run(build).is_success() {
+            if command(&alternative).run_capture(build).is_success() {
                 Some(PathBuf::from(alternative))
             } else {
                 None
diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs
index a60c0084f3d7f..22619e674f9a1 100644
--- a/src/bootstrap/src/utils/exec.rs
+++ b/src/bootstrap/src/utils/exec.rs
@@ -59,8 +59,6 @@ impl OutputMode {
 pub struct BootstrapCommand {
     command: Command,
     pub failure_behavior: BehaviorOnFailure,
-    pub stdout: OutputMode,
-    pub stderr: OutputMode,
     // Run the command even during dry run
     pub run_always: bool,
     // This field makes sure that each command is executed (or disarmed) before it is dropped,
@@ -135,22 +133,23 @@ impl BootstrapCommand {
         self
     }
 
-    /// Capture all output of the command, do not print it.
-    #[must_use]
-    pub fn capture(self) -> Self {
-        Self { stdout: OutputMode::Capture, stderr: OutputMode::Capture, ..self }
+    /// Run the command, while printing stdout and stderr.
+    /// Returns true if the command has succeeded.
+    #[track_caller]
+    pub fn run(&mut self, builder: &Build) -> bool {
+        builder.run(self, OutputMode::Print, OutputMode::Print).is_success()
     }
 
-    /// Capture stdout of the command, do not print it.
-    #[must_use]
-    pub fn capture_stdout(self) -> Self {
-        Self { stdout: OutputMode::Capture, ..self }
+    /// Run the command, while capturing and returning all its output.
+    #[track_caller]
+    pub fn run_capture(&mut self, builder: &Build) -> CommandOutput {
+        builder.run(self, OutputMode::Capture, OutputMode::Capture)
     }
 
-    /// Run the command, returning its output.
+    /// Run the command, while capturing and returning stdout, and printing stderr.
     #[track_caller]
-    pub fn run(&mut self, builder: &Build) -> CommandOutput {
-        builder.run(self)
+    pub fn run_capture_stdout(&mut self, builder: &Build) -> CommandOutput {
+        builder.run(self, OutputMode::Capture, OutputMode::Print)
     }
 
     /// Provides access to the stdlib Command inside.
@@ -189,11 +188,7 @@ impl BootstrapCommand {
 impl Debug for BootstrapCommand {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(f, "{:?}", self.command)?;
-        write!(
-            f,
-            " (failure_mode={:?}, stdout_mode={:?}, stderr_mode={:?})",
-            self.failure_behavior, self.stdout, self.stderr
-        )
+        write!(f, " (failure_mode={:?})", self.failure_behavior)
     }
 }
 
@@ -205,8 +200,6 @@ impl From<Command> for BootstrapCommand {
         Self {
             command,
             failure_behavior: BehaviorOnFailure::Exit,
-            stdout: OutputMode::Print,
-            stderr: OutputMode::Print,
             run_always: false,
             drop_bomb: DropBomb::arm(program),
         }
diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs
index 690d7318f9420..c6e7065667027 100644
--- a/src/bootstrap/src/utils/helpers.rs
+++ b/src/bootstrap/src/utils/helpers.rs
@@ -347,7 +347,7 @@ pub fn get_clang_cl_resource_dir(builder: &Builder<'_>, clang_cl_path: &str) ->
     let mut builtins_locator = command(clang_cl_path);
     builtins_locator.args(["/clang:-print-libgcc-file-name", "/clang:--rtlib=compiler-rt"]);
 
-    let clang_rt_builtins = builtins_locator.capture_stdout().run(builder).stdout();
+    let clang_rt_builtins = builtins_locator.run_capture_stdout(builder).stdout();
     let clang_rt_builtins = Path::new(clang_rt_builtins.trim());
     assert!(
         clang_rt_builtins.exists(),
@@ -372,9 +372,9 @@ fn lld_flag_no_threads(builder: &Builder<'_>, lld_mode: LldMode, is_windows: boo
     let (windows_flag, other_flag) = LLD_NO_THREADS.get_or_init(|| {
         let newer_version = match lld_mode {
             LldMode::External => {
-                let mut cmd = command("lld").capture_stdout();
+                let mut cmd = command("lld");
                 cmd.arg("-flavor").arg("ld").arg("--version");
-                let out = cmd.run(builder).stdout();
+                let out = cmd.run_capture_stdout(builder).stdout();
                 match (out.find(char::is_numeric), out.find('.')) {
                     (Some(b), Some(e)) => out.as_str()[b..e].parse::<i32>().ok().unwrap_or(14) > 10,
                     _ => true,
diff --git a/src/bootstrap/src/utils/tarball.rs b/src/bootstrap/src/utils/tarball.rs
index 28a295e3e2a5a..cef67ae4c371a 100644
--- a/src/bootstrap/src/utils/tarball.rs
+++ b/src/bootstrap/src/utils/tarball.rs
@@ -370,11 +370,10 @@ impl<'a> Tarball<'a> {
         if self.builder.rust_info().is_managed_git_subrepository() {
             // %ct means committer date
             let timestamp = helpers::git(Some(&self.builder.src))
-                .capture_stdout()
                 .arg("log")
                 .arg("-1")
                 .arg("--format=%ct")
-                .run(self.builder)
+                .run_capture_stdout(self.builder)
                 .stdout();
             cmd.args(["--override-file-mtime", timestamp.trim()]);
         }

From b5d4a7f83915ac50bb88c8a72f8cc1e753e10518 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= <jakub.beranek@vsb.cz>
Date: Sun, 7 Jul 2024 19:04:27 +0200
Subject: [PATCH 2/4] Make it easier to detect when bootstrap tries to read
 uncaptured stdout/stderr

If e.g. only stdout is captured, but the caller tries to read stderr, previously
they would get back an empty string. Now the code will explicitly panic when
accessing an uncaptured output stream.
---
 src/bootstrap/src/lib.rs        |  6 +++--
 src/bootstrap/src/utils/exec.rs | 46 ++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs
index 1d388767d7e37..899a80fa9c7d9 100644
--- a/src/bootstrap/src/lib.rs
+++ b/src/bootstrap/src/lib.rs
@@ -968,7 +968,9 @@ impl Build {
         let mut message = String::new();
         let output: CommandOutput = match output {
             // Command has succeeded
-            Ok(output) if output.status.success() => output.into(),
+            Ok(output) if output.status.success() => {
+                CommandOutput::from_output(output, stdout, stderr)
+            }
             // Command has started, but then it failed
             Ok(output) => {
                 writeln!(
@@ -982,7 +984,7 @@ Executed at: {executed_at}"#,
                 )
                 .unwrap();
 
-                let output: CommandOutput = output.into();
+                let output: CommandOutput = CommandOutput::from_output(output, stdout, stderr);
 
                 // If the output mode is OutputMode::Capture, we can now print the output.
                 // If it is OutputMode::Print, then the output has already been printed to
diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs
index 22619e674f9a1..d4ae8e26aaae6 100644
--- a/src/bootstrap/src/utils/exec.rs
+++ b/src/bootstrap/src/utils/exec.rs
@@ -223,17 +223,31 @@ pub fn command<S: AsRef<OsStr>>(program: S) -> BootstrapCommand {
 }
 
 /// Represents the output of an executed process.
-#[allow(unused)]
 pub struct CommandOutput {
     status: CommandStatus,
-    stdout: Vec<u8>,
-    stderr: Vec<u8>,
+    stdout: Option<Vec<u8>>,
+    stderr: Option<Vec<u8>>,
 }
 
 impl CommandOutput {
     #[must_use]
     pub fn did_not_start() -> Self {
-        Self { status: CommandStatus::DidNotStart, stdout: vec![], stderr: vec![] }
+        Self { status: CommandStatus::DidNotStart, stdout: None, stderr: None }
+    }
+
+    #[must_use]
+    pub fn from_output(output: Output, stdout: OutputMode, stderr: OutputMode) -> Self {
+        Self {
+            status: CommandStatus::Finished(output.status),
+            stdout: match stdout {
+                OutputMode::Print => None,
+                OutputMode::Capture => Some(output.stdout),
+            },
+            stderr: match stderr {
+                OutputMode::Print => None,
+                OutputMode::Capture => Some(output.stderr),
+            },
+        }
     }
 
     #[must_use]
@@ -259,7 +273,10 @@ impl CommandOutput {
 
     #[must_use]
     pub fn stdout(&self) -> String {
-        String::from_utf8(self.stdout.clone()).expect("Cannot parse process stdout as UTF-8")
+        String::from_utf8(
+            self.stdout.clone().expect("Accessing stdout of a command that did not capture stdout"),
+        )
+        .expect("Cannot parse process stdout as UTF-8")
     }
 
     #[must_use]
@@ -269,7 +286,10 @@ impl CommandOutput {
 
     #[must_use]
     pub fn stderr(&self) -> String {
-        String::from_utf8(self.stderr.clone()).expect("Cannot parse process stderr as UTF-8")
+        String::from_utf8(
+            self.stderr.clone().expect("Accessing stderr of a command that did not capture stderr"),
+        )
+        .expect("Cannot parse process stderr as UTF-8")
     }
 }
 
@@ -277,18 +297,8 @@ impl Default for CommandOutput {
     fn default() -> Self {
         Self {
             status: CommandStatus::Finished(ExitStatus::default()),
-            stdout: vec![],
-            stderr: vec![],
-        }
-    }
-}
-
-impl From<Output> for CommandOutput {
-    fn from(output: Output) -> Self {
-        Self {
-            status: CommandStatus::Finished(output.status),
-            stdout: output.stdout,
-            stderr: output.stderr,
+            stdout: Some(vec![]),
+            stderr: Some(vec![]),
         }
     }
 }

From 037b6261f897a516412f527e980d67eb9acf1413 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= <berykubik@gmail.com>
Date: Tue, 23 Jul 2024 08:43:02 +0200
Subject: [PATCH 3/4] Fix usages of old command API

---
 src/bootstrap/src/core/build_steps/tool.rs | 3 +--
 src/bootstrap/src/utils/helpers.rs         | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs
index 7f2eb9b7a8ca4..06bb8259fc427 100644
--- a/src/bootstrap/src/core/build_steps/tool.rs
+++ b/src/bootstrap/src/core/build_steps/tool.rs
@@ -589,8 +589,7 @@ impl Step for Rustdoc {
                 .arg("--")
                 .arg(librustdoc_src)
                 .arg(rustdoc_src)
-                .run(builder)
-                .is_success();
+                .run(builder);
 
             if !has_changes {
                 let precompiled_rustdoc = builder
diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs
index c6e7065667027..51c14ff6282bc 100644
--- a/src/bootstrap/src/utils/helpers.rs
+++ b/src/bootstrap/src/utils/helpers.rs
@@ -534,7 +534,7 @@ pub fn get_closest_merge_base_commit(
     author: &str,
     target_paths: &[PathBuf],
 ) -> Result<String, String> {
-    let mut git = git(source_dir).capture_stdout();
+    let mut git = git(source_dir);
 
     let merge_base = get_git_merge_base(config, source_dir).unwrap_or_else(|_| "HEAD".into());
 

From 17199d055964408c614adabff740cf8d44d37c2e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= <berykubik@gmail.com>
Date: Wed, 24 Jul 2024 18:06:52 +0200
Subject: [PATCH 4/4] Fix broken doc link

---
 src/bootstrap/src/utils/exec.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs
index d4ae8e26aaae6..3b04459db0660 100644
--- a/src/bootstrap/src/utils/exec.rs
+++ b/src/bootstrap/src/utils/exec.rs
@@ -50,7 +50,7 @@ impl OutputMode {
 /// If you want to delay failures until the end of bootstrap, use [delay_failure].
 ///
 /// By default, the command will print its stdout/stderr to stdout/stderr of bootstrap ([OutputMode::Print]).
-/// If you want to handle the output programmatically, use [BootstrapCommand::capture].
+/// If you want to handle the output programmatically, use [BootstrapCommand::run_capture].
 ///
 /// Bootstrap will print a debug log to stdout if the command fails and failure is not allowed.
 ///