mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #116581 - Kobzol:bootstrap-cmd-run, r=onur-ozkan

Browse Source 
Centralize command running in boostrap (part one)

This PR tries to consolidate the various `run, try_run, run_quiet, run_quiet_delaying_failure, run_delaying_failure` etc. methods on `Builder`. This PR only touches command execution which doesn't produce output that would be later read by bootstrap, and it also only refactors spawning of commands that happens after a builder is created (commands executed during download & git submodule checkout are left as-is, for now).

The `run_cmd` method is quite meaty, but I expect that it will be changing rapidly soon, so I considered it easy to kept everything in a single method, and only after things settle down a bit, then maybe again split it up a bit.

I still kept the original shortcut methods like `run_quiet_delaying_failure`, but they now only delegate to `run_cmd`. I tried to keep the original behavior (or as close to it as possible) for all the various commands, but it is a giant mess, so there may be some deviations. Notably, `cmd.output()` is now always called, instead of just `status()`, which was called previously in some situations.

Apart from the refactored methods, there is also `Config::try_run`, `check_run`, methods that run commands that produce output, oh my… that's left for follow-up PRs :)

The driving goal of this (and following) refactors is to centralize command execution in bootstrap on a single place, to make command mocking feasible.

r? `@onur-ozkan`
This commit is contained in:
bors 2023-10-26 20:15:16 +00:00
commit aa1a71e9e9
6 changed files with 159 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/bootstrap/src/core/build_steps/test.rs
View File

@ -27,6 +27,7 @@
use crate::core::config::TargetSelection;
use crate::utils;
use crate::utils::cache::{Interned, INTERNER};
use crate::utils::exec::BootstrapCommand;
use crate::utils::helpers::{
self, add_link_lib_path, dylib_path, dylib_path_var, output, t, up_to_date,
};
@ -808,8 +809,8 @@ fn run(self, builder: &Builder<'_>) {
let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host);
#[allow(deprecated)] // Clippy reports errors if it blessed the outputs
if builder.config.try_run(&mut cargo).is_ok() {
// Clippy reports errors if it blessed the outputs
if builder.run_cmd(BootstrapCommand::from(&mut cargo).allow_failure()) {
// The tests succeeded; nothing to do.
return;
}
@ -3094,7 +3095,7 @@ fn run(self, builder: &Builder<'_>) {
.arg("testsuite.extended_sysroot");
cargo.args(builder.config.test_args());
#[allow(deprecated)]
builder.config.try_run(&mut cargo.into()).unwrap();
let mut cmd: Command = cargo.into();
builder.run_cmd(BootstrapCommand::from(&mut cmd).fail_fast());
}
}

5
src/bootstrap/src/core/build_steps/tool.rs
View File

@ -8,6 +8,7 @@
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;
@ -108,8 +109,8 @@ fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
);
let mut cargo = Command::from(cargo);
#[allow(deprecated)] // we check this in `is_optional_tool` in a second
let is_expected = builder.config.try_run(&mut cargo).is_ok();
// we check this in `is_optional_tool` in a second
let is_expected = builder.run_cmd(BootstrapCommand::from(&mut cargo).allow_failure());
builder.save_toolstate(
tool,

131
src/bootstrap/src/lib.rs
View File

@ -23,11 +23,12 @@
use std::fs::{self, File};
use std::io;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::process::{Command, Output, Stdio};
use std::str;
use build_helper::ci::{gha, CiEnv};
use build_helper::exit;
use build_helper::util::fail;
use filetime::FileTime;
use once_cell::sync::OnceCell;
use termcolor::{ColorChoice, StandardStream, WriteColor};
@ -39,10 +40,8 @@
use crate::core::config::{DryRun, Target};
use crate::core::config::{LlvmLibunwind, TargetSelection};
use crate::utils::cache::{Interned, INTERNER};
use crate::utils::helpers::{
self, dir_is_empty, exe, libdir, mtime, output, run, run_suppressed, symlink_dir,
try_run_suppressed,
};
use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, OutputMode};
use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir};
mod core;
mod utils;
@ -580,15 +579,15 @@ pub(crate) fn update_submodule(&self, relative_path: &Path) {
}
// Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error).
#[allow(deprecated)] // diff-index reports the modifications through the exit status
let has_local_modifications = self
.config
.try_run(
// diff-index reports the modifications through the exit status
let has_local_modifications = !self.run_cmd(
BootstrapCommand::from(
Command::new("git")
.args(&["diff-index", "--quiet", "HEAD"])
.current_dir(&absolute_path),
)
.is_err();
.allow_failure(),
);
if has_local_modifications {
self.run(Command::new("git").args(&["stash", "push"]).current_dir(&absolute_path));
}
@ -921,55 +920,103 @@ fn rustc_snapshot_sysroot(&self) -> &Path {
/// Runs a command, printing out nice contextual information if it fails.
fn run(&self, cmd: &mut Command) {
if self.config.dry_run() {
return;
}
self.verbose(&format!("running: {cmd:?}"));
run(cmd, self.is_verbose())
self.run_cmd(BootstrapCommand::from(cmd).fail_fast().output_mode(
match self.is_verbose() {
true => OutputMode::PrintAll,
false => OutputMode::PrintOutput,
},
));
}
/// Runs a command, printing out contextual info if it fails, and delaying errors until the build finishes.
pub(crate) fn run_delaying_failure(&self, cmd: &mut Command) -> bool {
self.run_cmd(BootstrapCommand::from(cmd).delay_failure().output_mode(
match self.is_verbose() {
true => OutputMode::PrintAll,
false => OutputMode::PrintOutput,
},
))
}
/// Runs a command, printing out nice contextual information if it fails.
fn run_quiet(&self, cmd: &mut Command) {
if self.config.dry_run() {
return;
}
self.verbose(&format!("running: {cmd:?}"));
run_suppressed(cmd)
self.run_cmd(
BootstrapCommand::from(cmd).fail_fast().output_mode(OutputMode::SuppressOnSuccess),
);
}
/// Runs a command, printing out nice contextual information if it fails.
/// Exits if the command failed to execute at all, otherwise returns its
/// `status.success()`.
fn run_quiet_delaying_failure(&self, cmd: &mut Command) -> bool {
self.run_cmd(
BootstrapCommand::from(cmd).delay_failure().output_mode(OutputMode::SuppressOnSuccess),
)
}
/// A centralized function for running commands that do not return output.
pub(crate) fn run_cmd<'a, C: Into<BootstrapCommand<'a>>>(&self, cmd: C) -> bool {
if self.config.dry_run() {
return true;
}
if !self.fail_fast {
self.verbose(&format!("running: {cmd:?}"));
if !try_run_suppressed(cmd) {
let mut failures = self.delayed_failures.borrow_mut();
failures.push(format!("{cmd:?}"));
return false;
}
} else {
self.run_quiet(cmd);
}
true
}
/// Runs a command, printing out contextual info if it fails, and delaying errors until the build finishes.
pub(crate) fn run_delaying_failure(&self, cmd: &mut Command) -> bool {
if !self.fail_fast {
#[allow(deprecated)] // can't use Build::try_run, that's us
if self.config.try_run(cmd).is_err() {
let mut failures = self.delayed_failures.borrow_mut();
failures.push(format!("{cmd:?}"));
return false;
let command = cmd.into();
self.verbose(&format!("running: {command:?}"));
let (output, print_error) = match command.output_mode {
mode @ (OutputMode::PrintAll | OutputMode::PrintOutput) => (
command.command.status().map(|status| Output {
status,
stdout: Vec::new(),
stderr: Vec::new(),
}),
matches!(mode, OutputMode::PrintAll),
),
OutputMode::SuppressOnSuccess => (command.command.output(), true),
};
let output = match output {
Ok(output) => output,
Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", command, e)),
};
let result = if !output.status.success() {
if print_error {
println!(
"\n\ncommand did not execute successfully: {:?}\n\
expected success, got: {}\n\n\
stdout ----\n{}\n\
stderr ----\n{}\n\n",
command.command,
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
}
Err(())
} else {
self.run(cmd);
Ok(())
};
match result {
Ok(_) => true,
Err(_) => {
match command.failure_behavior {
BehaviorOnFailure::DelayFail => {
if self.fail_fast {
exit!(1);
}
let mut failures = self.delayed_failures.borrow_mut();
failures.push(format!("{command:?}"));
}
BehaviorOnFailure::Exit => {
exit!(1);
}
BehaviorOnFailure::Ignore => {}
}
false
}
}
true
}
pub fn is_verbose_than(&self, level: usize) -> bool {

60
src/bootstrap/src/utils/exec.rs Normal file
View File

@ -0,0 +1,60 @@
use std::process::Command;
/// What should be done when the command fails.
#[derive(Debug, Copy, Clone)]
pub enum BehaviorOnFailure {
/// Immediately stop bootstrap.
Exit,
/// Delay failure until the end of bootstrap invocation.
DelayFail,
/// Ignore the failure, the command can fail in an expected way.
Ignore,
}
/// How should the output of the command be handled.
#[derive(Debug, Copy, Clone)]
pub enum OutputMode {
/// Print both the output (by inheriting stdout/stderr) and also the command itself, if it
/// fails.
PrintAll,
/// Print the output (by inheriting stdout/stderr).
PrintOutput,
/// Suppress the output if the command succeeds, otherwise print the output.
SuppressOnSuccess,
}
/// Wrapper around `std::process::Command`.
#[derive(Debug)]
pub struct BootstrapCommand<'a> {
pub command: &'a mut Command,
pub failure_behavior: BehaviorOnFailure,
pub output_mode: OutputMode,
}
impl<'a> BootstrapCommand<'a> {
pub fn delay_failure(self) -> Self {
Self { failure_behavior: BehaviorOnFailure::DelayFail, ..self }
}
pub fn fail_fast(self) -> Self {
Self { failure_behavior: BehaviorOnFailure::Exit, ..self }
}
pub fn allow_failure(self) -> Self {
Self { failure_behavior: BehaviorOnFailure::Ignore, ..self }
}
pub fn output_mode(self, output_mode: OutputMode) -> Self {
Self { output_mode, ..self }
}
}
impl<'a> From<&'a mut Command> for BootstrapCommand<'a> {
fn from(command: &'a mut Command) -> Self {
Self {
command,
failure_behavior: BehaviorOnFailure::Exit,
output_mode: OutputMode::SuppressOnSuccess,
}
}
}

34
src/bootstrap/src/utils/helpers.rs
View File

@ -3,7 +3,7 @@
//! Simple things like testing the various filesystem operations here and there,
//! not a lot of interesting happenings here unfortunately.
use build_helper::util::{fail, try_run};
use build_helper::util::fail;
use std::env;
use std::fs;
use std::io;
@ -216,12 +216,6 @@ pub fn is_valid_test_suite_arg<'a, P: AsRef<Path>>(
}
}
pub fn run(cmd: &mut Command, print_cmd_on_fail: bool) {
if try_run(cmd, print_cmd_on_fail).is_err() {
crate::exit!(1);
}
}
pub fn check_run(cmd: &mut Command, print_cmd_on_fail: bool) -> bool {
let status = match cmd.status() {
Ok(status) => status,
@ -239,32 +233,6 @@ pub fn check_run(cmd: &mut Command, print_cmd_on_fail: bool) -> bool {
status.success()
}
pub fn run_suppressed(cmd: &mut Command) {
if !try_run_suppressed(cmd) {
crate::exit!(1);
}
}
pub fn try_run_suppressed(cmd: &mut Command) -> bool {
let output = match cmd.output() {
Ok(status) => status,
Err(e) => fail(&format!("failed to execute command: {cmd:?}\nerror: {e}")),
};
if !output.status.success() {
println!(
"\n\ncommand did not execute successfully: {:?}\n\
expected success, got: {}\n\n\
stdout ----\n{}\n\
stderr ----\n{}\n\n",
cmd,
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
}
output.status.success()
}
pub fn make(host: &str) -> PathBuf {
if host.contains("dragonfly")
|| host.contains("freebsd")

1
src/bootstrap/src/utils/mod.rs
View File

@ -6,6 +6,7 @@
pub(crate) mod cc_detect;
pub(crate) mod channel;
pub(crate) mod dylib;
pub(crate) mod exec;
pub(crate) mod helpers;
pub(crate) mod job;
#[cfg(feature = "build-metrics")]