Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of cases where llvm-tools-preview is not installed #219

Merged
merged 1 commit into from
Sep 10, 2022

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Sep 10, 2022

TL;DR: You no longer need to manually 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.

EDIT: If a toolchain is installed without via rustup, this works like CARGO_LLVM_COV_SETUP=no.

@taiki-e taiki-e added this to the v0.5 milestone Sep 10, 2022
@taiki-e taiki-e force-pushed the install branch 3 times, most recently from 0a9aad5 to 1d046b9 Compare September 10, 2022 06:25
@taiki-e
Copy link
Owner Author

taiki-e commented Sep 10, 2022

bors r+

bors bot added a commit that referenced this pull request Sep 10, 2022
219: Improved 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 <te316e89@gmail.com>
@bors
Copy link
Contributor

bors bot commented Sep 10, 2022

Canceled.

@taiki-e taiki-e changed the title Improved handling of cases where llvm-tools-preview is not installed Improve handling of cases where llvm-tools-preview is not installed Sep 10, 2022
@taiki-e
Copy link
Owner Author

taiki-e commented Sep 10, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 10, 2022

Build succeeded:

  • ci

@bors bors bot merged commit 46d36e9 into main Sep 10, 2022
@bors bors bot deleted the install branch September 10, 2022 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant