diff --git a/CHANGELOG.md b/CHANGELOG.md index 5544a1e4..66a6e49f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,17 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com ## [Unreleased] +- Improved handling of cases where `llvm-tools-preview` component is not installed. + + **TL;DR:** You no longer need to install `llvm-tools-preview` before running cargo-llvm-cov in most cases. + + The new logic is based on the logic used by Miri when it needs to install `rust-src` component and `xargo`. + + - 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). + + Previously, cargo-llvm-cov exit with error suggesting the installation of `llvm-tools-preview`. + - 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 index 8d893860..ff67ba34 100644 --- 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: ### Prerequisites -cargo-llvm-cov requires llvm-tools-preview: - -```sh -rustup component add llvm-tools-preview -``` - Running cargo-llvm-cov requires rustc 1.60+. diff --git a/src/context.rs b/src/context.rs index 582fc06e..fc29c11b 100644 --- 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,52 @@ 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, suggest install 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 + ); + ask_to_run( + &mut cmd, + true, + "install the `llvm-tools-preview` component for the selected 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 +216,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(()) +}