From 00dae097c4e0a87e468eec591ee8b00e2883c368 Mon Sep 17 00:00:00 2001 From: Jynn Nelson Date: Sun, 15 Mar 2026 08:27:58 +0100 Subject: [PATCH] Print the command and output even if it succeeds Previously, it was quite hard to get access to this output because it was printed to a buffer in memory and then discarded. This output is already hidden behind libtest's output capturing, we don't need to hide it twice. --- src/tools/run-make-support/src/command.rs | 8 +++++-- src/tools/run-make-support/src/run.rs | 28 ++++------------------- src/tools/run-make-support/src/targets.rs | 9 +------- src/tools/run-make-support/src/util.rs | 20 ++++++++++------ 4 files changed, 25 insertions(+), 40 deletions(-) diff --git a/src/tools/run-make-support/src/command.rs b/src/tools/run-make-support/src/command.rs index f4a09f9faae8f..0216ae7a30ddb 100644 --- a/src/tools/run-make-support/src/command.rs +++ b/src/tools/run-make-support/src/command.rs @@ -6,7 +6,7 @@ use std::{ffi, panic}; use build_helper::drop_bomb::DropBomb; -use crate::util::handle_failed_output; +use crate::util::{handle_failed_output, verbose_print_command}; use crate::{ assert_contains, assert_contains_regex, assert_equals, assert_not_contains, assert_not_contains_regex, @@ -156,7 +156,7 @@ impl Command { /// Inspect what the underlying [`std::process::Command`] is up to the /// current construction. - pub fn inspect(&mut self, inspector: I) -> &mut Self + pub fn inspect(&self, inspector: I) -> &Self where I: FnOnce(&StdCommand), { @@ -233,6 +233,8 @@ impl Command { let output = self.command_output(); if !output.status().success() { handle_failed_output(&self, output, panic::Location::caller().line()); + } else { + verbose_print_command(self, &output); } output } @@ -245,6 +247,8 @@ impl Command { let output = self.command_output(); if output.status().success() { handle_failed_output(&self, output, panic::Location::caller().line()); + } else { + verbose_print_command(self, &output); } output } diff --git a/src/tools/run-make-support/src/run.rs b/src/tools/run-make-support/src/run.rs index b95f3a5cfe5a8..43f0473ddf4a1 100644 --- a/src/tools/run-make-support/src/run.rs +++ b/src/tools/run-make-support/src/run.rs @@ -1,9 +1,8 @@ +use std::env; use std::ffi::{OsStr, OsString}; use std::path::PathBuf; -use std::{env, panic}; use crate::command::{Command, CompletedProcess}; -use crate::util::handle_failed_output; use crate::{cwd, env_var}; #[track_caller] @@ -59,43 +58,26 @@ fn run_common(name: &str, args: Option<&[&str]>) -> Command { }); cmd.env("LC_ALL", "C"); // force english locale + cmd.inspect(|std_cmd| eprintln!("running: {std_cmd:?}")); cmd } /// Run a built binary and make sure it succeeds. #[track_caller] pub fn run(name: &str) -> CompletedProcess { - let caller = panic::Location::caller(); - let mut cmd = run_common(name, None); - let output = cmd.run(); - if !output.status().success() { - handle_failed_output(&cmd, output, caller.line()); - } - output + run_common(name, None).run() } /// Run a built binary with one or more argument(s) and make sure it succeeds. #[track_caller] pub fn run_with_args(name: &str, args: &[&str]) -> CompletedProcess { - let caller = panic::Location::caller(); - let mut cmd = run_common(name, Some(args)); - let output = cmd.run(); - if !output.status().success() { - handle_failed_output(&cmd, output, caller.line()); - } - output + run_common(name, Some(args)).run() } /// Run a built binary and make sure it fails. #[track_caller] pub fn run_fail(name: &str) -> CompletedProcess { - let caller = panic::Location::caller(); - let mut cmd = run_common(name, None); - let output = cmd.run_fail(); - if output.status().success() { - handle_failed_output(&cmd, output, caller.line()); - } - output + run_common(name, None).run_fail() } /// Create a new custom [`Command`]. This should be preferred to creating [`std::process::Command`] diff --git a/src/tools/run-make-support/src/targets.rs b/src/tools/run-make-support/src/targets.rs index 6288f5f7c59da..bbc36889b1a4b 100644 --- a/src/tools/run-make-support/src/targets.rs +++ b/src/tools/run-make-support/src/targets.rs @@ -2,7 +2,6 @@ use std::panic; use crate::command::Command; use crate::env_var; -use crate::util::handle_failed_output; /// `TARGET` #[must_use] @@ -81,11 +80,5 @@ pub fn llvm_components_contain(component: &str) -> bool { #[track_caller] #[must_use] pub fn uname() -> String { - let caller = panic::Location::caller(); - let mut uname = Command::new("uname"); - let output = uname.run(); - if !output.status().success() { - handle_failed_output(&uname, output, caller.line()); - } - output.stdout_utf8() + Command::new("uname").run().stdout_utf8() } diff --git a/src/tools/run-make-support/src/util.rs b/src/tools/run-make-support/src/util.rs index 7908dc1f7b3d9..f44b3861d11d3 100644 --- a/src/tools/run-make-support/src/util.rs +++ b/src/tools/run-make-support/src/util.rs @@ -4,6 +4,18 @@ use crate::command::{Command, CompletedProcess}; use crate::env::env_var; use crate::path_helpers::cwd; +pub(crate) fn verbose_print_command(cmd: &Command, output: &CompletedProcess) { + cmd.inspect(|std_cmd| { + eprintln!("{std_cmd:?}"); + }); + eprintln!("output status: `{}`", output.status()); + eprintln!("=== STDOUT ===\n{}\n\n", output.stdout_utf8()); + eprintln!("=== STDERR ===\n{}\n\n", output.stderr_utf8()); + if !cmd.get_context().is_empty() { + eprintln!("Context:\n{}", cmd.get_context()); + } +} + /// If a given [`Command`] failed (as indicated by its [`CompletedProcess`]), verbose print the /// executed command, failure location, output status and stdout/stderr, and abort the process with /// exit code `1`. @@ -17,13 +29,7 @@ pub(crate) fn handle_failed_output( } else { eprintln!("command failed at line {caller_line_number}"); } - eprintln!("{cmd:?}"); - eprintln!("output status: `{}`", output.status()); - eprintln!("=== STDOUT ===\n{}\n\n", output.stdout_utf8()); - eprintln!("=== STDERR ===\n{}\n\n", output.stderr_utf8()); - if !cmd.get_context().is_empty() { - eprintln!("Context:\n{}", cmd.get_context()); - } + verbose_print_command(cmd, &output); std::process::exit(1) }