Skip to content

Commit

Permalink
use BootstrapCommand for tool::ToolBuild step
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
onur-ozkan committed May 30, 2024
1 parent e9768a3 commit e204a32
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 22 deletions.
17 changes: 11 additions & 6 deletions src/bootstrap/src/core/build_steps/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -145,7 +146,8 @@ impl Step for Std {
vec![],
true,
false,
);
)
.unwrap();
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -308,7 +311,8 @@ impl Step for CodegenBackend {
vec![],
true,
false,
);
)
.unwrap();
}
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 5 additions & 3 deletions src/bootstrap/src/core/build_steps/clippy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ impl Step for Std {
vec![],
true,
false,
);
)
.unwrap();
}
}

Expand Down Expand Up @@ -226,7 +227,8 @@ impl Step for Rustc {
vec![],
true,
false,
);
)
.unwrap();
}
}

Expand Down Expand Up @@ -295,7 +297,7 @@ macro_rules! lint_any {
vec![],
true,
false,
);
).unwrap();
}
}
)+
Expand Down
23 changes: 16 additions & 7 deletions src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -1907,7 +1910,8 @@ pub fn run_cargo(
additional_target_deps: Vec<(PathBuf, DependencyType)>,
is_check: bool,
rlib_only_metadata: bool,
) -> Vec<PathBuf> {
) -> Result<Vec<PathBuf>, 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
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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(
Expand Down
21 changes: 17 additions & 4 deletions src/bootstrap/src/core/build_steps/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/utils/exec.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down

0 comments on commit e204a32

Please sign in to comment.