diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index c3a3eddd16128..431c242608b70 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -12,7 +12,6 @@ use std::ffi::OsStr; use std::io::BufReader; use std::io::prelude::*; use std::path::{Path, PathBuf}; -use std::process::Stdio; use std::{env, fs, str}; use serde_derive::Deserialize; @@ -2507,7 +2506,6 @@ pub fn stream_cargo( #[cfg(feature = "tracing")] let _run_span = crate::trace_cmd!(cmd); - let cargo = cmd.as_command_mut(); // Instruct Cargo to give us json messages on stdout, critically leaving // stderr as piped so we can get those pretty colors. let mut message_format = if builder.config.json_output { @@ -2519,27 +2517,24 @@ pub fn stream_cargo( message_format.push_str(",json-diagnostic-"); message_format.push_str(s); } - cargo.arg("--message-format").arg(message_format).stdout(Stdio::piped()); + cmd.arg("--message-format").arg(message_format); for arg in tail_args { - cargo.arg(arg); + cmd.arg(arg); } - builder.verbose(|| println!("running: {cargo:?}")); + builder.verbose(|| println!("running: {cmd:?}")); - if builder.config.dry_run() { - return true; - } + let streaming_command = cmd.stream_capture_stdout(&builder.config.exec_ctx); - let mut child = match cargo.spawn() { - Ok(child) => child, - Err(e) => panic!("failed to execute command: {cargo:?}\nERROR: {e}"), + let Some(mut streaming_command) = streaming_command else { + return true; }; // Spawn Cargo slurping up its JSON output. We'll start building up the // `deps` array of all files it generated along with a `toplevel` array of // files we need to probe for later. - let stdout = BufReader::new(child.stdout.take().unwrap()); + let stdout = BufReader::new(streaming_command.stdout.take().unwrap()); for line in stdout.lines() { let line = t!(line); match serde_json::from_str::>(&line) { @@ -2556,13 +2551,14 @@ pub fn stream_cargo( } // Make sure Cargo actually succeeded after we read all of its stdout. - let status = t!(child.wait()); + let status = t!(streaming_command.wait()); if builder.is_verbose() && !status.success() { eprintln!( - "command did not execute successfully: {cargo:?}\n\ + "command did not execute successfully: {cmd:?}\n\ expected success, got: {status}" ); } + status.success() } diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index d092765ef762f..487077835ac6c 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -13,7 +13,9 @@ use std::fmt::{Debug, Formatter}; use std::hash::Hash; use std::panic::Location; use std::path::Path; -use std::process::{Child, Command, CommandArgs, CommandEnvs, ExitStatus, Output, Stdio}; +use std::process::{ + Child, ChildStderr, ChildStdout, Command, CommandArgs, CommandEnvs, ExitStatus, Output, Stdio, +}; use std::sync::{Arc, Mutex}; use build_helper::ci::CiEnv; @@ -209,15 +211,14 @@ impl<'a> BootstrapCommand { 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 { - // We proactively mark this command as executed since we can't be certain how the returned - // command will be handled. Caching must also be avoided here, as the inner command could be - // modified externally without us being aware. - self.mark_as_executed(); - self.do_not_cache(); - &mut self.command + /// Spawn the command in background, while capturing and returning stdout, and printing stderr. + /// Returns None in dry-mode + #[track_caller] + pub fn stream_capture_stdout( + &'a mut self, + exec_ctx: impl AsRef, + ) -> Option { + exec_ctx.as_ref().stream(self, OutputMode::Capture, OutputMode::Print) } /// Mark the command as being executed, disarming the drop bomb. @@ -449,6 +450,12 @@ enum CommandState<'a> { }, } +pub struct StreamingCommand { + child: Child, + pub stdout: Option, + pub stderr: Option, +} + #[must_use] pub struct DeferredCommand<'a> { state: CommandState<'a>, @@ -617,6 +624,33 @@ impl ExecutionContext { } exit!(1); } + + /// Spawns the command with configured stdout and stderr handling. + /// + /// Returns None if in dry-run mode or Panics if the command fails to spawn. + pub fn stream( + &self, + command: &mut BootstrapCommand, + stdout: OutputMode, + stderr: OutputMode, + ) -> Option { + command.mark_as_executed(); + if !command.run_in_dry_run && self.dry_run() { + return None; + } + let cmd = &mut command.command; + cmd.stdout(stdout.stdio()); + cmd.stderr(stderr.stdio()); + let child = cmd.spawn(); + let mut child = match child { + Ok(child) => child, + Err(e) => panic!("failed to execute command: {cmd:?}\nERROR: {e}"), + }; + + let stdout = child.stdout.take(); + let stderr = child.stderr.take(); + Some(StreamingCommand { child, stdout, stderr }) + } } impl AsRef for ExecutionContext { @@ -625,6 +659,12 @@ impl AsRef for ExecutionContext { } } +impl StreamingCommand { + pub fn wait(mut self) -> Result { + self.child.wait() + } +} + impl<'a> DeferredCommand<'a> { pub fn wait_for_output(self, exec_ctx: impl AsRef) -> CommandOutput { match self.state { diff --git a/src/bootstrap/src/utils/render_tests.rs b/src/bootstrap/src/utils/render_tests.rs index 051d7dd9fd4dd..934699d736b46 100644 --- a/src/bootstrap/src/utils/render_tests.rs +++ b/src/bootstrap/src/utils/render_tests.rs @@ -7,7 +7,7 @@ //! to reimplement all the rendering logic in this module because of that. use std::io::{BufRead, BufReader, Read, Write}; -use std::process::{ChildStdout, Stdio}; +use std::process::ChildStdout; use std::time::Duration; use termcolor::{Color, ColorSpec, WriteColor}; @@ -34,50 +34,44 @@ pub(crate) fn try_run_tests( cmd: &mut BootstrapCommand, stream: bool, ) -> bool { - if builder.config.dry_run() { - cmd.mark_as_executed(); + if run_tests(builder, cmd, stream) { return true; } - if !run_tests(builder, cmd, stream) { - if builder.fail_fast { - crate::exit!(1); - } else { - builder.config.exec_ctx().add_to_delay_failure(format!("{cmd:?}")); - false - } - } else { - true + if builder.fail_fast { + crate::exit!(1); } + + builder.config.exec_ctx().add_to_delay_failure(format!("{cmd:?}")); + + false } fn run_tests(builder: &Builder<'_>, cmd: &mut BootstrapCommand, stream: bool) -> bool { - let cmd = cmd.as_command_mut(); - cmd.stdout(Stdio::piped()); - builder.verbose(|| println!("running: {cmd:?}")); - let mut process = cmd.spawn().unwrap(); + let Some(mut streaming_command) = cmd.stream_capture_stdout(&builder.config.exec_ctx) else { + return true; + }; // This runs until the stdout of the child is closed, which means the child exited. We don't // run this on another thread since the builder is not Sync. - let renderer = Renderer::new(process.stdout.take().unwrap(), builder); + let renderer = Renderer::new(streaming_command.stdout.take().unwrap(), builder); if stream { renderer.stream_all(); } else { renderer.render_all(); } - let result = process.wait_with_output().unwrap(); - if !result.status.success() && builder.is_verbose() { + let status = streaming_command.wait().unwrap(); + if !status.success() && builder.is_verbose() { println!( "\n\ncommand did not execute successfully: {cmd:?}\n\ - expected success, got: {}", - result.status + expected success, got: {status}", ); } - result.status.success() + status.success() } struct Renderer<'a> {