Various environment variables-related cleanup

CHANGELOG.md

@@ -43,6 +43,14 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 - Support `--exclude-from-report` and `--ignore-run-fail` for `cargo llvm-cov run`.
 
+- Support relative path in `CARGO_LLVM_COV_TARGET_DIR`.
+
+- Add `LLVM_COV_FLAGS`/`LLVM_PROFDATA_FLAGS` environment variables to pass additional flags to llvm-cov/llvm-profdata in a space-separated list.
+
+- Deprecate `CARGO_LLVM_COV_FLAGS`/`CARGO_LLVM_PROFDATA_FLAGS` environment variables instead of `LLVM_COV_FLAGS`/`LLVM_PROFDATA_FLAGS` environment variables.
+
+- Document environment variables that cargo-llvm-cov reads.
+
 - Remove `cargo llvm-cov help` subcommand it was added automatically by clap. ([#197](https://github.com/taiki-e/cargo-llvm-cov/pull/197))
 
 - cargo-llvm-cov no longer maps the `--jobs` (`-j`) option to llvm-cov/llvm-profdata's `-num-threads` option.

README.md

@@ -25,6 +25,7 @@ This is a wrapper around rustc [`-C instrument-coverage`][instrument-coverage] a
   - [Exclude file from coverage](#exclude-file-from-coverage)
   - [Exclude function from coverage](#exclude-function-from-coverage)
   - [Continuous Integration](#continuous-integration)
+  - [Environment variables](#environment-variables)
 - [Installation](#installation)
 - [Known limitations](#known-limitations)
 - [Related Projects](#related-projects)
@@ -481,6 +482,19 @@ jobs:
 
 **Note:** Currently, only line coverage is available on Codecov. This is because `-C instrument-coverage` does not support branch coverage and Codecov does not support region coverage. See also [#8], [#12], and [#20].
 
+### Environment variables
+
+You can override these environment variables to change cargo-llvm-cov's behavior on your system:
+
+- `CARGO_LLVM_COV_TARGET_DIR` -- Location of where to place all generated artifacts, relative to the current working directory. Default to `<cargo_target_dir>/llvm-cov-target`.
+- `CARGO_LLVM_COV_SETUP` -- Control behavior if `llvm-tools-preview` component is not installed. See [#219] for more.
+- `LLVM_COV` -- Override the path to `llvm-cov`. You may need to specify both this and `LLVM_PROFDATA` environment variables if you are using [`--include-ffi` flag](#get-coverage-of-cc-code-linked-to-rust-librarybinary) or if you are using a toolchain installed without via rustup.
+- `LLVM_PROFDATA` -- Override the path to `llvm-profdata`. See `LLVM_COV` environment variable for more.
+- `LLVM_COV_FLAGS` -- A space-separated list of additional flags to pass to all `llvm-cov` invocations that cargo-llvm-cov performs. See [LLVM documentation](https://llvm.org/docs/CommandGuide/llvm-cov.html) for available options.
+- `LLVM_PROFDATA_FLAGS` -- A space-separated list of additional flags to pass to all `llvm-profdata` invocations that cargo-llvm-cov performs. See [LLVM documentation](https://llvm.org/docs/CommandGuide/llvm-profdata.html) for available options.
+
+See also [environment variables that Cargo reads](https://doc.rust-lang.org/nightly/cargo/reference/environment-variables.html#environment-variables-cargo-reads). cargo-llvm-cov respects many of them.
+
 ## Installation
 
 <!-- omit in toc -->
@@ -590,6 +604,7 @@ See also [the code-coverage-related issues reported in rust-lang/rust](https://g
 [#12]: https://github.com/taiki-e/cargo-llvm-cov/issues/12
 [#20]: https://github.com/taiki-e/cargo-llvm-cov/issues/20
 [#123]: https://github.com/taiki-e/cargo-llvm-cov/issues/123
+[#219]: https://github.com/taiki-e/cargo-llvm-cov/issues/219
 [cargo-hack]: https://github.com/taiki-e/cargo-hack
 [cargo-minimal-versions]: https://github.com/taiki-e/cargo-minimal-versions
 [codecov]: https://codecov.io

src/cargo.rs

@@ -1,4 +1,5 @@
 use std::{
+    convert::TryInto,
     ffi::OsStr,
     path::{Path, PathBuf},
 };
@@ -76,20 +77,18 @@ impl Workspace {
             );
         }
 
-        let target_dir = if let Some(path) =
-            env::var("CARGO_LLVM_COV_TARGET_DIR")?.map(Utf8PathBuf::from)
-        {
-            if path.is_relative() {
-                warn!("CARGO_LLVM_COV_TARGET_DIR with relative path may not work properly; consider using absolute path");
-            }
-            path
-        } else if show_env {
-            metadata.target_directory.clone()
-        } else {
-            // If we change RUSTFLAGS, all dependencies will be recompiled. Therefore,
-            // use a subdirectory of the target directory as the actual target directory.
-            metadata.target_directory.join("llvm-cov-target")
-        };
+        let target_dir =
+            if let Some(path) = env::var("CARGO_LLVM_COV_TARGET_DIR")?.map(Utf8PathBuf::from) {
+                let mut base: Utf8PathBuf = env::current_dir()?.try_into()?;
+                base.push(path);
+                base
+            } else if show_env {
+                metadata.target_directory.clone()
+            } else {
+                // If we change RUSTFLAGS, all dependencies will be recompiled. Therefore,
+                // use a subdirectory of the target directory as the actual target directory.
+                metadata.target_directory.join("llvm-cov-target")
+            };
         let output_dir = metadata.target_directory.join("llvm-cov");
         let doctests_dir = target_dir.join("doctestbins");
 

src/context.rs

@@ -33,12 +33,12 @@ pub(crate) struct Context {
     // Path to llvm-profdata, can be overridden with `LLVM_PROFDATA` environment variable.
     pub(crate) llvm_profdata: PathBuf,
 
-    /// `CARGO_LLVM_COV_FLAGS` environment variable to pass additional flags
-    /// to llvm-cov. (value: space-separated list)
-    pub(crate) cargo_llvm_cov_flags: Option<String>,
-    /// `CARGO_LLVM_PROFDATA_FLAGS` environment variable to pass additional flags
-    /// to llvm-profdata. (value: space-separated list)
-    pub(crate) cargo_llvm_profdata_flags: Option<String>,
+    /// `LLVM_COV_FLAGS` environment variable to pass additional flags to llvm-cov.
+    /// (value: space-separated list)
+    pub(crate) llvm_cov_flags: Option<String>,
+    /// `LLVM_PROFDATA_FLAGS` environment variable to pass additional flags to llvm-profdata.
+    /// (value: space-separated list)
+    pub(crate) llvm_profdata_flags: Option<String>,
 }
 
 impl Context {
@@ -156,6 +156,21 @@ impl Context {
 
         let build_script_re = pkg_hash_re(&ws, &workspace_members.included);
 
+        let mut llvm_cov_flags = env::var("LLVM_COV_FLAGS")?;
+        if llvm_cov_flags.is_none() {
+            llvm_cov_flags = env::var("CARGO_LLVM_COV_FLAGS")?;
+            if llvm_cov_flags.is_some() {
+                warn!("CARGO_LLVM_COV_FLAGS is deprecated; consider using LLVM_COV_FLAGS instead");
+            }
+        }
+        let mut llvm_profdata_flags = env::var("LLVM_PROFDATA_FLAGS")?;
+        if llvm_profdata_flags.is_none() {
+            llvm_profdata_flags = env::var("CARGO_LLVM_PROFDATA_FLAGS")?;
+            if llvm_profdata_flags.is_some() {
+                warn!("CARGO_LLVM_PROFDATA_FLAGS is deprecated; consider using LLVM_PROFDATA_FLAGS instead");
+            }
+        }
+
         Ok(Self {
             ws,
             args,
@@ -172,8 +187,8 @@ impl Context {
             },
             llvm_cov,
             llvm_profdata,
-            cargo_llvm_cov_flags: env::var("CARGO_LLVM_COV_FLAGS")?,
-            cargo_llvm_profdata_flags: env::var("CARGO_LLVM_PROFDATA_FLAGS")?,
+            llvm_cov_flags,
+            llvm_profdata_flags,
         })
     }
 

src/main.rs

@@ -505,7 +505,7 @@ fn merge_profraw(cx: &Context) -> Result<()> {
     if let Some(mode) = &cx.args.cov.failure_mode {
         cmd.arg(format!("-failure-mode={mode}"));
     }
-    if let Some(flags) = &cx.cargo_llvm_profdata_flags {
+    if let Some(flags) = &cx.llvm_profdata_flags {
         cmd.args(flags.split(' ').filter(|s| !s.trim().is_empty()));
     }
     if term::verbose() {
@@ -786,7 +786,7 @@ impl Format {
             Self::None => {}
         }
 
-        if let Some(flags) = &cx.cargo_llvm_cov_flags {
+        if let Some(flags) = &cx.llvm_cov_flags {
             cmd.args(flags.split(' ').filter(|s| !s.trim().is_empty()));
         }
 

src/process.rs

@@ -50,6 +50,8 @@ impl ProcessBuilder {
             display_env_vars: Cell::new(false),
         };
         this.env("CARGO_INCREMENTAL", "0");
+        this.env_remove("LLVM_COV_FLAGS");
+        this.env_remove("LLVM_PROFDATA_FLAGS");
         this
     }
 
@@ -74,11 +76,11 @@ impl ProcessBuilder {
         self
     }
 
-    // /// Remove a variable from the process's environment.
-    // pub(crate) fn env_remove(&mut self, key: impl Into<String>) -> &mut Self {
-    //     self.env.insert(key.into(), None);
-    //     self
-    // }
+    /// Remove a variable from the process's environment.
+    pub(crate) fn env_remove(&mut self, key: impl Into<String>) -> &mut Self {
+        self.env.insert(key.into(), None);
+        self
+    }
 
     /// Set the working directory where the process will execute.
     pub(crate) fn dir(&mut self, path: impl Into<PathBuf>) -> &mut Self {