From 3ef77cc42cb1ea1f07c1bd8c2ac514d15291106b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 22 Jun 2024 17:47:11 +0200 Subject: [PATCH] Remove various usages of the `output` function --- src/bootstrap/src/core/build_steps/compile.rs | 13 ++-- src/bootstrap/src/core/build_steps/dist.rs | 19 ++--- src/bootstrap/src/core/build_steps/format.rs | 33 ++++----- src/bootstrap/src/core/build_steps/llvm.rs | 14 ++-- src/bootstrap/src/core/build_steps/run.rs | 6 +- src/bootstrap/src/core/build_steps/suggest.rs | 25 +++---- .../src/core/build_steps/synthetic_targets.rs | 14 ++-- src/bootstrap/src/core/build_steps/test.rs | 70 ++++++++----------- src/bootstrap/src/core/build_steps/tool.rs | 5 +- src/bootstrap/src/core/builder.rs | 7 +- src/bootstrap/src/core/metadata.rs | 8 +-- src/bootstrap/src/core/sanity.rs | 4 +- src/bootstrap/src/lib.rs | 6 +- src/bootstrap/src/utils/cc_detect.rs | 8 +-- src/bootstrap/src/utils/exec.rs | 12 ++-- src/bootstrap/src/utils/helpers.rs | 8 ++- 16 files changed, 120 insertions(+), 132 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index c267d3b0c0c..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. @@ -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/format.rs b/src/bootstrap/src/core/build_steps/format.rs index 0372360c3cb..66286a52813 100644 --- a/src/bootstrap/src/core/build_steps/format.rs +++ b/src/bootstrap/src/core/build_steps/format.rs @@ -1,7 +1,8 @@ //! 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; @@ -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. @@ -175,14 +174,16 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) { ) .is_success(); if in_working_tree { - let untracked_paths_output = output( - &mut helpers::git(Some(&build.src)) - .arg("status") - .arg("--porcelain") - .arg("-z") - .arg("--untracked-files=normal") - .command, - ); + 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 ba21e2ad32d..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; @@ -478,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", @@ -528,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 { @@ -585,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 496fe446d72..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)); 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 9fc337dfd50..b063d0aaf4f 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -29,8 +29,7 @@ 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}; @@ -470,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() @@ -911,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)] @@ -953,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) })) } @@ -1811,26 +1804,15 @@ 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(); 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); } @@ -1886,8 +1868,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()) @@ -1906,7 +1892,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); } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 20c7a30f1a8..40184e016a6 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; @@ -926,7 +924,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/builder.rs b/src/bootstrap/src/core/builder.rs index 35de91c41d4..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); } } diff --git a/src/bootstrap/src/core/metadata.rs b/src/bootstrap/src/core/metadata.rs index 220eb5ba126..da269959e7b 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,8 @@ fn workspace_members(build: &Build) -> Vec { .arg("--no-deps") .arg("--manifest-path") .arg(build.src.join(manifest_path)); - let metadata_output = output(&mut cargo); + // FIXME: fix stderr + let metadata_output = build.run(cargo.capture()).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..d64923e69cb 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -24,6 +24,7 @@ use crate::builder::Kind; use crate::core::config::Target; +use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::output; use crate::Build; @@ -352,7 +353,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 ae3d18e8774..dfaa1e5eb12 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -861,14 +861,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() { 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/exec.rs b/src/bootstrap/src/utils/exec.rs index 5f9e1648161..bda6e0bfbac 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -51,11 +51,7 @@ pub struct BootstrapCommand { impl BootstrapCommand { pub fn new>(program: S) -> Self { - Self { - command: Command::new(program), - failure_behavior: BehaviorOnFailure::Exit, - output_mode: OutputMode::Print, - } + Command::new(program).into() } pub fn arg>(&mut self, arg: S) -> &mut Self { @@ -130,6 +126,12 @@ fn as_mut(&mut self) -> &mut BootstrapCommand { } } +impl From for BootstrapCommand { + fn from(command: Command) -> Self { + Self { command, failure_behavior: BehaviorOnFailure::Exit, output_mode: OutputMode::Print } + } +} + /// Represents the output of an executed process. #[allow(unused)] pub struct CommandOutput(Output); diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 9c40065dfc4..bbd4185874f 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()) )); } }