Auto merge of #2425 - RalfJung:hide-xargo, r=RalfJung

don't dump xargo output onto users of 'cargo miri test'

The xargo invocation prints a lot of details users probably won't care about, so let's hide them (unless the user did `cargo miri setup`, then we still print everything).

cargo-miri/bin.rs

@@ -497,9 +497,26 @@ path = "lib.rs"
     // Disable debug assertions in the standard library -- Miri is already slow enough.
     // But keep the overflow checks, they are cheap.
     command.env("RUSTFLAGS", "-Cdebug-assertions=off -Coverflow-checks=on");
+    // Manage the output the user sees.
+    if only_setup {
+        eprintln!("Preparing a sysroot for Miri...");
+    } else {
+        eprint!("Preparing a sysroot for Miri... ");
+        command.stdout(process::Stdio::null());
+        command.stderr(process::Stdio::null());
+    }
     // Finally run it!
     if command.status().expect("failed to run xargo").success().not() {
-        show_error(format!("failed to run xargo"));
+        if only_setup {
+            show_error(format!("failed to run xargo, see error details above"))
+        } else {
+            show_error(format!(
+                "failed to run xargo; run `cargo miri setup` to see the error details"
+            ))
+        }
+    }
+    if !only_setup {
+        eprintln!("done");
     }
 
     // That should be it! But we need to figure out where xargo built stuff.
@@ -510,10 +527,10 @@ path = "lib.rs"
     // Figure out what to print.
     let print_sysroot = only_setup && has_arg_flag("--print-sysroot"); // whether we just print the sysroot path
     if print_sysroot {
-        // Print just the sysroot and nothing else; this way we do not need any escaping.
+        // Print just the sysroot and nothing else to stdout; this way we do not need any escaping.
         println!("{}", sysroot.display());
     } else if only_setup {
-        println!("A libstd for Miri is now available in `{}`.", sysroot.display());
+        eprintln!("A sysroot for Miri is now available in `{}`.", sysroot.display());
     }
 }
 

ci.sh

@@ -21,6 +21,7 @@ function run_tests {
     echo "Testing host architecture"
   fi
 
+  ## ui test suite
   ./miri test --locked
   if [ -z "${MIRI_TEST_TARGET+exists}" ]; then
     # Only for host architecture: tests with optimizations (`-O` is what cargo passes, but crank MIR
@@ -30,15 +31,13 @@ function run_tests {
     MIRIFLAGS="-O -Zmir-opt-level=4" MIRI_SKIP_UI_CHECKS=1 ./miri test --locked -- tests/{pass,panic}
   fi
 
+  ## test-cargo-miri
   # On Windows, there is always "python", not "python3" or "python2".
   if command -v python3 > /dev/null; then
     PYTHON=python3
   else
     PYTHON=python
   fi
-
-  # "miri test" has built the sysroot for us, now this should pass without
-  # any interactive questions.
   ${PYTHON} test-cargo-miri/run-test.py
   echo
 

src/machine.rs

@@ -367,20 +367,14 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
             measureme::Profiler::new(out).expect("Couldn't create `measureme` profiler")
         });
         let rng = StdRng::seed_from_u64(config.seed.unwrap_or(0));
-        let stacked_borrows = if config.stacked_borrows {
-            Some(RefCell::new(stacked_borrows::GlobalStateInner::new(
+        let stacked_borrows = config.stacked_borrows.then(|| {
+            RefCell::new(stacked_borrows::GlobalStateInner::new(
                 config.tracked_pointer_tags.clone(),
                 config.tracked_call_ids.clone(),
                 config.retag_fields,
-            )))
-        } else {
-            None
-        };
-        let data_race = if config.data_race_detector {
-            Some(data_race::GlobalState::new(config))
-        } else {
-            None
-        };
+            ))
+        });
+        let data_race = config.data_race_detector.then(|| data_race::GlobalState::new(config));
         Evaluator {
             stacked_borrows,
             data_race,
@@ -691,32 +685,24 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         }
 
         let alloc = alloc.into_owned();
-        let stacks = if let Some(stacked_borrows) = &ecx.machine.stacked_borrows {
-            Some(Stacks::new_allocation(
+        let stacks = ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| {
+            Stacks::new_allocation(
                 id,
                 alloc.size(),
                 stacked_borrows,
                 kind,
                 ecx.machine.current_span(),
-            ))
-        } else {
-            None
-        };
-        let race_alloc = if let Some(data_race) = &ecx.machine.data_race {
-            Some(data_race::AllocExtra::new_allocation(
+            )
+        });
+        let race_alloc = ecx.machine.data_race.as_ref().map(|data_race| {
+            data_race::AllocExtra::new_allocation(
                 data_race,
                 &ecx.machine.threads,
                 alloc.size(),
                 kind,
-            ))
-        } else {
-            None
-        };
-        let buffer_alloc = if ecx.machine.weak_memory {
-            Some(weak_memory::AllocExtra::new_allocation())
-        } else {
-            None
-        };
+            )
+        });
+        let buffer_alloc = ecx.machine.weak_memory.then(weak_memory::AllocExtra::new_allocation);
         let alloc: Allocation<Provenance, Self::AllocExtra> = alloc.adjust_from_tcx(
             &ecx.tcx,
             AllocExtra {

test-cargo-miri/run-test.py

@@ -25,7 +25,12 @@ def cargo_miri(cmd, quiet = True):
 
 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)
+    str = re.sub("finished in \d+\.\d\ds", "finished in $TIME", str) # the time keeps changing, obviously
+    return str
+
+def normalize_stderr(str):
+    str = str.replace("Preparing a sysroot for Miri... done\n", "") # remove leading cargo-miri setup output
+    return str
 
 def check_output(actual, path, name):
     expected = open(path).read()
@@ -51,9 +56,8 @@ def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}):
         env=p_env,
     )
     (stdout, stderr) = p.communicate(input=stdin)
-    stdout = stdout.decode("UTF-8")
-    stderr = stderr.decode("UTF-8")
-    stdout = normalize_stdout(stdout)
+    stdout = normalize_stdout(stdout.decode("UTF-8"))
+    stderr = normalize_stderr(stderr.decode("UTF-8"))
 
     stdout_matches = check_output(stdout, stdout_ref, "stdout")
     stderr_matches = check_output(stderr, stderr_ref, "stderr")
@@ -175,10 +179,6 @@ os.environ["RUST_TEST_THREADS"] = "1" # avoid non-deterministic output due to co
 target_str = " for target {}".format(os.environ['MIRI_TEST_TARGET']) if 'MIRI_TEST_TARGET' in os.environ else ""
 print(CGREEN + CBOLD + "## Running `cargo miri` tests{}".format(target_str) + CEND)
 
-if not 'MIRI_SYSROOT' in os.environ:
-    # Make sure we got a working sysroot.
-    # (If the sysroot gets built later when output is compared, that leads to test failures.)
-    subprocess.run(cargo_miri("setup"), check=True)
 test_cargo_miri_run()
 test_cargo_miri_test()
 # Ensure we did not create anything outside the expected target dir.