Rollup merge of rust-lang#122460 - jieyouxu:rmake-example-refactor, r…

…=Nilstrieb

Rework rmake support library API

### Take 1: Strongly-typed API

Context: rust-lang#122448 (comment)

> My 2 cents: from my experience with writing similar "test DSLs", I would suggest to create these helpers as soon as possible in the process (basically the first time someone needs them, not only after N similar usages), and basically treat any imperative code in these high-level tests as a maintenance burden, basically making them as declarative as possible. Otherwise it might be a bit annoying to keep refactoring the tests later once such helpers are available.
>
> I would even discourage the arg method and create explicit methods for setting things like unpretty, the output file etc., but this might be more controversial, as it will make the invoked command-line arguments more opaque.

cc `@Kobzol` for the testing DSL suggestion.

Example:

```rs
let output = Rustc::new()
    .input_file("main.rs")
    .emit(&[EmitKind::Metadata])
    .extern_("stable", &stable_path)
    .output();
```

### Take 2: xshell-based macro API

Example:

```rs
let sh = Shell::new()?;
let stable_path = stable_path.to_string_lossy();
let output = cmd!(sh, "rustc main.rs --emit=metadata --extern stable={stable_path}").output()?;
```

### Take 3: Weakly-typed API with a few helper methods

```rs
let output = Rustc::new()
    .input("main.rs")
    .emit("metadata")
    .extern_("stable", &stable_path)
    .output();
```

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

@@ -1,10 +1,14 @@
+pub mod run;
+
 use std::env;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Output};
 
 pub use object;
 pub use wasmparser;
 
+pub use run::{run, run_fail};
+
 pub fn out_dir() -> PathBuf {
     env::var_os("TMPDIR").unwrap().into()
 }
@@ -25,65 +29,122 @@ fn handle_failed_output(cmd: &str, output: Output, caller_line_number: u32) -> !
     std::process::exit(1)
 }
 
-pub fn rustc() -> RustcInvocationBuilder {
-    RustcInvocationBuilder::new()
+/// Construct a new `rustc` invocation.
+pub fn rustc() -> Rustc {
+    Rustc::new()
 }
 
-pub fn aux_build() -> AuxBuildInvocationBuilder {
-    AuxBuildInvocationBuilder::new()
+/// Construct a new `rustc` aux-build invocation.
+pub fn aux_build() -> Rustc {
+    Rustc::new_aux_build()
 }
 
+/// A `rustc` invocation builder.
 #[derive(Debug)]
-pub struct RustcInvocationBuilder {
+pub struct Rustc {
     cmd: Command,
 }
 
-impl RustcInvocationBuilder {
-    fn new() -> Self {
+impl Rustc {
+    // `rustc` invocation constructor methods
+
+    /// Construct a new `rustc` invocation.
+    pub fn new() -> Self {
         let cmd = setup_common_build_cmd();
         Self { cmd }
     }
 
-    pub fn arg(&mut self, arg: &str) -> &mut RustcInvocationBuilder {
-        self.cmd.arg(arg);
+    /// Construct a new `rustc` invocation with `aux_build` preset (setting `--crate-type=lib`).
+    pub fn new_aux_build() -> Self {
+        let mut cmd = setup_common_build_cmd();
+        cmd.arg("--crate-type=lib");
+        Self { cmd }
+    }
+
+    // Argument provider methods
+
+    /// Configure the compilation environment.
+    pub fn cfg(&mut self, s: &str) -> &mut Self {
+        self.cmd.arg("--cfg");
+        self.cmd.arg(s);
         self
     }
 
-    pub fn args(&mut self, args: &[&str]) -> &mut RustcInvocationBuilder {
-        self.cmd.args(args);
+    /// Specify default optimization level `-O` (alias for `-C opt-level=2`).
+    pub fn opt(&mut self) -> &mut Self {
+        self.cmd.arg("-O");
         self
     }
 
-    #[track_caller]
-    pub fn run(&mut self) -> Output {
-        let caller_location = std::panic::Location::caller();
-        let caller_line_number = caller_location.line();
+    /// Specify type(s) of output files to generate.
+    pub fn emit(&mut self, kinds: &str) -> &mut Self {
+        self.cmd.arg(format!("--emit={kinds}"));
+        self
+    }
 
-        let output = self.cmd.output().unwrap();
-        if !output.status.success() {
-            handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number);
-        }
-        output
+    /// Specify where an external library is located.
+    pub fn extern_<P: AsRef<Path>>(&mut self, crate_name: &str, path: P) -> &mut Self {
+        assert!(
+            !crate_name.contains(|c: char| c.is_whitespace() || c == '\\' || c == '/'),
+            "crate name cannot contain whitespace or path separators"
+        );
+
+        let path = path.as_ref().to_string_lossy();
+
+        self.cmd.arg("--extern");
+        self.cmd.arg(format!("{crate_name}={path}"));
+
+        self
     }
-}
 
-#[derive(Debug)]
-pub struct AuxBuildInvocationBuilder {
-    cmd: Command,
-}
+    /// Specify path to the input file.
+    pub fn input<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
+        self.cmd.arg(path.as_ref());
+        self
+    }
 
-impl AuxBuildInvocationBuilder {
-    fn new() -> Self {
-        let mut cmd = setup_common_build_cmd();
-        cmd.arg("--crate-type=lib");
-        Self { cmd }
+    /// Specify target triple.
+    pub fn target(&mut self, target: &str) -> &mut Self {
+        assert!(!target.contains(char::is_whitespace), "target triple cannot contain spaces");
+        self.cmd.arg(format!("--target={target}"));
+        self
     }
 
-    pub fn arg(&mut self, arg: &str) -> &mut AuxBuildInvocationBuilder {
+    /// Generic command argument provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`.
+    /// This method will panic if a plain `-Z` or `-C` is passed, or if `-Z <name>` or `-C <name>`
+    /// is passed (note the space).
+    pub fn arg(&mut self, arg: &str) -> &mut Self {
+        assert!(
+            !(["-Z", "-C"].contains(&arg) || arg.starts_with("-Z ") || arg.starts_with("-C ")),
+            "use `-Zarg` or `-Carg` over split `-Z` `arg` or `-C` `arg`"
+        );
         self.cmd.arg(arg);
         self
     }
 
+    /// Generic command arguments provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`.
+    /// This method will panic if a plain `-Z` or `-C` is passed, or if `-Z <name>` or `-C <name>`
+    /// is passed (note the space).
+    pub fn args(&mut self, args: &[&str]) -> &mut Self {
+        for arg in args {
+            assert!(
+                !(["-Z", "-C"].contains(&arg) || arg.starts_with("-Z ") || arg.starts_with("-C ")),
+                "use `-Zarg` or `-Carg` over split `-Z` `arg` or `-C` `arg`"
+            );
+        }
+
+        self.cmd.args(args);
+        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();
@@ -95,66 +156,10 @@ impl AuxBuildInvocationBuilder {
         }
         output
     }
-}
-
-fn run_common(bin_name: &str) -> (Command, Output) {
-    let target = env::var("TARGET").unwrap();
-
-    let bin_name =
-        if target.contains("windows") { format!("{}.exe", bin_name) } else { bin_name.to_owned() };
-
-    let mut bin_path = PathBuf::new();
-    bin_path.push(env::var("TMPDIR").unwrap());
-    bin_path.push(&bin_name);
-    let ld_lib_path_envvar = env::var("LD_LIB_PATH_ENVVAR").unwrap();
-    let mut cmd = Command::new(bin_path);
-    cmd.env(&ld_lib_path_envvar, {
-        let mut paths = vec![];
-        paths.push(PathBuf::from(env::var("TMPDIR").unwrap()));
-        for p in env::split_paths(&env::var("TARGET_RPATH_ENV").unwrap()) {
-            paths.push(p.to_path_buf());
-        }
-        for p in env::split_paths(&env::var(&ld_lib_path_envvar).unwrap()) {
-            paths.push(p.to_path_buf());
-        }
-        env::join_paths(paths.iter()).unwrap()
-    });
 
-    if target.contains("windows") {
-        let mut paths = vec![];
-        for p in env::split_paths(&std::env::var("PATH").unwrap_or(String::new())) {
-            paths.push(p.to_path_buf());
-        }
-        paths.push(Path::new(&std::env::var("TARGET_RPATH_DIR").unwrap()).to_path_buf());
-        cmd.env("PATH", env::join_paths(paths.iter()).unwrap());
-    }
-
-    let output = cmd.output().unwrap();
-    (cmd, output)
-}
-
-/// Run a built binary and make sure it succeeds.
-#[track_caller]
-pub fn run(bin_name: &str) -> Output {
-    let caller_location = std::panic::Location::caller();
-    let caller_line_number = caller_location.line();
-
-    let (cmd, output) = run_common(bin_name);
-    if !output.status.success() {
-        handle_failed_output(&format!("{:#?}", cmd), output, caller_line_number);
-    }
-    output
-}
-
-/// Run a built binary and make sure it fails.
-#[track_caller]
-pub fn run_fail(bin_name: &str) -> Output {
-    let caller_location = std::panic::Location::caller();
-    let caller_line_number = caller_location.line();
-
-    let (cmd, output) = run_common(bin_name);
-    if output.status.success() {
-        handle_failed_output(&format!("{:#?}", cmd), output, caller_line_number);
+    /// 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
     }
-    output
 }

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

@@ -0,0 +1,67 @@
+use std::env;
+use std::path::{Path, PathBuf};
+use std::process::{Command, Output};
+
+use super::handle_failed_output;
+
+fn run_common(bin_name: &str) -> (Command, Output) {
+    let target = env::var("TARGET").unwrap();
+
+    let bin_name =
+        if target.contains("windows") { format!("{}.exe", bin_name) } else { bin_name.to_owned() };
+
+    let mut bin_path = PathBuf::new();
+    bin_path.push(env::var("TMPDIR").unwrap());
+    bin_path.push(&bin_name);
+    let ld_lib_path_envvar = env::var("LD_LIB_PATH_ENVVAR").unwrap();
+    let mut cmd = Command::new(bin_path);
+    cmd.env(&ld_lib_path_envvar, {
+        let mut paths = vec![];
+        paths.push(PathBuf::from(env::var("TMPDIR").unwrap()));
+        for p in env::split_paths(&env::var("TARGET_RPATH_ENV").unwrap()) {
+            paths.push(p.to_path_buf());
+        }
+        for p in env::split_paths(&env::var(&ld_lib_path_envvar).unwrap()) {
+            paths.push(p.to_path_buf());
+        }
+        env::join_paths(paths.iter()).unwrap()
+    });
+
+    if target.contains("windows") {
+        let mut paths = vec![];
+        for p in env::split_paths(&std::env::var("PATH").unwrap_or(String::new())) {
+            paths.push(p.to_path_buf());
+        }
+        paths.push(Path::new(&std::env::var("TARGET_RPATH_DIR").unwrap()).to_path_buf());
+        cmd.env("PATH", env::join_paths(paths.iter()).unwrap());
+    }
+
+    let output = cmd.output().unwrap();
+    (cmd, output)
+}
+
+/// Run a built binary and make sure it succeeds.
+#[track_caller]
+pub fn run(bin_name: &str) -> Output {
+    let caller_location = std::panic::Location::caller();
+    let caller_line_number = caller_location.line();
+
+    let (cmd, output) = run_common(bin_name);
+    if !output.status.success() {
+        handle_failed_output(&format!("{:#?}", cmd), output, caller_line_number);
+    }
+    output
+}
+
+/// Run a built binary and make sure it fails.
+#[track_caller]
+pub fn run_fail(bin_name: &str) -> Output {
+    let caller_location = std::panic::Location::caller();
+    let caller_line_number = caller_location.line();
+
+    let (cmd, output) = run_common(bin_name);
+    if output.status.success() {
+        handle_failed_output(&format!("{:#?}", cmd), output, caller_line_number);
+    }
+    output
+}

tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs

@@ -1,24 +1,25 @@
 // ignore-tidy-linelength
 
+// Check that the `CURRENT_RUSTC_VERSION` placeholder is correctly replaced by the current
+// `rustc` version and the `since` property in feature stability gating is properly respected.
+
 extern crate run_make_support;
 
 use std::path::PathBuf;
 
-use run_make_support::{aux_build, rustc};
+use run_make_support::{rustc, aux_build};
 
 fn main() {
-    aux_build()
-        .arg("--emit=metadata")
-        .arg("stable.rs")
-        .run();
+    aux_build().input("stable.rs").emit("metadata").run();
+
     let mut stable_path = PathBuf::from(env!("TMPDIR"));
     stable_path.push("libstable.rmeta");
+
     let output = rustc()
-        .arg("--emit=metadata")
-        .arg("--extern")
-        .arg(&format!("stable={}", &stable_path.to_string_lossy()))
-        .arg("main.rs")
-        .run();
+        .input("main.rs")
+        .emit("metadata")
+        .extern_("stable", &stable_path)
+        .output();
 
     let stderr = String::from_utf8_lossy(&output.stderr);
     let version = include_str!(concat!(env!("S"), "/src/version"));