Fix unconditional Skyframe invalidation with --lockfile_mode=update
#19842
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fmeum
requested review from
gregestren and
meteorcloudy
and removed request for
Wyverald and
meteorcloudy
github-actions
bot
added
awaiting-review
This was referenced Oct 17, 2023
meteorcloudy
approved these changes
Oct 17, 2023
Member
meteorcloudy
left a comment
meteorcloudy
left a comment
There was a problem hiding this comment.
Thanks for figuring this out!!
meteorcloudy
added
awaiting-PR-merge
Collaborator
Author
|
@meteorcloudy I flagged the original issue for Bazel 6.4.0 since I assume we want lockfiles to work well in 6.4.0 before we turn them on by default in 7.X.0. Do you agree? |
fmeum
force-pushed
the
19823-fix-config
branch
from
61d3807 to
fc1ca22
Compare
Collaborator
Author
|
I had to update the test to allow networking. |
Member
|
To fix the test failure, you'll have to add |
Member
|
Or enable internet access |
fmeum
force-pushed
the
19823-fix-config
branch
from
fc1ca22 to
64d52e2
Compare
Collaborator
Author
Nice, changed it to do that. |
Member
Ok, let's get this one in 6.4 |
Ensure that `BazelLockFileModule` only updates `MODULE.bazel.lock` if the content of the file needs to change. Every such update changes the file's metadata, which results in Skyframe invalidation of, in particular, all configurations. This broke `bazel config`, which uses `MemoizingEvaluator#getDoneValues()` to directly observe Skyframe state. Since this type of invalidation can also be caused by users and deviates from the usual "incremental correctness" guarantees provided by Bazel, let `config` show a descriptive error when no configurations are found.
fmeum
force-pushed
the
19823-fix-config
branch
from
64d52e2 to
7f0bba2
Compare
meteorcloudy
approved these changes
Oct 17, 2023
Member
|
I'm importing this one to accelerate the process. Sent out to @gregestren @katre for review |
github-actions
bot
removed
the
awaiting-PR-merge
fmeum
added a commit
to fmeum/bazel
that referenced
this pull request
Oct 17, 2023
…update Ensure that `BazelLockFileModule` only updates `MODULE.bazel.lock` if the content of the file needs to change. Every such update changes the file's metadata, which results in Skyframe invalidation of, in particular, all configurations. This broke `bazel config`, which uses `MemoizingEvaluator#getDoneValues()` to directly observe Skyframe state. Compared to the original commit bazelbuild@78db9ae, this cherry-pick does not include the change to `bazel config` as it may not be backwards compatible (changes the exit code in certain situations). Fixes bazelbuild#19823 Closes bazelbuild#19842. PiperOrigin-RevId: 574133346 Change-Id: I5886c91fc6b7b938a7dee59ea75aa7b8afb5b161 Fixes bazelbuild#19843
iancha1992
pushed a commit
that referenced
this pull request
Oct 17, 2023
#19848) …update Ensure that `BazelLockFileModule` only updates `MODULE.bazel.lock` if the content of the file needs to change. Every such update changes the file's metadata, which results in Skyframe invalidation of, in particular, all configurations. This broke `bazel config`, which uses `MemoizingEvaluator#getDoneValues()` to directly observe Skyframe state. Compared to the original commit 78db9ae, this cherry-pick does not include the change to `bazel config` as it may not be backwards compatible (changes the exit code in certain situations). Fixes #19823 Closes #19842. PiperOrigin-RevId: 574133346 Change-Id: I5886c91fc6b7b938a7dee59ea75aa7b8afb5b161 Fixes #19843
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensure that
BazelLockFileModuleonly updatesMODULE.bazel.lockif the content of the file needs to change. Every such update changes the file's metadata, which results in Skyframe invalidation of, in particular, all configurations. This brokebazel config, which usesMemoizingEvaluator#getDoneValues()to directly observe Skyframe state.Since this type of invalidation can also be caused by users and deviates from the usual "incremental correctness" guarantees provided by Bazel, let
configshow a descriptive error when no configurations are found.Fixes #19823