Auto merge of #3619 - RalfJung:print-sysroot, r=RalfJung

properly print error in 'cargo miri setup --print-sysroot'

Based on rustc-build-sysroot now putting the stderr into the error message.

Author: bors

src/tools/miri/cargo-miri/Cargo.lock

@@ -178,9 +178,9 @@ dependencies = [
 
 [[package]]
 name = "rustc-build-sysroot"
-version = "0.5.0"
+version = "0.5.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "de6077473f0c46779b49e4587a81f1b8919e0ec26630409ecfda0ba3259efb43"
+checksum = "fa3ca63cc537c1cb69e4c2c0afc5fda2ccd36ac84c97d5a4ae05e69b1c834afb"
 dependencies = [
  "anyhow",
  "rustc_version",

src/tools/miri/cargo-miri/Cargo.toml

@@ -18,7 +18,7 @@ directories = "5"
 rustc_version = "0.4"
 serde_json = "1.0.40"
 cargo_metadata = "0.18.0"
-rustc-build-sysroot = "0.5.0"
+rustc-build-sysroot = "0.5.2"
 
 # Enable some feature flags that dev-dependencies need but dependencies
 # do not.  This makes `./miri install` after `./miri build` faster.

src/tools/miri/cargo-miri/src/setup.rs

@@ -2,7 +2,6 @@
 
 use std::env;
 use std::ffi::OsStr;
-use std::fmt::Write;
 use std::path::PathBuf;
 use std::process::{self, Command};
 
@@ -24,6 +23,7 @@ pub fn setup(
     let only_setup = matches!(subcommand, MiriCommand::Setup);
     let ask_user = !only_setup;
     let print_sysroot = only_setup && has_arg_flag("--print-sysroot"); // whether we just print the sysroot path
+    let show_setup = only_setup && !print_sysroot;
     if !only_setup {
         if let Some(sysroot) = std::env::var_os("MIRI_SYSROOT") {
             // Skip setup step if MIRI_SYSROOT is explicitly set, *unless* we are `cargo miri setup`.
@@ -115,18 +115,16 @@ pub fn setup(
         // `config.toml`.
         command.env("RUSTC_WRAPPER", "");
 
-        if only_setup && !print_sysroot {
+        if show_setup {
             // Forward output. Even make it verbose, if requested.
+            command.stdout(process::Stdio::inherit());
+            command.stderr(process::Stdio::inherit());
             for _ in 0..verbose {
                 command.arg("-v");
             }
             if quiet {
                 command.arg("--quiet");
             }
-        } else {
-            // Suppress output.
-            command.stdout(process::Stdio::null());
-            command.stderr(process::Stdio::null());
         }
 
         command
@@ -137,22 +135,25 @@ pub fn setup(
     // not apply `RUSTFLAGS` to the sysroot either.
     let rustflags = &["-Cdebug-assertions=off", "-Coverflow-checks=on"];
 
+    let mut after_build_output = String::new(); // what should be printed when the build is done.
     let notify = || {
-        let mut msg = String::new();
-        write!(msg, "Preparing a sysroot for Miri (target: {target})").unwrap();
-        if verbose > 0 {
-            write!(msg, " in {}", sysroot_dir.display()).unwrap();
-        }
-        write!(msg, "...").unwrap();
-
-        if print_sysroot || quiet {
-            // Be silent.
-        } else if only_setup {
-            // We want to be explicit.
-            eprintln!("{msg}");
-        } else {
-            // We want to be quiet, but still let the user know that something is happening.
-            eprint!("{msg} ");
+        if !quiet {
+            eprint!("Preparing a sysroot for Miri (target: {target})");
+            if verbose > 0 {
+                eprint!(" in {}", sysroot_dir.display());
+            }
+            if show_setup {
+                // Cargo will print things, so we need to finish this line.
+                eprintln!("...");
+                after_build_output = format!(
+                    "A sysroot for Miri is now available in `{}`.\n",
+                    sysroot_dir.display()
+                );
+            } else {
+                // Keep all output on a single line.
+                eprint!("... ");
+                after_build_output = format!("done\n");
+            }
         }
     };
 
@@ -167,31 +168,17 @@ pub fn setup(
         .build_from_source(&rust_src);
     match status {
         Ok(SysrootStatus::AlreadyCached) =>
-            if only_setup && !(print_sysroot || quiet) {
+            if !quiet && show_setup {
                 eprintln!(
                     "A sysroot for Miri is already available in `{}`.",
                     sysroot_dir.display()
                 );
             },
         Ok(SysrootStatus::SysrootBuilt) => {
-            if print_sysroot || quiet {
-                // Be silent.
-            } else if only_setup {
-                eprintln!("A sysroot for Miri is now available in `{}`.", sysroot_dir.display());
-            } else {
-                eprintln!("done");
-            }
+            // Print what `notify` prepared.
+            eprint!("{after_build_output}");
         }
-        Err(err) =>
-            if print_sysroot {
-                show_error!("failed to build sysroot")
-            } else if only_setup {
-                show_error!("failed to build sysroot: {err:?}")
-            } else {
-                show_error!(
-                    "failed to build sysroot; run `cargo miri setup` to see the error details"
-                )
-            },
+        Err(err) => show_error!("failed to build sysroot: {err:?}"),
     }
 
     if print_sysroot {

src/tools/miri/miri-script/src/commands.rs

@@ -42,28 +42,19 @@ impl MiriEnv {
         let target_flag = &target_flag;
 
         if !quiet {
+            eprint!("$ cargo miri setup");
             if let Some(target) = target {
-                eprintln!("$ (building Miri sysroot for {})", target.to_string_lossy());
-            } else {
-                eprintln!("$ (building Miri sysroot)");
+                eprint!(" --target {target}", target = target.to_string_lossy());
             }
+            eprintln!();
         }
 
-        let output = cmd!(self.sh,
+        let mut cmd = cmd!(self.sh,
             "cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} --
              miri setup --print-sysroot {target_flag...}"
-        ).read();
-        let Ok(output) = output else {
-            // Run it again (without `--print-sysroot` or `--quiet`) so the user can see the error.
-            cmd!(
-                self.sh,
-                "cargo +{toolchain} run {cargo_extra_flags...} --manifest-path {manifest_path} --
-                miri setup {target_flag...}"
-            )
-            .run()
-            .with_context(|| "`cargo miri setup` failed")?;
-            panic!("`cargo miri setup` didn't fail again the 2nd time?");
-        };
+        );
+        cmd.set_quiet(quiet);
+        let output = cmd.read()?;
         self.sh.set_var("MIRI_SYSROOT", &output);
         Ok(output.into())
     }