run-make-support: arm command with drop bombs

- Update all command wrappers and command construction helpers with
  `#[track_caller]` where suitable to help the drop bomb panic message.
- Remove `Deref`/`DerefMut` for `Command` because it was causing issues
  with resolving to `std::process::Command` in a method call chain.

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

@@ -7,6 +7,7 @@ use crate::{bin_name, cygpath_windows, env_var, is_msvc, is_windows, uname};
 ///
 /// WARNING: This means that what flags are accepted by the underlying C compiler is
 /// platform- AND compiler-specific. Consult the relevant docs for `gcc`, `clang` and `mvsc`.
+#[track_caller]
 pub fn cc() -> Cc {
     Cc::new()
 }
@@ -25,6 +26,7 @@ impl Cc {
     ///
     /// WARNING: This means that what flags are accepted by the underlying C compile is
     /// platform- AND compiler-specific. Consult the relevant docs for `gcc`, `clang` and `mvsc`.
+    #[track_caller]
     pub fn new() -> Self {
         let compiler = env_var("CC");
 
@@ -84,11 +86,6 @@ impl Cc {
         self.cmd.arg(path.as_ref());
         self
     }
-
-    /// Get the [`Output`][::std::process::Output] of the finished process.
-    pub fn command_output(&mut self) -> ::std::process::Output {
-        self.cmd.output().expect("failed to get output of finished process")
-    }
 }
 
 /// `EXTRACFLAGS`

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

@@ -4,6 +4,7 @@ use crate::command::Command;
 use crate::{bin_name, env_var};
 
 /// Construct a new `clang` invocation. `clang` is not always available for all targets.
+#[track_caller]
 pub fn clang() -> Clang {
     Clang::new()
 }
@@ -18,6 +19,7 @@ crate::impl_common_helpers!(Clang);
 
 impl Clang {
     /// Construct a new `clang` invocation. `clang` is not always available for all targets.
+    #[track_caller]
     pub fn new() -> Self {
         let clang = env_var("CLANG");
         let cmd = Command::new(clang);

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

@@ -1,36 +1,107 @@
-use crate::{assert_not_contains, handle_failed_output};
+use std::ffi;
 use std::ffi::OsStr;
 use std::io::Write;
-use std::ops::{Deref, DerefMut};
+use std::panic;
+use std::path::Path;
 use std::process::{Command as StdCommand, ExitStatus, Output, Stdio};
 
-/// This is a custom command wrapper that simplifies working with commands
+use crate::drop_bomb::DropBomb;
-/// and makes it easier to ensure that we check the exit status of executed
+use crate::{assert_not_contains, handle_failed_output};
-/// processes.
+
+/// This is a custom command wrapper that simplifies working with commands and makes it easier to
+/// ensure that we check the exit status of executed processes.
+///
+/// # A [`Command`] must be executed
+///
+/// A [`Command`] is armed by a [`DropBomb`] on construction to enforce that it will be executed. If
+/// a [`Command`] is constructed but never executed, the drop bomb will explode and cause the test
+/// to panic. Execution methods [`run`] and [`run_fail`] will defuse the drop bomb. A test
+/// containing constructed but never executed commands is dangerous because it can give a false
+/// sense of confidence.
+///
+/// [`run`]: Self::run
+/// [`run_fail`]: Self::run_fail
 #[derive(Debug)]
 pub struct Command {
     cmd: StdCommand,
     stdin: Option<Box<[u8]>>,
+    drop_bomb: DropBomb,
 }
 
 impl Command {
-    pub fn new<S: AsRef<OsStr>>(program: S) -> Self {
+    #[track_caller]
-        Self { cmd: StdCommand::new(program), stdin: None }
+    pub fn new<P: AsRef<OsStr>>(program: P) -> Self {
+        let program = program.as_ref();
+        Self { cmd: StdCommand::new(program), stdin: None, drop_bomb: DropBomb::arm(program) }
     }
 
     pub fn set_stdin(&mut self, stdin: Box<[u8]>) {
         self.stdin = Some(stdin);
     }
 
+    /// Specify an environment variable.
+    pub fn env<K, V>(&mut self, key: K, value: V) -> &mut Self
+    where
+        K: AsRef<ffi::OsStr>,
+        V: AsRef<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<ffi::OsStr>,
+    {
+        self.cmd.env_remove(key);
+        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<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<ffi::OsStr>,
+    {
+        self.cmd.args(args);
+        self
+    }
+
+    /// Inspect what the underlying [`std::process::Command`] is up to the
+    /// current construction.
+    pub fn inspect<I>(&mut self, inspector: I) -> &mut Self
+    where
+        I: FnOnce(&StdCommand),
+    {
+        inspector(&self.cmd);
+        self
+    }
+
+    /// Set the path where the command will be run.
+    pub fn current_dir<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
+        self.cmd.current_dir(path);
+        self
+    }
+
     /// Run the constructed command and assert that it is successfully run.
     #[track_caller]
     pub fn run(&mut self) -> CompletedProcess {
-        let caller_location = std::panic::Location::caller();
-        let caller_line_number = caller_location.line();
-
         let output = self.command_output();
         if !output.status().success() {
-            handle_failed_output(&self, output, caller_line_number);
+            handle_failed_output(&self, output, panic::Location::caller().line());
         }
         output
     }
@@ -38,18 +109,16 @@ impl Command {
     /// Run the constructed command and assert that it does not successfully run.
     #[track_caller]
     pub fn run_fail(&mut self) -> CompletedProcess {
-        let caller_location = std::panic::Location::caller();
-        let caller_line_number = caller_location.line();
-
         let output = self.command_output();
         if output.status().success() {
-            handle_failed_output(&self, output, caller_line_number);
+            handle_failed_output(&self, output, panic::Location::caller().line());
         }
         output
     }
 
     #[track_caller]
-    pub(crate) fn command_output(&mut self) -> CompletedProcess {
+    fn command_output(&mut self) -> CompletedProcess {
+        self.drop_bomb.defuse();
         // let's make sure we piped all the input and outputs
         self.cmd.stdin(Stdio::piped());
         self.cmd.stdout(Stdio::piped());
@@ -71,20 +140,6 @@ impl Command {
     }
 }
 
-impl Deref for Command {
-    type Target = StdCommand;
-
-    fn deref(&self) -> &Self::Target {
-        &self.cmd
-    }
-}
-
-impl DerefMut for Command {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.cmd
-    }
-}
-
 /// Represents the result of an executed process.
 /// The various `assert_` helper methods should preferably be used for
 /// checking the contents of stdout/stderr.

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

@@ -2,9 +2,12 @@ use regex::Regex;
 use similar::TextDiff;
 use std::path::Path;
 
+use crate::drop_bomb::DropBomb;
+
 #[cfg(test)]
 mod tests;
 
+#[track_caller]
 pub fn diff() -> Diff {
     Diff::new()
 }
@@ -16,10 +19,12 @@ pub struct Diff {
     actual: Option<String>,
     actual_name: Option<String>,
     normalizers: Vec<(String, String)>,
+    drop_bomb: DropBomb,
 }
 
 impl Diff {
     /// Construct a bare `diff` invocation.
+    #[track_caller]
     pub fn new() -> Self {
         Self {
             expected: None,
@@ -27,6 +32,7 @@ impl Diff {
             actual: None,
             actual_name: None,
             normalizers: Vec::new(),
+            drop_bomb: DropBomb::arm("diff"),
         }
     }
 
@@ -79,9 +85,9 @@ impl Diff {
         self
     }
 
-    /// Executes the diff process, prints any differences to the standard error.
     #[track_caller]
-    pub fn run(&self) {
+    pub fn run(&mut self) {
+        self.drop_bomb.defuse();
         let expected = self.expected.as_ref().expect("expected text not set");
         let mut actual = self.actual.as_ref().expect("actual text not set").to_string();
         let expected_name = self.expected_name.as_ref().unwrap();

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

@@ -17,6 +17,7 @@ use std::env;
 use std::ffi::OsString;
 use std::fs;
 use std::io;
+use std::panic;
 use std::path::{Path, PathBuf};
 
 pub use gimli;
@@ -32,6 +33,7 @@ pub use run::{cmd, run, run_fail};
 pub use rustc::{aux_build, rustc, Rustc};
 pub use rustdoc::{bare_rustdoc, rustdoc, Rustdoc};
 
+#[track_caller]
 pub fn env_var(name: &str) -> String {
     match env::var(name) {
         Ok(v) => v,
@@ -39,6 +41,7 @@ pub fn env_var(name: &str) -> String {
     }
 }
 
+#[track_caller]
 pub fn env_var_os(name: &str) -> OsString {
     match env::var_os(name) {
         Some(v) => v,
@@ -66,11 +69,13 @@ pub fn is_darwin() -> bool {
     target().contains("darwin")
 }
 
+#[track_caller]
 pub fn python_command() -> Command {
     let python_path = env_var("PYTHON");
     Command::new(python_path)
 }
 
+#[track_caller]
 pub fn htmldocck() -> Command {
     let mut python = python_command();
     python.arg(source_root().join("src/etc/htmldocck.py"));
@@ -162,15 +167,13 @@ pub fn cwd() -> PathBuf {
 /// available on the platform!
 #[track_caller]
 pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
-    let caller_location = std::panic::Location::caller();
+    let caller = panic::Location::caller();
-    let caller_line_number = caller_location.line();
-
     let mut cygpath = Command::new("cygpath");
     cygpath.arg("-w");
     cygpath.arg(path.as_ref());
-    let output = cygpath.command_output();
+    let output = cygpath.run();
     if !output.status().success() {
-        handle_failed_output(&cygpath, output, caller_line_number);
+        handle_failed_output(&cygpath, output, caller.line());
     }
     // cygpath -w can attach a newline
     output.stdout_utf8().trim().to_string()
@@ -179,13 +182,11 @@ pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
 /// Run `uname`. This assumes that `uname` is available on the platform!
 #[track_caller]
 pub fn uname() -> String {
-    let caller_location = std::panic::Location::caller();
+    let caller = panic::Location::caller();
-    let caller_line_number = caller_location.line();
-
     let mut uname = Command::new("uname");
-    let output = uname.command_output();
+    let output = uname.run();
     if !output.status().success() {
-        handle_failed_output(&uname, output, caller_line_number);
+        handle_failed_output(&uname, output, caller.line());
     }
     output.stdout_utf8()
 }
@@ -393,7 +394,7 @@ macro_rules! impl_common_helpers {
             where
                 I: FnOnce(&::std::process::Command),
             {
-                inspector(&self.cmd);
+                self.cmd.inspect(inspector);
                 self
             }
 
@@ -410,7 +411,7 @@ macro_rules! impl_common_helpers {
             }
 
             /// Set the path where the command will be run.
-            pub fn current_dir<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
+            pub fn current_dir<P: AsRef<::std::path::Path>>(&mut self, path: P) -> &mut Self {
                 self.cmd.current_dir(path);
                 self
             }

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

@@ -5,6 +5,7 @@ use crate::env_var;
 
 /// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
 /// at `$LLVM_BIN_DIR/llvm-readobj`.
+#[track_caller]
 pub fn llvm_readobj() -> LlvmReadobj {
     LlvmReadobj::new()
 }
@@ -20,6 +21,7 @@ crate::impl_common_helpers!(LlvmReadobj);
 impl LlvmReadobj {
     /// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
     /// at `$LLVM_BIN_DIR/llvm-readobj`.
+    #[track_caller]
     pub fn new() -> Self {
         let llvm_bin_dir = env_var("LLVM_BIN_DIR");
         let llvm_bin_dir = PathBuf::from(llvm_bin_dir);

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

@@ -1,5 +1,6 @@
 use std::env;
 use std::ffi::OsStr;
+use std::panic;
 use std::path::{Path, PathBuf};
 
 use crate::command::{Command, CompletedProcess};
@@ -7,7 +8,8 @@ use crate::{cwd, env_var, is_windows, set_host_rpath};
 
 use super::handle_failed_output;
 
-fn run_common(name: &str) -> (Command, CompletedProcess) {
+#[track_caller]
+fn run_common(name: &str) -> Command {
     let mut bin_path = PathBuf::new();
     bin_path.push(cwd());
     bin_path.push(name);
@@ -34,19 +36,17 @@ fn run_common(name: &str) -> (Command, CompletedProcess) {
         cmd.env("PATH", env::join_paths(paths.iter()).unwrap());
     }
 
-    let output = cmd.command_output();
+    cmd
-    (cmd, output)
 }
 
 /// Run a built binary and make sure it succeeds.
 #[track_caller]
 pub fn run(name: &str) -> CompletedProcess {
-    let caller_location = std::panic::Location::caller();
+    let caller = panic::Location::caller();
-    let caller_line_number = caller_location.line();
+    let mut cmd = run_common(name);
-
+    let output = cmd.run();
-    let (cmd, output) = run_common(name);
     if !output.status().success() {
-        handle_failed_output(&cmd, output, caller_line_number);
+        handle_failed_output(&cmd, output, caller.line());
     }
     output
 }
@@ -54,18 +54,18 @@ pub fn run(name: &str) -> CompletedProcess {
 /// Run a built binary and make sure it fails.
 #[track_caller]
 pub fn run_fail(name: &str) -> CompletedProcess {
-    let caller_location = std::panic::Location::caller();
+    let caller = panic::Location::caller();
-    let caller_line_number = caller_location.line();
+    let mut cmd = run_common(name);
-
+    let output = cmd.run_fail();
-    let (cmd, output) = run_common(name);
     if output.status().success() {
-        handle_failed_output(&cmd, output, caller_line_number);
+        handle_failed_output(&cmd, output, caller.line());
     }
     output
 }
 
-/// Create a new custom Command.
+/// Create a new custom [`Command`]. This should be preferred to creating [`std::process::Command`]
-/// This should be preferred to creating `std::process::Command` directly.
+/// directly.
+#[track_caller]
 pub fn cmd<S: AsRef<OsStr>>(program: S) -> Command {
     let mut command = Command::new(program);
     set_host_rpath(&mut command);

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

@@ -5,11 +5,13 @@ use std::path::Path;
 use crate::{command, cwd, env_var, set_host_rpath};
 
 /// Construct a new `rustc` invocation.
+#[track_caller]
 pub fn rustc() -> Rustc {
     Rustc::new()
 }
 
 /// Construct a new `rustc` aux-build invocation.
+#[track_caller]
 pub fn aux_build() -> Rustc {
     Rustc::new_aux_build()
 }
@@ -22,6 +24,7 @@ pub struct Rustc {
 
 crate::impl_common_helpers!(Rustc);
 
+#[track_caller]
 fn setup_common() -> Command {
     let rustc = env_var("RUSTC");
     let mut cmd = Command::new(rustc);
@@ -34,12 +37,14 @@ impl Rustc {
     // `rustc` invocation constructor methods
 
     /// Construct a new `rustc` invocation.
+    #[track_caller]
     pub fn new() -> Self {
         let cmd = setup_common();
         Self { cmd }
     }
 
     /// Construct a new `rustc` invocation with `aux_build` preset (setting `--crate-type=lib`).
+    #[track_caller]
     pub fn new_aux_build() -> Self {
         let mut cmd = setup_common();
         cmd.arg("--crate-type=lib");

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

@@ -5,11 +5,13 @@ use crate::command::Command;
 use crate::{env_var, env_var_os, set_host_rpath};
 
 /// Construct a plain `rustdoc` invocation with no flags set.
+#[track_caller]
 pub fn bare_rustdoc() -> Rustdoc {
     Rustdoc::bare()
 }
 
 /// Construct a new `rustdoc` invocation with `-L $(TARGET_RPATH_DIR)` set.
+#[track_caller]
 pub fn rustdoc() -> Rustdoc {
     Rustdoc::new()
 }
@@ -21,6 +23,7 @@ pub struct Rustdoc {
 
 crate::impl_common_helpers!(Rustdoc);
 
+#[track_caller]
 fn setup_common() -> Command {
     let rustdoc = env_var("RUSTDOC");
     let mut cmd = Command::new(rustdoc);
@@ -30,12 +33,14 @@ fn setup_common() -> Command {
 
 impl Rustdoc {
     /// Construct a bare `rustdoc` invocation.
+    #[track_caller]
     pub fn bare() -> Self {
         let cmd = setup_common();
         Self { cmd }
     }
 
     /// Construct a `rustdoc` invocation with `-L $(TARGET_RPATH_DIR)` set.
+    #[track_caller]
     pub fn new() -> Self {
         let mut cmd = setup_common();
         let target_rpath_dir = env_var_os("TARGET_RPATH_DIR");