From e204a32f2d8cc9fe2a5530eafdb770ddf72f3568 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Thu, 30 May 2024 20:18:15 +0300 Subject: [PATCH] use `BootstrapCommand` for `tool::ToolBuild` step Previously, we were running bare commands for `ToolBuild` step and were unable to utilize many of the flags that are already handled by `compile::cargo_run`. This change improves `ToolBuild` to use `compile::cargo_run`, allowing us to fully benefit from all the flags supported by the bootstrap cargo. Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/check.rs | 17 +++++++++----- src/bootstrap/src/core/build_steps/clippy.rs | 8 ++++--- src/bootstrap/src/core/build_steps/compile.rs | 23 +++++++++++++------ src/bootstrap/src/core/build_steps/tool.rs | 21 +++++++++++++---- src/bootstrap/src/core/builder.rs | 2 +- src/bootstrap/src/utils/exec.rs | 2 +- 6 files changed, 51 insertions(+), 22 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/check.rs b/src/bootstrap/src/core/build_steps/check.rs index 8235d4634b753..ac5eb7ab7a16f 100644 --- a/src/bootstrap/src/core/build_steps/check.rs +++ b/src/bootstrap/src/core/build_steps/check.rs @@ -88,7 +88,8 @@ impl Step for Std { vec![], true, false, - ); + ) + .unwrap(); // We skip populating the sysroot in non-zero stage because that'll lead // to rlib/rmeta conflicts if std gets built during this session. @@ -145,7 +146,8 @@ impl Step for Std { vec![], true, false, - ); + ) + .unwrap(); } } @@ -242,7 +244,8 @@ impl Step for Rustc { vec![], true, false, - ); + ) + .unwrap(); let libdir = builder.sysroot_libdir(compiler, target); let hostdir = builder.sysroot_libdir(compiler, compiler.host); @@ -308,7 +311,8 @@ impl Step for CodegenBackend { vec![], true, false, - ); + ) + .unwrap(); } } @@ -374,7 +378,8 @@ impl Step for RustAnalyzer { vec![], true, false, - ); + ) + .unwrap(); /// Cargo's output path in a given stage, compiled by a particular /// compiler for the specified target. @@ -437,7 +442,7 @@ macro_rules! tool_check_step { vec![], true, false, - ); + ).unwrap(); /// Cargo's output path in a given stage, compiled by a particular /// compiler for the specified target. diff --git a/src/bootstrap/src/core/build_steps/clippy.rs b/src/bootstrap/src/core/build_steps/clippy.rs index 01b5e99116ff2..454755db7a107 100644 --- a/src/bootstrap/src/core/build_steps/clippy.rs +++ b/src/bootstrap/src/core/build_steps/clippy.rs @@ -152,7 +152,8 @@ impl Step for Std { vec![], true, false, - ); + ) + .unwrap(); } } @@ -226,7 +227,8 @@ impl Step for Rustc { vec![], true, false, - ); + ) + .unwrap(); } } @@ -295,7 +297,7 @@ macro_rules! lint_any { vec![], true, false, - ); + ).unwrap(); } } )+ diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 46949c510b533..daf00170128fd 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -27,6 +27,7 @@ use crate::core::builder::crate_description; use crate::core::builder::Cargo; use crate::core::builder::{Builder, Kind, PathSet, RunConfig, ShouldRun, Step, TaskPath}; use crate::core::config::{DebuginfoLevel, LlvmLibunwind, RustcLto, TargetSelection}; +use crate::utils::exec::BehaviorOnFailure; use crate::utils::helpers::{ exe, get_clang_cl_resource_dir, is_debug_info, is_dylib, output, symlink_dir, t, up_to_date, }; @@ -289,7 +290,8 @@ impl Step for Std { target_deps, self.is_for_mir_opt_tests, // is_check false, - ); + ) + .unwrap(); builder.ensure(StdLink::from_std( self, @@ -969,7 +971,8 @@ impl Step for Rustc { vec![], false, true, // Only ship rustc_driver.so and .rmeta files, not all intermediate .rlib files. - ); + ) + .unwrap(); // When building `librustc_driver.so` (like `libLLVM.so`) on linux, it can contain // unexpected debuginfo from dependencies, for example from the C++ standard library used in @@ -1381,7 +1384,7 @@ impl Step for CodegenBackend { let tmp_stamp = out_dir.join(".tmp.stamp"); let _guard = builder.msg_build(compiler, format_args!("codegen backend {backend}"), target); - let files = run_cargo(builder, cargo, vec![], &tmp_stamp, vec![], false, false); + let files = run_cargo(builder, cargo, vec![], &tmp_stamp, vec![], false, false).unwrap(); if builder.config.dry_run() { return; } @@ -1907,7 +1910,8 @@ pub fn run_cargo( additional_target_deps: Vec<(PathBuf, DependencyType)>, is_check: bool, rlib_only_metadata: bool, -) -> Vec { +) -> Result, String> { + let failure_behavior = cargo.bootstrap_command.failure_behavior; // `target_root_dir` looks like $dir/$target/release let target_root_dir = stamp.parent().unwrap(); // `target_deps_dir` looks like $dir/$target/release/deps @@ -2011,11 +2015,15 @@ pub fn run_cargo( }); if !ok { - crate::exit!(1); + if failure_behavior == BehaviorOnFailure::Ignore { + return Ok(Vec::new()); + } + + return Err("Command failed.".to_owned()); } if builder.config.dry_run() { - return Vec::new(); + return Ok(Vec::new()); } // Ok now we need to actually find all the files listed in `toplevel`. We've @@ -2038,6 +2046,7 @@ pub fn run_cargo( }); let path_to_add = match max { Some(triple) => triple.0.to_str().unwrap(), + None if failure_behavior == BehaviorOnFailure::Ignore => continue, None => panic!("no output generated for {prefix:?} {extension:?}"), }; if is_dylib(path_to_add) { @@ -2063,7 +2072,7 @@ pub fn run_cargo( new_contents.extend(b"\0"); } t!(fs::write(stamp, &new_contents)); - deps.into_iter().map(|(d, _)| d).collect() + Ok(deps.into_iter().map(|(d, _)| d).collect()) } pub fn stream_cargo( diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index d9609ff67b012..1be33e82cb2c5 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -9,7 +9,6 @@ use crate::core::builder; use crate::core::builder::{Builder, Cargo as CargoCommand, RunConfig, ShouldRun, Step}; use crate::core::config::TargetSelection; use crate::utils::channel::GitInfo; -use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{add_dylib_path, exe, t}; use crate::Compiler; use crate::Mode; @@ -97,9 +96,12 @@ impl Step for ToolBuild { self.source_type, &self.extra_features, ); + cargo.bootstrap_command = cargo.bootstrap_command.allow_failure(); + if !self.allow_features.is_empty() { cargo.allow_features(self.allow_features); } + let _guard = builder.msg_tool( Kind::Build, self.mode, @@ -109,9 +111,20 @@ impl Step for ToolBuild { &self.target, ); - let cargo = Command::from(cargo); - // we check this below - let build_success = builder.run_cmd(BootstrapCommand::from(cargo).allow_failure()); + let stamp = builder + .cargo_out(compiler, Mode::ToolRustc, target) + .join(format!(".{}-check.stamp", self.tool)); + + let build_success = compile::run_cargo( + builder, + cargo, + builder.config.free_args.clone(), + &stamp, + vec![], + false, + false, + ) + .is_ok(); builder.save_toolstate( tool, diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 8695e7c4c2d91..ea99c2bea1930 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -2383,7 +2383,7 @@ impl HostFlags { #[derive(Debug)] pub struct Cargo { - bootstrap_command: BootstrapCommand, + pub(crate) bootstrap_command: BootstrapCommand, compiler: Compiler, target: TargetSelection, rustflags: Rustflags, diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 81ef7a9ac8e3d..958875c344197 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -1,7 +1,7 @@ use std::process::Command; /// What should be done when the command fails. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, PartialEq)] pub enum BehaviorOnFailure { /// Immediately stop bootstrap. Exit,