diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index de3b938e427..01b25224de4 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -29,7 +29,7 @@ use crate::core::config::{DebuginfoLevel, LlvmLibunwind, RustcLto, TargetSelection}; use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{ - exe, get_clang_cl_resource_dir, is_debug_info, is_dylib, output, symlink_dir, t, up_to_date, + exe, get_clang_cl_resource_dir, is_debug_info, is_dylib, symlink_dir, t, up_to_date, }; use crate::LLVM_TOOLS; use crate::{CLang, Compiler, DependencyType, GitRepo, Mode}; @@ -1488,10 +1488,10 @@ pub fn compiler_file( if builder.config.dry_run() { return PathBuf::new(); } - let mut cmd = Command::new(compiler); + let mut cmd = BootstrapCommand::new(compiler); cmd.args(builder.cflags(target, GitRepo::Rustc, c)); cmd.arg(format!("-print-file-name={file}")); - let out = output(&mut cmd); + let out = builder.run(cmd.capture_stdout()).stdout(); PathBuf::from(out.trim()) } @@ -1836,7 +1836,9 @@ fn run(self, builder: &Builder<'_>) -> Compiler { let llvm::LlvmResult { llvm_config, .. } = builder.ensure(llvm::Llvm { target: target_compiler.host }); if !builder.config.dry_run() && builder.config.llvm_tools_enabled { - let llvm_bin_dir = output(Command::new(llvm_config).arg("--bindir")); + let llvm_bin_dir = builder + .run(BootstrapCommand::new(llvm_config).capture_stdout().arg("--bindir")) + .stdout(); let llvm_bin_dir = Path::new(llvm_bin_dir.trim()); // Since we've already built the LLVM tools, install them to the sysroot. @@ -2080,7 +2082,7 @@ pub fn stream_cargo( tail_args: Vec, cb: &mut dyn FnMut(CargoMessage<'_>), ) -> bool { - let mut cargo = BootstrapCommand::from(cargo).command; + let mut cargo = cargo.into_cmd().command; // 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 { @@ -2161,8 +2163,7 @@ pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path) } let previous_mtime = FileTime::from_last_modification_time(&path.metadata().unwrap()); - // NOTE: `output` will propagate any errors here. - output(Command::new("strip").arg("--strip-debug").arg(path)); + builder.run(BootstrapCommand::new("strip").capture().arg("--strip-debug").arg(path)); // After running `strip`, we have to set the file modification time to what it was before, // otherwise we risk Cargo invalidating its fingerprint and rebuilding the world next time diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index 08795efe5bb..3dc871b1ca9 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -14,7 +14,6 @@ use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; -use std::process::Command; use object::read::archive::ArchiveFile; use object::BinaryFormat; @@ -28,7 +27,7 @@ use crate::utils::channel::{self, Info}; use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{ - exe, is_dylib, move_file, output, t, target_supports_cranelift_backend, timeit, + exe, is_dylib, move_file, t, target_supports_cranelift_backend, timeit, }; use crate::utils::tarball::{GeneratedTarball, OverlayKind, Tarball}; use crate::{Compiler, DependencyType, Mode, LLVM_TOOLS}; @@ -181,9 +180,9 @@ fn make_win_dist( } //Ask gcc where it keeps its stuff - let mut cmd = Command::new(builder.cc(target)); + let mut cmd = BootstrapCommand::new(builder.cc(target)); cmd.arg("-print-search-dirs"); - let gcc_out = output(&mut cmd); + let gcc_out = builder.run(cmd.capture_stdout()).stdout(); let mut bin_path: Vec<_> = env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect(); let mut lib_path = Vec::new(); @@ -1024,7 +1023,7 @@ fn run(self, builder: &Builder<'_>) -> GeneratedTarball { } // Vendor all Cargo dependencies - let mut cmd = Command::new(&builder.initial_cargo); + let mut cmd = BootstrapCommand::new(&builder.initial_cargo); cmd.arg("vendor") .arg("--versioned-dirs") .arg("--sync") @@ -1062,7 +1061,7 @@ fn run(self, builder: &Builder<'_>) -> GeneratedTarball { } let config = if !builder.config.dry_run() { - t!(String::from_utf8(t!(cmd.output()).stdout)) + builder.run(cmd.capture()).stdout() } else { String::new() }; @@ -2079,10 +2078,14 @@ fn maybe_install_llvm( } else if let llvm::LlvmBuildStatus::AlreadyBuilt(llvm::LlvmResult { llvm_config, .. }) = llvm::prebuilt_llvm_config(builder, target) { - let mut cmd = Command::new(llvm_config); + let mut cmd = BootstrapCommand::new(llvm_config); cmd.arg("--libfiles"); builder.verbose(|| println!("running {cmd:?}")); - let files = if builder.config.dry_run() { "".into() } else { output(&mut cmd) }; + let files = if builder.config.dry_run() { + "".into() + } else { + builder.run(cmd.capture_stdout()).stdout() + }; let build_llvm_out = &builder.llvm_out(builder.config.build); let target_llvm_out = &builder.llvm_out(target); for file in files.trim_end().split(' ') { diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index 4a5af25b3b2..823e842693e 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/src/bootstrap/src/core/build_steps/doc.rs @@ -738,7 +738,7 @@ fn doc_std( format!("library{} in {} format", crate_description(requested_crates), format.as_str()); let _guard = builder.msg_doc(compiler, description, target); - builder.run(cargo); + builder.run(cargo.into_cmd()); builder.cp_link_r(&out_dir, out); } @@ -863,7 +863,7 @@ fn run(self, builder: &Builder<'_>) { let proc_macro_out_dir = builder.stage_out(compiler, Mode::Rustc).join("doc"); symlink_dir_force(&builder.config, &out, &proc_macro_out_dir); - builder.run(cargo); + builder.run(cargo.into_cmd()); if !builder.config.dry_run() { // Sanity check on linked compiler crates @@ -996,7 +996,7 @@ fn run(self, builder: &Builder<'_>) { symlink_dir_force(&builder.config, &out, &proc_macro_out_dir); let _guard = builder.msg_doc(compiler, stringify!($tool).to_lowercase(), target); - builder.run(cargo); + builder.run(cargo.into_cmd()); if !builder.config.dry_run() { // Sanity check on linked doc directories diff --git a/src/bootstrap/src/core/build_steps/format.rs b/src/bootstrap/src/core/build_steps/format.rs index f5e34672686..66286a52813 100644 --- a/src/bootstrap/src/core/build_steps/format.rs +++ b/src/bootstrap/src/core/build_steps/format.rs @@ -1,13 +1,14 @@ //! Runs rustfmt on the repository. use crate::core::builder::Builder; -use crate::utils::helpers::{self, output, program_out_of_date, t}; +use crate::utils::exec::BootstrapCommand; +use crate::utils::helpers::{self, program_out_of_date, t}; use build_helper::ci::CiEnv; use build_helper::git::get_git_modified_files; use ignore::WalkBuilder; use std::collections::VecDeque; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Command; use std::sync::mpsc::SyncSender; use std::sync::Mutex; @@ -53,19 +54,17 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F fn get_rustfmt_version(build: &Builder<'_>) -> Option<(String, PathBuf)> { let stamp_file = build.out.join("rustfmt.stamp"); - let mut cmd = Command::new(match build.initial_rustfmt() { + let mut cmd = BootstrapCommand::new(match build.initial_rustfmt() { Some(p) => p, None => return None, }); cmd.arg("--version"); - let output = match cmd.output() { - Ok(status) => status, - Err(_) => return None, - }; - if !output.status.success() { + + let output = build.run(cmd.capture().allow_failure()); + if output.is_failure() { return None; } - Some((String::from_utf8(output.stdout).unwrap(), stamp_file)) + Some((output.stdout(), stamp_file)) } /// Return whether the format cache can be reused. @@ -160,36 +159,31 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) { override_builder.add(&format!("!{ignore}")).expect(&ignore); } } - let git_available = match helpers::git(None) - .arg("--version") - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - { - Ok(status) => status.success(), - Err(_) => false, - }; + let git_available = + build.run(helpers::git(None).capture().allow_failure().arg("--version")).is_success(); let mut adjective = None; if git_available { - let in_working_tree = match helpers::git(Some(&build.src)) - .arg("rev-parse") - .arg("--is-inside-work-tree") - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - { - Ok(status) => status.success(), - Err(_) => false, - }; - if in_working_tree { - let untracked_paths_output = output( + let in_working_tree = build + .run( helpers::git(Some(&build.src)) - .arg("status") - .arg("--porcelain") - .arg("-z") - .arg("--untracked-files=normal"), - ); + .capture() + .allow_failure() + .arg("rev-parse") + .arg("--is-inside-work-tree"), + ) + .is_success(); + if in_working_tree { + let untracked_paths_output = build + .run( + helpers::git(Some(&build.src)) + .capture_stdout() + .arg("status") + .arg("--porcelain") + .arg("-z") + .arg("--untracked-files=normal"), + ) + .stdout(); let untracked_paths: Vec<_> = untracked_paths_output .split_terminator('\0') .filter_map( diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 8e6795b11bd..4f1c1f87d1c 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -14,7 +14,6 @@ use std::fs::{self, File}; use std::io; use std::path::{Path, PathBuf}; -use std::process::Command; use std::sync::OnceLock; use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; @@ -25,6 +24,7 @@ }; use crate::{generate_smart_stamp_hash, CLang, GitRepo, Kind}; +use crate::utils::exec::BootstrapCommand; use build_helper::ci::CiEnv; use build_helper::git::get_git_merge_base; @@ -172,7 +172,7 @@ pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String { // the LLVM shared object file is named `LLVM-12-rust-{version}-nightly` config.src.join("src/version"), ]); - output(&mut rev_list).trim().to_owned() + output(&mut rev_list.command).trim().to_owned() } else if let Some(info) = channel::read_commit_info_file(&config.src) { info.sha.trim().to_owned() } else { @@ -253,7 +253,8 @@ pub(crate) fn is_ci_llvm_modified(config: &Config) -> bool { // We assume we have access to git, so it's okay to unconditionally pass // `true` here. let llvm_sha = detect_llvm_sha(config, true); - let head_sha = output(helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD")); + let head_sha = + output(&mut helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD").command); let head_sha = head_sha.trim(); llvm_sha == head_sha } @@ -477,7 +478,9 @@ fn run(self, builder: &Builder<'_>) -> LlvmResult { let LlvmResult { llvm_config, .. } = builder.ensure(Llvm { target: builder.config.build }); if !builder.config.dry_run() { - let llvm_bindir = output(Command::new(&llvm_config).arg("--bindir")); + let llvm_bindir = builder + .run(BootstrapCommand::new(&llvm_config).capture_stdout().arg("--bindir")) + .stdout(); let host_bin = Path::new(llvm_bindir.trim()); cfg.define( "LLVM_TABLEGEN", @@ -527,8 +530,8 @@ fn run(self, builder: &Builder<'_>) -> LlvmResult { // Helper to find the name of LLVM's shared library on darwin and linux. let find_llvm_lib_name = |extension| { - let mut cmd = Command::new(&res.llvm_config); - let version = output(cmd.arg("--version")); + let cmd = BootstrapCommand::new(&res.llvm_config); + let version = builder.run(cmd.capture_stdout().arg("--version")).stdout(); let major = version.split('.').next().unwrap(); match &llvm_version_suffix { @@ -584,8 +587,8 @@ fn check_llvm_version(builder: &Builder<'_>, llvm_config: &Path) { return; } - let mut cmd = Command::new(llvm_config); - let version = output(cmd.arg("--version")); + let cmd = BootstrapCommand::new(llvm_config); + let version = builder.run(cmd.capture_stdout().arg("--version")).stdout(); let mut parts = version.split('.').take(2).filter_map(|s| s.parse::().ok()); if let (Some(major), Some(_minor)) = (parts.next(), parts.next()) { if major >= 17 { diff --git a/src/bootstrap/src/core/build_steps/run.rs b/src/bootstrap/src/core/build_steps/run.rs index 22d5efa5d95..316a03a2171 100644 --- a/src/bootstrap/src/core/build_steps/run.rs +++ b/src/bootstrap/src/core/build_steps/run.rs @@ -4,7 +4,6 @@ //! If it can be reached from `./x.py run` it can go here. use std::path::PathBuf; -use std::process::Command; use crate::core::build_steps::dist::distdir; use crate::core::build_steps::test; @@ -12,7 +11,7 @@ use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::core::config::flags::get_completion; use crate::core::config::TargetSelection; -use crate::utils::helpers::output; +use crate::utils::exec::BootstrapCommand; use crate::Mode; #[derive(Debug, PartialOrd, Ord, Clone, Hash, PartialEq, Eq)] @@ -41,7 +40,8 @@ fn run(self, builder: &Builder<'_>) { panic!("\n\nfailed to specify `dist.upload-addr` in `config.toml`\n\n") }); - let today = output(Command::new("date").arg("+%Y-%m-%d")); + let today = + builder.run(BootstrapCommand::new("date").capture_stdout().arg("+%Y-%m-%d")).stdout(); cmd.arg(sign); cmd.arg(distdir(builder)); @@ -158,7 +158,7 @@ fn run(self, builder: &Builder<'_>) { // after another --, so this must be at the end. miri.args(builder.config.args()); - builder.run(miri); + builder.run(miri.into_cmd()); } } diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index 947c74f32c9..e6a09e8cb8e 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -484,6 +484,7 @@ fn run(self, builder: &Builder<'_>) -> Self::Output { fn install_git_hook_maybe(config: &Config) -> io::Result<()> { let git = helpers::git(Some(&config.src)) .args(["rev-parse", "--git-common-dir"]) + .command .output() .map(|output| { assert!(output.status.success(), "failed to run `git`"); diff --git a/src/bootstrap/src/core/build_steps/suggest.rs b/src/bootstrap/src/core/build_steps/suggest.rs index 7c5e0d4e13e..5412b3c807b 100644 --- a/src/bootstrap/src/core/build_steps/suggest.rs +++ b/src/bootstrap/src/core/build_steps/suggest.rs @@ -13,24 +13,15 @@ pub fn suggest(builder: &Builder<'_>, run: bool) { let git_config = builder.config.git_config(); let suggestions = builder - .tool_cmd(Tool::SuggestTests) - .env("SUGGEST_TESTS_GIT_REPOSITORY", git_config.git_repository) - .env("SUGGEST_TESTS_NIGHTLY_BRANCH", git_config.nightly_branch) - .command - .output() - .expect("failed to run `suggest-tests` tool"); + .run( + builder + .tool_cmd(Tool::SuggestTests) + .capture_stdout() + .env("SUGGEST_TESTS_GIT_REPOSITORY", git_config.git_repository) + .env("SUGGEST_TESTS_NIGHTLY_BRANCH", git_config.nightly_branch), + ) + .stdout(); - if !suggestions.status.success() { - println!("failed to run `suggest-tests` tool ({})", suggestions.status); - println!( - "`suggest_tests` stdout:\n{}`suggest_tests` stderr:\n{}", - String::from_utf8(suggestions.stdout).unwrap(), - String::from_utf8(suggestions.stderr).unwrap() - ); - panic!("failed to run `suggest-tests`"); - } - - let suggestions = String::from_utf8(suggestions.stdout).unwrap(); let suggestions = suggestions .lines() .map(|line| { diff --git a/src/bootstrap/src/core/build_steps/synthetic_targets.rs b/src/bootstrap/src/core/build_steps/synthetic_targets.rs index 281a9b093b9..a115ba2d8b2 100644 --- a/src/bootstrap/src/core/build_steps/synthetic_targets.rs +++ b/src/bootstrap/src/core/build_steps/synthetic_targets.rs @@ -9,8 +9,8 @@ use crate::core::builder::{Builder, ShouldRun, Step}; use crate::core::config::TargetSelection; +use crate::utils::exec::BootstrapCommand; use crate::Compiler; -use std::process::{Command, Stdio}; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub(crate) struct MirOptPanicAbortSyntheticTarget { @@ -56,7 +56,7 @@ fn create_synthetic_target( return TargetSelection::create_synthetic(&name, path.to_str().unwrap()); } - let mut cmd = Command::new(builder.rustc(compiler)); + let mut cmd = BootstrapCommand::new(builder.rustc(compiler)); cmd.arg("--target").arg(base.rustc_target_arg()); cmd.args(["-Zunstable-options", "--print", "target-spec-json"]); @@ -64,14 +64,8 @@ fn create_synthetic_target( // we cannot use nightly features. So `RUSTC_BOOTSTRAP` is needed here. cmd.env("RUSTC_BOOTSTRAP", "1"); - cmd.stdout(Stdio::piped()); - - let output = cmd.spawn().unwrap().wait_with_output().unwrap(); - if !output.status.success() { - panic!("failed to gather the target spec for {base}"); - } - - let mut spec: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let output = builder.run(cmd.capture()).stdout(); + let mut spec: serde_json::Value = serde_json::from_slice(output.as_bytes()).unwrap(); let spec_map = spec.as_object_mut().unwrap(); // The `is-builtin` attribute of a spec needs to be removed, otherwise rustc will complain. diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 1460a229019..dc53bd3cfa7 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -26,11 +26,10 @@ use crate::core::config::flags::get_completion; use crate::core::config::flags::Subcommand; use crate::core::config::TargetSelection; -use crate::utils::exec::{BootstrapCommand, OutputMode}; +use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{ self, add_link_lib_path, add_rustdoc_cargo_linker_args, dylib_path, dylib_path_var, - linker_args, linker_flags, output, t, target_supports_cranelift_backend, up_to_date, - LldThreads, + linker_args, linker_flags, t, target_supports_cranelift_backend, up_to_date, LldThreads, }; use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests}; use crate::{envify, CLang, DocTests, GitRepo, Mode}; @@ -150,16 +149,13 @@ fn run(self, builder: &Builder<'_>) { builder.default_doc(&[]); // Build the linkchecker before calling `msg`, since GHA doesn't support nested groups. - let mut linkchecker = builder.tool_cmd(Tool::Linkchecker); + let linkchecker = builder.tool_cmd(Tool::Linkchecker); // Run the linkchecker. let _guard = builder.msg(Kind::Test, compiler.stage, "Linkcheck", bootstrap_host, bootstrap_host); let _time = helpers::timeit(builder); - builder.run( - BootstrapCommand::from(linkchecker.arg(builder.out.join(host.triple).join("doc"))) - .delay_failure(), - ); + builder.run(linkchecker.delay_failure().arg(builder.out.join(host.triple).join("doc"))); } fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { @@ -217,10 +213,7 @@ fn run(self, builder: &Builder<'_>) { )); builder.run( - BootstrapCommand::from( - builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)), - ) - .delay_failure(), + builder.tool_cmd(Tool::HtmlChecker).delay_failure().arg(builder.doc_out(self.target)), ); } } @@ -260,14 +253,13 @@ fn run(self, builder: &Builder<'_>) { let _time = helpers::timeit(builder); let mut cmd = builder.tool_cmd(Tool::CargoTest); - let cmd = cmd - .arg(&cargo) + cmd.arg(&cargo) .arg(&out_dir) .args(builder.config.test_args()) .env("RUSTC", builder.rustc(compiler)) .env("RUSTDOC", builder.rustdoc(compiler)); - add_rustdoc_cargo_linker_args(cmd, builder, compiler.host, LldThreads::No); - builder.run(BootstrapCommand::from(cmd).delay_failure()); + add_rustdoc_cargo_linker_args(&mut cmd, builder, compiler.host, LldThreads::No); + builder.run(cmd.delay_failure()); } } @@ -477,19 +469,12 @@ pub fn build_miri_sysroot( // We re-use the `cargo` from above. cargo.arg("--print-sysroot"); - // FIXME: Is there a way in which we can re-use the usual `run` helpers? if builder.config.dry_run() { String::new() } else { builder.verbose(|| println!("running: {cargo:?}")); - let out = cargo - .command - .output() - .expect("We already ran `cargo miri setup` before and that worked"); - assert!(out.status.success(), "`cargo miri setup` returned with non-0 exit code"); + let stdout = builder.run(cargo.capture_stdout()).stdout(); // Output is "\n". - let stdout = String::from_utf8(out.stdout) - .expect("`cargo miri setup` stdout is not valid UTF-8"); let sysroot = stdout.trim_end(); builder.verbose(|| println!("`cargo miri setup --print-sysroot` said: {sysroot:?}")); sysroot.to_owned() @@ -763,12 +748,12 @@ fn run(self, builder: &Builder<'_>) { cargo.env("HOST_LIBS", host_libs); cargo.add_rustc_lib_path(builder); - let mut cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder); + let cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder); let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host); // Clippy reports errors if it blessed the outputs - if builder.run(BootstrapCommand::from(&mut cargo).allow_failure()).is_success() { + if builder.run(cargo.allow_failure()).is_success() { // The tests succeeded; nothing to do. return; } @@ -821,7 +806,7 @@ fn run(self, builder: &Builder<'_>) { .env("RUSTC_BOOTSTRAP", "1"); cmd.args(linker_args(builder, self.compiler.host, LldThreads::No)); - builder.run(BootstrapCommand::from(&mut cmd).delay_failure()); + builder.run(cmd.delay_failure()); } } @@ -918,25 +903,26 @@ fn run(self, builder: &Builder<'_>) { } } -fn get_browser_ui_test_version_inner(npm: &Path, global: bool) -> Option { - let mut command = Command::new(npm); +fn get_browser_ui_test_version_inner( + builder: &Builder<'_>, + npm: &Path, + global: bool, +) -> Option { + let mut command = BootstrapCommand::new(npm).capture(); command.arg("list").arg("--parseable").arg("--long").arg("--depth=0"); if global { command.arg("--global"); } - let lines = command - .output() - .map(|output| String::from_utf8_lossy(&output.stdout).into_owned()) - .unwrap_or_default(); + let lines = builder.run(command.allow_failure()).stdout(); lines .lines() .find_map(|l| l.split(':').nth(1)?.strip_prefix("browser-ui-test@")) .map(|v| v.to_owned()) } -fn get_browser_ui_test_version(npm: &Path) -> Option { - get_browser_ui_test_version_inner(npm, false) - .or_else(|| get_browser_ui_test_version_inner(npm, true)) +fn get_browser_ui_test_version(builder: &Builder<'_>, npm: &Path) -> Option { + get_browser_ui_test_version_inner(builder, npm, false) + .or_else(|| get_browser_ui_test_version_inner(builder, npm, true)) } #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -960,7 +946,7 @@ fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { .config .npm .as_ref() - .map(|p| get_browser_ui_test_version(p).is_some()) + .map(|p| get_browser_ui_test_version(builder, p).is_some()) .unwrap_or(false) })) } @@ -1099,7 +1085,7 @@ fn run(self, builder: &Builder<'_>) { } builder.info("tidy check"); - builder.run(BootstrapCommand::from(&mut cmd).delay_failure()); + builder.run(cmd.delay_failure()); builder.info("x.py completions check"); let [bash, zsh, fish, powershell] = ["x.py.sh", "x.py.zsh", "x.py.fish", "x.py.ps1"] @@ -1306,7 +1292,7 @@ fn run(self, builder: &Builder<'_>) -> PathBuf { &[], ); - builder.run(cargo); + builder.run(cargo.into_cmd()); let lib_name = "librun_make_support.rlib"; let lib = builder.tools_dir(self.compiler).join(lib_name); @@ -1818,26 +1804,16 @@ fn run(self, builder: &Builder<'_>) { } let lldb_exe = builder.config.lldb.clone().unwrap_or_else(|| PathBuf::from("lldb")); - let lldb_version = Command::new(&lldb_exe) - .arg("--version") - .output() - .map(|output| { - (String::from_utf8_lossy(&output.stdout).to_string(), output.status.success()) - }) - .ok() - .and_then(|(output, success)| if success { Some(output) } else { None }); + let lldb_version = builder + .run(BootstrapCommand::new(&lldb_exe).capture().allow_failure().arg("--version")) + .stdout_if_ok() + .and_then(|v| if v.trim().is_empty() { None } else { Some(v) }); if let Some(ref vers) = lldb_version { - let run = |cmd: &mut Command| { - cmd.output().map(|output| { - String::from_utf8_lossy(&output.stdout) - .lines() - .next() - .unwrap_or_else(|| panic!("{:?} failed {:?}", cmd, output)) - .to_string() - }) - }; cmd.arg("--lldb-version").arg(vers); - let lldb_python_dir = run(Command::new(&lldb_exe).arg("-P")).ok(); + let lldb_python_dir = builder + .run(BootstrapCommand::new(&lldb_exe).allow_failure().capture_stdout().arg("-P")) + .stdout_if_ok() + .map(|p| p.lines().next().expect("lldb Python dir not found").to_string()); if let Some(ref dir) = lldb_python_dir { cmd.arg("--lldb-python-dir").arg(dir); } @@ -1893,8 +1869,12 @@ fn run(self, builder: &Builder<'_>) { let llvm::LlvmResult { llvm_config, .. } = builder.ensure(llvm::Llvm { target: builder.config.build }); if !builder.config.dry_run() { - let llvm_version = output(Command::new(&llvm_config).arg("--version")); - let llvm_components = output(Command::new(&llvm_config).arg("--components")); + let llvm_version = builder + .run(BootstrapCommand::new(&llvm_config).capture_stdout().arg("--version")) + .stdout(); + let llvm_components = builder + .run(BootstrapCommand::new(&llvm_config).capture_stdout().arg("--components")) + .stdout(); // Remove trailing newline from llvm-config output. cmd.arg("--llvm-version") .arg(llvm_version.trim()) @@ -1913,7 +1893,9 @@ fn run(self, builder: &Builder<'_>) { // separate compilations. We can add LLVM's library path to the // platform-specific environment variable as a workaround. if !builder.config.dry_run() && suite.ends_with("fulldeps") { - let llvm_libdir = output(Command::new(&llvm_config).arg("--libdir")); + let llvm_libdir = builder + .run(BootstrapCommand::new(&llvm_config).capture_stdout().arg("--libdir")) + .stdout(); add_link_lib_path(vec![llvm_libdir.trim().into()], &mut cmd); } @@ -2187,7 +2169,7 @@ fn run_ext_doc(self, builder: &Builder<'_>) { compiler.host, ); let _time = helpers::timeit(builder); - let cmd = BootstrapCommand::from(&mut rustbook_cmd).delay_failure(); + let cmd = rustbook_cmd.delay_failure(); let toolstate = if builder.run(cmd).is_success() { ToolState::TestPass } else { ToolState::TestFail }; builder.save_toolstate(self.name, toolstate); @@ -2318,7 +2300,7 @@ fn run(self, builder: &Builder<'_>) { let guard = builder.msg(Kind::Test, compiler.stage, "error-index", compiler.host, compiler.host); let _time = helpers::timeit(builder); - builder.run(BootstrapCommand::from(&mut tool).output_mode(OutputMode::OnlyOnFailure)); + builder.run(tool.capture()); drop(guard); // The tests themselves need to link to std, so make sure it is // available. @@ -2349,7 +2331,7 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) -> cmd = cmd.delay_failure(); if !builder.config.verbose_tests { - cmd = cmd.quiet(); + cmd = cmd.capture(); } builder.run(cmd).is_success() } @@ -2375,10 +2357,13 @@ fn run(self, builder: &Builder<'_>) { builder.update_submodule(&relative_path); let src = builder.src.join(relative_path); - let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook); - let cmd = BootstrapCommand::from(rustbook_cmd.arg("linkcheck").arg(&src)).delay_failure(); - let toolstate = - if builder.run(cmd).is_success() { ToolState::TestPass } else { ToolState::TestFail }; + let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook).delay_failure(); + rustbook_cmd.arg("linkcheck").arg(&src); + let toolstate = if builder.run(rustbook_cmd).is_success() { + ToolState::TestPass + } else { + ToolState::TestFail + }; builder.save_toolstate("rustc-dev-guide", toolstate); } } @@ -3347,7 +3332,7 @@ fn run(self, builder: &Builder<'_>) { .arg("testsuite.extended_sysroot"); cargo.args(builder.config.test_args()); - builder.run(cargo); + builder.run(cargo.into_cmd()); } } @@ -3472,6 +3457,6 @@ fn run(self, builder: &Builder<'_>) { .arg("--std-tests"); cargo.args(builder.config.test_args()); - builder.run(cargo); + builder.run(cargo.into_cmd()); } } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index b3464043912..5fb282db90a 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -1,7 +1,6 @@ use std::env; use std::fs; use std::path::{Path, PathBuf}; -use std::process::Command; use crate::core::build_steps::compile; use crate::core::build_steps::toolstate::ToolState; @@ -10,7 +9,6 @@ use crate::core::config::TargetSelection; use crate::utils::channel::GitInfo; use crate::utils::exec::BootstrapCommand; -use crate::utils::helpers::output; use crate::utils::helpers::{add_dylib_path, exe, t}; use crate::Compiler; use crate::Mode; @@ -246,6 +244,7 @@ macro_rules! bootstrap_tool { ; )+) => { #[derive(PartialEq, Eq, Clone)] + #[allow(dead_code)] pub enum Tool { $( $name, @@ -603,7 +602,7 @@ fn run(self, builder: &Builder<'_>) -> PathBuf { &self.compiler.host, &target, ); - builder.run(cargo); + builder.run(cargo.into_cmd()); // Cargo adds a number of paths to the dylib search path on windows, which results in // the wrong rustdoc being executed. To avoid the conflicting rustdocs, we name the "tool" @@ -858,7 +857,7 @@ fn run(self, builder: &Builder<'_>) -> PathBuf { &self.extra_features, ); - builder.run(cargo); + builder.run(cargo.into_cmd()); let tool_out = builder .cargo_out(self.compiler, Mode::ToolRustc, self.target) @@ -926,7 +925,8 @@ fn run(self, builder: &Builder<'_>) -> LibcxxVersion { } } - let version_output = output(&mut Command::new(executable)); + let version_output = + builder.run(BootstrapCommand::new(executable).capture_stdout()).stdout(); let version_str = version_output.split_once("version:").unwrap().1; let version = version_str.trim().parse::().unwrap(); diff --git a/src/bootstrap/src/core/build_steps/toolstate.rs b/src/bootstrap/src/core/build_steps/toolstate.rs index 9e6d03349b5..e3e7931a5a2 100644 --- a/src/bootstrap/src/core/build_steps/toolstate.rs +++ b/src/bootstrap/src/core/build_steps/toolstate.rs @@ -101,8 +101,13 @@ fn print_error(tool: &str, submodule: &str) { fn check_changed_files(toolstates: &HashMap, ToolState>) { // Changed files - let output = - helpers::git(None).arg("diff").arg("--name-status").arg("HEAD").arg("HEAD^").output(); + let output = helpers::git(None) + .arg("diff") + .arg("--name-status") + .arg("HEAD") + .arg("HEAD^") + .command + .output(); let output = match output { Ok(o) => o, Err(e) => { @@ -324,6 +329,7 @@ fn checkout_toolstate_repo() { .arg("--depth=1") .arg(toolstate_repo()) .arg(TOOLSTATE_DIR) + .command .status(); let success = match status { Ok(s) => s.success(), @@ -337,7 +343,8 @@ fn checkout_toolstate_repo() { /// Sets up config and authentication for modifying the toolstate repo. fn prepare_toolstate_config(token: &str) { fn git_config(key: &str, value: &str) { - let status = helpers::git(None).arg("config").arg("--global").arg(key).arg(value).status(); + let status = + helpers::git(None).arg("config").arg("--global").arg(key).arg(value).command.status(); let success = match status { Ok(s) => s.success(), Err(_) => false, @@ -406,6 +413,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) { .arg("-a") .arg("-m") .arg(&message) + .command .status()); if !status.success() { success = true; @@ -416,6 +424,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) { .arg("push") .arg("origin") .arg("master") + .command .status()); // If we successfully push, exit. if status.success() { @@ -428,12 +437,14 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) { .arg("fetch") .arg("origin") .arg("master") + .command .status()); assert!(status.success()); let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR))) .arg("reset") .arg("--hard") .arg("origin/master") + .command .status()); assert!(status.success()); } @@ -449,7 +460,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) { /// `publish_toolstate.py` script if the PR passes all tests and is merged to /// master. fn publish_test_results(current_toolstate: &ToolstateData) { - let commit = t!(helpers::git(None).arg("rev-parse").arg("HEAD").output()); + let commit = t!(helpers::git(None).arg("rev-parse").arg("HEAD").command.output()); let commit = t!(String::from_utf8(commit.stdout)); let toolstate_serialized = t!(serde_json::to_string(¤t_toolstate)); diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 58d6e7a58e3..4da912994c3 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -8,7 +8,6 @@ use std::hash::Hash; use std::ops::Deref; use std::path::{Path, PathBuf}; -use std::process::Command; use std::time::{Duration, Instant}; use crate::core::build_steps::tool::{self, SourceType}; @@ -20,7 +19,7 @@ use crate::prepare_behaviour_dump_dir; use crate::utils::cache::Cache; use crate::utils::helpers::{self, add_dylib_path, add_link_lib_path, exe, linker_args}; -use crate::utils::helpers::{check_cfg_arg, libdir, linker_flags, output, t, LldThreads}; +use crate::utils::helpers::{check_cfg_arg, libdir, linker_flags, t, LldThreads}; use crate::EXTRA_CHECK_CFGS; use crate::{Build, CLang, Crate, DocTests, GitRepo, Mode}; @@ -1919,7 +1918,9 @@ fn cargo( // platform-specific environment variable as a workaround. if mode == Mode::ToolRustc || mode == Mode::Codegen { if let Some(llvm_config) = self.llvm_config(target) { - let llvm_libdir = output(Command::new(llvm_config).arg("--libdir")); + let llvm_libdir = self + .run(BootstrapCommand::new(llvm_config).capture_stdout().arg("--libdir")) + .stdout(); add_link_lib_path(vec![llvm_libdir.trim().into()], &mut cargo); } } @@ -2398,6 +2399,10 @@ pub fn new( cargo } + pub fn into_cmd(self) -> BootstrapCommand { + self.into() + } + /// Same as `Cargo::new` except this one doesn't configure the linker with `Cargo::configure_linker` pub fn new_for_mir_opt_tests( builder: &Builder<'_>, @@ -2622,9 +2627,3 @@ fn from(mut cargo: Cargo) -> BootstrapCommand { cargo.command } } - -impl From for Command { - fn from(cargo: Cargo) -> Command { - BootstrapCommand::from(cargo).command - } -} diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 948f97e746f..10ac6c93e9a 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1259,6 +1259,7 @@ pub(crate) fn parse_inner(args: &[String], get_toml: impl Fn(&Path) -> TomlConfi cmd.arg("rev-parse").arg("--show-cdup"); // Discard stderr because we expect this to fail when building from a tarball. let output = cmd + .command .stderr(std::process::Stdio::null()) .output() .ok() @@ -2141,7 +2142,7 @@ pub(crate) fn read_file_by_commit(&self, file: &Path, commit: &str) -> String { let mut git = helpers::git(Some(&self.src)); git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap())); - output(&mut git) + output(&mut git.command) } /// Bootstrap embeds a version number into the name of shared libraries it uploads in CI. @@ -2445,8 +2446,9 @@ fn download_ci_rustc_commit(&self, download_rustc: Option) -> Opti }; // Handle running from a directory other than the top level - let top_level = - output(helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"])); + let top_level = output( + &mut helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]).command, + ); let top_level = top_level.trim_end(); let compiler = format!("{top_level}/compiler/"); let library = format!("{top_level}/library/"); @@ -2454,10 +2456,11 @@ fn download_ci_rustc_commit(&self, download_rustc: Option) -> Opti // Look for a version to compare to based on the current commit. // Only commits merged by bors will have CI artifacts. let merge_base = output( - helpers::git(Some(&self.src)) + &mut helpers::git(Some(&self.src)) .arg("rev-list") .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email)) - .args(["-n1", "--first-parent", "HEAD"]), + .args(["-n1", "--first-parent", "HEAD"]) + .command, ); let commit = merge_base.trim_end(); if commit.is_empty() { @@ -2471,6 +2474,7 @@ fn download_ci_rustc_commit(&self, download_rustc: Option) -> Opti // Warn if there were changes to the compiler or standard library since the ancestor commit. let has_changes = !t!(helpers::git(Some(&self.src)) .args(["diff-index", "--quiet", commit, "--", &compiler, &library]) + .command .status()) .success(); if has_changes { @@ -2542,17 +2546,19 @@ pub fn last_modified_commit( if_unchanged: bool, ) -> Option { // Handle running from a directory other than the top level - let top_level = - output(helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"])); + let top_level = output( + &mut helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]).command, + ); let top_level = top_level.trim_end(); // Look for a version to compare to based on the current commit. // Only commits merged by bors will have CI artifacts. let merge_base = output( - helpers::git(Some(&self.src)) + &mut helpers::git(Some(&self.src)) .arg("rev-list") .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email)) - .args(["-n1", "--first-parent", "HEAD"]), + .args(["-n1", "--first-parent", "HEAD"]) + .command, ); let commit = merge_base.trim_end(); if commit.is_empty() { @@ -2571,7 +2577,7 @@ pub fn last_modified_commit( git.arg(format!("{top_level}/{path}")); } - let has_changes = !t!(git.status()).success(); + let has_changes = !t!(git.command.status()).success(); if has_changes { if if_unchanged { if self.verbose > 0 { diff --git a/src/bootstrap/src/core/metadata.rs b/src/bootstrap/src/core/metadata.rs index 220eb5ba126..49d17107125 100644 --- a/src/bootstrap/src/core/metadata.rs +++ b/src/bootstrap/src/core/metadata.rs @@ -1,9 +1,8 @@ use std::path::PathBuf; -use std::process::Command; use serde_derive::Deserialize; -use crate::utils::helpers::output; +use crate::utils::exec::BootstrapCommand; use crate::{t, Build, Crate}; /// For more information, see the output of @@ -71,7 +70,7 @@ pub fn build(build: &mut Build) { /// particular crate (e.g., `x build sysroot` to build library/sysroot). fn workspace_members(build: &Build) -> Vec { let collect_metadata = |manifest_path| { - let mut cargo = Command::new(&build.initial_cargo); + let mut cargo = BootstrapCommand::new(&build.initial_cargo); cargo // Will read the libstd Cargo.toml // which uses the unstable `public-dependency` feature. @@ -82,7 +81,7 @@ fn workspace_members(build: &Build) -> Vec { .arg("--no-deps") .arg("--manifest-path") .arg(build.src.join(manifest_path)); - let metadata_output = output(&mut cargo); + let metadata_output = build.run(cargo.capture_stdout().run_always()).stdout(); let Output { packages, .. } = t!(serde_json::from_str(&metadata_output)); packages }; diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 5a0be2948a1..78862ccb8cd 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -13,7 +13,6 @@ use std::ffi::{OsStr, OsString}; use std::fs; use std::path::PathBuf; -use std::process::Command; #[cfg(not(feature = "bootstrap-self-test"))] use crate::builder::Builder; @@ -24,7 +23,7 @@ use crate::builder::Kind; use crate::core::config::Target; -use crate::utils::helpers::output; +use crate::utils::exec::BootstrapCommand; use crate::Build; pub struct Finder { @@ -209,11 +208,14 @@ pub fn check(build: &mut Build) { .or_else(|| cmd_finder.maybe_have("reuse")); #[cfg(not(feature = "bootstrap-self-test"))] - let stage0_supported_target_list: HashSet = - output(Command::new(&build.config.initial_rustc).args(["--print", "target-list"])) - .lines() - .map(|s| s.to_string()) - .collect(); + let stage0_supported_target_list: HashSet = crate::utils::helpers::output( + &mut BootstrapCommand::new(&build.config.initial_rustc) + .args(["--print", "target-list"]) + .command, + ) + .lines() + .map(|s| s.to_string()) + .collect(); // We're gonna build some custom C code here and there, host triples // also build some C++ shims for LLVM so we need a C++ compiler. @@ -352,7 +354,8 @@ pub fn check(build: &mut Build) { // 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 = output(Command::new("cmake").arg("--help")); + let out = + build.run(BootstrapCommand::new("cmake").capture_stdout().arg("--help")).stdout(); if !out.contains("Visual Studio") { panic!( " diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index c12449fdc4a..ffdd9bc885a 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -30,7 +30,6 @@ use build_helper::ci::{gha, CiEnv}; use build_helper::exit; -use build_helper::util::fail; use filetime::FileTime; use sha2::digest::Digest; use termcolor::{ColorChoice, StandardStream, WriteColor}; @@ -42,7 +41,7 @@ use crate::core::config::{flags, LldMode}; use crate::core::config::{DryRun, Target}; use crate::core::config::{LlvmLibunwind, TargetSelection}; -use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode}; +use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, CommandOutput}; use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir}; mod core; @@ -494,11 +493,12 @@ pub(crate) fn update_submodule(&self, relative_path: &Path) { let submodule_git = || helpers::git(Some(&absolute_path)); // Determine commit checked out in submodule. - let checked_out_hash = output(submodule_git().args(["rev-parse", "HEAD"])); + let checked_out_hash = output(&mut submodule_git().args(["rev-parse", "HEAD"]).command); let checked_out_hash = checked_out_hash.trim_end(); // Determine commit that the submodule *should* have. - let recorded = - output(helpers::git(Some(&self.src)).args(["ls-tree", "HEAD"]).arg(relative_path)); + let recorded = output( + &mut helpers::git(Some(&self.src)).args(["ls-tree", "HEAD"]).arg(relative_path).command, + ); let actual_hash = recorded .split_whitespace() .nth(2) @@ -521,6 +521,7 @@ pub(crate) fn update_submodule(&self, relative_path: &Path) { let current_branch = { let output = helpers::git(Some(&self.src)) .args(["symbolic-ref", "--short", "HEAD"]) + .command .stderr(Stdio::inherit()) .output(); let output = t!(output); @@ -546,17 +547,14 @@ pub(crate) fn update_submodule(&self, relative_path: &Path) { git }; // NOTE: doesn't use `try_run` because this shouldn't print an error if it fails. - if !update(true).status().map_or(false, |status| status.success()) { + if !update(true).command.status().map_or(false, |status| status.success()) { self.run(update(false)); } // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error). // diff-index reports the modifications through the exit status let has_local_modifications = self - .run( - BootstrapCommand::from(submodule_git().args(["diff-index", "--quiet", "HEAD"])) - .allow_failure(), - ) + .run(submodule_git().allow_failure().args(["diff-index", "--quiet", "HEAD"])) .is_failure(); if has_local_modifications { self.run(submodule_git().args(["stash", "push"])); @@ -577,12 +575,15 @@ pub fn update_existing_submodules(&self) { if !self.config.submodules(self.rust_info()) { return; } - let output = output( - helpers::git(Some(&self.src)) - .args(["config", "--file"]) - .arg(self.config.src.join(".gitmodules")) - .args(["--get-regexp", "path"]), - ); + let output = self + .run( + helpers::git(Some(&self.src)) + .capture() + .args(["config", "--file"]) + .arg(self.config.src.join(".gitmodules")) + .args(["--get-regexp", "path"]), + ) + .stdout(); for line in output.lines() { // Look for `submodule.$name.path = $path` // Sample output: `submodule.src/rust-installer.path src/tools/rust-installer` @@ -859,14 +860,16 @@ fn llvm_filecheck(&self, target: TargetSelection) -> PathBuf { if let Some(s) = target_config.and_then(|c| c.llvm_filecheck.as_ref()) { s.to_path_buf() } else if let Some(s) = target_config.and_then(|c| c.llvm_config.as_ref()) { - let llvm_bindir = output(Command::new(s).arg("--bindir")); + let llvm_bindir = + self.run(BootstrapCommand::new(s).capture_stdout().arg("--bindir")).stdout(); let filecheck = Path::new(llvm_bindir.trim()).join(exe("FileCheck", target)); if filecheck.exists() { filecheck } else { // On Fedora the system LLVM installs FileCheck in the // llvm subdirectory of the libdir. - let llvm_libdir = output(Command::new(s).arg("--libdir")); + let llvm_libdir = + self.run(BootstrapCommand::new(s).capture_stdout().arg("--libdir")).stdout(); let lib_filecheck = Path::new(llvm_libdir.trim()).join("llvm").join(exe("FileCheck", target)); if lib_filecheck.exists() { @@ -932,66 +935,79 @@ fn rustc_snapshot_sysroot(&self) -> &Path { /// Execute a command and return its output. /// This method should be used for all command executions in bootstrap. - fn run>(&self, command: C) -> CommandOutput { - if self.config.dry_run() { + fn run>(&self, mut command: C) -> CommandOutput { + let command = command.as_mut(); + if self.config.dry_run() && !command.run_always { return CommandOutput::default(); } - let mut command = command.into(); - self.verbose(|| println!("running: {command:?}")); - let output_mode = command.output_mode.unwrap_or_else(|| match self.is_verbose() { - true => OutputMode::All, - false => OutputMode::OnlyOutput, - }); - let (output, print_error): (io::Result, bool) = match output_mode { - mode @ (OutputMode::All | OutputMode::OnlyOutput) => ( - command.command.status().map(|status| status.into()), - matches!(mode, OutputMode::All), - ), - OutputMode::OnlyOnFailure => (command.command.output().map(|o| o.into()), true), - }; + command.command.stdout(command.stdout.stdio()); + command.command.stderr(command.stderr.stdio()); - let output = match output { - Ok(output) => output, - Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", command, e)), + let output = command.command.output(); + + use std::fmt::Write; + + let mut message = String::new(); + let output: CommandOutput = match output { + // Command has succeeded + Ok(output) if output.status.success() => output.into(), + // Command has started, but then it failed + Ok(output) => { + writeln!( + message, + "\n\nCommand {command:?} did not execute successfully.\ + \nExpected success, got: {}", + output.status, + ) + .unwrap(); + + let output: CommandOutput = output.into(); + + // If the output mode is OutputMode::Capture, we can now print the output. + // If it is OutputMode::Print, then the output has already been printed to + // stdout/stderr, and we thus don't have anything captured to print anyway. + if command.stdout.captures() { + writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap(); + } + if command.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() + } }; if !output.is_success() { - if print_error { - println!( - "\n\nCommand did not execute successfully.\ - \nExpected success, got: {}", - output.status(), - ); - - if !self.is_verbose() { - println!("Add `-v` to see more details.\n"); - } - - self.verbose(|| { - println!( - "\nSTDOUT ----\n{}\n\ - STDERR ----\n{}\n", - output.stdout(), - output.stderr(), - ) - }); - } - match command.failure_behavior { BehaviorOnFailure::DelayFail => { if self.fail_fast { + println!("{message}"); exit!(1); } let mut failures = self.delayed_failures.borrow_mut(); - failures.push(format!("{command:?}")); + failures.push(message); } BehaviorOnFailure::Exit => { + println!("{message}"); exit!(1); } - BehaviorOnFailure::Ignore => {} + 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 @@ -1480,14 +1496,18 @@ fn extract_beta_rev_from_file>(version_file: P) -> Option // Figure out how many merge commits happened since we branched off master. // That's our beta number! // (Note that we use a `..` range, not the `...` symmetric difference.) - output( - helpers::git(Some(&self.src)).arg("rev-list").arg("--count").arg("--merges").arg( - format!( + self.run( + helpers::git(Some(&self.src)) + .capture() + .arg("rev-list") + .arg("--count") + .arg("--merges") + .arg(format!( "refs/remotes/origin/{}..HEAD", self.config.stage0_metadata.config.nightly_branch - ), - ), + )), ) + .stdout() }); let n = count.trim().parse().unwrap(); self.prerelease_version.set(Some(n)); @@ -1914,6 +1934,7 @@ fn envify(s: &str) -> String { pub fn generate_smart_stamp_hash(dir: &Path, additional_input: &str) -> String { let diff = helpers::git(Some(dir)) .arg("diff") + .command .output() .map(|o| String::from_utf8(o.stdout).unwrap_or_default()) .unwrap_or_default(); @@ -1923,6 +1944,7 @@ pub fn generate_smart_stamp_hash(dir: &Path, additional_input: &str) -> String { .arg("--porcelain") .arg("-z") .arg("--untracked-files=normal") + .command .output() .map(|o| String::from_utf8(o.stdout).unwrap_or_default()) .unwrap_or_default(); diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 540b8671333..b80df545202 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -23,11 +23,10 @@ use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::process::Command; use std::{env, iter}; use crate::core::config::TargetSelection; -use crate::utils::helpers::output; +use crate::utils::exec::BootstrapCommand; use crate::{Build, CLang, GitRepo}; // The `cc` crate doesn't provide a way to obtain a path to the detected archiver, @@ -183,14 +182,15 @@ fn default_compiler( return None; } - let output = output(c.to_command().arg("--version")); + let cmd = BootstrapCommand::from(c.to_command()); + let output = build.run(cmd.capture_stdout().arg("--version")).stdout(); let i = output.find(" 4.")?; match output[i + 3..].chars().next().unwrap() { '0'..='6' => {} _ => return None, } let alternative = format!("e{gnu_compiler}"); - if Command::new(&alternative).output().is_ok() { + if build.run(BootstrapCommand::new(&alternative).capture()).is_success() { Some(PathBuf::from(alternative)) } else { None diff --git a/src/bootstrap/src/utils/channel.rs b/src/bootstrap/src/utils/channel.rs index ce82c52f049..2ca86bdb0ed 100644 --- a/src/bootstrap/src/utils/channel.rs +++ b/src/bootstrap/src/utils/channel.rs @@ -45,7 +45,7 @@ pub fn new(omit_git_hash: bool, dir: &Path) -> GitInfo { } // Make sure git commands work - match helpers::git(Some(dir)).arg("rev-parse").output() { + match helpers::git(Some(dir)).arg("rev-parse").command.output() { Ok(ref out) if out.status.success() => {} _ => return GitInfo::Absent, } @@ -58,15 +58,17 @@ pub fn new(omit_git_hash: bool, dir: &Path) -> GitInfo { // Ok, let's scrape some info let ver_date = output( - helpers::git(Some(dir)) + &mut helpers::git(Some(dir)) .arg("log") .arg("-1") .arg("--date=short") - .arg("--pretty=format:%cd"), + .arg("--pretty=format:%cd") + .command, + ); + let ver_hash = output(&mut helpers::git(Some(dir)).arg("rev-parse").arg("HEAD").command); + let short_ver_hash = output( + &mut helpers::git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD").command, ); - let ver_hash = output(helpers::git(Some(dir)).arg("rev-parse").arg("HEAD")); - let short_ver_hash = - output(helpers::git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD")); GitInfo::Present(Some(Info { commit_date: ver_date.trim().to_string(), sha: ver_hash.trim().to_string(), diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 8bcb2301f1a..086d10c6351 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -1,6 +1,6 @@ use std::ffi::OsStr; use std::path::Path; -use std::process::{Command, CommandArgs, CommandEnvs, ExitStatus, Output}; +use std::process::{Command, CommandArgs, CommandEnvs, ExitStatus, Output, Stdio}; /// What should be done when the command fails. #[derive(Debug, Copy, Clone)] @@ -13,16 +13,30 @@ pub enum BehaviorOnFailure { Ignore, } -/// How should the output of the command be handled. +/// How should the output of a specific stream of the command (stdout/stderr) be handled +/// (whether it should be captured or printed). #[derive(Debug, Copy, Clone)] pub enum OutputMode { - /// Print both the output (by inheriting stdout/stderr) and also the command itself, if it - /// fails. - All, - /// Print the output (by inheriting stdout/stderr). - OnlyOutput, - /// Suppress the output if the command succeeds, otherwise print the output. - OnlyOnFailure, + /// Prints the stream by inheriting it from the bootstrap process. + Print, + /// Captures the stream into memory. + Capture, +} + +impl OutputMode { + pub fn captures(&self) -> bool { + match self { + OutputMode::Print => false, + OutputMode::Capture => true, + } + } + + pub fn stdio(&self) -> Stdio { + match self { + OutputMode::Print => Stdio::inherit(), + OutputMode::Capture => Stdio::piped(), + } + } } /// Wrapper around `std::process::Command`. @@ -31,10 +45,10 @@ pub enum OutputMode { /// If you want to allow failures, use [allow_failure]. /// If you want to delay failures until the end of bootstrap, use [delay_failure]. /// -/// By default, the command will print its stdout/stderr to stdout/stderr of bootstrap -/// ([OutputMode::OnlyOutput]). If bootstrap uses verbose mode, then it will also print the -/// command itself in case of failure ([OutputMode::All]). -/// If you want to handle the output programmatically, use `output_mode(OutputMode::OnlyOnFailure)`. +/// By default, the command will print its stdout/stderr to stdout/stderr of bootstrap ([OutputMode::Print]). +/// If you want to handle the output programmatically, use [BootstrapCommand::capture]. +/// +/// Bootstrap will print a debug log to stdout if the command fails and failure is not allowed. /// /// [allow_failure]: BootstrapCommand::allow_failure /// [delay_failure]: BootstrapCommand::delay_failure @@ -42,7 +56,10 @@ pub enum OutputMode { pub struct BootstrapCommand { pub command: Command, pub failure_behavior: BehaviorOnFailure, - pub output_mode: Option, + pub stdout: OutputMode, + pub stderr: OutputMode, + // Run the command even during dry run + pub run_always: bool, } impl BootstrapCommand { @@ -103,95 +120,110 @@ pub fn allow_failure(self) -> Self { Self { failure_behavior: BehaviorOnFailure::Ignore, ..self } } - /// Do not print the output of the command, unless it fails. - pub fn quiet(self) -> Self { - self.output_mode(OutputMode::OnlyOnFailure) + pub fn run_always(&mut self) -> &mut Self { + self.run_always = true; + self } - pub fn output_mode(self, output_mode: OutputMode) -> Self { - Self { output_mode: Some(output_mode), ..self } + /// Capture all output of the command, do not print it. + pub fn capture(self) -> Self { + Self { stdout: OutputMode::Capture, stderr: OutputMode::Capture, ..self } + } + + /// Capture stdout of the command, do not print it. + pub fn capture_stdout(self) -> Self { + Self { stdout: OutputMode::Capture, ..self } } } -/// This implementation is temporary, until all `Command` invocations are migrated to -/// `BootstrapCommand`. -impl<'a> From<&'a mut Command> for BootstrapCommand { - fn from(command: &'a mut Command) -> Self { - // This is essentially a manual `Command::clone` - let mut cmd = Command::new(command.get_program()); - if let Some(dir) = command.get_current_dir() { - cmd.current_dir(dir); - } - cmd.args(command.get_args()); - for (key, value) in command.get_envs() { - match value { - Some(value) => { - cmd.env(key, value); - } - None => { - cmd.env_remove(key); - } - } - } - - cmd.into() - } -} - -/// This implementation is temporary, until all `Command` invocations are migrated to -/// `BootstrapCommand`. -impl<'a> From<&'a mut BootstrapCommand> for BootstrapCommand { - fn from(command: &'a mut BootstrapCommand) -> Self { - BootstrapCommand::from(&mut command.command) +/// This implementation exists to make it possible to pass both [BootstrapCommand] and +/// `&mut BootstrapCommand` to `Build.run()`. +impl AsMut for BootstrapCommand { + fn as_mut(&mut self) -> &mut BootstrapCommand { + self } } impl From for BootstrapCommand { fn from(command: Command) -> Self { - Self { command, failure_behavior: BehaviorOnFailure::Exit, output_mode: None } + Self { + command, + failure_behavior: BehaviorOnFailure::Exit, + stdout: OutputMode::Print, + stderr: OutputMode::Print, + run_always: false, + } } } +/// Represents the current status of `BootstrapCommand`. +enum CommandStatus { + /// The command has started and finished with some status. + Finished(ExitStatus), + /// It was not even possible to start the command. + DidNotStart, +} + /// Represents the output of an executed process. #[allow(unused)] -pub struct CommandOutput(Output); +pub struct CommandOutput { + status: CommandStatus, + stdout: Vec, + stderr: Vec, +} impl CommandOutput { + pub fn did_not_start() -> Self { + Self { status: CommandStatus::DidNotStart, stdout: vec![], stderr: vec![] } + } + pub fn is_success(&self) -> bool { - self.0.status.success() + match self.status { + CommandStatus::Finished(status) => status.success(), + CommandStatus::DidNotStart => false, + } } pub fn is_failure(&self) -> bool { !self.is_success() } - pub fn status(&self) -> ExitStatus { - self.0.status + pub fn status(&self) -> Option { + match self.status { + CommandStatus::Finished(status) => Some(status), + CommandStatus::DidNotStart => None, + } } pub fn stdout(&self) -> String { - String::from_utf8(self.0.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + String::from_utf8(self.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + } + + pub fn stdout_if_ok(&self) -> Option { + if self.is_success() { Some(self.stdout()) } else { None } } pub fn stderr(&self) -> String { - String::from_utf8(self.0.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + String::from_utf8(self.stderr.clone()).expect("Cannot parse process stderr as UTF-8") } } impl Default for CommandOutput { fn default() -> Self { - Self(Output { status: Default::default(), stdout: vec![], stderr: vec![] }) + Self { + status: CommandStatus::Finished(ExitStatus::default()), + stdout: vec![], + stderr: vec![], + } } } impl From for CommandOutput { fn from(output: Output) -> Self { - Self(output) - } -} - -impl From for CommandOutput { - fn from(status: ExitStatus) -> Self { - Self(Output { status, stdout: vec![], stderr: vec![] }) + Self { + status: CommandStatus::Finished(output.status), + stdout: output.stdout, + stderr: output.stderr, + } } } diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index adf18c0ace1..2575b7939b4 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -360,7 +360,7 @@ pub fn get_clang_cl_resource_dir(clang_cl_path: &str) -> PathBuf { /// Returns a flag that configures LLD to use only a single thread. /// If we use an external LLD, we need to find out which version is it to know which flag should we /// pass to it (LLD older than version 10 had a different flag). -fn lld_flag_no_threads(lld_mode: LldMode, is_windows: bool) -> &'static str { +fn lld_flag_no_threads(builder: &Builder<'_>, lld_mode: LldMode, is_windows: bool) -> &'static str { static LLD_NO_THREADS: OnceLock<(&'static str, &'static str)> = OnceLock::new(); let new_flags = ("/threads:1", "--threads=1"); @@ -369,7 +369,9 @@ fn lld_flag_no_threads(lld_mode: LldMode, is_windows: bool) -> &'static str { let (windows_flag, other_flag) = LLD_NO_THREADS.get_or_init(|| { let newer_version = match lld_mode { LldMode::External => { - let out = output(Command::new("lld").arg("-flavor").arg("ld").arg("--version")); + let mut cmd = BootstrapCommand::new("lld").capture_stdout(); + cmd.arg("-flavor").arg("ld").arg("--version"); + let out = builder.run(cmd).stdout(); match (out.find(char::is_numeric), out.find('.')) { (Some(b), Some(e)) => out.as_str()[b..e].parse::().ok().unwrap_or(14) > 10, _ => true, @@ -431,7 +433,7 @@ pub fn linker_flags( if matches!(lld_threads, LldThreads::No) { args.push(format!( "-Clink-arg=-Wl,{}", - lld_flag_no_threads(builder.config.lld_mode, target.is_windows()) + lld_flag_no_threads(builder, builder.config.lld_mode, target.is_windows()) )); } } @@ -492,14 +494,15 @@ pub fn check_cfg_arg(name: &str, values: Option<&[&str]>) -> String { format!("--check-cfg=cfg({name}{next})") } -/// Prepares `Command` that runs git inside the source directory if given. +/// Prepares `BootstrapCommand` that runs git inside the source directory if given. /// /// Whenever a git invocation is needed, this function should be preferred over -/// manually building a git `Command`. This approach allows us to manage bootstrap-specific -/// needs/hacks from a single source, rather than applying them on next to every `Command::new("git")`, -/// which is painful to ensure that the required change is applied on each one of them correctly. -pub fn git(source_dir: Option<&Path>) -> Command { - let mut git = Command::new("git"); +/// manually building a git `BootstrapCommand`. This approach allows us to manage +/// bootstrap-specific needs/hacks from a single source, rather than applying them on next to every +/// `BootstrapCommand::new("git")`, which is painful to ensure that the required change is applied +/// on each one of them correctly. +pub fn git(source_dir: Option<&Path>) -> BootstrapCommand { + let mut git = BootstrapCommand::new("git"); if let Some(source_dir) = source_dir { git.current_dir(source_dir); diff --git a/src/bootstrap/src/utils/tarball.rs b/src/bootstrap/src/utils/tarball.rs index 3c15bb296f3..1a35dc1fd38 100644 --- a/src/bootstrap/src/utils/tarball.rs +++ b/src/bootstrap/src/utils/tarball.rs @@ -370,7 +370,11 @@ fn run(self, build_cli: impl FnOnce(&Tarball<'a>, &mut BootstrapCommand)) -> Gen if self.builder.rust_info().is_managed_git_subrepository() { // %ct means committer date let timestamp = helpers::output( - helpers::git(Some(&self.builder.src)).arg("log").arg("-1").arg("--format=%ct"), + &mut helpers::git(Some(&self.builder.src)) + .arg("log") + .arg("-1") + .arg("--format=%ct") + .command, ); cmd.args(["--override-file-mtime", timestamp.trim()]); }