Remove temporary BootstrapCommand trait impls

This commit is contained in:
Jakub Beránek 2024-06-22 12:32:55 +02:00 committed by Jakub Beránek
parent a34d0a8d5f
commit b14ff77c04
9 changed files with 114 additions and 143 deletions

View File

@ -2080,7 +2080,7 @@ pub fn stream_cargo(
tail_args: Vec<String>,
cb: &mut dyn FnMut(CargoMessage<'_>),
) -> bool {
let mut cargo = BootstrapCommand::from(cargo).command;
let mut cargo = cargo.into_cmd().command;
// Instruct Cargo to give us json messages on stdout, critically leaving
// stderr as piped so we can get those pretty colors.
let mut message_format = if builder.config.json_output {

View File

@ -738,7 +738,7 @@ fn doc_std(
format!("library{} in {} format", crate_description(requested_crates), format.as_str());
let _guard = builder.msg_doc(compiler, description, target);
builder.run(cargo);
builder.run(cargo.into_cmd());
builder.cp_link_r(&out_dir, out);
}
@ -863,7 +863,7 @@ fn run(self, builder: &Builder<'_>) {
let proc_macro_out_dir = builder.stage_out(compiler, Mode::Rustc).join("doc");
symlink_dir_force(&builder.config, &out, &proc_macro_out_dir);
builder.run(cargo);
builder.run(cargo.into_cmd());
if !builder.config.dry_run() {
// Sanity check on linked compiler crates
@ -996,7 +996,7 @@ fn run(self, builder: &Builder<'_>) {
symlink_dir_force(&builder.config, &out, &proc_macro_out_dir);
let _guard = builder.msg_doc(compiler, stringify!($tool).to_lowercase(), target);
builder.run(cargo);
builder.run(cargo.into_cmd());
if !builder.config.dry_run() {
// Sanity check on linked doc directories

View File

@ -160,16 +160,15 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
override_builder.add(&format!("!{ignore}")).expect(&ignore);
}
}
let git_available = build
.run(helpers::git(None).print_on_failure().allow_failure().arg("--version"))
.is_success();
let git_available =
build.run(helpers::git(None).capture().allow_failure().arg("--version")).is_success();
let mut adjective = None;
if git_available {
let in_working_tree = build
.run(
helpers::git(Some(&build.src))
.print_on_failure()
.capture()
.allow_failure()
.arg("rev-parse")
.arg("--is-inside-work-tree"),

View File

@ -158,7 +158,7 @@ fn run(self, builder: &Builder<'_>) {
// after another --, so this must be at the end.
miri.args(builder.config.args());
builder.run(miri);
builder.run(miri.into_cmd());
}
}

View File

@ -26,7 +26,7 @@
use crate::core::config::flags::get_completion;
use crate::core::config::flags::Subcommand;
use crate::core::config::TargetSelection;
use crate::utils::exec::{BootstrapCommand, OutputMode};
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,
@ -150,16 +150,13 @@ fn run(self, builder: &Builder<'_>) {
builder.default_doc(&[]);
// Build the linkchecker before calling `msg`, since GHA doesn't support nested groups.
let mut linkchecker = builder.tool_cmd(Tool::Linkchecker);
let linkchecker = builder.tool_cmd(Tool::Linkchecker);
// Run the linkchecker.
let _guard =
builder.msg(Kind::Test, compiler.stage, "Linkcheck", bootstrap_host, bootstrap_host);
let _time = helpers::timeit(builder);
builder.run(
BootstrapCommand::from(linkchecker.arg(builder.out.join(host.triple).join("doc")))
.delay_failure(),
);
builder.run(linkchecker.delay_failure().arg(builder.out.join(host.triple).join("doc")));
}
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@ -217,10 +214,7 @@ fn run(self, builder: &Builder<'_>) {
));
builder.run(
BootstrapCommand::from(
builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)),
)
.delay_failure(),
builder.tool_cmd(Tool::HtmlChecker).delay_failure().arg(builder.doc_out(self.target)),
);
}
}
@ -260,14 +254,13 @@ fn run(self, builder: &Builder<'_>) {
let _time = helpers::timeit(builder);
let mut cmd = builder.tool_cmd(Tool::CargoTest);
let cmd = cmd
.arg(&cargo)
cmd.arg(&cargo)
.arg(&out_dir)
.args(builder.config.test_args())
.env("RUSTC", builder.rustc(compiler))
.env("RUSTDOC", builder.rustdoc(compiler));
add_rustdoc_cargo_linker_args(cmd, builder, compiler.host, LldThreads::No);
builder.run(BootstrapCommand::from(cmd).delay_failure());
add_rustdoc_cargo_linker_args(&mut cmd, builder, compiler.host, LldThreads::No);
builder.run(cmd.delay_failure());
}
}
@ -763,12 +756,12 @@ fn run(self, builder: &Builder<'_>) {
cargo.env("HOST_LIBS", host_libs);
cargo.add_rustc_lib_path(builder);
let mut cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder);
let cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder);
let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host);
// Clippy reports errors if it blessed the outputs
if builder.run(BootstrapCommand::from(&mut cargo).allow_failure()).is_success() {
if builder.run(cargo.allow_failure()).is_success() {
// The tests succeeded; nothing to do.
return;
}
@ -821,7 +814,7 @@ fn run(self, builder: &Builder<'_>) {
.env("RUSTC_BOOTSTRAP", "1");
cmd.args(linker_args(builder, self.compiler.host, LldThreads::No));
builder.run(BootstrapCommand::from(&mut cmd).delay_failure());
builder.run(cmd.delay_failure());
}
}
@ -1099,7 +1092,7 @@ fn run(self, builder: &Builder<'_>) {
}
builder.info("tidy check");
builder.run(BootstrapCommand::from(&mut cmd).delay_failure());
builder.run(cmd.delay_failure());
builder.info("x.py completions check");
let [bash, zsh, fish, powershell] = ["x.py.sh", "x.py.zsh", "x.py.fish", "x.py.ps1"]
@ -1306,7 +1299,7 @@ fn run(self, builder: &Builder<'_>) -> PathBuf {
&[],
);
builder.run(cargo);
builder.run(cargo.into_cmd());
let lib_name = "librun_make_support.rlib";
let lib = builder.tools_dir(self.compiler).join(lib_name);
@ -2187,7 +2180,7 @@ fn run_ext_doc(self, builder: &Builder<'_>) {
compiler.host,
);
let _time = helpers::timeit(builder);
let cmd = BootstrapCommand::from(&mut rustbook_cmd).delay_failure();
let cmd = rustbook_cmd.delay_failure();
let toolstate =
if builder.run(cmd).is_success() { ToolState::TestPass } else { ToolState::TestFail };
builder.save_toolstate(self.name, toolstate);
@ -2318,7 +2311,7 @@ fn run(self, builder: &Builder<'_>) {
let guard =
builder.msg(Kind::Test, compiler.stage, "error-index", compiler.host, compiler.host);
let _time = helpers::timeit(builder);
builder.run(BootstrapCommand::from(&mut tool).output_mode(OutputMode::OnlyOnFailure));
builder.run(tool.capture());
drop(guard);
// The tests themselves need to link to std, so make sure it is
// available.
@ -2349,7 +2342,7 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) ->
cmd = cmd.delay_failure();
if !builder.config.verbose_tests {
cmd = cmd.print_on_failure();
cmd = cmd.capture();
}
builder.run(cmd).is_success()
}
@ -2375,10 +2368,13 @@ fn run(self, builder: &Builder<'_>) {
builder.update_submodule(&relative_path);
let src = builder.src.join(relative_path);
let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook);
let cmd = BootstrapCommand::from(rustbook_cmd.arg("linkcheck").arg(&src)).delay_failure();
let toolstate =
if builder.run(cmd).is_success() { ToolState::TestPass } else { ToolState::TestFail };
let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook).delay_failure();
rustbook_cmd.arg("linkcheck").arg(&src);
let toolstate = if builder.run(rustbook_cmd).is_success() {
ToolState::TestPass
} else {
ToolState::TestFail
};
builder.save_toolstate("rustc-dev-guide", toolstate);
}
}
@ -3347,7 +3343,7 @@ fn run(self, builder: &Builder<'_>) {
.arg("testsuite.extended_sysroot");
cargo.args(builder.config.test_args());
builder.run(cargo);
builder.run(cargo.into_cmd());
}
}
@ -3472,6 +3468,6 @@ fn run(self, builder: &Builder<'_>) {
.arg("--std-tests");
cargo.args(builder.config.test_args());
builder.run(cargo);
builder.run(cargo.into_cmd());
}
}

View File

@ -603,7 +603,7 @@ fn run(self, builder: &Builder<'_>) -> PathBuf {
&self.compiler.host,
&target,
);
builder.run(cargo);
builder.run(cargo.into_cmd());
// Cargo adds a number of paths to the dylib search path on windows, which results in
// the wrong rustdoc being executed. To avoid the conflicting rustdocs, we name the "tool"
@ -858,7 +858,7 @@ fn run(self, builder: &Builder<'_>) -> PathBuf {
&self.extra_features,
);
builder.run(cargo);
builder.run(cargo.into_cmd());
let tool_out = builder
.cargo_out(self.compiler, Mode::ToolRustc, self.target)

View File

@ -2398,6 +2398,10 @@ pub fn new(
cargo
}
pub fn into_cmd(self) -> BootstrapCommand {
self.into()
}
/// Same as `Cargo::new` except this one doesn't configure the linker with `Cargo::configure_linker`
pub fn new_for_mir_opt_tests(
builder: &Builder<'_>,
@ -2622,9 +2626,3 @@ fn from(mut cargo: Cargo) -> BootstrapCommand {
cargo.command
}
}
impl From<Cargo> for Command {
fn from(cargo: Cargo) -> Command {
BootstrapCommand::from(cargo).command
}
}

View File

@ -555,10 +555,7 @@ 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).
// diff-index reports the modifications through the exit status
let has_local_modifications = self
.run(
BootstrapCommand::from(submodule_git().args(["diff-index", "--quiet", "HEAD"]))
.allow_failure(),
)
.run(submodule_git().allow_failure().args(["diff-index", "--quiet", "HEAD"]))
.is_failure();
if has_local_modifications {
self.run(submodule_git().args(["stash", "push"]));
@ -582,7 +579,7 @@ pub fn update_existing_submodules(&self) {
let output = self
.run(
helpers::git(Some(&self.src))
.quiet()
.capture()
.args(["config", "--file"])
.arg(self.config.src.join(".gitmodules"))
.args(["--get-regexp", "path"]),
@ -937,69 +934,71 @@ fn rustc_snapshot_sysroot(&self) -> &Path {
/// Execute a command and return its output.
/// This method should be used for all command executions in bootstrap.
fn run<C: Into<BootstrapCommand>>(&self, command: C) -> CommandOutput {
fn run<C: AsMut<BootstrapCommand>>(&self, mut command: C) -> CommandOutput {
if self.config.dry_run() {
return CommandOutput::default();
}
let mut command = command.into();
let command = command.as_mut();
self.verbose(|| println!("running: {command:?}"));
let output_mode = command.output_mode.unwrap_or_else(|| match self.is_verbose() {
true => OutputMode::All,
false => OutputMode::OnlyOutput,
});
let (output, print_error): (io::Result<CommandOutput>, bool) = match output_mode {
mode @ (OutputMode::All | OutputMode::OnlyOutput) => (
command.command.status().map(|status| status.into()),
matches!(mode, OutputMode::All),
),
mode @ (OutputMode::OnlyOnFailure | OutputMode::Quiet) => (
command.command.output().map(|o| o.into()),
matches!(mode, OutputMode::OnlyOnFailure),
),
let output: io::Result<CommandOutput> = match command.output_mode {
OutputMode::Print => command.command.status().map(|status| status.into()),
OutputMode::CaptureAll => command.command.output().map(|o| o.into()),
OutputMode::CaptureStdout => {
command.command.stderr(Stdio::inherit());
command.command.output().map(|o| o.into())
}
};
let output = match output {
Ok(output) => output,
Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", command, e)),
Err(e) => fail(&format!("failed to execute command: {command:?}\nerror: {e}")),
};
if !output.is_success() {
if print_error {
println!(
"\n\nCommand did not execute successfully.\
\nExpected success, got: {}",
output.status(),
);
use std::fmt::Write;
if !self.is_verbose() {
println!("Add `-v` to see more details.\n");
// Here we build an error message, and below we decide if it should be printed or not.
let mut message = String::new();
writeln!(
message,
"\n\nCommand {command:?} did not execute successfully.\
\nExpected success, got: {}",
output.status(),
)
.unwrap();
// If the output mode is OutputMode::Print, the output has already been printed to
// stdout/stderr, and we thus don't have anything captured to print anyway.
if matches!(command.output_mode, OutputMode::CaptureAll | OutputMode::CaptureStdout) {
writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap();
// Stderr is added to the message only if it was captured
if matches!(command.output_mode, OutputMode::CaptureAll) {
writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap();
}
self.verbose(|| {
println!(
"\nSTDOUT ----\n{}\n\
STDERR ----\n{}\n",
output.stdout(),
output.stderr(),
)
});
}
match command.failure_behavior {
BehaviorOnFailure::DelayFail => {
if self.fail_fast {
println!("{message}");
exit!(1);
}
let mut failures = self.delayed_failures.borrow_mut();
failures.push(format!("{command:?}"));
failures.push(message);
}
BehaviorOnFailure::Exit => {
println!("{message}");
exit!(1);
}
BehaviorOnFailure::Ignore => {}
BehaviorOnFailure::Ignore => {
// If failures are allowed, either the error has been printed already
// (OutputMode::Print) or the user used a capture output mode and wants to
// handle the error output on their own.
}
}
}
output
@ -1490,7 +1489,7 @@ fn extract_beta_rev_from_file<P: AsRef<Path>>(version_file: P) -> Option<String>
// (Note that we use a `..` range, not the `...` symmetric difference.)
self.run(
helpers::git(Some(&self.src))
.quiet()
.capture()
.arg("rev-list")
.arg("--count")
.arg("--merges")

View File

@ -13,18 +13,19 @@ pub enum BehaviorOnFailure {
Ignore,
}
/// How should the output of the command be handled.
/// How should the output of the command be handled (whether it should be captured or printed).
#[derive(Debug, Copy, Clone)]
pub enum OutputMode {
/// Print both the output (by inheriting stdout/stderr) and also the command itself, if it
/// fails.
All,
/// Print the output (by inheriting stdout/stderr).
OnlyOutput,
/// Suppress the output if the command succeeds, otherwise print the output.
OnlyOnFailure,
/// Suppress the output of the command.
Quiet,
/// Prints the stdout/stderr of the command to stdout/stderr of bootstrap (by inheriting these
/// streams).
/// Corresponds to calling `cmd.status()`.
Print,
/// Captures the stdout and stderr of the command into memory.
/// Corresponds to calling `cmd.output()`.
CaptureAll,
/// Captures the stdout of the command into memory, inherits stderr.
/// Corresponds to calling `cmd.output()`.
CaptureStdout,
}
/// Wrapper around `std::process::Command`.
@ -34,10 +35,10 @@ pub enum OutputMode {
/// If you want to delay failures until the end of bootstrap, use [delay_failure].
///
/// By default, the command will print its stdout/stderr to stdout/stderr of bootstrap
/// ([OutputMode::OnlyOutput]). If bootstrap uses verbose mode, then it will also print the
/// command itself in case of failure ([OutputMode::All]).
/// If you want to handle the output programmatically, use `output_mode(OutputMode::OnlyOnFailure)`,
/// which will print the output only if the command fails.
/// ([OutputMode::Print]).
/// If you want to handle the output programmatically, use [BootstrapCommand::capture].
///
/// Bootstrap will print a debug log to stdout if the command fails and failure is not allowed.
///
/// [allow_failure]: BootstrapCommand::allow_failure
/// [delay_failure]: BootstrapCommand::delay_failure
@ -45,12 +46,16 @@ pub enum OutputMode {
pub struct BootstrapCommand {
pub command: Command,
pub failure_behavior: BehaviorOnFailure,
pub output_mode: Option<OutputMode>,
pub output_mode: OutputMode,
}
impl BootstrapCommand {
pub fn new<S: AsRef<OsStr>>(program: S) -> Self {
Command::new(program).into()
Self {
command: Command::new(program),
failure_behavior: BehaviorOnFailure::Exit,
output_mode: OutputMode::Print,
}
}
pub fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Self {
@ -106,52 +111,22 @@ pub fn allow_failure(self) -> Self {
Self { failure_behavior: BehaviorOnFailure::Ignore, ..self }
}
/// Do not print the output of the command, unless it fails.
pub fn print_on_failure(self) -> Self {
self.output_mode(OutputMode::OnlyOnFailure)
/// Capture the output of the command, do not print it.
pub fn capture(self) -> Self {
Self { output_mode: OutputMode::CaptureAll, ..self }
}
/// Do not print the output of the command.
pub fn quiet(self) -> Self {
self.output_mode(OutputMode::Quiet)
}
pub fn output_mode(self, output_mode: OutputMode) -> Self {
Self { output_mode: Some(output_mode), ..self }
/// Capture stdout of the command, do not print it.
pub fn capture_stdout(self) -> Self {
Self { output_mode: OutputMode::CaptureStdout, ..self }
}
}
/// FIXME: This implementation is temporary, until all `Command` invocations are migrated to
/// `BootstrapCommand`.
impl<'a> From<&'a mut BootstrapCommand> for BootstrapCommand {
fn from(command: &'a mut BootstrapCommand) -> Self {
// This is essentially a manual `Command::clone`
let mut cmd = Command::new(command.command.get_program());
if let Some(dir) = command.command.get_current_dir() {
cmd.current_dir(dir);
}
cmd.args(command.command.get_args());
for (key, value) in command.command.get_envs() {
match value {
Some(value) => {
cmd.env(key, value);
}
None => {
cmd.env_remove(key);
}
}
}
Self {
command: cmd,
output_mode: command.output_mode,
failure_behavior: command.failure_behavior,
}
}
}
impl From<Command> for BootstrapCommand {
fn from(command: Command) -> Self {
Self { command, failure_behavior: BehaviorOnFailure::Exit, output_mode: None }
/// This implementation exists to make it possible to pass both [BootstrapCommand] and
/// `&mut BootstrapCommand` to `Build.run()`.
impl AsMut<BootstrapCommand> for BootstrapCommand {
fn as_mut(&mut self) -> &mut BootstrapCommand {
self
}
}
@ -176,6 +151,10 @@ pub fn stdout(&self) -> String {
String::from_utf8(self.0.stdout.clone()).expect("Cannot parse process stdout as UTF-8")
}
pub fn stdout_if_ok(&self) -> Option<String> {
if self.is_success() { Some(self.stdout()) } else { None }
}
pub fn stderr(&self) -> String {
String::from_utf8(self.0.stderr.clone()).expect("Cannot parse process stderr as UTF-8")
}