Rollup merge of #123699 - jieyouxu:rmake-refactor, r=Mark-Simulacrum

run-make-support: tidy up support library

- Make `handle_failed_output` take `&Command` instead of having the caller keep doing `format!("{:#?}", s)`.
- Introduce a helper macro for implementing common command wrappers, such as `arg`, `args`, `run`, `run_fail`.
    - Use the helper macro on existing command wrappers and remove manual copy-pasta'd implementations.

src/tools/run-make-support/src/cc.rs

@@ -1,6 +1,6 @@
 use std::env;
 use std::path::Path;
-use std::process::{Command, Output};
+use std::process::Command;
 
 use crate::{bin_name, cygpath_windows, handle_failed_output, is_msvc, is_windows, tmp_dir, uname};
 
@@ -19,6 +19,8 @@ pub struct Cc {
     cmd: Command,
 }
 
+crate::impl_common_helpers!(Cc);
+
 impl Cc {
     /// Construct a new platform-specific C compiler invocation.
     ///
@@ -43,22 +45,6 @@ impl Cc {
         self
     }
 
-    /// Add a *platform-and-compiler-specific* argument. Please consult the docs for the various
-    /// possible C compilers on the various platforms to check which arguments are legal for
-    /// which compiler.
-    pub fn arg(&mut self, flag: &str) -> &mut Self {
-        self.cmd.arg(flag);
-        self
-    }
-
-    /// Add multiple *platform-and-compiler-specific* arguments. Please consult the docs for the
-    /// various possible C compilers on the various platforms to check which arguments are legal
-    /// for which compiler.
-    pub fn args(&mut self, args: &[&str]) -> &mut Self {
-        self.cmd.args(args);
-        self
-    }
-
     /// Specify `-o` or `-Fe`/`-Fo` depending on platform/compiler. This assumes that the executable
     /// is under `$TMPDIR`.
     pub fn out_exe(&mut self, name: &str) -> &mut Self {
@@ -85,25 +71,6 @@ impl Cc {
 
         self
     }
-
-    /// Run the constructed C invocation command and assert that it is successfully run.
-    #[track_caller]
-    pub fn run(&mut self) -> Output {
-        let caller_location = std::panic::Location::caller();
-        let caller_line_number = caller_location.line();
-
-        let output = self.cmd.output().unwrap();
-        if !output.status.success() {
-            handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number);
-        }
-        output
-    }
-
-    /// Inspect what the underlying [`Command`] is up to the current construction.
-    pub fn inspect(&mut self, f: impl FnOnce(&Command)) -> &mut Self {
-        f(&self.cmd);
-        self
-    }
 }
 
 /// `EXTRACFLAGS`

src/tools/run-make-support/src/lib.rs

@@ -1,3 +1,8 @@
+//! `run-make-support` is a support library for run-make tests. It provides command wrappers and
+//! convenience utility functions to help test writers reduce duplication. The support library
+//! notably is built via cargo: this means that if your test wants some non-trivial utility, such
+//! as `object` or `wasmparser`, they can be re-exported and be made available through this library.
+
 pub mod cc;
 pub mod run;
 pub mod rustc;
@@ -82,7 +87,7 @@ pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
     cygpath.arg(path.as_ref());
     let output = cygpath.output().unwrap();
     if !output.status.success() {
-        handle_failed_output(&format!("{:#?}", cygpath), output, caller_line_number);
+        handle_failed_output(&cygpath, output, caller_line_number);
     }
     let s = String::from_utf8(output.stdout).unwrap();
     // cygpath -w can attach a newline
@@ -98,18 +103,18 @@ pub fn uname() -> String {
     let mut uname = Command::new("uname");
     let output = uname.output().unwrap();
     if !output.status.success() {
-        handle_failed_output(&format!("{:#?}", uname), output, caller_line_number);
+        handle_failed_output(&uname, output, caller_line_number);
     }
     String::from_utf8(output.stdout).unwrap()
 }
 
-fn handle_failed_output(cmd: &str, output: Output, caller_line_number: u32) -> ! {
+fn handle_failed_output(cmd: &Command, output: Output, caller_line_number: u32) -> ! {
     if output.status.success() {
-        eprintln!("command incorrectly succeeded at line {caller_line_number}");
+        eprintln!("command unexpectedly succeeded at line {caller_line_number}");
     } else {
         eprintln!("command failed at line {caller_line_number}");
     }
-    eprintln!("{cmd}");
+    eprintln!("{cmd:?}");
     eprintln!("output status: `{}`", output.status);
     eprintln!("=== STDOUT ===\n{}\n\n", String::from_utf8(output.stdout).unwrap());
     eprintln!("=== STDERR ===\n{}\n\n", String::from_utf8(output.stderr).unwrap());
@@ -129,3 +134,127 @@ pub fn set_host_rpath(cmd: &mut Command) {
         env::join_paths(paths.iter()).unwrap()
     });
 }
+
+/// Implement common helpers for command wrappers. This assumes that the command wrapper is a struct
+/// containing a `cmd: Command` field. The provided helpers are:
+///
+/// 1. Generic argument acceptors: `arg` and `args` (delegated to [`Command`]). These are intended
+///    to be *fallback* argument acceptors, when specific helpers don't make sense. Prefer to add
+///    new specific helper methods over relying on these generic argument providers.
+/// 2. Environment manipulation methods: `env`, `env_remove` and `env_clear`: these delegate to
+///    methods of the same name on [`Command`].
+/// 3. Output and execution: `output`, `run` and `run_fail` are provided. `output` waits for the
+///    command to finish running and returns the process's [`Output`]. `run` and `run_fail` are
+///    higher-level convenience methods which waits for the command to finish running and assert
+///    that the command successfully ran or failed as expected. Prefer `run` and `run_fail` when
+///    possible.
+///
+/// Example usage:
+///
+/// ```ignore (illustrative)
+/// struct CommandWrapper { cmd: Command }
+///
+/// crate::impl_common_helpers!(CommandWrapper);
+///
+/// impl CommandWrapper {
+///     // ... additional specific helper methods
+/// }
+/// ```
+///
+/// [`Command`]: ::std::process::Command
+/// [`Output`]: ::std::process::Output
+macro_rules! impl_common_helpers {
+    ($wrapper: ident) => {
+        impl $wrapper {
+            /// Specify an environment variable.
+            pub fn env<K, V>(&mut self, key: K, value: V) -> &mut Self
+            where
+                K: AsRef<::std::ffi::OsStr>,
+                V: AsRef<::std::ffi::OsStr>,
+            {
+                self.cmd.env(key, value);
+                self
+            }
+
+            /// Remove an environmental variable.
+            pub fn env_remove<K>(&mut self, key: K) -> &mut Self
+            where
+                K: AsRef<::std::ffi::OsStr>,
+            {
+                self.cmd.env_remove(key);
+                self
+            }
+
+            /// Clear all environmental variables.
+            pub fn env_var(&mut self) -> &mut Self {
+                self.cmd.env_clear();
+                self
+            }
+
+            /// Generic command argument provider. Prefer specific helper methods if possible.
+            /// Note that for some executables, arguments might be platform specific. For C/C++
+            /// compilers, arguments might be platform *and* compiler specific.
+            pub fn arg<S>(&mut self, arg: S) -> &mut Self
+            where
+                S: AsRef<::std::ffi::OsStr>,
+            {
+                self.cmd.arg(arg);
+                self
+            }
+
+            /// Generic command arguments provider. Prefer specific helper methods if possible.
+            /// Note that for some executables, arguments might be platform specific. For C/C++
+            /// compilers, arguments might be platform *and* compiler specific.
+            pub fn args<S>(&mut self, args: &[S]) -> &mut Self
+            where
+                S: AsRef<::std::ffi::OsStr>,
+            {
+                self.cmd.args(args);
+                self
+            }
+
+            /// Inspect what the underlying [`Command`][::std::process::Command] is up to the
+            /// current construction.
+            pub fn inspect<I>(&mut self, inspector: I) -> &mut Self
+            where
+                I: FnOnce(&::std::process::Command),
+            {
+                inspector(&self.cmd);
+                self
+            }
+
+            /// Get the [`Output`][::std::process::Output] of the finished process.
+            pub fn output(&mut self) -> ::std::process::Output {
+                self.cmd.output().expect("failed to get output of finished process")
+            }
+
+            /// Run the constructed command and assert that it is successfully run.
+            #[track_caller]
+            pub fn run(&mut self) -> ::std::process::Output {
+                let caller_location = ::std::panic::Location::caller();
+                let caller_line_number = caller_location.line();
+
+                let output = self.cmd.output().unwrap();
+                if !output.status.success() {
+                    handle_failed_output(&self.cmd, output, caller_line_number);
+                }
+                output
+            }
+
+            /// Run the constructed command and assert that it does not successfully run.
+            #[track_caller]
+            pub fn run_fail(&mut self) -> ::std::process::Output {
+                let caller_location = ::std::panic::Location::caller();
+                let caller_line_number = caller_location.line();
+
+                let output = self.cmd.output().unwrap();
+                if output.status.success() {
+                    handle_failed_output(&self.cmd, output, caller_line_number);
+                }
+                output
+            }
+        }
+    };
+}
+
+pub(crate) use impl_common_helpers;

src/tools/run-make-support/src/run.rs

@@ -45,7 +45,7 @@ pub fn run(name: &str) -> Output {
 
     let (cmd, output) = run_common(name);
     if !output.status.success() {
-        handle_failed_output(&format!("{:#?}", cmd), output, caller_line_number);
+        handle_failed_output(&cmd, output, caller_line_number);
     }
     output
 }
@@ -58,7 +58,7 @@ pub fn run_fail(name: &str) -> Output {
 
     let (cmd, output) = run_common(name);
     if output.status.success() {
-        handle_failed_output(&format!("{:#?}", cmd), output, caller_line_number);
+        handle_failed_output(&cmd, output, caller_line_number);
     }
     output
 }

src/tools/run-make-support/src/rustc.rs

@@ -1,5 +1,5 @@
 use std::env;
-use std::ffi::{OsStr, OsString};
+use std::ffi::OsString;
 use std::path::Path;
 use std::process::{Command, Output};
 
@@ -21,6 +21,8 @@ pub struct Rustc {
     cmd: Command,
 }
 
+crate::impl_common_helpers!(Rustc);
+
 fn setup_common() -> Command {
     let rustc = env::var("RUSTC").unwrap();
     let mut cmd = Command::new(rustc);
@@ -133,12 +135,6 @@ impl Rustc {
         self
     }
 
-    /// Generic command argument provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`.
-    pub fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Self {
-        self.cmd.arg(arg);
-        self
-    }
-
     /// Specify the crate type.
     pub fn crate_type(&mut self, crate_type: &str) -> &mut Self {
         self.cmd.arg("--crate-type");
@@ -153,64 +149,15 @@ impl Rustc {
         self
     }
 
-    /// Generic command arguments provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`.
-    pub fn args<S: AsRef<OsStr>>(&mut self, args: &[S]) -> &mut Self {
-        self.cmd.args(args);
-        self
-    }
-
-    pub fn env(&mut self, name: impl AsRef<OsStr>, value: impl AsRef<OsStr>) -> &mut Self {
-        self.cmd.env(name, value);
-        self
-    }
-
-    // Command inspection, output and running helper methods
-
-    /// Get the [`Output`][std::process::Output] of the finished `rustc` process.
-    pub fn output(&mut self) -> Output {
-        self.cmd.output().unwrap()
-    }
-
-    /// Run the constructed `rustc` command and assert that it is successfully run.
-    #[track_caller]
-    pub fn run(&mut self) -> Output {
-        let caller_location = std::panic::Location::caller();
-        let caller_line_number = caller_location.line();
-
-        let output = self.cmd.output().unwrap();
-        if !output.status.success() {
-            handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number);
-        }
-        output
-    }
-
-    #[track_caller]
-    pub fn run_fail(&mut self) -> Output {
-        let caller_location = std::panic::Location::caller();
-        let caller_line_number = caller_location.line();
-
-        let output = self.cmd.output().unwrap();
-        if output.status.success() {
-            handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number);
-        }
-        output
-    }
-
     #[track_caller]
     pub fn run_fail_assert_exit_code(&mut self, code: i32) -> Output {
         let caller_location = std::panic::Location::caller();
         let caller_line_number = caller_location.line();
 
         let output = self.cmd.output().unwrap();
         if output.status.code().unwrap() != code {
-            handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number);
+            handle_failed_output(&self.cmd, output, caller_line_number);
         }
         output
     }
-
-    /// Inspect what the underlying [`Command`] is up to the current construction.
-    pub fn inspect(&mut self, f: impl FnOnce(&Command)) -> &mut Self {
-        f(&self.cmd);
-        self
-    }
 }

src/tools/run-make-support/src/rustdoc.rs

@@ -1,5 +1,4 @@
 use std::env;
-use std::ffi::OsStr;
 use std::path::Path;
 use std::process::{Command, Output};
 
@@ -20,6 +19,8 @@ pub struct Rustdoc {
     cmd: Command,
 }
 
+crate::impl_common_helpers!(Rustdoc);
+
 fn setup_common() -> Command {
     let rustdoc = env::var("RUSTDOC").unwrap();
     let mut cmd = Command::new(rustdoc);
@@ -61,33 +62,14 @@ impl Rustdoc {
         self
     }
 
-    /// Generic command argument provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`.
-    pub fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Self {
-        self.cmd.arg(arg);
-        self
-    }
-
-    /// Run the build `rustdoc` command and assert that the run is successful.
-    #[track_caller]
-    pub fn run(&mut self) -> Output {
-        let caller_location = std::panic::Location::caller();
-        let caller_line_number = caller_location.line();
-
-        let output = self.cmd.output().unwrap();
-        if !output.status.success() {
-            handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number);
-        }
-        output
-    }
-
     #[track_caller]
     pub fn run_fail_assert_exit_code(&mut self, code: i32) -> Output {
         let caller_location = std::panic::Location::caller();
         let caller_line_number = caller_location.line();
 
         let output = self.cmd.output().unwrap();
         if output.status.code().unwrap() != code {
-            handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number);
+            handle_failed_output(&self.cmd, output, caller_line_number);
         }
         output
     }