Auto merge of #126197 - jieyouxu:rmake-must-use, r=<try>

run-make: annotate library with `#[must_use]` and enforce `unused_must_use` in rmake.rs

Commits up to 706abf6 are from #125752.

This PR adds `#[must_use]` annotations to functions of the `run_make_support` library where it makes sense, and adjusts compiletest to compile rmake.rs with `-Dunused_must_use`.

The rationale is that it's highly likely that unused `#[must_use]` values in rmake.rs test files are bugs. For example, unused fs/io results are often crucial to the correctness of the test and often unchecked fs/io results allow the test to silently pass where it would've failed if the result was checked.

try-job: test-various
try-job: x86_64-msvc

Author: bors

Cargo.lock

@@ -3452,7 +3452,7 @@ dependencies = [
 
 [[package]]
 name = "run_make_support"
-version = "0.0.0"
+version = "0.2.0"
 dependencies = [
  "gimli 0.28.1",
  "object 0.34.0",

src/tools/compiletest/src/runtest.rs

@@ -3551,6 +3551,10 @@ impl<'test> TestCx<'test> {
             .env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path))
             .env("LLVM_COMPONENTS", &self.config.llvm_components);
 
+        // In test code we want to be very pedantic about values being silently discarded that are
+        // annotated with `#[must_use]`.
+        cmd.arg("-Dunused_must_use");
+
         if std::env::var_os("COMPILETEST_FORCE_STAGE0").is_some() {
             let mut stage0_sysroot = build_root.clone();
             stage0_sysroot.push("stage0-sysroot");

src/tools/run-make-support/CHANGELOG.md

@@ -0,0 +1,73 @@
+# Changelog
+
+All notable changes to the `run_make_support` library should be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the support
+library should adhere to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) even if it's
+not intended for public consumption (it's moreso to help internally, to help test writers track
+changes to the support library).
+
+This support library will probably never reach 1.0. Please bump the minor version in `Cargo.toml` if
+you make any breaking changes or other significant changes, or bump the patch version for bug fixes.
+
+## [0.2.0] - 2024-06-09
+
+### Changed
+
+- Marked many functions with `#[must_use]`, and rmake.rs are now compiled with `-Dmust_use`.
+
+## [0.1.0] - 2024-06-09
+
+### Changed
+
+- Use *drop bombs* to enforce that commands are executed; a command invocation will panic if it is
+  constructed but never executed. Execution methods `Command::{run, run_fail}` will defuse the drop
+  bomb.
+- Added `Command` helpers that forward to `std::process::Command` counterparts.
+
+### Removed
+
+- The `env_var` method which was incorrectly named and is `env_clear` underneath and is a footgun
+  from `impl_common_helpers`. For example, removing `TMPDIR` on Unix and `TMP`/`TEMP` breaks
+  `std::env::temp_dir` and wrecks anything using that, such as rustc's codgen.
+- Removed `Deref`/`DerefMut` for `run_make_support::Command` -> `std::process::Command` because it
+  causes a method chain like `htmldocck().arg().run()` to fail, because `arg()` resolves to
+  `std::process::Command` which also returns a `&mut std::process::Command`, causing the `run()` to
+  be not found.
+
+## [0.0.0] - 2024-06-09
+
+Consider this version to contain all changes made to the support library before we started to track
+changes in this changelog.
+
+### Added
+
+- Custom command wrappers around `std::process::Command` (`run_make_support::Command`) and custom
+  wrapper around `std::process::Output` (`CompletedProcess`) to make it more convenient to work with
+  commands and their output, and help avoid forgetting to check for exit status.
+    - `Command`: `set_stdin`, `run`, `run_fail`.
+    - `CompletedProcess`: `std{err,out}_utf8`, `status`, `assert_std{err,out}_{equals, contains,
+      not_contains}`, `assert_exit_code`.
+- `impl_common_helpers` macro to avoid repeating adding common convenience methods, including:
+    - Environment manipulation methods: `env`, `env_remove`
+    - Command argument providers: `arg`, `args`
+    - Common invocation inspection (of the command invocation up until `inspect` is called):
+      `inspect`
+    - Execution methods: `run` (for commands expected to succeed execution, exit status `0`) and
+      `run_fail` (for commands expected to fail execution, exit status non-zero).
+- Command wrappers around: `rustc`, `clang`, `cc`, `rustc`, `rustdoc`, `llvm-readobj`.
+- Thin helpers to construct `python` and `htmldocck` commands.
+- `run` and `run_fail` (like `Command::{run, run_fail}`) for running binaries, which sets suitable
+  env vars (like `LD_LIB_PATH` or equivalent, `TARGET_RPATH_ENV`, `PATH` on Windows).
+- Pseudo command `diff` which has similar functionality as the cli util but not the same API.
+- Convenience panic-on-fail helpers `env_var`, `env_var_os`, `cwd` for their `std::env` conterparts.
+- Convenience panic-on-fail helpers for reading respective env vars: `target`, `source_root`.
+- Platform check helpers: `is_windows`, `is_msvc`, `cygpath_windows`, `uname`.
+- fs helpers: `copy_dir_all`.
+- `recursive_diff` helper.
+- Generic `assert_not_contains` helper.
+- Scoped run-with-teardown helper `run_in_tmpdir` which is designed to run commands in a temporary
+  directory that is cleared when closure returns.
+- Helpers for constructing the name of binaries and libraries: `rust_lib_name`, `static_lib_name`,
+  `bin_name`, `dynamic_lib_name`.
+- Re-export libraries: `gimli`, `object`, `regex`, `wasmparsmer`.

src/tools/run-make-support/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "run_make_support"
-version = "0.0.0"
+version = "0.2.0"
 edition = "2021"
 
 [dependencies]

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

@@ -7,13 +7,15 @@ 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()
 }
 
 /// A platform-specific C compiler invocation builder. The specific C compiler used is
 /// passed down from compiletest.
 #[derive(Debug)]
+#[must_use]
 pub struct Cc {
     cmd: Command,
 }
@@ -25,6 +27,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 +87,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,12 +4,14 @@ 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()
 }
 
 /// A `clang` invocation builder.
 #[derive(Debug)]
+#[must_use]
 pub struct Clang {
     cmd: Command,
 }
@@ -18,6 +20,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,55 +1,125 @@
-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
-/// and makes it easier to ensure that we check the exit status of executed
-/// processes.
+use crate::drop_bomb::DropBomb;
+use crate::{assert_not_contains, handle_failed_output};
+
+/// 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 [`Commmand`] 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
 #[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 {
-        Self { cmd: StdCommand::new(program), stdin: None }
+    #[track_caller]
+    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();
-
+        self.drop_bomb.defuse();
         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
     }
 
     /// 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();
-
+        self.drop_bomb.defuse();
         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 {
         // let's make sure we piped all the input and outputs
         self.cmd.stdin(Stdio::piped());
         self.cmd.stdout(Stdio::piped());
@@ -71,20 +141,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.
@@ -93,14 +149,17 @@ pub struct CompletedProcess {
 }
 
 impl CompletedProcess {
+    #[must_use]
     pub fn stdout_utf8(&self) -> String {
         String::from_utf8(self.output.stdout.clone()).expect("stdout is not valid UTF-8")
     }
 
+    #[must_use]
     pub fn stderr_utf8(&self) -> String {
         String::from_utf8(self.output.stderr.clone()).expect("stderr is not valid UTF-8")
     }
 
+    #[must_use]
     pub fn status(&self) -> ExitStatus {
         self.output.status
     }

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

@@ -2,31 +2,38 @@ 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()
 }
 
 #[derive(Debug)]
+#[must_use]
 pub struct Diff {
     expected: Option<String>,
     expected_name: Option<String>,
     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,
             expected_name: None,
             actual: None,
             actual_name: None,
             normalizers: Vec::new(),
+            drop_bomb: DropBomb::arm("diff"),
         }
     }
 
@@ -79,9 +86,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/drop_bomb/mod.rs

@@ -0,0 +1,50 @@
+//! This module implements "drop bombs" intended for use by command wrappers to ensure that the
+//! constructed commands are *eventually* executed. This is exactly like `rustc_errors::Diag` where
+//! we force every `Diag` to be consumed or we emit a bug, but we panic instead.
+//!
+//! This is adapted from <https://docs.rs/drop_bomb/latest/drop_bomb/> and simplified for our
+//! purposes.
+
+use std::ffi::{OsStr, OsString};
+use std::panic;
+
+#[cfg(test)]
+mod tests;
+
+#[derive(Debug)]
+pub(crate) struct DropBomb {
+    command: OsString,
+    defused: bool,
+    armed_line: u32,
+}
+
+impl DropBomb {
+    /// Arm a [`DropBomb`]. If the value is dropped without being [`defused`][Self::defused], then
+    /// it will panic. It is expected that the command wrapper uses `#[track_caller]` to help
+    /// propagate the caller info from rmake.rs.
+    #[track_caller]
+    pub(crate) fn arm<S: AsRef<OsStr>>(command: S) -> DropBomb {
+        DropBomb {
+            command: command.as_ref().into(),
+            defused: false,
+            armed_line: panic::Location::caller().line(),
+        }
+    }
+
+    /// Defuse the [`DropBomb`]. This will prevent the drop bomb from panicking when dropped.
+    pub(crate) fn defuse(&mut self) {
+        self.defused = true;
+    }
+}
+
+impl Drop for DropBomb {
+    fn drop(&mut self) {
+        if !self.defused && !std::thread::panicking() {
+            panic!(
+                "command constructed but not executed at line {}: `{}`",
+                self.armed_line,
+                self.command.to_string_lossy()
+            )
+        }
+    }
+}

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

@@ -0,0 +1,15 @@
+use super::DropBomb;
+
+#[test]
+#[should_panic]
+fn test_arm() {
+    let bomb = DropBomb::arm("hi :3");
+    drop(bomb); // <- armed bomb should explode when not defused
+}
+
+#[test]
+fn test_defuse() {
+    let mut bomb = DropBomb::arm("hi :3");
+    bomb.defuse();
+    drop(bomb); // <- defused bomb should not explode
+}

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

@@ -7,6 +7,7 @@ pub mod cc;
 pub mod clang;
 mod command;
 pub mod diff;
+mod drop_bomb;
 pub mod llvm_readobj;
 pub mod run;
 pub mod rustc;
@@ -16,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;
@@ -31,13 +33,17 @@ pub use run::{cmd, run, run_fail};
 pub use rustc::{aux_build, rustc, Rustc};
 pub use rustdoc::{bare_rustdoc, rustdoc, Rustdoc};
 
+#[track_caller]
+#[must_use]
 pub fn env_var(name: &str) -> String {
     match env::var(name) {
         Ok(v) => v,
         Err(err) => panic!("failed to retrieve environment variable {name:?}: {err:?}"),
     }
 }
 
+#[track_caller]
+#[must_use]
 pub fn env_var_os(name: &str) -> OsString {
     match env::var_os(name) {
         Some(v) => v,
@@ -46,42 +52,52 @@ pub fn env_var_os(name: &str) -> OsString {
 }
 
 /// `TARGET`
+#[must_use]
 pub fn target() -> String {
     env_var("TARGET")
 }
 
 /// Check if target is windows-like.
+#[must_use]
 pub fn is_windows() -> bool {
     target().contains("windows")
 }
 
 /// Check if target uses msvc.
+#[must_use]
 pub fn is_msvc() -> bool {
     target().contains("msvc")
 }
 
 /// Check if target uses macOS.
+#[must_use]
 pub fn is_darwin() -> bool {
     target().contains("darwin")
 }
 
+#[track_caller]
+#[must_use]
 pub fn python_command() -> Command {
     let python_path = env_var("PYTHON");
     Command::new(python_path)
 }
 
+#[track_caller]
+#[must_use]
 pub fn htmldocck() -> Command {
     let mut python = python_command();
     python.arg(source_root().join("src/etc/htmldocck.py"));
     python
 }
 
 /// Path to the root rust-lang/rust source checkout.
+#[must_use]
 pub fn source_root() -> PathBuf {
     env_var("SOURCE_ROOT").into()
 }
 
 /// Construct the static library name based on the platform.
+#[must_use]
 pub fn static_lib_name(name: &str) -> String {
     // See tools.mk (irrelevant lines omitted):
     //
@@ -106,6 +122,7 @@ pub fn static_lib_name(name: &str) -> String {
 }
 
 /// Construct the dynamic library name based on the platform.
+#[must_use]
 pub fn dynamic_lib_name(name: &str) -> String {
     // See tools.mk (irrelevant lines omitted):
     //
@@ -132,6 +149,7 @@ pub fn dynamic_lib_name(name: &str) -> String {
     }
 }
 
+#[must_use]
 pub fn dynamic_lib_extension() -> &'static str {
     if is_darwin() {
         "dylib"
@@ -143,48 +161,49 @@ pub fn dynamic_lib_extension() -> &'static str {
 }
 
 /// Construct a rust library (rlib) name.
+#[must_use]
 pub fn rust_lib_name(name: &str) -> String {
     format!("lib{name}.rlib")
 }
 
 /// Construct the binary name based on platform.
+#[must_use]
 pub fn bin_name(name: &str) -> String {
     if is_windows() { format!("{name}.exe") } else { name.to_string() }
 }
 
 /// Return the current working directory.
+#[must_use]
 pub fn cwd() -> PathBuf {
     env::current_dir().unwrap()
 }
 
 /// Use `cygpath -w` on a path to get a Windows path string back. This assumes that `cygpath` is
 /// available on the platform!
 #[track_caller]
+#[must_use]
 pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
-    let caller_location = std::panic::Location::caller();
-    let caller_line_number = caller_location.line();
-
+    let caller = panic::Location::caller();
     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()
 }
 
 /// Run `uname`. This assumes that `uname` is available on the platform!
 #[track_caller]
+#[must_use]
 pub fn uname() -> String {
-    let caller_location = std::panic::Location::caller();
-    let caller_line_number = caller_location.line();
-
+    let caller = panic::Location::caller();
     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()
 }
@@ -372,12 +391,6 @@ macro_rules! impl_common_helpers {
                 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.
@@ -406,7 +419,7 @@ macro_rules! impl_common_helpers {
             where
                 I: FnOnce(&::std::process::Command),
             {
-                inspector(&self.cmd);
+                self.cmd.inspect(inspector);
                 self
             }
 
@@ -423,7 +436,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,12 +5,14 @@ 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()
 }
 
 /// A `llvm-readobj` invocation builder.
 #[derive(Debug)]
+#[must_use]
 pub struct LlvmReadobj {
     cmd: Command,
 }
@@ -20,6 +22,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,13 +1,15 @@
 use std::env;
 use std::ffi::OsStr;
+use std::panic;
 use std::path::{Path, PathBuf};
 
 use crate::command::{Command, CompletedProcess};
 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,38 +36,36 @@ fn run_common(name: &str) -> (Command, CompletedProcess) {
         cmd.env("PATH", env::join_paths(paths.iter()).unwrap());
     }
 
-    let output = cmd.command_output();
-    (cmd, output)
+    cmd
 }
 
 /// 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_line_number = caller_location.line();
-
-    let (cmd, output) = run_common(name);
+    let caller = panic::Location::caller();
+    let mut cmd = run_common(name);
+    let output = cmd.run();
     if !output.status().success() {
-        handle_failed_output(&cmd, output, caller_line_number);
+        handle_failed_output(&cmd, output, caller.line());
     }
     output
 }
 
 /// 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_line_number = caller_location.line();
-
-    let (cmd, output) = run_common(name);
+    let caller = panic::Location::caller();
+    let mut cmd = run_common(name);
+    let output = cmd.run_fail();
     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.
-/// This should be preferred to creating `std::process::Command` directly.
+/// Create a new custom [`Command`]. This should be preferred to creating [`std::process::Command`]
+/// 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,23 +5,27 @@ 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()
 }
 
 /// A `rustc` invocation builder.
 #[derive(Debug)]
+#[must_use]
 pub struct Rustc {
     cmd: Command,
 }
 
 crate::impl_common_helpers!(Rustc);
 
+#[track_caller]
 fn setup_common() -> Command {
     let rustc = env_var("RUSTC");
     let mut cmd = Command::new(rustc);
@@ -34,12 +38,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

@@ -10,17 +10,20 @@ pub fn bare_rustdoc() -> Rustdoc {
 }
 
 /// Construct a new `rustdoc` invocation with `-L $(TARGET_RPATH_DIR)` set.
+#[track_caller]
 pub fn rustdoc() -> Rustdoc {
     Rustdoc::new()
 }
 
 #[derive(Debug)]
+#[must_use]
 pub struct Rustdoc {
     cmd: Command,
 }
 
 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");

tests/run-make/c-link-to-rust-staticlib/rmake.rs

@@ -10,6 +10,6 @@ fn main() {
     rustc().input("foo.rs").run();
     cc().input("bar.c").input(static_lib_name("foo")).out_exe("bar").args(&extra_c_flags()).run();
     run("bar");
-    fs::remove_file(static_lib_name("foo"));
+    fs::remove_file(static_lib_name("foo")).unwrap();
     run("bar");
 }

tests/run-make/compiler-builtins/rmake.rs

@@ -35,7 +35,7 @@ fn main() {
     let rustc = env_var("RUSTC");
     let bootstrap_cargo = env_var("BOOTSTRAP_CARGO");
     let mut cmd = cmd(bootstrap_cargo);
-    cmd.args([
+    cmd.args(&[
         "build",
         "--manifest-path",
         manifest_path.to_str().unwrap(),

tests/run-make/doctests-keep-binaries/rmake.rs

@@ -10,7 +10,7 @@ fn setup_test_env<F: FnOnce(&Path, &Path)>(callback: F) {
     create_dir(&out_dir).expect("failed to create doctests folder");
     rustc().input("t.rs").crate_type("rlib").run();
     callback(&out_dir, Path::new("libt.rlib"));
-    remove_dir_all(out_dir);
+    remove_dir_all(out_dir).unwrap();
 }
 
 fn check_generated_binaries() {
@@ -58,6 +58,6 @@ fn main() {
             .extern_("t", "libt.rlib")
             .run();
 
-        remove_dir_all(run_dir_path);
+        remove_dir_all(run_dir_path).unwrap();
     });
 }

tests/run-make/doctests-runtool/rmake.rs

@@ -31,6 +31,6 @@ fn main() {
         .extern_("t", "libt.rlib")
         .run();
 
-    remove_dir_all(run_dir);
-    remove_dir_all(run_tool);
+    remove_dir_all(run_dir).unwrap();
+    remove_dir_all(run_tool).unwrap();
 }

tests/run-make/reset-codegen-1/rmake.rs

@@ -28,11 +28,20 @@ fn main() {
         ("link-output", Some("link")),
         ("obj-output", Some("obj")),
         ("dep-output", Some("dep-info")),
-        ("multi-output", Some("asm,obj")),
     ];
     for (output_file, emit) in flags {
         fs::remove_file(output_file).unwrap_or_default();
         compile(output_file, emit);
-        fs::remove_file(output_file);
+        eprintln!("{output_file}");
+        fs::remove_file(output_file).unwrap();
+    }
+
+    {
+        // multi-output has two output artifacts with extensions.
+        fs::remove_file("multi-output.s").unwrap_or_default();
+        fs::remove_file("multi-output.o").unwrap_or_default();
+        compile("multi-output", Some("asm,obj"));
+        fs::remove_file("multi-output.s").unwrap();
+        fs::remove_file("multi-output.o").unwrap();
     }
 }

tests/run-make/rustdoc-map-file/rmake.rs

@@ -9,5 +9,5 @@ fn main() {
         .output(&out_dir)
         .run();
     // FIXME (GuillaumeGomez): Port the python script to Rust as well.
-    assert!(python_command().arg("validate_json.py").arg(&out_dir).status().unwrap().success());
+    python_command().arg("validate_json.py").arg(&out_dir).run();
 }

tests/run-make/rustdoc-scrape-examples-macros/rmake.rs

@@ -56,5 +56,5 @@ fn main() {
         .arg(&ex_dir)
         .run();
 
-    assert!(htmldocck().arg(out_dir).arg("src/lib.rs").status().unwrap().success());
+    htmldocck().arg(out_dir).arg("src/lib.rs").run();
 }

tests/run-make/rustdoc-scrape-examples-remap/scrape.rs

@@ -45,5 +45,5 @@ pub fn scrape(extra_args: &[&str]) {
     }
     rustdoc.run();
 
-    assert!(htmldocck().arg(out_dir).arg("src/lib.rs").status().unwrap().success());
+    htmldocck().arg(out_dir).arg("src/lib.rs").run();
 }

tests/run-make/rustdoc-themes/rmake.rs

@@ -28,5 +28,5 @@ fn main() {
     std::fs::write(&test_css, test_content).unwrap();
 
     rustdoc().output(&out_dir).input("foo.rs").arg("--theme").arg(&test_css).run();
-    assert!(htmldocck().arg(out_dir).arg("foo.rs").status().unwrap().success());
+    htmldocck().arg(out_dir).arg("foo.rs").run();
 }

tests/run-make/rustdoc-with-out-dir-option/rmake.rs

@@ -3,5 +3,5 @@ use run_make_support::{htmldocck, rustdoc};
 fn main() {
     let out_dir = "rustdoc";
     rustdoc().input("src/lib.rs").crate_name("foobar").crate_type("lib").output(&out_dir).run();
-    assert!(htmldocck().arg(out_dir).arg("src/lib.rs").status().unwrap().success());
+    htmldocck().arg(out_dir).arg("src/lib.rs").run();
 }

tests/run-make/rustdoc-with-output-option/rmake.rs

@@ -12,5 +12,5 @@ fn main() {
         .arg(&out_dir)
         .run();
 
-    assert!(htmldocck().arg(out_dir).arg("src/lib.rs").status().unwrap().success());
+    htmldocck().arg(out_dir).arg("src/lib.rs").run();
 }

tests/run-make/rustdoc-with-short-out-dir-option/rmake.rs

@@ -12,5 +12,5 @@ fn main() {
         .arg(&out_dir)
         .run();
 
-    assert!(htmldocck().arg(out_dir).arg("src/lib.rs").status().unwrap().success());
+    htmldocck().arg(out_dir).arg("src/lib.rs").run();
 }