From e994534242e742d52c7bab0f0637bc4ed14ec90d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Tue, 9 Apr 2024 21:11:37 +0000 Subject: [PATCH 1/4] run-make-support: make `handle_failed_output` take a `&Command` --- src/tools/run-make-support/src/lib.rs | 10 +++++----- src/tools/run-make-support/src/run.rs | 4 ++-- src/tools/run-make-support/src/rustc.rs | 2 +- src/tools/run-make-support/src/rustdoc.rs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index e0a278d634c25..f4d2634c470ec 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -82,7 +82,7 @@ pub fn cygpath_windows>(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 +98,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()); diff --git a/src/tools/run-make-support/src/run.rs b/src/tools/run-make-support/src/run.rs index e33ea9d6e40a3..9aad91f1b4630 100644 --- a/src/tools/run-make-support/src/run.rs +++ b/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 } diff --git a/src/tools/run-make-support/src/rustc.rs b/src/tools/run-make-support/src/rustc.rs index b76711b4e979a..e9552cd881f6e 100644 --- a/src/tools/run-make-support/src/rustc.rs +++ b/src/tools/run-make-support/src/rustc.rs @@ -203,7 +203,7 @@ impl Rustc { 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 } diff --git a/src/tools/run-make-support/src/rustdoc.rs b/src/tools/run-make-support/src/rustdoc.rs index 02af216d80506..fc1d99b70a2f1 100644 --- a/src/tools/run-make-support/src/rustdoc.rs +++ b/src/tools/run-make-support/src/rustdoc.rs @@ -87,7 +87,7 @@ impl Rustdoc { 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 } From b22099d4e0be8819b1cf476d32d02459a594278b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Sat, 13 Apr 2024 14:49:44 +0000 Subject: [PATCH 2/4] run-make-support: introduce macro for common methods to avoid repetition Add a helper macro for adding common methods to command wrappers. Common methods include helpers that delegate to `Command` and running methods. - `arg` and `args` (delegates to `Command`) - `env`, `env_remove` and `env_clear` (delegates to `Command`) - `output`, `run` and `run_fail` This helps to avoid needing to copy-pasta / reimplement these common methods on a new command wrapper, which hopefully reduces the friction for run-make test writers wanting to introduce new command wrappers. --- src/tools/run-make-support/src/lib.rs | 124 ++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index f4d2634c470ec..504bd6030d9e2 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -129,3 +129,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(&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(&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(&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(&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(&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; From 3d115b9cc9cf511b49d2e4812ae6a530bc6f61df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Wed, 10 Apr 2024 13:41:51 +0000 Subject: [PATCH 3/4] run-make-support: use macro to implement common methods Removes the manual copy-pasta'd implementation of common methods. --- src/tools/run-make-support/src/cc.rs | 39 ++------------- src/tools/run-make-support/src/rustc.rs | 59 ++--------------------- src/tools/run-make-support/src/rustdoc.rs | 22 +-------- 3 files changed, 8 insertions(+), 112 deletions(-) diff --git a/src/tools/run-make-support/src/cc.rs b/src/tools/run-make-support/src/cc.rs index 2c9ad4f17006c..a2d51902652bc 100644 --- a/src/tools/run-make-support/src/cc.rs +++ b/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` diff --git a/src/tools/run-make-support/src/rustc.rs b/src/tools/run-make-support/src/rustc.rs index e9552cd881f6e..ebda151b908b0 100644 --- a/src/tools/run-make-support/src/rustc.rs +++ b/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>(&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,49 +149,6 @@ impl Rustc { self } - /// Generic command arguments provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`. - pub fn args>(&mut self, args: &[S]) -> &mut Self { - self.cmd.args(args); - self - } - - pub fn env(&mut self, name: impl AsRef, value: impl AsRef) -> &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(); @@ -207,10 +160,4 @@ impl Rustc { } 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 - } } diff --git a/src/tools/run-make-support/src/rustdoc.rs b/src/tools/run-make-support/src/rustdoc.rs index fc1d99b70a2f1..1054ac83c103c 100644 --- a/src/tools/run-make-support/src/rustdoc.rs +++ b/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,25 +62,6 @@ impl Rustdoc { self } - /// Generic command argument provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`. - pub fn arg>(&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(); From a67a119424377ec53a3b36bca251a30637ae42dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Wed, 10 Apr 2024 13:59:58 +0000 Subject: [PATCH 4/4] run-make-support: add some top-level docs --- src/tools/run-make-support/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index 504bd6030d9e2..47b46a0a699c1 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/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;