From e31709345c82542e14eae079303c1b09a42a811f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 4 Jul 2024 20:12:19 +0200 Subject: [PATCH] Port all remaining simple command usages in bootstrap to `BootstrapCommand` --- src/bootstrap/src/core/build_steps/compile.rs | 26 ++++------- src/bootstrap/src/core/build_steps/llvm.rs | 2 +- src/bootstrap/src/core/build_steps/setup.rs | 46 +++++++++---------- src/bootstrap/src/core/build_steps/test.rs | 14 ++---- src/bootstrap/src/utils/helpers.rs | 6 +-- 5 files changed, 40 insertions(+), 54 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 039a7b2fc276..e6c15f27ea91 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -14,7 +14,7 @@ use std::fs; use std::io::prelude::*; use std::io::BufReader; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Stdio; use std::str; use serde_derive::Deserialize; @@ -696,10 +696,10 @@ fn copy_sanitizers( || target == "x86_64-apple-ios" { // Update the library’s install name to reflect that it has been renamed. - apple_darwin_update_library_name(&dst, &format!("@rpath/{}", &runtime.name)); + apple_darwin_update_library_name(builder, &dst, &format!("@rpath/{}", &runtime.name)); // Upon renaming the install name, the code signature of the file will invalidate, // so we will sign it again. - apple_darwin_sign_file(&dst); + apple_darwin_sign_file(builder, &dst); } target_deps.push(dst); @@ -708,25 +708,17 @@ fn copy_sanitizers( target_deps } -fn apple_darwin_update_library_name(library_path: &Path, new_name: &str) { - let status = Command::new("install_name_tool") - .arg("-id") - .arg(new_name) - .arg(library_path) - .status() - .expect("failed to execute `install_name_tool`"); - assert!(status.success()); +fn apple_darwin_update_library_name(builder: &Builder<'_>, library_path: &Path, new_name: &str) { + command("install_name_tool").arg("-id").arg(new_name).arg(library_path).run(builder); } -fn apple_darwin_sign_file(file_path: &Path) { - let status = Command::new("codesign") +fn apple_darwin_sign_file(builder: &Builder<'_>, file_path: &Path) { + command("codesign") .arg("-f") // Force to rewrite the existing signature .arg("-s") .arg("-") .arg(file_path) - .status() - .expect("failed to execute `codesign`"); - assert!(status.success()); + .run(builder); } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -1172,7 +1164,7 @@ fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelect if builder.config.llvm_profile_generate && target.is_msvc() { if let Some(ref clang_cl_path) = builder.config.llvm_clang_cl { // Add clang's runtime library directory to the search path - let clang_rt_dir = get_clang_cl_resource_dir(clang_cl_path); + let clang_rt_dir = get_clang_cl_resource_dir(builder, clang_cl_path); llvm_linker_flags.push_str(&format!("-L{}", clang_rt_dir.display())); } } diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 41dff2123f13..876d73dce3ce 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -912,7 +912,7 @@ impl Step for Lld { if let Some(clang_cl_path) = builder.config.llvm_clang_cl.as_ref() { // Find clang's runtime library directory and push that as a search path to the // cmake linker flags. - let clang_rt_dir = get_clang_cl_resource_dir(clang_cl_path); + let clang_rt_dir = get_clang_cl_resource_dir(builder, clang_cl_path); ldflags.push_all(format!("/libpath:{}", clang_rt_dir.display())); } } diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index 29cc5e00637e..e155b3025783 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -8,6 +8,7 @@ use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::t; use crate::utils::change_tracker::CONFIG_CHANGE_HISTORY; +use crate::utils::exec::command; use crate::utils::helpers::{self, hex_encode}; use crate::Config; use sha2::Digest; @@ -16,7 +17,6 @@ use std::fmt::Write as _; use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf, MAIN_SEPARATOR_STR}; -use std::process::Command; use std::str::FromStr; use std::{fmt, fs, io}; @@ -266,20 +266,16 @@ impl Step for Link { } let stage_path = ["build", config.build.rustc_target_arg(), "stage1"].join(MAIN_SEPARATOR_STR); - if !rustup_installed() { + if !rustup_installed(builder) { eprintln!("`rustup` is not installed; cannot link `stage1` toolchain"); } else if stage_dir_exists(&stage_path[..]) && !config.dry_run() { - attempt_toolchain_link(&stage_path[..]); + attempt_toolchain_link(builder, &stage_path[..]); } } } -fn rustup_installed() -> bool { - Command::new("rustup") - .arg("--version") - .stdout(std::process::Stdio::null()) - .output() - .map_or(false, |output| output.status.success()) +fn rustup_installed(builder: &Builder<'_>) -> bool { + command("rustup").capture_stdout().arg("--version").run(builder).is_success() } fn stage_dir_exists(stage_path: &str) -> bool { @@ -289,8 +285,8 @@ fn stage_dir_exists(stage_path: &str) -> bool { } } -fn attempt_toolchain_link(stage_path: &str) { - if toolchain_is_linked() { +fn attempt_toolchain_link(builder: &Builder<'_>, stage_path: &str) { + if toolchain_is_linked(builder) { return; } @@ -301,7 +297,7 @@ fn attempt_toolchain_link(stage_path: &str) { return; } - if try_link_toolchain(stage_path) { + if try_link_toolchain(builder, stage_path) { println!( "Added `stage1` rustup toolchain; try `cargo +stage1 build` on a separate rust project to run a newly-built toolchain" ); @@ -315,14 +311,16 @@ fn attempt_toolchain_link(stage_path: &str) { } } -fn toolchain_is_linked() -> bool { - match Command::new("rustup") +fn toolchain_is_linked(builder: &Builder<'_>) -> bool { + match command("rustup") + .capture_stdout() + .allow_failure() .args(["toolchain", "list"]) - .stdout(std::process::Stdio::piped()) - .output() + .run(builder) + .stdout_if_ok() { - Ok(toolchain_list) => { - if !String::from_utf8_lossy(&toolchain_list.stdout).contains("stage1") { + Some(toolchain_list) => { + if !toolchain_list.contains("stage1") { return false; } // The toolchain has already been linked. @@ -330,7 +328,7 @@ fn toolchain_is_linked() -> bool { "`stage1` toolchain already linked; not attempting to link `stage1` toolchain" ); } - Err(_) => { + None => { // In this case, we don't know if the `stage1` toolchain has been linked; // but `rustup` failed, so let's not go any further. println!( @@ -341,12 +339,12 @@ fn toolchain_is_linked() -> bool { true } -fn try_link_toolchain(stage_path: &str) -> bool { - Command::new("rustup") - .stdout(std::process::Stdio::null()) +fn try_link_toolchain(builder: &Builder<'_>, stage_path: &str) -> bool { + command("rustup") + .capture_stdout() .args(["toolchain", "link", "stage1", stage_path]) - .output() - .map_or(false, |output| output.status.success()) + .run(builder) + .is_success() } fn ensure_stage1_toolchain_placeholder_exists(stage_path: &str) -> bool { diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 4d69d6b52311..118b6b876ed1 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -9,7 +9,6 @@ use std::ffi::OsString; use std::fs; use std::iter; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; use clap_complete::shells; @@ -169,12 +168,8 @@ You can skip linkcheck with --skip src/tools/linkchecker" } } -fn check_if_tidy_is_installed() -> bool { - Command::new("tidy") - .arg("--version") - .stdout(Stdio::null()) - .status() - .map_or(false, |status| status.success()) +fn check_if_tidy_is_installed(builder: &Builder<'_>) -> bool { + command("tidy").capture_stdout().allow_failure().arg("--version").run(builder).is_success() } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -188,8 +183,9 @@ impl Step for HtmlCheck { const ONLY_HOSTS: bool = true; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + let builder = run.builder; let run = run.path("src/tools/html-checker"); - run.lazy_default_condition(Box::new(check_if_tidy_is_installed)) + run.lazy_default_condition(Box::new(|| check_if_tidy_is_installed(builder))) } fn make_run(run: RunConfig<'_>) { @@ -197,7 +193,7 @@ impl Step for HtmlCheck { } fn run(self, builder: &Builder<'_>) { - if !check_if_tidy_is_installed() { + if !check_if_tidy_is_installed(builder) { eprintln!("not running HTML-check tool because `tidy` is missing"); eprintln!( "You need the HTML tidy tool https://www.html-tidy.org/, this tool is *not* part of the rust project and needs to be installed separately, for example via your package manager." diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 15133e441b49..3c82fa189be3 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -338,13 +338,13 @@ fn dir_up_to_date(src: &Path, threshold: SystemTime) -> bool { /// When `clang-cl` is used with instrumentation, we need to add clang's runtime library resource /// directory to the linker flags, otherwise there will be linker errors about the profiler runtime /// missing. This function returns the path to that directory. -pub fn get_clang_cl_resource_dir(clang_cl_path: &str) -> PathBuf { +pub fn get_clang_cl_resource_dir(builder: &Builder<'_>, clang_cl_path: &str) -> PathBuf { // Similar to how LLVM does it, to find clang's library runtime directory: // - we ask `clang-cl` to locate the `clang_rt.builtins` lib. - let mut builtins_locator = Command::new(clang_cl_path); + let mut builtins_locator = command(clang_cl_path); builtins_locator.args(["/clang:-print-libgcc-file-name", "/clang:--rtlib=compiler-rt"]); - let clang_rt_builtins = output(&mut builtins_locator); + let clang_rt_builtins = builtins_locator.capture_stdout().run(builder).stdout(); let clang_rt_builtins = Path::new(clang_rt_builtins.trim()); assert!( clang_rt_builtins.exists(),