Skip to content

Commit

Permalink
Improved handling of cases where llvm-tools-preview is not installed
Browse files Browse the repository at this point in the history
  • Loading branch information
taiki-e committed Sep 10, 2022
1 parent bd1bac6 commit 0a9aad5
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 38 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 1 addition & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 -->
Expand Down
102 changes: 71 additions & 31 deletions src/context.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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))
}
};

Expand Down Expand Up @@ -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(())
}

0 comments on commit 0a9aad5

Please sign in to comment.