Auto merge of #2373 - rust-lang:test_dependencies, r=RalfJung

Allow ui tests to have dependencies in a reliable way

This completely sidesteps the issue that compiletest-rs has where old artifacts of a dependency cause `multiple available crates of name XXX` errors. At this point I think we've reached feature parity for clippy, too, so I'm going to try publishing a version once this is merged.

cargo-miri/bin.rs

@@ -3,6 +3,7 @@
 
 mod version;
 
+use std::collections::HashMap;
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fmt::Write as _;
@@ -114,10 +115,14 @@ fn show_error(msg: String) -> ! {
     std::process::exit(1)
 }
 
-// Determines whether a `--flag` is present.
+/// Determines whether a `--flag` is present.
 fn has_arg_flag(name: &str) -> bool {
-    let mut args = std::env::args().take_while(|val| val != "--");
-    args.any(|val| val == name)
+    num_arg_flag(name) > 0
+}
+
+/// Determines how many times a `--flag` is present.
+fn num_arg_flag(name: &str) -> usize {
+    std::env::args().take_while(|val| val != "--").filter(|val| val == name).count()
 }
 
 /// Yields all values of command line flag `name` as `Ok(arg)`, and all other arguments except
@@ -588,7 +593,7 @@ fn phase_cargo_miri(mut args: env::Args) {
                 "`cargo miri` supports the following subcommands: `run`, `test`, and `setup`."
             )),
     };
-    let verbose = has_arg_flag("-v");
+    let verbose = num_arg_flag("-v");
 
     // We always setup.
     setup(&subcommand);
@@ -685,15 +690,15 @@ fn phase_cargo_miri(mut args: env::Args) {
     cmd.env("MIRI_LOCAL_CRATES", local_crates(&metadata));
 
     // Run cargo.
-    if verbose {
+    if verbose > 0 {
         eprintln!("[cargo-miri miri] RUSTC_WRAPPER={:?}", cargo_miri_path);
         eprintln!("[cargo-miri miri] {}={:?}", target_runner_env_name, cargo_miri_path);
         if *target != host {
             eprintln!("[cargo-miri miri] {}={:?}", host_runner_env_name, cargo_miri_path);
         }
         eprintln!("[cargo-miri miri] RUSTDOC={:?}", cargo_miri_path);
         eprintln!("[cargo-miri miri] {:?}", cmd);
-        cmd.env("MIRI_VERBOSE", ""); // This makes the other phases verbose.
+        cmd.env("MIRI_VERBOSE", verbose.to_string()); // This makes the other phases verbose.
     }
     exec(cmd)
 }
@@ -752,7 +757,8 @@ fn phase_rustc(mut args: env::Args, phase: RustcPhase) {
         }
     }
 
-    let verbose = std::env::var_os("MIRI_VERBOSE").is_some();
+    let verbose = std::env::var("MIRI_VERBOSE")
+        .map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer"));
     let target_crate = is_target_crate();
     let print = get_arg_flag_value("--print").is_some() || has_arg_flag("-vV"); // whether this is cargo/xargo invoking rustc to get some infos
 
@@ -761,13 +767,13 @@ fn phase_rustc(mut args: env::Args, phase: RustcPhase) {
         // https://github.com/rust-lang/miri/issues/1724#issuecomment-787115693
         // As we store a JSON file instead of building the crate here, an empty file is fine.
         let dep_info_name = out_filename("", ".d");
-        if verbose {
+        if verbose > 0 {
             eprintln!("[cargo-miri rustc] writing stub dep-info to `{}`", dep_info_name.display());
         }
         File::create(dep_info_name).expect("failed to create fake .d file");
 
         let filename = out_filename("", "");
-        if verbose {
+        if verbose > 0 {
             eprintln!("[cargo-miri rustc] writing run info to `{}`", filename.display());
         }
         info.store(&filename);
@@ -810,7 +816,7 @@ fn phase_rustc(mut args: env::Args, phase: RustcPhase) {
             cmd.args(&env.args);
             cmd.env("MIRI_BE_RUSTC", "target");
 
-            if verbose {
+            if verbose > 0 {
                 eprintln!(
                     "[cargo-miri rustc] captured input:\n{}",
                     std::str::from_utf8(&env.stdin).unwrap()
@@ -877,6 +883,15 @@ fn phase_rustc(mut args: env::Args, phase: RustcPhase) {
             cmd.arg("-C").arg("panic=abort");
         }
     } else {
+        // For host crates (but not when we are printing), we might still have to set the sysroot.
+        if !print {
+            // When we're running `cargo-miri` from `x.py` we need to pass the sysroot explicitly as rustc
+            // can't figure out the sysroot on its own unless it's from rustup.
+            if let Some(sysroot) = std::env::var_os("SYSROOT") {
+                cmd.arg("--sysroot").arg(sysroot);
+            }
+        }
+
         // For host crates or when we are printing, just forward everything.
         cmd.args(args);
     }
@@ -888,8 +903,14 @@ fn phase_rustc(mut args: env::Args, phase: RustcPhase) {
     cmd.env("MIRI_BE_RUSTC", if target_crate { "target" } else { "host" });
 
     // Run it.
-    if verbose {
-        eprintln!("[cargo-miri rustc] {:?}", cmd);
+    if verbose > 0 {
+        eprint!("[cargo-miri rustc] ");
+        if verbose > 1 {
+            for (key, value) in env_vars_from_cmd(&cmd) {
+                eprintln!("{key}={value:?} \\");
+            }
+        }
+        eprintln!("{:?}", cmd);
     }
     exec(cmd);
 
@@ -908,6 +929,23 @@ fn phase_rustc(mut args: env::Args, phase: RustcPhase) {
     }
 }
 
+fn env_vars_from_cmd(cmd: &Command) -> Vec<(String, String)> {
+    let mut envs = HashMap::new();
+    for (key, value) in std::env::vars() {
+        envs.insert(key, value);
+    }
+    for (key, value) in cmd.get_envs() {
+        if let Some(value) = value {
+            envs.insert(key.to_str().unwrap().into(), value.to_str().unwrap().to_owned());
+        } else {
+            envs.remove(key.to_str().unwrap());
+        }
+    }
+    let mut envs: Vec<_> = envs.into_iter().collect();
+    envs.sort();
+    envs
+}
+
 #[derive(Debug, Copy, Clone, PartialEq)]
 enum RunnerPhase {
     /// `cargo` is running a binary

test-cargo-miri/Cargo.toml

@@ -19,8 +19,6 @@ issue_rust_86261 = { path = "issue-rust-86261" }
 
 [dev-dependencies]
 rand = { version = "0.8", features = ["small_rng"] }
-getrandom_1 = { package = "getrandom", version = "0.1" }
-getrandom_2 = { package = "getrandom", version = "0.2" }
 serde_derive = "1.0" # not actually used, but exercises some unique code path (`--extern` .so file)
 page_size = "0.4.1"
 

test-cargo-miri/run-test.py

@@ -5,7 +5,7 @@
 and the working directory to contain the cargo-miri-test project.
 '''
 
-import sys, subprocess, os, re
+import sys, subprocess, os, re, difflib
 
 CGREEN  = '\33[32m'
 CBOLD   = '\33[1m'
@@ -27,6 +27,17 @@ def normalize_stdout(str):
     str = str.replace("src\\", "src/") # normalize paths across platforms
     return re.sub("finished in \d+\.\d\ds", "finished in $TIME", str)
 
+def check_output(actual, path, name):
+    expected = open(path).read()
+    if expected == actual:
+        return True
+    print(f"{path} did not match reference!")
+    print(f"--- BEGIN diff {name} ---")
+    for text in difflib.unified_diff(expected.split("\n"), actual.split("\n")):
+        print(text)
+    print(f"--- END diff {name} ---")
+    return False
+
 def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}):
     print("Testing {}...".format(name))
     ## Call `cargo miri`, capture all output
@@ -42,17 +53,14 @@ def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}):
     (stdout, stderr) = p.communicate(input=stdin)
     stdout = stdout.decode("UTF-8")
     stderr = stderr.decode("UTF-8")
-    if p.returncode == 0 and normalize_stdout(stdout) == open(stdout_ref).read() and stderr == open(stderr_ref).read():
+    stdout = normalize_stdout(stdout)
+
+    stdout_matches = check_output(stdout, stdout_ref, "stdout")
+    stderr_matches = check_output(stderr, stderr_ref, "stderr")
+
+    if p.returncode == 0 and stdout_matches and stderr_matches:
         # All good!
         return
-    # Show output
-    print("Test stdout or stderr did not match reference!")
-    print("--- BEGIN test stdout ---")
-    print(stdout, end="")
-    print("--- END test stdout ---")
-    print("--- BEGIN test stderr ---")
-    print(stderr, end="")
-    print("--- END test stderr ---")
     fail("exit code was {}".format(p.returncode))
 
 def test_no_rebuild(name, cmd, env={}):

test-cargo-miri/tests/test.rs

@@ -20,15 +20,9 @@ fn does_not_work_on_miri() {
     assert!(&x as *const _ as usize % 4 < 4);
 }
 
-// We also use this to test some external crates, that we cannot depend on in the compiletest suite.
-
+// Make sure integration tests can access dev-dependencies
 #[test]
 fn entropy_rng() {
-    // Test `getrandom` directly (in multiple different versions).
-    let mut data = vec![0; 16];
-    getrandom_1::getrandom(&mut data).unwrap();
-    getrandom_2::getrandom(&mut data).unwrap();
-
     // Try seeding with "real" entropy.
     let mut rng = SmallRng::from_entropy();
     let _val = rng.gen::<i32>();