search

taiki-e / cargo-llvm-cov

Cargo subcommand to easily use LLVM source-based code coverage (-C instrument-coverage).
Apache License 2.0
858 stars 57 forks source link

`target.'cfg(all())'.rustflags` not supported (yet)? #332

Closed smoelius closed 6 months ago

smoelius commented 6 months ago

I have a test that involves the following .cargo/config.toml file:

[target.'cfg(all())']
rustflags = ["-C", "linker=dylint-link"]

# For Rust versions 1.74.0 and onward, the following alternative can be used
# (see https://github.com/rust-lang/cargo/pull/12535):
# linker = "dylint-link"

The test works fine when run under cargo test, but fails when run under cargo-llvm-cov.

On the other hand, if you comment out the rustflags... line in the above, and uncomment the the # linker... line, the test works under cargo-llvm-cov.

Is it possible cargo-llvm-cov doesn't yet support target.'cfg(all())'.rustflags?

The following commands should reproduce the issue:

cargo install dylint-link
git clone https://github.com/trailofbits/dylint
cd dylint
cargo test -p cargo-dylint --test custom_toolchain
cargo llvm-cov -p cargo-dylint --test custom_toolchain
taiki-e commented 6 months ago

TR; DR: The workaround is using --coverage-target-only flag.

Is it possible cargo-llvm-cov doesn't yet support target.'cfg(all())'.rustflags?

No. cfg(all()) is evaluated to true correctly https://github.com/taiki-e/cargo-config2/blob/e64df187d824d2dbca3afcfc256ae370d224e2d6/src/cfg_expr/tests/eval.rs#L32.

The problem is that the config (that sets linker=dylint-link) is not the config of the workspace or package being compiled by cargo-llvm-cov, but a local config of a crate compiled by internally invoked (by test) commands. This is not taken into account where cargo-llvm-cov is invoked; cargo-llvm-cov passes rustflags as an environment variable (RUSTFLAGS by default) rather than config so that internally invoked commands can also participate in coverage, but because RUSTFLAGS environment variable take precedence over config files, internally invoked commands will ignore their local config will be ignored.

The workaround here is to have the cargo-llvm-cov set the TARGET_*_RUSTFLAGS instead of RUSTFLAGS to be merged with target.'cfg(,..)'.rustflags, which can be done by passing the --coverage-target-only flag (https://github.com/taiki-e/cargo-llvm-cov/pull/167).

Ideally, I would like Cargo to support merging of rustflags by environment variables (https://github.com/rust-lang/cargo/issues/5376)...

smoelius commented 6 months ago

Thanks for the detailed explanation, @taiki-e. Your solution worked perfectly.