Add spawn APIs for BootstrapCommand to support deferred command execution

src/bootstrap/src/utils/channel.rs

@@ -11,7 +11,7 @@ use std::path::Path;
 use super::execution_context::ExecutionContext;
 use super::helpers;
 use crate::Build;
-use crate::utils::helpers::{start_process, t};
+use crate::utils::helpers::t;
 
 #[derive(Clone, Default)]
 pub enum GitInfo {
@@ -46,7 +46,7 @@ impl GitInfo {
 
         let mut git_command = helpers::git(Some(dir));
         git_command.arg("rev-parse");
-        let output = git_command.allow_failure().run_capture(exec_ctx);
+        let output = git_command.allow_failure().run_capture(&exec_ctx);
 
         if output.is_failure() {
             return GitInfo::Absent;
@@ -59,23 +59,32 @@ impl GitInfo {
         }
 
         // Ok, let's scrape some info
-        let ver_date = start_process(
-            helpers::git(Some(dir))
-                .arg("log")
-                .arg("-1")
-                .arg("--date=short")
-                .arg("--pretty=format:%cd")
-                .as_command_mut(),
-        );
+        // We use the command's spawn API to execute these commands concurrently, which leads to performance improvements.
+        let mut git_log_cmd = helpers::git(Some(dir));
+        let ver_date = git_log_cmd
+            .arg("log")
+            .arg("-1")
+            .arg("--date=short")
+            .arg("--pretty=format:%cd")
+            .run_always()
+            .start_capture_stdout(&exec_ctx);
+
+        let mut git_hash_cmd = helpers::git(Some(dir));
         let ver_hash =
-            start_process(helpers::git(Some(dir)).arg("rev-parse").arg("HEAD").as_command_mut());
-        let short_ver_hash = start_process(
-            helpers::git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD").as_command_mut(),
-        );
+            git_hash_cmd.arg("rev-parse").arg("HEAD").run_always().start_capture_stdout(&exec_ctx);
+
+        let mut git_short_hash_cmd = helpers::git(Some(dir));
+        let short_ver_hash = git_short_hash_cmd
+            .arg("rev-parse")
+            .arg("--short=9")
+            .arg("HEAD")
+            .run_always()
+            .start_capture_stdout(&exec_ctx);
+
         GitInfo::Present(Some(Info {
-            commit_date: ver_date().trim().to_string(),
-            sha: ver_hash().trim().to_string(),
-            short_sha: short_ver_hash().trim().to_string(),
+            commit_date: ver_date.wait_for_output(&exec_ctx).stdout().trim().to_string(),
+            sha: ver_hash.wait_for_output(&exec_ctx).stdout().trim().to_string(),
+            short_sha: short_ver_hash.wait_for_output(&exec_ctx).stdout().trim().to_string(),
         }))
     }
 

src/bootstrap/src/utils/exec.rs

@@ -2,7 +2,6 @@
 //!
 //! This module provides a structured way to execute and manage commands efficiently,
 //! ensuring controlled failure handling and output management.
-
 use std::ffi::OsStr;
 use std::fmt::{Debug, Formatter};
 use std::path::Path;
@@ -11,7 +10,7 @@ use std::process::{Command, CommandArgs, CommandEnvs, ExitStatus, Output, Stdio}
 use build_helper::ci::CiEnv;
 use build_helper::drop_bomb::DropBomb;
 
-use super::execution_context::ExecutionContext;
+use super::execution_context::{DeferredCommand, ExecutionContext};
 
 /// What should be done when the command fails.
 #[derive(Debug, Copy, Clone)]
@@ -73,7 +72,7 @@ pub struct BootstrapCommand {
     drop_bomb: DropBomb,
 }
 
-impl BootstrapCommand {
+impl<'a> BootstrapCommand {
     #[track_caller]
     pub fn new<S: AsRef<OsStr>>(program: S) -> Self {
         Command::new(program).into()
@@ -158,6 +157,24 @@ impl BootstrapCommand {
         exec_ctx.as_ref().run(self, OutputMode::Capture, OutputMode::Print)
     }
 
+    /// Spawn the command in background, while capturing and returning all its output.
+    #[track_caller]
+    pub fn start_capture(
+        &'a mut self,
+        exec_ctx: impl AsRef<ExecutionContext>,
+    ) -> DeferredCommand<'a> {
+        exec_ctx.as_ref().start(self, OutputMode::Capture, OutputMode::Capture)
+    }
+
+    /// Spawn the command in background, while capturing and returning stdout, and printing stderr.
+    #[track_caller]
+    pub fn start_capture_stdout(
+        &'a mut self,
+        exec_ctx: impl AsRef<ExecutionContext>,
+    ) -> DeferredCommand<'a> {
+        exec_ctx.as_ref().start(self, OutputMode::Capture, OutputMode::Print)
+    }
+
     /// Provides access to the stdlib Command inside.
     /// FIXME: This function should be eventually removed from bootstrap.
     pub fn as_command_mut(&mut self) -> &mut Command {

src/bootstrap/src/utils/execution_context.rs

@@ -3,6 +3,8 @@
 //! This module provides the [`ExecutionContext`] type, which holds global configuration
 //! relevant during the execution of commands in bootstrap. This includes dry-run
 //! mode, verbosity level, and behavior on failure.
+use std::panic::Location;
+use std::process::Child;
 use std::sync::{Arc, Mutex};
 
 use crate::core::config::DryRun;
@@ -80,23 +82,24 @@ impl ExecutionContext {
     /// Note: Ideally, you should use one of the BootstrapCommand::run* functions to
     /// execute commands. They internally call this method.
     #[track_caller]
-    pub fn run(
+    pub fn start<'a>(
         &self,
-        command: &mut BootstrapCommand,
+        command: &'a mut BootstrapCommand,
         stdout: OutputMode,
         stderr: OutputMode,
-    ) -> CommandOutput {
+    ) -> DeferredCommand<'a> {
         command.mark_as_executed();
+
+        let created_at = command.get_created_location();
+        let executed_at = std::panic::Location::caller();
+
         if self.dry_run() && !command.run_always {
-            return CommandOutput::default();
+            return DeferredCommand { process: None, stdout, stderr, command, executed_at };
         }
 
         #[cfg(feature = "tracing")]
         let _run_span = trace_cmd!(command);
 
-        let created_at = command.get_created_location();
-        let executed_at = std::panic::Location::caller();
-
         self.verbose(|| {
             println!("running: {command:?} (created at {created_at}, executed at {executed_at})")
         });
@@ -105,38 +108,106 @@ impl ExecutionContext {
         cmd.stdout(stdout.stdio());
         cmd.stderr(stderr.stdio());
 
-        let output = cmd.output();
+        let child = cmd.spawn().unwrap();
+
+        DeferredCommand { process: Some(child), stdout, stderr, command, executed_at }
+    }
+
+    /// Execute a command and return its output.
+    /// Note: Ideally, you should use one of the BootstrapCommand::run* functions to
+    /// execute commands. They internally call this method.
+    #[track_caller]
+    pub fn run(
+        &self,
+        command: &mut BootstrapCommand,
+        stdout: OutputMode,
+        stderr: OutputMode,
+    ) -> CommandOutput {
+        self.start(command, stdout, stderr).wait_for_output(self)
+    }
+
+    fn fail(&self, message: &str, output: CommandOutput) -> ! {
+        if self.is_verbose() {
+            println!("{message}");
+        } else {
+            let (stdout, stderr) = (output.stdout_if_present(), output.stderr_if_present());
+            // If the command captures output, the user would not see any indication that
+            // it has failed. In this case, print a more verbose error, since to provide more
+            // context.
+            if stdout.is_some() || stderr.is_some() {
+                if let Some(stdout) = output.stdout_if_present().take_if(|s| !s.trim().is_empty()) {
+                    println!("STDOUT:\n{stdout}\n");
+                }
+                if let Some(stderr) = output.stderr_if_present().take_if(|s| !s.trim().is_empty()) {
+                    println!("STDERR:\n{stderr}\n");
+                }
+                println!("Command has failed. Rerun with -v to see more details.");
+            } else {
+                println!("Command has failed. Rerun with -v to see more details.");
+            }
+        }
+        exit!(1);
+    }
+}
+
+impl AsRef<ExecutionContext> for ExecutionContext {
+    fn as_ref(&self) -> &ExecutionContext {
+        self
+    }
+}
+
+pub struct DeferredCommand<'a> {
+    process: Option<Child>,
+    command: &'a mut BootstrapCommand,
+    stdout: OutputMode,
+    stderr: OutputMode,
+    executed_at: &'a Location<'a>,
+}
+
+impl<'a> DeferredCommand<'a> {
+    pub fn wait_for_output(mut self, exec_ctx: impl AsRef<ExecutionContext>) -> CommandOutput {
+        if self.process.is_none() {
+            return CommandOutput::default();
+        }
+
+        let exec_ctx = exec_ctx.as_ref();
+
+        let output = self.process.take().unwrap().wait_with_output();
+
+        let created_at = self.command.get_created_location();
+        let executed_at = self.executed_at;
 
         use std::fmt::Write;
 
         let mut message = String::new();
         let output: CommandOutput = match output {
             // Command has succeeded
             Ok(output) if output.status.success() => {
-                CommandOutput::from_output(output, stdout, stderr)
+                CommandOutput::from_output(output, self.stdout, self.stderr)
             }
             // Command has started, but then it failed
             Ok(output) => {
                 writeln!(
                     message,
                     r#"
-Command {command:?} did not execute successfully.
+Command {:?} did not execute successfully.
 Expected success, got {}
 Created at: {created_at}
 Executed at: {executed_at}"#,
-                    output.status,
+                    self.command, output.status,
                 )
                 .unwrap();
 
-                let output: CommandOutput = CommandOutput::from_output(output, stdout, stderr);
+                let output: CommandOutput =
+                    CommandOutput::from_output(output, self.stdout, self.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
                 // stdout/stderr, and we thus don't have anything captured to print anyway.
-                if stdout.captures() {
+                if self.stdout.captures() {
                     writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap();
                 }
-                if stderr.captures() {
+                if self.stderr.captures() {
                     writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap();
                 }
                 output
@@ -145,52 +216,26 @@ Executed at: {executed_at}"#,
             Err(e) => {
                 writeln!(
                     message,
-                    "\n\nCommand {command:?} did not execute successfully.\
-            \nIt was not possible to execute the command: {e:?}"
+                    "\n\nCommand {:?} did not execute successfully.\
+            \nIt was not possible to execute the command: {e:?}",
+                    self.command
                 )
                 .unwrap();
-                CommandOutput::did_not_start(stdout, stderr)
-            }
-        };
-
-        let fail = |message: &str, output: CommandOutput| -> ! {
-            if self.is_verbose() {
-                println!("{message}");
-            } else {
-                let (stdout, stderr) = (output.stdout_if_present(), output.stderr_if_present());
-                // If the command captures output, the user would not see any indication that
-                // it has failed. In this case, print a more verbose error, since to provide more
-                // context.
-                if stdout.is_some() || stderr.is_some() {
-                    if let Some(stdout) =
-                        output.stdout_if_present().take_if(|s| !s.trim().is_empty())
-                    {
-                        println!("STDOUT:\n{stdout}\n");
-                    }
-                    if let Some(stderr) =
-                        output.stderr_if_present().take_if(|s| !s.trim().is_empty())
-                    {
-                        println!("STDERR:\n{stderr}\n");
-                    }
-                    println!("Command {command:?} has failed. Rerun with -v to see more details.");
-                } else {
-                    println!("Command has failed. Rerun with -v to see more details.");
-                }
+                CommandOutput::did_not_start(self.stdout, self.stderr)
             }
-            exit!(1);
         };
 
         if !output.is_success() {
-            match command.failure_behavior {
+            match self.command.failure_behavior {
                 BehaviorOnFailure::DelayFail => {
-                    if self.fail_fast {
-                        fail(&message, output);
+                    if exec_ctx.fail_fast {
+                        exec_ctx.fail(&message, output);
                     }
 
-                    self.add_to_delay_failure(message);
+                    exec_ctx.add_to_delay_failure(message);
                 }
                 BehaviorOnFailure::Exit => {
-                    fail(&message, output);
+                    exec_ctx.fail(&message, output);
                 }
                 BehaviorOnFailure::Ignore => {
                     // If failures are allowed, either the error has been printed already

src/bootstrap/src/utils/helpers.rs

@@ -5,13 +5,11 @@
 
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
 use std::sync::OnceLock;
 use std::thread::panicking;
 use std::time::{Instant, SystemTime, UNIX_EPOCH};
 use std::{env, fs, io, panic, str};
 
-use build_helper::util::fail;
 use object::read::archive::ArchiveFile;
 
 use crate::LldMode;
@@ -282,33 +280,6 @@ pub fn make(host: &str) -> PathBuf {
     }
 }
 
-/// Spawn a process and return a closure that will wait for the process
-/// to finish and then return its output. This allows the spawned process
-/// to do work without immediately blocking bootstrap.
-#[track_caller]
-pub fn start_process(cmd: &mut Command) -> impl FnOnce() -> String + use<> {
-    let child = match cmd.stderr(Stdio::inherit()).stdout(Stdio::piped()).spawn() {
-        Ok(child) => child,
-        Err(e) => fail(&format!("failed to execute command: {cmd:?}\nERROR: {e}")),
-    };
-
-    let command = format!("{cmd:?}");
-
-    move || {
-        let output = child.wait_with_output().unwrap();
-
-        if !output.status.success() {
-            panic!(
-                "command did not execute successfully: {}\n\
-                 expected success, got: {}",
-                command, output.status
-            );
-        }
-
-        String::from_utf8(output.stdout).unwrap()
-    }
-}
-
 /// Returns the last-modified time for `path`, or zero if it doesn't exist.
 pub fn mtime(path: &Path) -> SystemTime {
     fs::metadata(path).and_then(|f| f.modified()).unwrap_or(UNIX_EPOCH)