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

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

Closed
smoelius opened this issue Jan 5, 2024 · 2 comments
Closed

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

smoelius opened this issue Jan 5, 2024 · 2 comments
Labels
C-question Category: A question

Comments

@smoelius
Copy link
Contributor

smoelius commented Jan 5, 2024

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
Copy link
Owner

taiki-e commented Jan 6, 2024

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 (#167).

Ideally, I would like Cargo to support merging of rustflags by environment variables (rust-lang/cargo#5376)...

@taiki-e taiki-e added the C-question Category: A question label Jan 6, 2024
@smoelius
Copy link
Contributor Author

smoelius commented Jan 7, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-question Category: A question
Projects
None yet
Development

No branches or pull requests

2 participants