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

Error on potential unsupported /showIncludes lines #19580

Closed

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Sep 21, 2023

This error ensures that removing the English language pack for MSVC
without also refetching @local_config_cc doesn't result in silently
incorrect incremental builds.

I considered making this a warning instead, but that would either result
in warning spam (if a false positive) or drown in the unfiltered MSVC
output (if a true positive). Making this an error is better for
correctness and will lead users to report any false positive matches.

Fixes #19439 (comment)

@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Rules-CPP Issues for C++ rules labels Sep 21, 2023
@fmeum fmeum force-pushed the 19439-warn-on-undetected-show-includes branch 3 times, most recently from e1ae2d4 to b00f9c6 Compare September 21, 2023 13:44
@fmeum fmeum changed the title Warn on potential unsupported /showIncludes lines Error on potential unsupported /showIncludes lines Sep 21, 2023
@fmeum fmeum force-pushed the 19439-warn-on-undetected-show-includes branch from b00f9c6 to c47c9a8 Compare September 21, 2023 13:47
@fmeum fmeum requested a review from meteorcloudy September 21, 2023 13:47
@fmeum
Copy link
Collaborator Author

fmeum commented Sep 21, 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 Sep 21, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Sep 21, 2023

@meteorcloudy I tested this manually by uninstalling my English language pack and then doing a build without prior clean with just a French language pack.

@iancha1992
Copy link
Member

@bazel-io fork 6.4.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 Sep 21, 2023
This error ensures that removing the English language pack for MSVC
without also refetching `@local_config_cc` doesn't result in silently
incorrect incremental builds.

I considered making this a warning instead, but that would either result
in warning spam (if a false positive) or drown in the unfiltered MSVC
output (if a true positive). Making this an error is better for
correctness and will lead users to report any false positive matches.
@fmeum fmeum force-pushed the 19439-warn-on-undetected-show-includes branch from c47c9a8 to 50b5b03 Compare September 21, 2023 18:42
@fmeum fmeum requested a review from meteorcloudy September 21, 2023 18:43
Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!!

@meteorcloudy meteorcloudy 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 Sep 22, 2023
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 25, 2023
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Sep 25, 2023
This error ensures that removing the English language pack for MSVC
without also refetching `@local_config_cc` doesn't result in silently
incorrect incremental builds.

I considered making this a warning instead, but that would either result
in warning spam (if a false positive) or drown in the unfiltered MSVC
output (if a true positive). Making this an error is better for
correctness and will lead users to report any false positive matches.

Fixes bazelbuild#19439 (comment)

Closes bazelbuild#19580.

PiperOrigin-RevId: 568133958
Change-Id: If43924da6ba2f332edf4db0aed24056aa89fbb75
@fmeum fmeum deleted the 19439-warn-on-undetected-show-includes branch September 25, 2023 11:20
meteorcloudy pushed a commit that referenced this pull request Sep 25, 2023
This error ensures that removing the English language pack for MSVC
without also refetching `@local_config_cc` doesn't result in silently
incorrect incremental builds.

I considered making this a warning instead, but that would either result
in warning spam (if a false positive) or drown in the unfiltered MSVC
output (if a true positive). Making this an error is better for
correctness and will lead users to report any false positive matches.

Fixes
#19439 (comment)

Closes #19580.

Commit
a0497a0

PiperOrigin-RevId: 568133958
Change-Id: If43924da6ba2f332edf4db0aed24056aa89fbb75

Co-authored-by: Fabian Meumertzheim <[email protected]>
@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 6.4.0 RC2. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cc_libaray target won't be rebuilt when header files are modified
4 participants