Skip to content

Commit

Permalink
Auto merge of #126121 - Kobzol:runmake-cmd-wrapper, r=jieyouxu
Browse files Browse the repository at this point in the history
Add a custom Command wrapper to `run-make-support`

This should make it easier to make sure that we check process exit codes, and it should also make checking of stdout/stderr less verbose and more explicit in run-make tests. I prefer the `run()/run_fail().assert(...)` style to something like `run_fail_assert_exit_code`, because the former is more composable.

Regarding #125747, I'm not sure if we really need a custom trait, I think that we can get far enough with just `Deref` on the `Cc/Clang/Rustc/Rustdoc/...` structs. But now that these structs don't even need `command_output` anymore, I think that they are fine-ish as they are with the macro.

Related issues: #125617, #125747

Fixes: #125617 (because `command_output` is no longer a public method)

r? `@jieyouxu`
  • Loading branch information
bors committed Jun 8, 2024
2 parents 565cadb + 0a190e8 commit f21554f
Show file tree
Hide file tree
Showing 30 changed files with 289 additions and 247 deletions.
4 changes: 2 additions & 2 deletions src/tools/run-make-support/src/cc.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::path::Path;
use std::process::Command;

use crate::{bin_name, cygpath_windows, env_var, handle_failed_output, is_msvc, is_windows, uname};
use crate::command::Command;
use crate::{bin_name, cygpath_windows, env_var, is_msvc, is_windows, uname};

/// Construct a new platform-specific C compiler invocation.
///
Expand Down
9 changes: 2 additions & 7 deletions src/tools/run-make-support/src/clang.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::path::Path;
use std::process::Command;

use crate::{bin_name, env_var, handle_failed_output};
use crate::command::Command;
use crate::{bin_name, env_var};

/// Construct a new `clang` invocation. `clang` is not always available for all targets.
pub fn clang() -> Clang {
Expand Down Expand Up @@ -68,9 +68,4 @@ impl Clang {
self.cmd.arg(format!("-fuse-ld={ld}"));
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")
}
}
151 changes: 151 additions & 0 deletions src/tools/run-make-support/src/command.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
use crate::{assert_not_contains, handle_failed_output};
use std::ffi::OsStr;
use std::io::Write;
use std::ops::{Deref, DerefMut};
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.
#[derive(Debug)]
pub struct Command {
cmd: StdCommand,
stdin: Option<Box<[u8]>>,
}

impl Command {
pub fn new<S: AsRef<OsStr>>(program: S) -> Self {
Self { cmd: StdCommand::new(program), stdin: None }
}

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

/// 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);
}
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();

let output = self.command_output();
if output.status().success() {
handle_failed_output(&self, output, caller_line_number);
}
output
}

#[track_caller]
pub(crate) 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());
self.cmd.stderr(Stdio::piped());

let output = if let Some(input) = &self.stdin {
let mut child = self.cmd.spawn().unwrap();

{
let mut stdin = child.stdin.take().unwrap();
stdin.write_all(input.as_ref()).unwrap();
}

child.wait_with_output().expect("failed to get output of finished process")
} else {
self.cmd.output().expect("failed to get output of finished process")
};
output.into()
}
}

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.
pub struct CompletedProcess {
output: Output,
}

impl CompletedProcess {
pub fn stdout_utf8(&self) -> String {
String::from_utf8(self.output.stdout.clone()).expect("stdout is not valid UTF-8")
}

pub fn stderr_utf8(&self) -> String {
String::from_utf8(self.output.stderr.clone()).expect("stderr is not valid UTF-8")
}

pub fn status(&self) -> ExitStatus {
self.output.status
}

/// Checks that trimmed `stdout` matches trimmed `content`.
#[track_caller]
pub fn assert_stdout_equals<S: AsRef<str>>(self, content: S) -> Self {
assert_eq!(self.stdout_utf8().trim(), content.as_ref().trim());
self
}

#[track_caller]
pub fn assert_stdout_not_contains<S: AsRef<str>>(self, needle: S) -> Self {
assert_not_contains(&self.stdout_utf8(), needle.as_ref());
self
}

/// Checks that trimmed `stderr` matches trimmed `content`.
#[track_caller]
pub fn assert_stderr_equals<S: AsRef<str>>(self, content: S) -> Self {
assert_eq!(self.stderr_utf8().trim(), content.as_ref().trim());
self
}

#[track_caller]
pub fn assert_stderr_contains<S: AsRef<str>>(self, needle: S) -> Self {
assert!(self.stderr_utf8().contains(needle.as_ref()));
self
}

#[track_caller]
pub fn assert_stderr_not_contains<S: AsRef<str>>(self, needle: S) -> Self {
assert_not_contains(&self.stdout_utf8(), needle.as_ref());
self
}

#[track_caller]
pub fn assert_exit_code(self, code: i32) -> Self {
assert!(self.output.status.code() == Some(code));
self
}
}

impl From<Output> for CompletedProcess {
fn from(output: Output) -> Self {
Self { output }
}
}
51 changes: 19 additions & 32 deletions src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

pub mod cc;
pub mod clang;
mod command;
pub mod diff;
pub mod llvm_readobj;
pub mod run;
Expand All @@ -16,7 +17,6 @@ use std::ffi::OsString;
use std::fs;
use std::io;
use std::path::{Path, PathBuf};
use std::process::{Command, Output};

pub use gimli;
pub use object;
Expand All @@ -27,7 +27,7 @@ pub use cc::{cc, extra_c_flags, extra_cxx_flags, Cc};
pub use clang::{clang, Clang};
pub use diff::{diff, Diff};
pub use llvm_readobj::{llvm_readobj, LlvmReadobj};
pub use run::{run, run_fail};
pub use run::{cmd, run, run_fail};
pub use rustc::{aux_build, rustc, Rustc};
pub use rustdoc::{bare_rustdoc, rustdoc, Rustdoc};

Expand Down Expand Up @@ -167,13 +167,12 @@ pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
let mut cygpath = Command::new("cygpath");
cygpath.arg("-w");
cygpath.arg(path.as_ref());
let output = cygpath.output().unwrap();
if !output.status.success() {
let output = cygpath.command_output();
if !output.status().success() {
handle_failed_output(&cygpath, output, caller_line_number);
}
let s = String::from_utf8(output.stdout).unwrap();
// cygpath -w can attach a newline
s.trim().to_string()
output.stdout_utf8().trim().to_string()
}

/// Run `uname`. This assumes that `uname` is available on the platform!
Expand All @@ -183,23 +182,23 @@ pub fn uname() -> String {
let caller_line_number = caller_location.line();

let mut uname = Command::new("uname");
let output = uname.output().unwrap();
if !output.status.success() {
let output = uname.command_output();
if !output.status().success() {
handle_failed_output(&uname, output, caller_line_number);
}
String::from_utf8(output.stdout).unwrap()
output.stdout_utf8()
}

fn handle_failed_output(cmd: &Command, output: Output, caller_line_number: u32) -> ! {
if output.status.success() {
fn handle_failed_output(cmd: &Command, output: CompletedProcess, caller_line_number: u32) -> ! {
if output.status().success() {
eprintln!("command unexpectedly succeeded at line {caller_line_number}");
} else {
eprintln!("command failed at line {caller_line_number}");
}
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());
eprintln!("output status: `{}`", output.status());
eprintln!("=== STDOUT ===\n{}\n\n", output.stdout_utf8());
eprintln!("=== STDERR ===\n{}\n\n", output.stderr_utf8());
std::process::exit(1)
}

Expand Down Expand Up @@ -286,6 +285,7 @@ pub fn read_dir<F: Fn(&Path)>(dir: impl AsRef<Path>, callback: F) {
}

/// Check that `haystack` does not contain `needle`. Panic otherwise.
#[track_caller]
pub fn assert_not_contains(haystack: &str, needle: &str) {
if haystack.contains(needle) {
eprintln!("=== HAYSTACK ===");
Expand Down Expand Up @@ -412,28 +412,14 @@ macro_rules! impl_common_helpers {

/// 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.command_output();
if !output.status.success() {
handle_failed_output(&self.cmd, output, caller_line_number);
}
output
pub fn run(&mut self) -> crate::command::CompletedProcess {
self.cmd.run()
}

/// 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.command_output();
if output.status.success() {
handle_failed_output(&self.cmd, output, caller_line_number);
}
output
pub fn run_fail(&mut self) -> crate::command::CompletedProcess {
self.cmd.run_fail()
}

/// Set the path where the command will be run.
Expand All @@ -445,4 +431,5 @@ macro_rules! impl_common_helpers {
};
}

use crate::command::{Command, CompletedProcess};
pub(crate) use impl_common_helpers;
10 changes: 2 additions & 8 deletions src/tools/run-make-support/src/llvm_readobj.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::path::{Path, PathBuf};
use std::process::Command;

use crate::{env_var, handle_failed_output};
use crate::command::Command;
use crate::env_var;

/// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
/// at `$LLVM_BIN_DIR/llvm-readobj`.
Expand Down Expand Up @@ -39,10 +39,4 @@ impl LlvmReadobj {
self.cmd.arg("--file-header");
self
}

/// Get the [`Output`][::std::process::Output] of the finished process.
#[track_caller]
pub fn command_output(&mut self) -> ::std::process::Output {
self.cmd.output().expect("failed to get output of finished process")
}
}
25 changes: 17 additions & 8 deletions src/tools/run-make-support/src/run.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use std::env;
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::process::{Command, Output};

use crate::{cwd, env_var, is_windows};
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, Output) {
fn run_common(name: &str) -> (Command, CompletedProcess) {
let mut bin_path = PathBuf::new();
bin_path.push(cwd());
bin_path.push(name);
Expand All @@ -33,32 +34,40 @@ fn run_common(name: &str) -> (Command, Output) {
cmd.env("PATH", env::join_paths(paths.iter()).unwrap());
}

let output = cmd.output().unwrap();
let output = cmd.command_output();
(cmd, output)
}

/// Run a built binary and make sure it succeeds.
#[track_caller]
pub fn run(name: &str) -> Output {
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);
if !output.status.success() {
if !output.status().success() {
handle_failed_output(&cmd, output, caller_line_number);
}
output
}

/// Run a built binary and make sure it fails.
#[track_caller]
pub fn run_fail(name: &str) -> Output {
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);
if output.status.success() {
if output.status().success() {
handle_failed_output(&cmd, output, caller_line_number);
}
output
}

/// Create a new custom Command.
/// This should be preferred to creating `std::process::Command` directly.
pub fn cmd<S: AsRef<OsStr>>(program: S) -> Command {
let mut command = Command::new(program);
set_host_rpath(&mut command);
command
}
Loading

0 comments on commit f21554f

Please sign in to comment.