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

Emit LLVM coverage for source file paths with a tmp segment #16852

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Nov 25, 2022

The LLVM LCOV coverage collection logic attempted to filter out source files under /tmp/, but instead filtered out all source files with paths containing the substring /tmp/. Under macOS, where Bazel's output base lies under /private/var/tmp, this matched every output base absolute path.

The LLVM LCOV coverage collection logic attempted to filter out source
files under `/tmp/`, but instead filtered out all source files with
paths containing the substring `/tmp/`. Under macOS, where Bazel's
output base lies under `/private/var/tmp`, this matched every output
base absolute path.
@fmeum fmeum requested a review from lberki as a code owner November 25, 2022 12:51
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 25, 2022

@c-mita Could you review this PR?

@sgowroji sgowroji added team-Rules-Server Issues for serverside rules included with Bazel awaiting-review PR is awaiting review from an assigned reviewer labels Nov 28, 2022
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 9, 2022

@c-mita Friendly ping

@c-mita c-mita self-requested a review December 14, 2022 14:52
Copy link
Member

@c-mita c-mita left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 3, 2023

@c-mita @sgowroji Friendly ping, does this need to be transitioned to "awaiting-merge"?

@sgowroji
Copy link
Member

sgowroji commented Jan 3, 2023

Hello @c-mita, Can you please confirm to import the above PR changes.

@c-mita c-mita added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jan 9, 2023
@c-mita
Copy link
Member

c-mita commented Jan 9, 2023

Oops. Yes, this should be imported.

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 9, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 9, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 10, 2023
@fmeum fmeum deleted the fix-cc-coverage-regex branch January 10, 2023 14:14
@ShreeM01
Copy link
Contributor

@bazel-io fork 6.1.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 18, 2023
ShreeM01 added a commit that referenced this pull request Jan 26, 2023
The LLVM LCOV coverage collection logic attempted to filter out source files under `/tmp/`, but instead filtered out all source files with paths containing the substring `/tmp/`. Under macOS, where Bazel's output base lies under `/private/var/tmp`, this matched every output base absolute path.

Closes #16852.

PiperOrigin-RevId: 500973551
Change-Id: I6c890f38c19eedec83cdf0ea99f021eb85f4e697

Co-authored-by: Fabian Meumertzheim <[email protected]>
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
The LLVM LCOV coverage collection logic attempted to filter out source files under `/tmp/`, but instead filtered out all source files with paths containing the substring `/tmp/`. Under macOS, where Bazel's output base lies under `/private/var/tmp`, this matched every output base absolute path.

Closes #16852.

PiperOrigin-RevId: 500973551
Change-Id: I6c890f38c19eedec83cdf0ea99f021eb85f4e697
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Server Issues for serverside rules included with Bazel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants