Skip to content

Commit 54e7044

Browse files
jieyouxujieyouxu
committedJun 11, 2024
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.
1 parent a3feeb3 commit 54e7044

File tree

9 files changed

+137
-64
lines changed

9 files changed

+137
-64
lines changed
 

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

+2-5
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::{bin_name, cygpath_windows, env_var, is_msvc, is_windows, uname};
77
///
88
/// WARNING: This means that what flags are accepted by the underlying C compiler is
99
/// platform- AND compiler-specific. Consult the relevant docs for `gcc`, `clang` and `mvsc`.
10+
#[track_caller]
1011
pub fn cc() -> Cc {
1112
Cc::new()
1213
}
@@ -25,6 +26,7 @@ impl Cc {
2526
///
2627
/// WARNING: This means that what flags are accepted by the underlying C compile is
2728
/// platform- AND compiler-specific. Consult the relevant docs for `gcc`, `clang` and `mvsc`.
29+
#[track_caller]
2830
pub fn new() -> Self {
2931
let compiler = env_var("CC");
3032

@@ -84,11 +86,6 @@ impl Cc {
8486
self.cmd.arg(path.as_ref());
8587
self
8688
}
87-
88-
/// Get the [`Output`][::std::process::Output] of the finished process.
89-
pub fn command_output(&mut self) -> ::std::process::Output {
90-
self.cmd.output().expect("failed to get output of finished process")
91-
}
9289
}
9390

9491
/// `EXTRACFLAGS`

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

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::command::Command;
44
use crate::{bin_name, env_var};
55

66
/// Construct a new `clang` invocation. `clang` is not always available for all targets.
7+
#[track_caller]
78
pub fn clang() -> Clang {
89
Clang::new()
910
}
@@ -18,6 +19,7 @@ crate::impl_common_helpers!(Clang);
1819

1920
impl Clang {
2021
/// Construct a new `clang` invocation. `clang` is not always available for all targets.
22+
#[track_caller]
2123
pub fn new() -> Self {
2224
let clang = env_var("CLANG");
2325
let cmd = Command::new(clang);

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

+85-30
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,124 @@
1-
use crate::{assert_not_contains, handle_failed_output};
1+
use std::ffi;
22
use std::ffi::OsStr;
33
use std::io::Write;
4-
use std::ops::{Deref, DerefMut};
4+
use std::panic;
5+
use std::path::Path;
56
use std::process::{Command as StdCommand, ExitStatus, Output, Stdio};
67

7-
/// This is a custom command wrapper that simplifies working with commands
8-
/// and makes it easier to ensure that we check the exit status of executed
9-
/// processes.
8+
use crate::drop_bomb::DropBomb;
9+
use crate::{assert_not_contains, handle_failed_output};
10+
11+
/// This is a custom command wrapper that simplifies working with commands and makes it easier to
12+
/// ensure that we check the exit status of executed processes.
13+
///
14+
/// # A [`Command`] must be executed
15+
///
16+
/// A [`Command`] is armed by a [`DropBomb`] on construction to enforce that it will be executed. If
17+
/// a [`Command`] is constructed but never executed, the drop bomb will explode and cause the test
18+
/// to panic. Execution methods [`run`] and [`run_fail`] will defuse the drop bomb. A test
19+
/// containing constructed but never executed commands is dangerous because it can give a false
20+
/// sense of confidence.
21+
///
22+
/// [`run`]: Self::run
23+
/// [`run_fail`]: Self::run_fail
1024
#[derive(Debug)]
1125
pub struct Command {
1226
cmd: StdCommand,
1327
stdin: Option<Box<[u8]>>,
28+
drop_bomb: DropBomb,
1429
}
1530

1631
impl Command {
17-
pub fn new<S: AsRef<OsStr>>(program: S) -> Self {
18-
Self { cmd: StdCommand::new(program), stdin: None }
32+
#[track_caller]
33+
pub fn new<P: AsRef<OsStr>>(program: P) -> Self {
34+
let program = program.as_ref();
35+
Self { cmd: StdCommand::new(program), stdin: None, drop_bomb: DropBomb::arm(program) }
1936
}
2037

2138
pub fn set_stdin(&mut self, stdin: Box<[u8]>) {
2239
self.stdin = Some(stdin);
2340
}
2441

42+
/// Specify an environment variable.
43+
pub fn env<K, V>(&mut self, key: K, value: V) -> &mut Self
44+
where
45+
K: AsRef<ffi::OsStr>,
46+
V: AsRef<ffi::OsStr>,
47+
{
48+
self.cmd.env(key, value);
49+
self
50+
}
51+
52+
/// Remove an environmental variable.
53+
pub fn env_remove<K>(&mut self, key: K) -> &mut Self
54+
where
55+
K: AsRef<ffi::OsStr>,
56+
{
57+
self.cmd.env_remove(key);
58+
self
59+
}
60+
61+
/// Generic command argument provider. Prefer specific helper methods if possible.
62+
/// Note that for some executables, arguments might be platform specific. For C/C++
63+
/// compilers, arguments might be platform *and* compiler specific.
64+
pub fn arg<S>(&mut self, arg: S) -> &mut Self
65+
where
66+
S: AsRef<ffi::OsStr>,
67+
{
68+
self.cmd.arg(arg);
69+
self
70+
}
71+
72+
/// Generic command arguments provider. Prefer specific helper methods if possible.
73+
/// Note that for some executables, arguments might be platform specific. For C/C++
74+
/// compilers, arguments might be platform *and* compiler specific.
75+
pub fn args<S>(&mut self, args: &[S]) -> &mut Self
76+
where
77+
S: AsRef<ffi::OsStr>,
78+
{
79+
self.cmd.args(args);
80+
self
81+
}
82+
83+
/// Inspect what the underlying [`std::process::Command`] is up to the
84+
/// current construction.
85+
pub fn inspect<I>(&mut self, inspector: I) -> &mut Self
86+
where
87+
I: FnOnce(&StdCommand),
88+
{
89+
inspector(&self.cmd);
90+
self
91+
}
92+
93+
/// Set the path where the command will be run.
94+
pub fn current_dir<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
95+
self.cmd.current_dir(path);
96+
self
97+
}
98+
2599
/// Run the constructed command and assert that it is successfully run.
26100
#[track_caller]
27101
pub fn run(&mut self) -> CompletedProcess {
28-
let caller_location = std::panic::Location::caller();
29-
let caller_line_number = caller_location.line();
30-
31102
let output = self.command_output();
32103
if !output.status().success() {
33-
handle_failed_output(&self, output, caller_line_number);
104+
handle_failed_output(&self, output, panic::Location::caller().line());
34105
}
35106
output
36107
}
37108

38109
/// Run the constructed command and assert that it does not successfully run.
39110
#[track_caller]
40111
pub fn run_fail(&mut self) -> CompletedProcess {
41-
let caller_location = std::panic::Location::caller();
42-
let caller_line_number = caller_location.line();
43-
44112
let output = self.command_output();
45113
if output.status().success() {
46-
handle_failed_output(&self, output, caller_line_number);
114+
handle_failed_output(&self, output, panic::Location::caller().line());
47115
}
48116
output
49117
}
50118

51119
#[track_caller]
52-
pub(crate) fn command_output(&mut self) -> CompletedProcess {
120+
fn command_output(&mut self) -> CompletedProcess {
121+
self.drop_bomb.defuse();
53122
// let's make sure we piped all the input and outputs
54123
self.cmd.stdin(Stdio::piped());
55124
self.cmd.stdout(Stdio::piped());
@@ -71,20 +140,6 @@ impl Command {
71140
}
72141
}
73142

74-
impl Deref for Command {
75-
type Target = StdCommand;
76-
77-
fn deref(&self) -> &Self::Target {
78-
&self.cmd
79-
}
80-
}
81-
82-
impl DerefMut for Command {
83-
fn deref_mut(&mut self) -> &mut Self::Target {
84-
&mut self.cmd
85-
}
86-
}
87-
88143
/// Represents the result of an executed process.
89144
/// The various `assert_` helper methods should preferably be used for
90145
/// checking the contents of stdout/stderr.

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

+8-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ use regex::Regex;
22
use similar::TextDiff;
33
use std::path::Path;
44

5+
use crate::drop_bomb::DropBomb;
6+
57
#[cfg(test)]
68
mod tests;
79

10+
#[track_caller]
811
pub fn diff() -> Diff {
912
Diff::new()
1013
}
@@ -16,17 +19,20 @@ pub struct Diff {
1619
actual: Option<String>,
1720
actual_name: Option<String>,
1821
normalizers: Vec<(String, String)>,
22+
drop_bomb: DropBomb,
1923
}
2024

2125
impl Diff {
2226
/// Construct a bare `diff` invocation.
27+
#[track_caller]
2328
pub fn new() -> Self {
2429
Self {
2530
expected: None,
2631
expected_name: None,
2732
actual: None,
2833
actual_name: None,
2934
normalizers: Vec::new(),
35+
drop_bomb: DropBomb::arm("diff"),
3036
}
3137
}
3238

@@ -79,9 +85,9 @@ impl Diff {
7985
self
8086
}
8187

82-
/// Executes the diff process, prints any differences to the standard error.
8388
#[track_caller]
84-
pub fn run(&self) {
89+
pub fn run(&mut self) {
90+
self.drop_bomb.defuse();
8591
let expected = self.expected.as_ref().expect("expected text not set");
8692
let mut actual = self.actual.as_ref().expect("actual text not set").to_string();
8793
let expected_name = self.expected_name.as_ref().unwrap();

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

+13-12
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use std::env;
1717
use std::ffi::OsString;
1818
use std::fs;
1919
use std::io;
20+
use std::panic;
2021
use std::path::{Path, PathBuf};
2122

2223
pub use gimli;
@@ -32,13 +33,15 @@ pub use run::{cmd, run, run_fail};
3233
pub use rustc::{aux_build, rustc, Rustc};
3334
pub use rustdoc::{bare_rustdoc, rustdoc, Rustdoc};
3435

36+
#[track_caller]
3537
pub fn env_var(name: &str) -> String {
3638
match env::var(name) {
3739
Ok(v) => v,
3840
Err(err) => panic!("failed to retrieve environment variable {name:?}: {err:?}"),
3941
}
4042
}
4143

44+
#[track_caller]
4245
pub fn env_var_os(name: &str) -> OsString {
4346
match env::var_os(name) {
4447
Some(v) => v,
@@ -66,11 +69,13 @@ pub fn is_darwin() -> bool {
6669
target().contains("darwin")
6770
}
6871

72+
#[track_caller]
6973
pub fn python_command() -> Command {
7074
let python_path = env_var("PYTHON");
7175
Command::new(python_path)
7276
}
7377

78+
#[track_caller]
7479
pub fn htmldocck() -> Command {
7580
let mut python = python_command();
7681
python.arg(source_root().join("src/etc/htmldocck.py"));
@@ -162,15 +167,13 @@ pub fn cwd() -> PathBuf {
162167
/// available on the platform!
163168
#[track_caller]
164169
pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
165-
let caller_location = std::panic::Location::caller();
166-
let caller_line_number = caller_location.line();
167-
170+
let caller = panic::Location::caller();
168171
let mut cygpath = Command::new("cygpath");
169172
cygpath.arg("-w");
170173
cygpath.arg(path.as_ref());
171-
let output = cygpath.command_output();
174+
let output = cygpath.run();
172175
if !output.status().success() {
173-
handle_failed_output(&cygpath, output, caller_line_number);
176+
handle_failed_output(&cygpath, output, caller.line());
174177
}
175178
// cygpath -w can attach a newline
176179
output.stdout_utf8().trim().to_string()
@@ -179,13 +182,11 @@ pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
179182
/// Run `uname`. This assumes that `uname` is available on the platform!
180183
#[track_caller]
181184
pub fn uname() -> String {
182-
let caller_location = std::panic::Location::caller();
183-
let caller_line_number = caller_location.line();
184-
185+
let caller = panic::Location::caller();
185186
let mut uname = Command::new("uname");
186-
let output = uname.command_output();
187+
let output = uname.run();
187188
if !output.status().success() {
188-
handle_failed_output(&uname, output, caller_line_number);
189+
handle_failed_output(&uname, output, caller.line());
189190
}
190191
output.stdout_utf8()
191192
}
@@ -393,7 +394,7 @@ macro_rules! impl_common_helpers {
393394
where
394395
I: FnOnce(&::std::process::Command),
395396
{
396-
inspector(&self.cmd);
397+
self.cmd.inspect(inspector);
397398
self
398399
}
399400

@@ -410,7 +411,7 @@ macro_rules! impl_common_helpers {
410411
}
411412

412413
/// Set the path where the command will be run.
413-
pub fn current_dir<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
414+
pub fn current_dir<P: AsRef<::std::path::Path>>(&mut self, path: P) -> &mut Self {
414415
self.cmd.current_dir(path);
415416
self
416417
}

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

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use crate::env_var;
55

66
/// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
77
/// at `$LLVM_BIN_DIR/llvm-readobj`.
8+
#[track_caller]
89
pub fn llvm_readobj() -> LlvmReadobj {
910
LlvmReadobj::new()
1011
}
@@ -20,6 +21,7 @@ crate::impl_common_helpers!(LlvmReadobj);
2021
impl LlvmReadobj {
2122
/// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
2223
/// at `$LLVM_BIN_DIR/llvm-readobj`.
24+
#[track_caller]
2325
pub fn new() -> Self {
2426
let llvm_bin_dir = env_var("LLVM_BIN_DIR");
2527
let llvm_bin_dir = PathBuf::from(llvm_bin_dir);

0 commit comments

Comments
 (0)