diff --git a/src/bootstrap/src/utils/channel.rs b/src/bootstrap/src/utils/channel.rs index 38f250af42f08..b28ab57377408 100644 --- a/src/bootstrap/src/utils/channel.rs +++ b/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(), })) } diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index f297300e34a85..eb9802bf2e1ba 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/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>(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, + ) -> 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, + ) -> 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 { diff --git a/src/bootstrap/src/utils/execution_context.rs b/src/bootstrap/src/utils/execution_context.rs index a5e1e9bcc07df..5b9fef3f8248b 100644 --- a/src/bootstrap/src/utils/execution_context.rs +++ b/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,92 +108,149 @@ impl ExecutionContext { cmd.stdout(stdout.stdio()); cmd.stderr(stderr.stdio()); - let output = cmd.output(); + let child = cmd.spawn(); - use std::fmt::Write; + DeferredCommand { process: Some(child), stdout, stderr, command, executed_at } + } - 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) + /// 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."); } - // Command has started, but then it failed - Ok(output) => { - writeln!( - message, - r#" -Command {command:?} did not execute successfully. + } + exit!(1); + } +} + +impl AsRef for ExecutionContext { + fn as_ref(&self) -> &ExecutionContext { + self + } +} + +pub struct DeferredCommand<'a> { + process: Option>, + 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) -> CommandOutput { + let exec_ctx = exec_ctx.as_ref(); + + let process = match self.process.take() { + Some(p) => p, + None => return CommandOutput::default(), + }; + + let created_at = self.command.get_created_location(); + let executed_at = self.executed_at; + + let mut message = String::new(); + + let output = match process { + Ok(child) => match child.wait_with_output() { + Ok(result) if result.status.success() => { + // Successful execution + CommandOutput::from_output(result, self.stdout, self.stderr) + } + Ok(result) => { + // Command ran but failed + use std::fmt::Write; + + writeln!( + message, + r#" +Command {:?} did not execute successfully. Expected success, got {} Created at: {created_at} Executed at: {executed_at}"#, - output.status, - ) - .unwrap(); + self.command, result.status, + ) + .unwrap(); + + let output = CommandOutput::from_output(result, self.stdout, self.stderr); - let output: CommandOutput = CommandOutput::from_output(output, stdout, stderr); + if self.stdout.captures() { + writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap(); + } + if self.stderr.captures() { + writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap(); + } - // 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() { - writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap(); + output } - if stderr.captures() { - writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap(); + Err(e) => { + // Failed to wait for output + use std::fmt::Write; + + writeln!( + message, + "\n\nCommand {:?} did not execute successfully.\ + \nIt was not possible to execute the command: {e:?}", + self.command + ) + .unwrap(); + + CommandOutput::did_not_start(self.stdout, self.stderr) } - output - } - // The command did not even start + }, Err(e) => { + // Failed to spawn the command + use std::fmt::Write; + 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 @@ -199,6 +259,7 @@ Executed at: {executed_at}"#, } } } + output } } diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index f4be22f1e649e..2f18fb6031829 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/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)