Repository: rust-lang/rust
Pull request: run_make_support: 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.

<!-- homu-ignore:start -->
<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r? <reviewer name>
-->
<!-- homu-ignore:end -->


Changed files:
filename: src/tools/run-make-support/src/command.rs
patch: @@ -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<I>(&mut self, inspector: I) -> &mut Self
+    pub fn inspect<I>(&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
     }

filename: src/tools/run-make-support/src/run.rs
patch: @@ -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`]

filename: src/tools/run-make-support/src/targets.rs
patch: @@ -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()
 }

filename: src/tools/run-make-support/src/util.rs
patch: @@ -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)
 }