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

Do not attempt to instrument rust code for coverage if rust_toolchain.llvm-cov is None #1432

Merged
merged 2 commits into from
Jul 5, 2022

Conversation

krasimirgg
Copy link
Collaborator

No functional changes intended.

This PR updates rules_rust so that if no llvm-cov is passed to the rust_toolchain, rust code is not attempted to be instrumented for coverage. The PR 7465c1a added rust test coverage support. Before that, rust test code was not being instrumented for coverage.

We've got a case where the underlying rust toolchain doesn't support coverage instrumentation, in particular the toolchain is compiled without support for the profiler runtime, which is required by -C instrument-coverage (https://doc.rust-lang.org/rustc/instrument-coverage.html#how-it-works). Before this PR, the -C instrument-coverage rust arg was added unconditionally to the rustc invocation, which fails if the rustc didn't support it. The ability to disable rust coverage instrumentation is useful in such cases, where there is also code in other languages which is instrumented.

@krasimirgg krasimirgg requested a review from scentini July 5, 2022 10:01
@krasimirgg krasimirgg marked this pull request as ready for review July 5, 2022 11:53
Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable, thanks!

@scentini scentini merged commit da75146 into bazelbuild:main Jul 5, 2022
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.

3 participants