Skip to content

Don't update the baseline configuration with --noallow_analysis_cache_discard#28510

Closed
fmeum wants to merge 1 commit intobazelbuild:masterfrom
fmeum:skygraft-fix-no-discard
Closed

Don't update the baseline configuration with --noallow_analysis_cache_discard#28510
fmeum wants to merge 1 commit intobazelbuild:masterfrom
fmeum:skygraft-fix-no-discard

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Feb 3, 2026

Before this change, large parts of the analysis cache would be invalidated even with --noallow_analysis_cache_discard.

@fmeum fmeum requested a review from a team as a code owner February 3, 2026 18:49
@fmeum fmeum requested review from aranguyen, gregestren and susinmotion and removed request for a team and aranguyen February 3, 2026 18:49
@fmeum
Copy link
Collaborator Author

fmeum commented Feb 3, 2026

@bazel-io fork 9.1.0

@github-actions github-actions bot added team-Configurability platforms, toolchains, cquery, select(), config transitions awaiting-review PR is awaiting review from an assigned reviewer labels Feb 3, 2026
…e_discard`

Before this change, large parts of the analysis cache would be invalidated even with `--noallow_analysis_cache_discard`.
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request fixes an issue where the analysis cache was being invalidated even when --noallow_analysis_cache_discard was used. The fix correctly moves the call to setBaselineConfiguration to after the check for whether the cache should be discarded, preventing premature invalidation. The accompanying test changes are excellent, verifying not only that the build fails as expected but also that the cached configured target is not invalidated. The changes are correct and improve the reliability of the analysis cache behavior.

@fmeum
Copy link
Collaborator Author

fmeum commented Feb 3, 2026

@bazel-io fork 9.0.1

@gregestren
Copy link
Contributor

gregestren commented Feb 10, 2026

Before this change, large parts of the analysis cache would be invalidated even with --noallow_analysis_cache_discard.

This change just moves setting the baseline configuration from before the --noanalysis_cache_discard check until after?

@fmeum
Copy link
Collaborator Author

fmeum commented Feb 11, 2026

Yes, that's all it takes to avoid the invalidation

@iancha1992
Copy link
Member

@gregestren Should we import this now?

@iancha1992 iancha1992 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 Mar 12, 2026
@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 Mar 16, 2026
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Mar 16, 2026
…e_discard` (bazelbuild#28510)

Before this change, large parts of the analysis cache would be invalidated even with `--noallow_analysis_cache_discard`.

Closes bazelbuild#28510.

PiperOrigin-RevId: 884451995
Change-Id: Ie03a441be4189cd852afaea68a7069c3fd603638
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Configurability platforms, toolchains, cquery, select(), config transitions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants