From 3ab5e60d18a7f6ed016974cced19e9f372517976 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 8 Jun 2025 11:58:19 +0530 Subject: [PATCH 01/14] Add execution context --- src/bootstrap/src/utils/exec.rs | 26 ++- src/bootstrap/src/utils/execution_context.rs | 198 +++++++++++++++++++ 2 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 src/bootstrap/src/utils/execution_context.rs diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 64e46f105638f..c03fd2772ad89 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -11,6 +11,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 crate::Build; /// What should be done when the command fails. @@ -125,7 +126,6 @@ impl BootstrapCommand { Self { failure_behavior: BehaviorOnFailure::DelayFail, ..self } } - #[expect(dead_code)] pub fn fail_fast(self) -> Self { Self { failure_behavior: BehaviorOnFailure::Exit, ..self } } @@ -140,6 +140,29 @@ impl BootstrapCommand { self } + #[track_caller] + pub fn run_exec_ctx(&mut self, exec_ctx: impl AsRef) -> bool { + exec_ctx.as_ref().run(self, OutputMode::Print, OutputMode::Print).is_success() + } + + /// Run the command, while capturing and returning all its output. + #[track_caller] + pub fn run_capture_exec_ctx( + &mut self, + exec_ctx: impl AsRef, + ) -> CommandOutput { + exec_ctx.as_ref().run(self, OutputMode::Capture, OutputMode::Capture) + } + + /// Run the command, while capturing and returning stdout, and printing stderr. + #[track_caller] + pub fn run_capture_stdout_exec_ctx( + &mut self, + exec_ctx: impl AsRef, + ) -> CommandOutput { + exec_ctx.as_ref().run(self, OutputMode::Capture, OutputMode::Print) + } + /// Run the command, while printing stdout and stderr. /// Returns true if the command has succeeded. #[track_caller] @@ -280,7 +303,6 @@ impl CommandOutput { !self.is_success() } - #[expect(dead_code)] pub fn status(&self) -> Option { match self.status { CommandStatus::Finished(status) => Some(status), diff --git a/src/bootstrap/src/utils/execution_context.rs b/src/bootstrap/src/utils/execution_context.rs new file mode 100644 index 0000000000000..17af98babb496 --- /dev/null +++ b/src/bootstrap/src/utils/execution_context.rs @@ -0,0 +1,198 @@ +//! Shared execution context for running bootstrap commands. +//! +//! 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::sync::{Arc, Mutex}; + +use crate::core::config::DryRun; +use crate::{BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode, exit}; + +#[derive(Clone, Default)] +pub struct ExecutionContext { + dry_run: DryRun, + verbose: u8, + pub fail_fast: bool, + delayed_failures: Arc>>, +} + +impl ExecutionContext { + pub fn new() -> Self { + ExecutionContext::default() + } + + pub fn dry_run(&self) -> bool { + match self.dry_run { + DryRun::Disabled => false, + DryRun::SelfCheck | DryRun::UserSelected => true, + } + } + + pub fn verbose(&self, f: impl Fn()) { + if self.is_verbose() { + f() + } + } + + pub fn is_verbose(&self) -> bool { + self.verbose > 0 + } + + pub fn fail_fast(&self) -> bool { + self.fail_fast + } + + pub fn set_dry_run(&mut self, value: DryRun) { + self.dry_run = value; + } + + pub fn set_verbose(&mut self, value: u8) { + self.verbose = value; + } + + pub fn set_fail_fast(&mut self, value: bool) { + self.fail_fast = value; + } + + pub fn add_to_delay_failure(&self, message: String) { + self.delayed_failures.lock().unwrap().push(message); + } + + pub fn report_failures_and_exit(&self) { + let failures = self.delayed_failures.lock().unwrap(); + if failures.is_empty() { + return; + } + eprintln!("\n{} command(s) did not execute successfully:\n", failures.len()); + for failure in &*failures { + eprintln!(" - {failure}"); + } + exit!(1); + } + + /// 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 { + command.mark_as_executed(); + if self.dry_run() && !command.run_always { + return CommandOutput::default(); + } + + #[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})") + }); + + let cmd = command.as_command_mut(); + cmd.stdout(stdout.stdio()); + cmd.stderr(stderr.stdio()); + + let output = cmd.output(); + + 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) + } + // Command has started, but then it failed + Ok(output) => { + writeln!( + message, + r#" +Command {command:?} did not execute successfully. +Expected success, got {} +Created at: {created_at} +Executed at: {executed_at}"#, + output.status, + ) + .unwrap(); + + 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 + // stdout/stderr, and we thus don't have anything captured to print anyway. + if stdout.captures() { + writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap(); + } + if stderr.captures() { + writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap(); + } + output + } + // The command did not even start + Err(e) => { + writeln!( + message, + "\n\nCommand {command:?} did not execute successfully.\ + \nIt was not possible to execute the command: {e:?}" + ) + .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."); + } + } + exit!(1); + }; + + if !output.is_success() { + match command.failure_behavior { + BehaviorOnFailure::DelayFail => { + if self.fail_fast { + fail(&message, output); + } + + self.add_to_delay_failure(message); + } + BehaviorOnFailure::Exit => { + fail(&message, output); + } + BehaviorOnFailure::Ignore => { + // If failures are allowed, either the error has been printed already + // (OutputMode::Print) or the user used a capture output mode and wants to + // handle the error output on their own. + } + } + } + output + } +} From 81ee86b23b356aca3d131fa6271b19d55f9751c3 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 8 Jun 2025 12:02:58 +0530 Subject: [PATCH 02/14] add execution context to bootstrap workflow --- src/bootstrap/src/bin/main.rs | 4 ++-- src/bootstrap/src/core/builder/mod.rs | 11 +++++++++++ src/bootstrap/src/core/config/config.rs | 23 +++++++++++++++++++++-- src/bootstrap/src/core/config/flags.rs | 15 +++++++++++---- src/bootstrap/src/utils/mod.rs | 1 + 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/src/bin/main.rs b/src/bootstrap/src/bin/main.rs index 833f80279517a..9f1fde74359c7 100644 --- a/src/bootstrap/src/bin/main.rs +++ b/src/bootstrap/src/bin/main.rs @@ -29,9 +29,9 @@ fn main() { } debug!("parsing flags"); - let flags = Flags::parse(&args); + let (flags, exec_ctx) = Flags::parse(&args); debug!("parsing config based on flags"); - let config = Config::parse(flags); + let config = Config::parse(flags, exec_ctx); let mut build_lock; let _build_lock_guard; diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index 19b79bfe818c2..e8686c75e02f6 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -22,6 +22,7 @@ use crate::core::config::flags::Subcommand; use crate::core::config::{DryRun, TargetSelection}; use crate::utils::cache::Cache; use crate::utils::exec::{BootstrapCommand, command}; +use crate::utils::execution_context::ExecutionContext; use crate::utils::helpers::{self, LldThreads, add_dylib_path, exe, libdir, linker_args, t}; use crate::{Build, Crate, trace}; @@ -1633,4 +1634,14 @@ impl<'a> Builder<'a> { self.info(&format!("{err}\n")); } } + + pub fn exec_ctx(&self) -> &ExecutionContext { + &self.config.exec_ctx + } +} + +impl<'a> AsRef for Builder<'a> { + fn as_ref(&self) -> &ExecutionContext { + self.exec_ctx() + } } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index a92d58ef9e87e..d7e6107defaa3 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -47,6 +47,7 @@ use crate::core::config::{ }; use crate::core::download::is_download_ci_available; use crate::utils::channel; +use crate::utils::execution_context::ExecutionContext; use crate::utils::helpers::exe; use crate::{Command, GitInfo, OnceLock, TargetSelection, check_ci_llvm, helpers, output, t}; @@ -304,6 +305,8 @@ pub struct Config { /// This is mostly for RA as building the stage1 compiler to check the library tree /// on each code change might be too much for some computers. pub skip_std_check_if_no_download_rustc: bool, + + pub exec_ctx: ExecutionContext, } impl Config { @@ -364,8 +367,8 @@ impl Config { feature = "tracing", instrument(target = "CONFIG_HANDLING", level = "trace", name = "Config::parse", skip_all) )] - pub fn parse(flags: Flags) -> Config { - Self::parse_inner(flags, Self::get_toml) + pub fn parse(flags: Flags, exec_ctx: ExecutionContext) -> Config { + Self::parse_inner(flags, Self::get_toml, exec_ctx) } #[cfg_attr( @@ -380,8 +383,10 @@ impl Config { pub(crate) fn parse_inner( mut flags: Flags, get_toml: impl Fn(&Path) -> Result, + exec_ctx: ExecutionContext, ) -> Config { let mut config = Config::default_opts(); + config.exec_ctx = exec_ctx; // Set flags. config.paths = std::mem::take(&mut flags.paths); @@ -1741,4 +1746,18 @@ impl Config { _ => !self.is_system_llvm(target), } } + + pub fn exec_ctx(&self) -> &ExecutionContext { + &self.exec_ctx + } + + pub fn git_info(&self, omit_git_hash: bool, dir: &Path) -> GitInfo { + GitInfo::new(omit_git_hash, dir, self) + } +} + +impl AsRef for Config { + fn as_ref(&self) -> &ExecutionContext { + &self.exec_ctx + } } diff --git a/src/bootstrap/src/core/config/flags.rs b/src/bootstrap/src/core/config/flags.rs index 30617f58d4306..a50ff4caaf463 100644 --- a/src/bootstrap/src/core/config/flags.rs +++ b/src/bootstrap/src/core/config/flags.rs @@ -14,7 +14,8 @@ use crate::core::build_steps::setup::Profile; use crate::core::builder::{Builder, Kind}; use crate::core::config::Config; use crate::core::config::target_selection::{TargetSelectionList, target_selection_list}; -use crate::{Build, DocTests}; +use crate::utils::execution_context::ExecutionContext; +use crate::{Build, DocTests, DryRun}; #[derive(Copy, Clone, Default, Debug, ValueEnum)] pub enum Color { @@ -209,7 +210,8 @@ impl Flags { HelpVerboseOnly::try_parse_from(normalize_args(args)) { println!("NOTE: updating submodules before printing available paths"); - let config = Config::parse(Self::parse(&[String::from("build")])); + let (flags, exec_ctx) = Self::parse(&[String::from("build")]); + let config = Config::parse(flags, exec_ctx); let build = Build::new(config); let paths = Builder::get_help(&build, subcommand); if let Some(s) = paths { @@ -227,8 +229,13 @@ impl Flags { feature = "tracing", instrument(level = "trace", name = "Flags::parse", skip_all, fields(args = ?args)) )] - pub fn parse(args: &[String]) -> Self { - Flags::parse_from(normalize_args(args)) + pub fn parse(args: &[String]) -> (Self, ExecutionContext) { + let mut exec_ctx = ExecutionContext::new(); + let flags = Flags::parse_from(normalize_args(args)); + exec_ctx.set_dry_run(if flags.dry_run { DryRun::UserSelected } else { DryRun::Disabled }); + exec_ctx.set_verbose(flags.verbose); + exec_ctx.set_fail_fast(flags.cmd.fail_fast()); + (flags, exec_ctx) } } diff --git a/src/bootstrap/src/utils/mod.rs b/src/bootstrap/src/utils/mod.rs index 169fcec303e90..5a0b90801e73a 100644 --- a/src/bootstrap/src/utils/mod.rs +++ b/src/bootstrap/src/utils/mod.rs @@ -8,6 +8,7 @@ pub(crate) mod cc_detect; pub(crate) mod change_tracker; pub(crate) mod channel; pub(crate) mod exec; +pub(crate) mod execution_context; pub(crate) mod helpers; pub(crate) mod job; pub(crate) mod render_tests; From def44885ee91b059d60834dfd9afe667da888575 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 8 Jun 2025 12:03:45 +0530 Subject: [PATCH 03/14] move git command to new execution context --- src/bootstrap/src/core/build_steps/tool.rs | 3 +-- src/bootstrap/src/core/config/config.rs | 24 ++++++++++++---------- src/bootstrap/src/lib.rs | 13 +++++++++++- src/bootstrap/src/utils/channel.rs | 18 +++++++++++----- 4 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 9861637d8c822..717accb399adc 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -23,7 +23,6 @@ use crate::core::builder::{ Builder, Cargo as CargoCommand, RunConfig, ShouldRun, Step, cargo_profile_var, }; use crate::core::config::{DebuginfoLevel, RustcLto, TargetSelection}; -use crate::utils::channel::GitInfo; use crate::utils::exec::{BootstrapCommand, command}; use crate::utils::helpers::{add_dylib_path, exe, t}; use crate::{Compiler, FileType, Kind, Mode, gha}; @@ -278,7 +277,7 @@ pub fn prepare_tool_cargo( cargo.env("CFG_VER_DESCRIPTION", description); } - let info = GitInfo::new(builder.config.omit_git_hash, &dir); + let info = builder.config.git_info(builder.config.omit_git_hash, &dir); if let Some(sha) = info.sha() { cargo.env("CFG_COMMIT_HASH", sha); } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index d7e6107defaa3..ee068f2bf52f2 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -547,7 +547,7 @@ impl Config { build.cargo = build.cargo.take().or(std::env::var_os("CARGO").map(|p| p.into())); } - if GitInfo::new(false, &config.src).is_from_tarball() && toml.profile.is_none() { + if config.git_info(false, &config.src).is_from_tarball() && toml.profile.is_none() { toml.profile = Some("dist".into()); } @@ -850,19 +850,21 @@ impl Config { let default = config.channel == "dev"; config.omit_git_hash = toml.rust.as_ref().and_then(|r| r.omit_git_hash).unwrap_or(default); - config.rust_info = GitInfo::new(config.omit_git_hash, &config.src); - config.cargo_info = GitInfo::new(config.omit_git_hash, &config.src.join("src/tools/cargo")); + config.rust_info = config.git_info(config.omit_git_hash, &config.src); + config.cargo_info = + config.git_info(config.omit_git_hash, &config.src.join("src/tools/cargo")); config.rust_analyzer_info = - GitInfo::new(config.omit_git_hash, &config.src.join("src/tools/rust-analyzer")); + config.git_info(config.omit_git_hash, &config.src.join("src/tools/rust-analyzer")); config.clippy_info = - GitInfo::new(config.omit_git_hash, &config.src.join("src/tools/clippy")); - config.miri_info = GitInfo::new(config.omit_git_hash, &config.src.join("src/tools/miri")); + config.git_info(config.omit_git_hash, &config.src.join("src/tools/clippy")); + config.miri_info = + config.git_info(config.omit_git_hash, &config.src.join("src/tools/miri")); config.rustfmt_info = - GitInfo::new(config.omit_git_hash, &config.src.join("src/tools/rustfmt")); + config.git_info(config.omit_git_hash, &config.src.join("src/tools/rustfmt")); config.enzyme_info = - GitInfo::new(config.omit_git_hash, &config.src.join("src/tools/enzyme")); - config.in_tree_llvm_info = GitInfo::new(false, &config.src.join("src/llvm-project")); - config.in_tree_gcc_info = GitInfo::new(false, &config.src.join("src/gcc")); + config.git_info(config.omit_git_hash, &config.src.join("src/tools/enzyme")); + config.in_tree_llvm_info = config.git_info(false, &config.src.join("src/llvm-project")); + config.in_tree_gcc_info = config.git_info(false, &config.src.join("src/gcc")); config.vendor = vendor.unwrap_or( config.rust_info.is_from_tarball() @@ -1329,7 +1331,7 @@ impl Config { // NOTE: The check for the empty directory is here because when running x.py the first time, // the submodule won't be checked out. Check it out now so we can build it. - if !GitInfo::new(false, &absolute_path).is_managed_git_subrepository() + if !self.git_info(false, &absolute_path).is_managed_git_subrepository() && !helpers::dir_is_empty(&absolute_path) { return; diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 07772b8932d9d..9bace4eb77ce0 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -32,6 +32,7 @@ use cc::Tool; use termcolor::{ColorChoice, StandardStream, WriteColor}; use utils::build_stamp::BuildStamp; use utils::channel::GitInfo; +use utils::execution_context::ExecutionContext; use crate::core::builder; use crate::core::builder::Kind; @@ -616,7 +617,7 @@ impl Build { return; } - if GitInfo::new(false, Path::new(submodule)).is_managed_git_subrepository() { + if config.git_info(false, Path::new(submodule)).is_managed_git_subrepository() { config.update_submodule(submodule); } } @@ -2015,6 +2016,16 @@ to download LLVM rather than building it. stream.reset().unwrap(); result } + + pub fn exec_ctx(&self) -> &ExecutionContext { + &self.config.exec_ctx + } +} + +impl AsRef for Build { + fn as_ref(&self) -> &ExecutionContext { + &self.config.exec_ctx + } } #[cfg(unix)] diff --git a/src/bootstrap/src/utils/channel.rs b/src/bootstrap/src/utils/channel.rs index 4a9ecc7a4f8a1..333c04c3c2641 100644 --- a/src/bootstrap/src/utils/channel.rs +++ b/src/bootstrap/src/utils/channel.rs @@ -8,6 +8,7 @@ use std::fs; use std::path::Path; +use super::execution_context::ExecutionContext; use super::helpers; use crate::Build; use crate::utils::helpers::{start_process, t}; @@ -34,7 +35,7 @@ pub struct Info { } impl GitInfo { - pub fn new(omit_git_hash: bool, dir: &Path) -> GitInfo { + pub fn new(omit_git_hash: bool, dir: &Path, exec_ctx: impl AsRef) -> GitInfo { // See if this even begins to look like a git dir if !dir.join(".git").exists() { match read_commit_info_file(dir) { @@ -43,11 +44,18 @@ impl GitInfo { } } - // Make sure git commands work - match helpers::git(Some(dir)).arg("rev-parse").as_command_mut().output() { - Ok(ref out) if out.status.success() => {} - _ => return GitInfo::Absent, + let mut git_command = helpers::git(Some(dir)); + git_command.arg("rev-parse"); + let output = git_command.allow_failure().run_capture_stdout_exec_ctx(exec_ctx); + + if output.is_failure() { + return GitInfo::Absent; } + // Make sure git commands work + // match helpers::git(Some(dir)).arg("rev-parse").as_command_mut().output() { + // Ok(ref out) if out.status.success() => {} + // _ => return GitInfo::Absent, + // } // If we're ignoring the git info, we don't actually need to collect it, just make sure this // was a git repo in the first place. From 8bd81699d977657e9571758e413e7cf1595626d4 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 6 Jun 2025 21:16:47 +0530 Subject: [PATCH 04/14] moved render_tests in utils to new execution context --- src/bootstrap/src/utils/render_tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bootstrap/src/utils/render_tests.rs b/src/bootstrap/src/utils/render_tests.rs index 418f3ff975d94..77e645a9e3cb8 100644 --- a/src/bootstrap/src/utils/render_tests.rs +++ b/src/bootstrap/src/utils/render_tests.rs @@ -43,8 +43,7 @@ pub(crate) fn try_run_tests( if builder.fail_fast { crate::exit!(1); } else { - let mut failures = builder.delayed_failures.borrow_mut(); - failures.push(format!("{cmd:?}")); + builder.config.exec_ctx().add_to_delay_failure(format!("{cmd:?}")); false } } else { From 81abbe1f12e417275aa96ad1deb146d20e1b673d Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 6 Jun 2025 22:19:31 +0530 Subject: [PATCH 05/14] moved sanity command to exec context --- src/bootstrap/src/core/sanity.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index af4ec679d080d..4ef9316d00b68 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -361,8 +361,11 @@ 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").arg("--help").run_always().run_capture_stdout(build).stdout(); + let out = command("cmake") + .arg("--help") + .run_always() + .run_capture_stdout_exec_ctx(&build) + .stdout(); if !out.contains("Visual Studio") { panic!( " From 2d1ca83ab58c2510350975dc219faa6a80885c0f Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 6 Jun 2025 22:36:48 +0530 Subject: [PATCH 06/14] moved curl to use new execution_context --- src/bootstrap/src/core/download.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index ba00b405c61d3..ccb03ad21e589 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -22,8 +22,7 @@ fn try_run(config: &Config, cmd: &mut Command) -> Result<(), ()> { config.try_run(cmd) } -fn extract_curl_version(out: &[u8]) -> semver::Version { - let out = String::from_utf8_lossy(out); +fn extract_curl_version(out: String) -> semver::Version { // The output should look like this: "curl .. ..." out.lines() .next() @@ -32,12 +31,15 @@ fn extract_curl_version(out: &[u8]) -> semver::Version { .unwrap_or(semver::Version::new(1, 0, 0)) } -fn curl_version() -> semver::Version { - let mut curl = Command::new("curl"); +fn curl_version(config: &Config) -> semver::Version { + let mut curl = command("curl"); curl.arg("-V"); - let Ok(out) = curl.output() else { return semver::Version::new(1, 0, 0) }; - let out = out.stdout; - extract_curl_version(&out) + let curl = curl.run_capture_stdout_exec_ctx(config); + if curl.is_failure() { + return semver::Version::new(1, 0, 0); + } + let output = curl.stdout(); + extract_curl_version(output) } /// Generic helpers that are useful anywhere in bootstrap. @@ -267,7 +269,7 @@ impl Config { curl.arg("--progress-bar"); } // --retry-all-errors was added in 7.71.0, don't use it if curl is old. - if curl_version() >= semver::Version::new(7, 71, 0) { + if curl_version(self) >= semver::Version::new(7, 71, 0) { curl.arg("--retry-all-errors"); } curl.arg(url); From d544c2b8d005c6f5de6796f6a9c9dd3407153c59 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 7 Jun 2025 00:23:44 +0530 Subject: [PATCH 07/14] covert uname to new extext method --- src/bootstrap/src/core/download.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index ccb03ad21e589..8d75fb48ccee6 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -3,7 +3,7 @@ use std::ffi::OsString; use std::fs::{self, File}; use std::io::{BufRead, BufReader, BufWriter, ErrorKind, Write}; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Command; use std::sync::OnceLock; use xz2::bufread::XzDecoder; @@ -87,20 +87,14 @@ impl Config { /// on NixOS fn should_fix_bins_and_dylibs(&self) -> bool { let val = *SHOULD_FIX_BINS_AND_DYLIBS.get_or_init(|| { - match Command::new("uname").arg("-s").stderr(Stdio::inherit()).output() { - Err(_) => return false, - Ok(output) if !output.status.success() => return false, - Ok(output) => { - let mut os_name = output.stdout; - if os_name.last() == Some(&b'\n') { - os_name.pop(); - } - if os_name != b"Linux" { - return false; - } - } + let uname = command("uname").arg("-s").run_capture_stdout_exec_ctx(self); + if uname.is_failure() { + return false; + } + let output = uname.stdout(); + if !output.starts_with("Linux") { + return false; } - // If the user has asked binaries to be patched for Nix, then // don't check for NixOS or `/lib`. // NOTE: this intentionally comes after the Linux check: From bae39b8f10b0ee748c6420ba4ab71662e3c17b09 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 7 Jun 2025 00:42:19 +0530 Subject: [PATCH 08/14] move all download.rs method to new execution context command invocation --- src/bootstrap/src/core/config/config.rs | 12 --------- src/bootstrap/src/core/download.rs | 35 +++++++++---------------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index ee068f2bf52f2..cb905a099ba88 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1034,18 +1034,6 @@ impl Config { self.explicit_stage_from_cli || self.explicit_stage_from_config } - /// Runs a command, printing out nice contextual information if it fails. - /// Exits if the command failed to execute at all, otherwise returns its - /// `status.success()`. - #[deprecated = "use `Builder::try_run` instead where possible"] - pub(crate) fn try_run(&self, cmd: &mut Command) -> Result<(), ()> { - if self.dry_run() { - return Ok(()); - } - self.verbose(|| println!("running: {cmd:?}")); - build_helper::util::try_run(cmd, self.is_verbose()) - } - pub(crate) fn test_args(&self) -> Vec<&str> { let mut test_args = match self.cmd { Subcommand::Test { ref test_args, .. } diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 8d75fb48ccee6..48604023c97af 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -3,7 +3,6 @@ use std::ffi::OsString; use std::fs::{self, File}; use std::io::{BufRead, BufReader, BufWriter, ErrorKind, Write}; use std::path::{Path, PathBuf}; -use std::process::Command; use std::sync::OnceLock; use xz2::bufread::XzDecoder; @@ -16,12 +15,6 @@ use crate::{Config, t}; static SHOULD_FIX_BINS_AND_DYLIBS: OnceLock = OnceLock::new(); -/// `Config::try_run` wrapper for this module to avoid warnings on `try_run`, since we don't have access to a `builder` yet. -fn try_run(config: &Config, cmd: &mut Command) -> Result<(), ()> { - #[expect(deprecated)] - config.try_run(cmd) -} - fn extract_curl_version(out: String) -> semver::Version { // The output should look like this: "curl .. ..." out.lines() @@ -169,23 +162,18 @@ impl Config { ]; } "; - nix_build_succeeded = try_run( - self, - Command::new("nix-build").args([ - Path::new("-E"), - Path::new(NIX_EXPR), - Path::new("-o"), - &nix_deps_dir, - ]), - ) - .is_ok(); + nix_build_succeeded = command("nix-build") + .allow_failure() + .args([Path::new("-E"), Path::new(NIX_EXPR), Path::new("-o"), &nix_deps_dir]) + .run_capture_stdout_exec_ctx(self) + .is_success(); nix_deps_dir }); if !nix_build_succeeded { return; } - let mut patchelf = Command::new(nix_deps_dir.join("bin/patchelf")); + let mut patchelf = command(nix_deps_dir.join("bin/patchelf")); patchelf.args(&[ OsString::from("--add-rpath"), OsString::from(t!(fs::canonicalize(nix_deps_dir)).join("lib")), @@ -196,8 +184,8 @@ impl Config { let dynamic_linker = t!(fs::read_to_string(dynamic_linker_path)); patchelf.args(["--set-interpreter", dynamic_linker.trim_end()]); } - - let _ = try_run(self, patchelf.arg(fname)); + patchelf.arg(fname); + let _ = patchelf.run_capture_stdout_exec_ctx(self); } fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) { @@ -271,7 +259,7 @@ impl Config { if self.build.contains("windows-msvc") { eprintln!("Fallback to PowerShell"); for _ in 0..3 { - if try_run(self, Command::new("PowerShell.exe").args([ + let powershell = command("PowerShell.exe").args([ "/nologo", "-Command", "[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12;", @@ -279,9 +267,12 @@ impl Config { "(New-Object System.Net.WebClient).DownloadFile('{}', '{}')", url, tempfile.to_str().expect("invalid UTF-8 not supported with powershell downloads"), ), - ])).is_err() { + ]).run_capture_stdout_exec_ctx(self); + + if powershell.is_failure() { return; } + eprintln!("\nspurious failure, trying again"); } } From 746276cfb26a3a8ceab2fed795ab76b1989f9650 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 7 Jun 2025 07:46:38 +0530 Subject: [PATCH 09/14] moved git command to new exec context --- src/bootstrap/src/core/config/config.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index cb905a099ba88..c9f37332a285e 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -445,14 +445,9 @@ impl Config { // has already been (kinda-cross-)compiled to Windows land, we require a normal Windows path. cmd.arg("rev-parse").arg("--show-cdup"); // Discard stderr because we expect this to fail when building from a tarball. - let output = cmd - .as_command_mut() - .stderr(std::process::Stdio::null()) - .output() - .ok() - .and_then(|output| if output.status.success() { Some(output) } else { None }); - if let Some(output) = output { - let git_root_relative = String::from_utf8(output.stdout).unwrap(); + let output = cmd.run_capture_stdout_exec_ctx(&config); + if output.is_success() { + let git_root_relative = output.stdout(); // We need to canonicalize this path to make sure it uses backslashes instead of forward slashes, // and to resolve any relative components. let git_root = env::current_dir() From 98be2a0498529db0f40628bf9ecef0ae833d937d Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 7 Jun 2025 09:40:48 +0530 Subject: [PATCH 10/14] move all config command invocation to new execution context invocation --- src/bootstrap/src/core/config/config.rs | 55 ++++++++++++++----------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index c9f37332a285e..e37101381ddb7 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -24,7 +24,7 @@ use std::{cmp, env, fs}; use build_helper::ci::CiEnv; use build_helper::exit; -use build_helper::git::{GitConfig, PathFreshness, check_path_modifications, output_result}; +use build_helper::git::{GitConfig, PathFreshness, check_path_modifications}; use serde::Deserialize; #[cfg(feature = "tracing")] use tracing::{instrument, span}; @@ -47,9 +47,10 @@ use crate::core::config::{ }; use crate::core::download::is_download_ci_available; use crate::utils::channel; +use crate::utils::exec::command; use crate::utils::execution_context::ExecutionContext; use crate::utils::helpers::exe; -use crate::{Command, GitInfo, OnceLock, TargetSelection, check_ci_llvm, helpers, output, t}; +use crate::{GitInfo, OnceLock, TargetSelection, check_ci_llvm, helpers, t}; /// Each path in this list is considered "allowed" in the `download-rustc="if-unchanged"` logic. /// This means they can be modified and changes to these paths should never trigger a compiler build @@ -445,7 +446,7 @@ impl Config { // has already been (kinda-cross-)compiled to Windows land, we require a normal Windows path. cmd.arg("rev-parse").arg("--show-cdup"); // Discard stderr because we expect this to fail when building from a tarball. - let output = cmd.run_capture_stdout_exec_ctx(&config); + let output = cmd.allow_failure().run_capture_stdout_exec_ctx(&config); if output.is_success() { let git_root_relative = output.stdout(); // We need to canonicalize this path to make sure it uses backslashes instead of forward slashes, @@ -749,7 +750,12 @@ impl Config { }; config.initial_sysroot = t!(PathBuf::from_str( - output(Command::new(&config.initial_rustc).args(["--print", "sysroot"])).trim() + command(&config.initial_rustc) + .args(["--print", "sysroot"]) + .run_always() + .run_capture_stdout_exec_ctx(&config) + .stdout() + .trim() )); config.initial_cargo_clippy = cargo_clippy; @@ -1062,7 +1068,7 @@ impl Config { let mut git = helpers::git(Some(&self.src)); git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap())); - output(git.as_command_mut()) + git.allow_failure().run_capture_stdout_exec_ctx(self).stdout() } /// Bootstrap embeds a version number into the name of shared libraries it uploads in CI. @@ -1333,16 +1339,20 @@ impl Config { }; // Determine commit checked out in submodule. - let checked_out_hash = output(submodule_git().args(["rev-parse", "HEAD"]).as_command_mut()); + let checked_out_hash = submodule_git() + .allow_failure() + .args(["rev-parse", "HEAD"]) + .run_capture_stdout_exec_ctx(self) + .stdout(); let checked_out_hash = checked_out_hash.trim_end(); // Determine commit that the submodule *should* have. - let recorded = output( - helpers::git(Some(&self.src)) - .run_always() - .args(["ls-tree", "HEAD"]) - .arg(relative_path) - .as_command_mut(), - ); + let recorded = helpers::git(Some(&self.src)) + .allow_failure() + .run_always() + .args(["ls-tree", "HEAD"]) + .arg(relative_path) + .run_capture_stdout_exec_ctx(self) + .stdout(); let actual_hash = recorded .split_whitespace() @@ -1366,20 +1376,18 @@ impl Config { let update = |progress: bool| { // 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 = output_result( - helpers::git(Some(&self.src)) - .allow_failure() - .run_always() - .args(["symbolic-ref", "--short", "HEAD"]) - .as_command_mut(), - ) - .map(|b| b.trim().to_owned()); + let current_branch = helpers::git(Some(&self.src)) + .allow_failure() + .run_always() + .args(["symbolic-ref", "--short", "HEAD"]) + .run_capture_exec_ctx(self); let mut git = helpers::git(Some(&self.src)).allow_failure(); git.run_always(); - if let Ok(branch) = current_branch { + if current_branch.is_success() { // If there is a tag named after the current branch, git will try to disambiguate by prepending `heads/` to the branch name. // This syntax isn't accepted by `branch.{branch}`. Strip it. + let branch = current_branch.stdout(); let branch = branch.strip_prefix("heads/").unwrap_or(&branch); git.arg("-c").arg(format!("branch.{branch}.remote=origin")); } @@ -1425,7 +1433,8 @@ impl Config { return; } - let stage0_output = output(Command::new(program_path).arg("--version")); + let stage0_output = + command(program_path).arg("--version").run_capture_stdout_exec_ctx(self).stdout(); let mut stage0_output = stage0_output.lines().next().unwrap().split(' '); let stage0_name = stage0_output.next().unwrap(); From 50725f325b32d3de2a0f159c7b90b21cfc91a2d3 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 7 Jun 2025 15:05:04 +0530 Subject: [PATCH 11/14] move all commands to new execution context --- src/bootstrap/src/core/config/config.rs | 14 +-- src/bootstrap/src/core/download.rs | 10 +- src/bootstrap/src/core/sanity.rs | 7 +- src/bootstrap/src/lib.rs | 127 ------------------------ src/bootstrap/src/utils/channel.rs | 2 +- src/bootstrap/src/utils/exec.rs | 34 +------ 6 files changed, 20 insertions(+), 174 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index e37101381ddb7..8b5e252d1d7bf 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -446,7 +446,7 @@ impl Config { // has already been (kinda-cross-)compiled to Windows land, we require a normal Windows path. cmd.arg("rev-parse").arg("--show-cdup"); // Discard stderr because we expect this to fail when building from a tarball. - let output = cmd.allow_failure().run_capture_stdout_exec_ctx(&config); + let output = cmd.allow_failure().run_capture_stdout(&config); if output.is_success() { let git_root_relative = output.stdout(); // We need to canonicalize this path to make sure it uses backslashes instead of forward slashes, @@ -753,7 +753,7 @@ impl Config { command(&config.initial_rustc) .args(["--print", "sysroot"]) .run_always() - .run_capture_stdout_exec_ctx(&config) + .run_capture_stdout(&config) .stdout() .trim() )); @@ -1068,7 +1068,7 @@ impl Config { let mut git = helpers::git(Some(&self.src)); git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap())); - git.allow_failure().run_capture_stdout_exec_ctx(self).stdout() + git.allow_failure().run_capture_stdout(self).stdout() } /// Bootstrap embeds a version number into the name of shared libraries it uploads in CI. @@ -1342,7 +1342,7 @@ impl Config { let checked_out_hash = submodule_git() .allow_failure() .args(["rev-parse", "HEAD"]) - .run_capture_stdout_exec_ctx(self) + .run_capture_stdout(self) .stdout(); let checked_out_hash = checked_out_hash.trim_end(); // Determine commit that the submodule *should* have. @@ -1351,7 +1351,7 @@ impl Config { .run_always() .args(["ls-tree", "HEAD"]) .arg(relative_path) - .run_capture_stdout_exec_ctx(self) + .run_capture_stdout(self) .stdout(); let actual_hash = recorded @@ -1380,7 +1380,7 @@ impl Config { .allow_failure() .run_always() .args(["symbolic-ref", "--short", "HEAD"]) - .run_capture_exec_ctx(self); + .run_capture(self); let mut git = helpers::git(Some(&self.src)).allow_failure(); git.run_always(); @@ -1434,7 +1434,7 @@ impl Config { } let stage0_output = - command(program_path).arg("--version").run_capture_stdout_exec_ctx(self).stdout(); + command(program_path).arg("--version").run_capture_stdout(self).stdout(); let mut stage0_output = stage0_output.lines().next().unwrap().split(' '); let stage0_name = stage0_output.next().unwrap(); diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 48604023c97af..c518cc8bcd488 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -27,7 +27,7 @@ fn extract_curl_version(out: String) -> semver::Version { fn curl_version(config: &Config) -> semver::Version { let mut curl = command("curl"); curl.arg("-V"); - let curl = curl.run_capture_stdout_exec_ctx(config); + let curl = curl.run_capture_stdout(config); if curl.is_failure() { return semver::Version::new(1, 0, 0); } @@ -80,7 +80,7 @@ impl Config { /// on NixOS fn should_fix_bins_and_dylibs(&self) -> bool { let val = *SHOULD_FIX_BINS_AND_DYLIBS.get_or_init(|| { - let uname = command("uname").arg("-s").run_capture_stdout_exec_ctx(self); + let uname = command("uname").arg("-s").run_capture_stdout(self); if uname.is_failure() { return false; } @@ -165,7 +165,7 @@ impl Config { nix_build_succeeded = command("nix-build") .allow_failure() .args([Path::new("-E"), Path::new(NIX_EXPR), Path::new("-o"), &nix_deps_dir]) - .run_capture_stdout_exec_ctx(self) + .run_capture_stdout(self) .is_success(); nix_deps_dir }); @@ -185,7 +185,7 @@ impl Config { patchelf.args(["--set-interpreter", dynamic_linker.trim_end()]); } patchelf.arg(fname); - let _ = patchelf.run_capture_stdout_exec_ctx(self); + let _ = patchelf.run_capture_stdout(self); } fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) { @@ -267,7 +267,7 @@ impl Config { "(New-Object System.Net.WebClient).DownloadFile('{}', '{}')", url, tempfile.to_str().expect("invalid UTF-8 not supported with powershell downloads"), ), - ]).run_capture_stdout_exec_ctx(self); + ]).run_capture_stdout(self); if powershell.is_failure() { return; diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 4ef9316d00b68..f55d924fdd8a3 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -361,11 +361,8 @@ 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") - .arg("--help") - .run_always() - .run_capture_stdout_exec_ctx(&build) - .stdout(); + let out = + command("cmake").arg("--help").run_always().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 9bace4eb77ce0..c8b6d59f38f7e 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -941,133 +941,6 @@ impl Build { }) } - /// 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] - 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(); - } - - #[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})") - }); - - let cmd = command.as_command_mut(); - cmd.stdout(stdout.stdio()); - cmd.stderr(stderr.stdio()); - - let output = cmd.output(); - - 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) - } - // Command has started, but then it failed - Ok(output) => { - writeln!( - message, - r#" -Command {command:?} did not execute successfully. -Expected success, got {} -Created at: {created_at} -Executed at: {executed_at}"#, - output.status, - ) - .unwrap(); - - 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 - // stdout/stderr, and we thus don't have anything captured to print anyway. - if stdout.captures() { - writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap(); - } - if stderr.captures() { - writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap(); - } - output - } - // The command did not even start - Err(e) => { - writeln!( - message, - "\n\nCommand {command:?} did not execute successfully.\ - \nIt was not possible to execute the command: {e:?}" - ) - .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."); - } - } - exit!(1); - }; - - if !output.is_success() { - match command.failure_behavior { - BehaviorOnFailure::DelayFail => { - if self.fail_fast { - fail(&message, output); - } - - let mut failures = self.delayed_failures.borrow_mut(); - failures.push(message); - } - BehaviorOnFailure::Exit => { - fail(&message, output); - } - BehaviorOnFailure::Ignore => { - // If failures are allowed, either the error has been printed already - // (OutputMode::Print) or the user used a capture output mode and wants to - // handle the error output on their own. - } - } - } - output - } - /// Check if verbosity is greater than the `level` pub fn is_verbose_than(&self, level: usize) -> bool { self.verbosity > level diff --git a/src/bootstrap/src/utils/channel.rs b/src/bootstrap/src/utils/channel.rs index 333c04c3c2641..fec9f068f6dfe 100644 --- a/src/bootstrap/src/utils/channel.rs +++ b/src/bootstrap/src/utils/channel.rs @@ -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_stdout_exec_ctx(exec_ctx); + let output = git_command.allow_failure().run_capture_stdout(exec_ctx); if output.is_failure() { return GitInfo::Absent; diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index c03fd2772ad89..f297300e34a85 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -12,7 +12,6 @@ use build_helper::ci::CiEnv; use build_helper::drop_bomb::DropBomb; use super::execution_context::ExecutionContext; -use crate::Build; /// What should be done when the command fails. #[derive(Debug, Copy, Clone)] @@ -140,48 +139,25 @@ impl BootstrapCommand { self } + /// Run the command, while printing stdout and stderr. + /// Returns true if the command has succeeded. #[track_caller] - pub fn run_exec_ctx(&mut self, exec_ctx: impl AsRef) -> bool { + pub fn run(&mut self, exec_ctx: impl AsRef) -> bool { exec_ctx.as_ref().run(self, OutputMode::Print, OutputMode::Print).is_success() } /// Run the command, while capturing and returning all its output. #[track_caller] - pub fn run_capture_exec_ctx( - &mut self, - exec_ctx: impl AsRef, - ) -> CommandOutput { + pub fn run_capture(&mut self, exec_ctx: impl AsRef) -> CommandOutput { exec_ctx.as_ref().run(self, OutputMode::Capture, OutputMode::Capture) } /// Run the command, while capturing and returning stdout, and printing stderr. #[track_caller] - pub fn run_capture_stdout_exec_ctx( - &mut self, - exec_ctx: impl AsRef, - ) -> CommandOutput { + pub fn run_capture_stdout(&mut self, exec_ctx: impl AsRef) -> CommandOutput { exec_ctx.as_ref().run(self, OutputMode::Capture, OutputMode::Print) } - /// 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() - } - - /// 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, while capturing and returning stdout, and printing stderr. - #[track_caller] - pub fn run_capture_stdout(&mut self, builder: &Build) -> CommandOutput { - builder.run(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 { From f3e1eb1dcade3e846c8624d2db85fc2d11a6d82a Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 8 Jun 2025 11:00:42 +0530 Subject: [PATCH 12/14] update dry_run value in exec_ctx and start forwarding exec_ctx verbose methods via config --- src/bootstrap/src/core/config/config.rs | 9 ++------- src/bootstrap/src/core/download.rs | 2 +- src/bootstrap/src/lib.rs | 2 ++ src/bootstrap/src/utils/channel.rs | 5 ----- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 8b5e252d1d7bf..6b6d9c9e5ba40 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1025,10 +1025,7 @@ impl Config { } pub fn dry_run(&self) -> bool { - match self.dry_run { - DryRun::Disabled => false, - DryRun::SelfCheck | DryRun::UserSelected => true, - } + self.exec_ctx.dry_run() } pub fn is_explicit_stage(&self) -> bool { @@ -1256,9 +1253,7 @@ impl Config { /// Runs a function if verbosity is greater than 0 pub fn verbose(&self, f: impl Fn()) { - if self.is_verbose() { - f() - } + self.exec_ctx.verbose(f); } pub fn any_sanitizers_to_build(&self) -> bool { diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index c518cc8bcd488..16d097661b1d4 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -38,7 +38,7 @@ fn curl_version(config: &Config) -> semver::Version { /// Generic helpers that are useful anywhere in bootstrap. impl Config { pub fn is_verbose(&self) -> bool { - self.verbose > 0 + self.exec_ctx.is_verbose() } pub(crate) fn create>(&self, path: P, s: &str) { diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index c8b6d59f38f7e..75dee300ec5e6 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -673,6 +673,7 @@ impl Build { let _sanity_check_span = span!(tracing::Level::DEBUG, "(1) executing dry-run sanity-check").entered(); self.config.dry_run = DryRun::SelfCheck; + self.config.exec_ctx.set_dry_run(DryRun::SelfCheck); let builder = builder::Builder::new(self); builder.execute_cli(); } @@ -683,6 +684,7 @@ impl Build { let _actual_run_span = span!(tracing::Level::DEBUG, "(2) executing actual run").entered(); self.config.dry_run = DryRun::Disabled; + self.config.exec_ctx.set_dry_run(DryRun::Disabled); let builder = builder::Builder::new(self); builder.execute_cli(); } diff --git a/src/bootstrap/src/utils/channel.rs b/src/bootstrap/src/utils/channel.rs index fec9f068f6dfe..8bd090e1a0176 100644 --- a/src/bootstrap/src/utils/channel.rs +++ b/src/bootstrap/src/utils/channel.rs @@ -51,11 +51,6 @@ impl GitInfo { if output.is_failure() { return GitInfo::Absent; } - // Make sure git commands work - // match helpers::git(Some(dir)).arg("rev-parse").as_command_mut().output() { - // Ok(ref out) if out.status.success() => {} - // _ => return GitInfo::Absent, - // } // If we're ignoring the git info, we don't actually need to collect it, just make sure this // was a git repo in the first place. From e9ced508f4feeadd9d5b63aa752c1d39dcfbbb6c Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Mon, 9 Jun 2025 20:34:30 +0530 Subject: [PATCH 13/14] remove execution context from flag module and correct the command invocation according to suggestions --- src/bootstrap/src/bin/main.rs | 4 ++-- src/bootstrap/src/core/config/config.rs | 16 ++++++++-------- src/bootstrap/src/core/config/flags.rs | 16 +++++----------- src/bootstrap/src/core/download.rs | 6 +++--- src/bootstrap/src/utils/channel.rs | 2 +- 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/bootstrap/src/bin/main.rs b/src/bootstrap/src/bin/main.rs index 9f1fde74359c7..833f80279517a 100644 --- a/src/bootstrap/src/bin/main.rs +++ b/src/bootstrap/src/bin/main.rs @@ -29,9 +29,9 @@ fn main() { } debug!("parsing flags"); - let (flags, exec_ctx) = Flags::parse(&args); + let flags = Flags::parse(&args); debug!("parsing config based on flags"); - let config = Config::parse(flags, exec_ctx); + let config = Config::parse(flags); let mut build_lock; let _build_lock_guard; diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 6b6d9c9e5ba40..8d244f438f0fb 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -368,7 +368,11 @@ impl Config { feature = "tracing", instrument(target = "CONFIG_HANDLING", level = "trace", name = "Config::parse", skip_all) )] - pub fn parse(flags: Flags, exec_ctx: ExecutionContext) -> Config { + pub fn parse(flags: Flags) -> Config { + let mut exec_ctx = ExecutionContext::new(); + exec_ctx.set_dry_run(if flags.dry_run { DryRun::UserSelected } else { DryRun::Disabled }); + exec_ctx.set_verbose(flags.verbose); + exec_ctx.set_fail_fast(flags.cmd.fail_fast()); Self::parse_inner(flags, Self::get_toml, exec_ctx) } @@ -1065,7 +1069,7 @@ impl Config { let mut git = helpers::git(Some(&self.src)); git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap())); - git.allow_failure().run_capture_stdout(self).stdout() + git.run_capture_stdout(self).stdout() } /// Bootstrap embeds a version number into the name of shared libraries it uploads in CI. @@ -1334,15 +1338,11 @@ impl Config { }; // Determine commit checked out in submodule. - let checked_out_hash = submodule_git() - .allow_failure() - .args(["rev-parse", "HEAD"]) - .run_capture_stdout(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)) - .allow_failure() .run_always() .args(["ls-tree", "HEAD"]) .arg(relative_path) diff --git a/src/bootstrap/src/core/config/flags.rs b/src/bootstrap/src/core/config/flags.rs index a50ff4caaf463..bc4fa73a36266 100644 --- a/src/bootstrap/src/core/config/flags.rs +++ b/src/bootstrap/src/core/config/flags.rs @@ -14,8 +14,7 @@ use crate::core::build_steps::setup::Profile; use crate::core::builder::{Builder, Kind}; use crate::core::config::Config; use crate::core::config::target_selection::{TargetSelectionList, target_selection_list}; -use crate::utils::execution_context::ExecutionContext; -use crate::{Build, DocTests, DryRun}; +use crate::{Build, DocTests}; #[derive(Copy, Clone, Default, Debug, ValueEnum)] pub enum Color { @@ -210,8 +209,8 @@ impl Flags { HelpVerboseOnly::try_parse_from(normalize_args(args)) { println!("NOTE: updating submodules before printing available paths"); - let (flags, exec_ctx) = Self::parse(&[String::from("build")]); - let config = Config::parse(flags, exec_ctx); + let flags = Self::parse(&[String::from("build")]); + let config = Config::parse(flags); let build = Build::new(config); let paths = Builder::get_help(&build, subcommand); if let Some(s) = paths { @@ -229,13 +228,8 @@ impl Flags { feature = "tracing", instrument(level = "trace", name = "Flags::parse", skip_all, fields(args = ?args)) )] - pub fn parse(args: &[String]) -> (Self, ExecutionContext) { - let mut exec_ctx = ExecutionContext::new(); - let flags = Flags::parse_from(normalize_args(args)); - exec_ctx.set_dry_run(if flags.dry_run { DryRun::UserSelected } else { DryRun::Disabled }); - exec_ctx.set_verbose(flags.verbose); - exec_ctx.set_fail_fast(flags.cmd.fail_fast()); - (flags, exec_ctx) + pub fn parse(args: &[String]) -> Self { + Flags::parse_from(normalize_args(args)) } } diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 16d097661b1d4..f349b9a87edb0 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -80,7 +80,7 @@ impl Config { /// on NixOS fn should_fix_bins_and_dylibs(&self) -> bool { let val = *SHOULD_FIX_BINS_AND_DYLIBS.get_or_init(|| { - let uname = command("uname").arg("-s").run_capture_stdout(self); + let uname = command("uname").allow_failure().arg("-s").run_capture_stdout(self); if uname.is_failure() { return false; } @@ -185,7 +185,7 @@ impl Config { patchelf.args(["--set-interpreter", dynamic_linker.trim_end()]); } patchelf.arg(fname); - let _ = patchelf.run_capture_stdout(self); + let _ = patchelf.allow_failure().run_capture_stdout(self); } fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) { @@ -259,7 +259,7 @@ impl Config { if self.build.contains("windows-msvc") { eprintln!("Fallback to PowerShell"); for _ in 0..3 { - let powershell = command("PowerShell.exe").args([ + let powershell = command("PowerShell.exe").allow_failure().args([ "/nologo", "-Command", "[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12;", diff --git a/src/bootstrap/src/utils/channel.rs b/src/bootstrap/src/utils/channel.rs index 8bd090e1a0176..38f250af42f08 100644 --- a/src/bootstrap/src/utils/channel.rs +++ b/src/bootstrap/src/utils/channel.rs @@ -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_stdout(exec_ctx); + let output = git_command.allow_failure().run_capture(exec_ctx); if output.is_failure() { return GitInfo::Absent; From 51fbd145f95cb193bde3c675a9236e2777973d07 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Mon, 9 Jun 2025 21:11:45 +0530 Subject: [PATCH 14/14] Initialize the execution context in parse_inner, start using dry run from the execution context, add getters and setters in the config, and update the tests and other relevant areas accordingly. --- src/bootstrap/src/core/builder/cargo.rs | 2 +- src/bootstrap/src/core/builder/mod.rs | 6 ++++-- src/bootstrap/src/core/builder/tests.rs | 2 +- src/bootstrap/src/core/config/config.rs | 22 +++++++++++++------- src/bootstrap/src/lib.rs | 21 +++++-------------- src/bootstrap/src/utils/execution_context.rs | 4 ++++ 6 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index 1e9af68a92df6..6026765d32a40 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -551,7 +551,7 @@ impl Builder<'_> { let libdir = self.rustc_libdir(compiler); let sysroot_str = sysroot.as_os_str().to_str().expect("sysroot should be UTF-8"); - if self.is_verbose() && !matches!(self.config.dry_run, DryRun::SelfCheck) { + if self.is_verbose() && !matches!(self.config.get_dry_run(), DryRun::SelfCheck) { println!("using sysroot {sysroot_str}"); } diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index e8686c75e02f6..8b1520de3a854 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -443,13 +443,15 @@ impl StepDescription { fn is_excluded(&self, builder: &Builder<'_>, pathset: &PathSet) -> bool { if builder.config.skip.iter().any(|e| pathset.has(e, builder.kind)) { - if !matches!(builder.config.dry_run, DryRun::SelfCheck) { + if !matches!(builder.config.get_dry_run(), DryRun::SelfCheck) { println!("Skipping {pathset:?} because it is excluded"); } return true; } - if !builder.config.skip.is_empty() && !matches!(builder.config.dry_run, DryRun::SelfCheck) { + if !builder.config.skip.is_empty() + && !matches!(builder.config.get_dry_run(), DryRun::SelfCheck) + { builder.verbose(|| { println!( "{:?} not skipped for {:?} -- not in {:?}", diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index baa22fc7f72af..a264d772c5629 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -22,7 +22,7 @@ fn configure_with_args(cmd: &[String], host: &[&str], target: &[&str]) -> Config let mut config = Config::parse(Flags::parse(cmd)); // don't save toolstates config.save_toolstates = None; - config.dry_run = DryRun::SelfCheck; + config.set_dry_run(DryRun::SelfCheck); // Ignore most submodules, since we don't need them for a dry run, and the // tests run much faster without them. diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 8d244f438f0fb..8f3c95ab5bf4c 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -131,7 +131,6 @@ pub struct Config { pub jobs: Option, pub cmd: Subcommand, pub incremental: bool, - pub dry_run: DryRun, pub dump_bootstrap_shims: bool, /// Arguments appearing after `--` to be forwarded to tools, /// e.g. `--fix-broken` or test arguments. @@ -364,16 +363,20 @@ impl Config { } } + pub fn set_dry_run(&mut self, dry_run: DryRun) { + self.exec_ctx.set_dry_run(dry_run); + } + + pub fn get_dry_run(&self) -> &DryRun { + self.exec_ctx.get_dry_run() + } + #[cfg_attr( feature = "tracing", instrument(target = "CONFIG_HANDLING", level = "trace", name = "Config::parse", skip_all) )] pub fn parse(flags: Flags) -> Config { - let mut exec_ctx = ExecutionContext::new(); - exec_ctx.set_dry_run(if flags.dry_run { DryRun::UserSelected } else { DryRun::Disabled }); - exec_ctx.set_verbose(flags.verbose); - exec_ctx.set_fail_fast(flags.cmd.fail_fast()); - Self::parse_inner(flags, Self::get_toml, exec_ctx) + Self::parse_inner(flags, Self::get_toml) } #[cfg_attr( @@ -388,9 +391,12 @@ impl Config { pub(crate) fn parse_inner( mut flags: Flags, get_toml: impl Fn(&Path) -> Result, - exec_ctx: ExecutionContext, ) -> Config { let mut config = Config::default_opts(); + let mut exec_ctx = ExecutionContext::new(); + exec_ctx.set_verbose(flags.verbose); + exec_ctx.set_fail_fast(flags.cmd.fail_fast()); + config.exec_ctx = exec_ctx; // Set flags. @@ -420,7 +426,7 @@ impl Config { config.on_fail = flags.on_fail; config.cmd = flags.cmd; config.incremental = flags.incremental; - config.dry_run = if flags.dry_run { DryRun::UserSelected } else { DryRun::Disabled }; + config.set_dry_run(if flags.dry_run { DryRun::UserSelected } else { DryRun::Disabled }); config.dump_bootstrap_shims = flags.dump_bootstrap_shims; config.keep_stage = flags.keep_stage; config.keep_stage_std = flags.keep_stage_std; diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 75dee300ec5e6..ea688f6f86bd1 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -196,7 +196,6 @@ pub struct Build { crates: HashMap, crate_paths: HashMap, is_sudo: bool, - delayed_failures: RefCell>, prerelease_version: Cell>, #[cfg(feature = "build-metrics")] @@ -457,7 +456,6 @@ impl Build { crates: HashMap::new(), crate_paths: HashMap::new(), is_sudo, - delayed_failures: RefCell::new(Vec::new()), prerelease_version: Cell::new(None), #[cfg(feature = "build-metrics")] @@ -672,8 +670,7 @@ impl Build { #[cfg(feature = "tracing")] let _sanity_check_span = span!(tracing::Level::DEBUG, "(1) executing dry-run sanity-check").entered(); - self.config.dry_run = DryRun::SelfCheck; - self.config.exec_ctx.set_dry_run(DryRun::SelfCheck); + self.config.set_dry_run(DryRun::SelfCheck); let builder = builder::Builder::new(self); builder.execute_cli(); } @@ -683,8 +680,7 @@ impl Build { #[cfg(feature = "tracing")] let _actual_run_span = span!(tracing::Level::DEBUG, "(2) executing actual run").entered(); - self.config.dry_run = DryRun::Disabled; - self.config.exec_ctx.set_dry_run(DryRun::Disabled); + self.config.set_dry_run(DryRun::Disabled); let builder = builder::Builder::new(self); builder.execute_cli(); } @@ -700,14 +696,7 @@ impl Build { debug!("checking for postponed test failures from `test --no-fail-fast`"); // Check for postponed failures from `test --no-fail-fast`. - let failures = self.delayed_failures.borrow(); - if !failures.is_empty() { - eprintln!("\n{} command(s) did not execute successfully:\n", failures.len()); - for failure in failures.iter() { - eprintln!(" - {failure}\n"); - } - exit!(1); - } + self.config.exec_ctx().report_failures_and_exit(); #[cfg(feature = "build-metrics")] self.metrics.persist(self); @@ -956,7 +945,7 @@ impl Build { } fn info(&self, msg: &str) { - match self.config.dry_run { + match self.config.get_dry_run() { DryRun::SelfCheck => (), DryRun::Disabled | DryRun::UserSelected => { println!("{msg}"); @@ -1079,7 +1068,7 @@ impl Build { #[track_caller] fn group(&self, msg: &str) -> Option { - match self.config.dry_run { + match self.config.get_dry_run() { DryRun::SelfCheck => None, DryRun::Disabled | DryRun::UserSelected => Some(gha::group(msg)), } diff --git a/src/bootstrap/src/utils/execution_context.rs b/src/bootstrap/src/utils/execution_context.rs index 17af98babb496..d12c02c161dfa 100644 --- a/src/bootstrap/src/utils/execution_context.rs +++ b/src/bootstrap/src/utils/execution_context.rs @@ -28,6 +28,10 @@ impl ExecutionContext { } } + pub fn get_dry_run(&self) -> &DryRun { + &self.dry_run + } + pub fn verbose(&self, f: impl Fn()) { if self.is_verbose() { f()