Merge #219

219: Improve handling of cases where llvm-tools-preview is not installed r=taiki-e a=taiki-e **TL;DR:** You no longer need to install `llvm-tools-preview` before running cargo-llvm-cov in most cases. Previously, cargo-llvm-cov exit with error suggesting the installation of `llvm-tools-preview`. The new logic is based on the logic used by Miri when `rust-src` component or `xargo` is not installed. - cargo-llvm-cov asks the user if they want to install the `llvm-tools-preview`, and if the user allows it, cargo-llvm-cov will install the `llvm-tools-preview`. - On CI, cargo-llvm-cov will install the `llvm-tools-preview` without asking the user (interactive prompts don't work on CI). Additionally, this adds an environment variable to control the behavior here. - If `CARGO_LLVM_COV_SETUP=no` environment variable is set, cargo-llvm-cov exit with error suggesting the installation of `llvm-tools-preview`. This matches with the previous behavior. - If `CARGO_LLVM_COV_SETUP=yes` environment variable is set, cargo-llvm-cov will install the `llvm-tools-preview` without asking the user. This matches with the behavior on CI. Co-authored-by: Taiki Endo <[email protected]>
taiki-e · Sep 10, 2022 · 46d36e9 · 46d36e9
2 parents bd1bac6 + c6663e7
commit 46d36e9
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,16 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 ## [Unreleased]
 
+- Improve handling of cases where `llvm-tools-preview` component is not installed. ([#219](https://github.com/taiki-e/cargo-llvm-cov/pull/219))
+
+  **TL;DR:** You no longer need to install `llvm-tools-preview` before running cargo-llvm-cov in most cases.
+
+  Previously, cargo-llvm-cov exit with error suggesting the installation of `llvm-tools-preview`.
+
+  The new logic is based on the logic used by Miri when `rust-src` component or `xargo` is not installed.
+
+  See [#219](https://github.com/taiki-e/cargo-llvm-cov/pull/219) for more.
+
 - Fix various CLI-related bugs. ([#197](https://github.com/taiki-e/cargo-llvm-cov/pull/197))
 
   This fixes various bugs related to subcommands (especially `nextest`). The following is a partial list:

diff --git a/README.md b/README.md
@@ -466,7 +466,7 @@ jobs:
     steps:
       - uses: actions/checkout@v3
       - name: Install Rust
-        run: rustup toolchain install stable --component llvm-tools-preview
+        run: rustup update stable
       - name: Install cargo-llvm-cov
         uses: taiki-e/install-action@cargo-llvm-cov
       - name: Generate code coverage
@@ -486,12 +486,6 @@ jobs:
 <!-- omit in toc -->
 ### Prerequisites
 
-cargo-llvm-cov requires llvm-tools-preview:
-
-```sh
-rustup component add llvm-tools-preview
-```
-
 Running cargo-llvm-cov requires rustc 1.60+.
 
 <!-- omit in toc -->

diff --git a/src/context.rs b/src/context.rs
@@ -1,4 +1,8 @@
-use std::{ffi::OsString, path::PathBuf};
+use std::{
+    ffi::OsString,
+    io::{self, Write},
+    path::PathBuf,
+};
 
 use anyhow::{bail, Result};
 use camino::Utf8PathBuf;
@@ -79,41 +83,75 @@ impl Context {
         let mut rustlib: Utf8PathBuf = ws.rustc_print("target-libdir")?.into();
         rustlib.pop(); // lib
         rustlib.push("bin");
-        let llvm_cov: PathBuf = match env::var_os("LLVM_COV") {
-            Some(llvm_cov) => llvm_cov.into(),
-            None => {
-                let llvm_cov = rustlib.join(format!("llvm-cov{}", env::consts::EXE_SUFFIX));
-                // Check if required tools are installed.
-                if !llvm_cov.exists() {
-                    let sysroot: Utf8PathBuf = ws.rustc_print("sysroot")?.into();
-                    let toolchain = sysroot.file_name().unwrap();
-                    // Include --toolchain flag in the suggestion because the user may be
-                    // using toolchain override shorthand (+toolchain).
-                    bail!(
-                        "failed to find llvm-tools-preview, please install llvm-tools-preview \
-                         with `rustup component add llvm-tools-preview --toolchain {toolchain}`",
-                    );
+        let (llvm_cov, llvm_profdata): (PathBuf, PathBuf) = match (
+            env::var_os("LLVM_COV").map(PathBuf::from),
+            env::var_os("LLVM_PROFDATA").map(PathBuf::from),
+        ) {
+            (Some(llvm_cov), Some(llvm_profdata)) => (llvm_cov, llvm_profdata),
+            (llvm_cov_env, llvm_profdata_env) => {
+                if llvm_cov_env.is_some() {
+                    warn!("setting only LLVM_COV environment variable may not work properly; consider setting both LLVM_COV and LLVM_PROFDATA environment variables");
+                } else if llvm_profdata_env.is_some() {
+                    warn!("setting only LLVM_PROFDATA environment variable may not work properly; consider setting both LLVM_COV and LLVM_PROFDATA environment variables");
                 }
-                llvm_cov.into()
-            }
-        };
-        let llvm_profdata: PathBuf = match env::var_os("LLVM_PROFDATA") {
-            Some(llvm_profdata) => llvm_profdata.into(),
-            None => {
-                let llvm_profdata =
-                    rustlib.join(format!("llvm-profdata{}", env::consts::EXE_SUFFIX));
+                let llvm_cov: PathBuf =
+                    rustlib.join(format!("llvm-cov{}", env::consts::EXE_SUFFIX)).into();
+                let llvm_profdata: PathBuf =
+                    rustlib.join(format!("llvm-profdata{}", env::consts::EXE_SUFFIX)).into();
                 // Check if required tools are installed.
-                if !llvm_profdata.exists() {
+                if !llvm_cov.exists() || !llvm_profdata.exists() {
                     let sysroot: Utf8PathBuf = ws.rustc_print("sysroot")?.into();
                     let toolchain = sysroot.file_name().unwrap();
-                    // Include --toolchain flag in the suggestion because the user may be
-                    // using toolchain override shorthand (+toolchain).
-                    bail!(
-                        "failed to find llvm-tools-preview, please install llvm-tools-preview \
-                         with `rustup component add llvm-tools-preview --toolchain {toolchain}`",
-                    );
+                    if cmd!("rustup", "toolchain", "list")
+                        .read()
+                        .map_or(false, |t| t.contains(toolchain))
+                    {
+                        // If toolchain is installed from rustup and llvm-tools-preview is not installed,
+                        // suggest installing llvm-tools-preview via rustup.
+                        // Include --toolchain flag because the user may be using toolchain
+                        // override shorthand (+toolchain).
+                        let mut cmd = cmd!(
+                            "rustup",
+                            "component",
+                            "add",
+                            "llvm-tools-preview",
+                            "--toolchain",
+                            toolchain
+                        );
+                        match env::var_os("CARGO_LLVM_COV_SETUP") {
+                            None => {
+                                ask_to_run(
+                                    &mut cmd,
+                                    true,
+                                    "install the `llvm-tools-preview` component for the selected toolchain",
+                                )?;
+                            }
+                            Some(ref v) if v == "yes" => {
+                                ask_to_run(
+                                    &mut cmd,
+                                    false,
+                                    "install the `llvm-tools-preview` component for the selected toolchain",
+                                )?;
+                            }
+                            Some(v) => {
+                                if v != "no" {
+                                    warn!(
+                                        "CARGO_LLVM_COV_SETUP must be yes or no, but found `{v:?}`"
+                                    );
+                                }
+                                bail!(
+                                    "failed to find llvm-tools-preview, please install llvm-tools-preview \
+                                     with `rustup component add llvm-tools-preview --toolchain {toolchain}`",
+                                );
+                            }
+                        }
+                    } else {
+                        bail!(
+                            "failed to find llvm-tools-preview, please install llvm-tools-preview, or set LLVM_COV and LLVM_PROFDATA environment variables",
+                        );
+                    }
                 }
-                llvm_profdata.into()
+                (llvm_cov_env.unwrap_or(llvm_cov), llvm_profdata_env.unwrap_or(llvm_profdata))
             }
         };
 
@@ -201,3 +239,28 @@ impl WorkspaceMembers {
         Self { excluded, included }
     }
 }
+
+// Adapted from https://github.com/rust-lang/miri/blob/dba35d2be72f4b78343d1a0f0b4737306f310672/cargo-miri/src/util.rs#L181-L204
+fn ask_to_run(cmd: &mut ProcessBuilder, ask: bool, text: &str) -> Result<()> {
+    // Disable interactive prompts in CI (GitHub Actions, Travis, AppVeyor, etc).
+    // Azure doesn't set `CI` though (nothing to see here, just Microsoft being Microsoft),
+    // so we also check their `TF_BUILD`.
+    let is_ci = env::var_os("CI").is_some() || env::var_os("TF_BUILD").is_some();
+    if ask && !is_ci {
+        let mut buf = String::new();
+        print!("I will run {} to {}.\nProceed? [Y/n] ", cmd, text);
+        io::stdout().flush()?;
+        io::stdin().read_line(&mut buf)?;
+        match buf.trim().to_lowercase().as_str() {
+            // Proceed.
+            "" | "y" | "yes" => {}
+            "n" | "no" => bail!("aborting as per your request"),
+            a => bail!("invalid answer `{}`", a),
+        };
+    } else {
+        info!("running {} to {}", cmd, text);
+    }
+
+    cmd.run()?;
+    Ok(())
+}