Fix unconditional Skyframe invalidation with `--lockfile_mode=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.
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.
Fixes #19823
Closes #19842.
PiperOrigin-RevId: 574133346
Change-Id: I5886c91fc6b7b938a7dee59ea75aa7b8afb5b161
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java
index a664ae3..4036eb3 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java
@@ -106,8 +106,13 @@
combineModuleExtensions(lockfile.getModuleExtensions(), oldExtensionUsages))
.build();
- // Write the new value to the file
- updateLockfile(lockfilePath, lockfile);
+ // Write the new value to the file, but only if needed. This is not just a performance
+ // optimization: whenever the lockfile is updated, most Skyframe nodes will be marked as dirty
+ // on the next build, which breaks commands such as `bazel config` that rely on
+ // com.google.devtools.build.skyframe.MemoizingEvaluator#getDoneValues.
+ if (!lockfile.equals(oldLockfile)) {
+ updateLockfile(lockfilePath, lockfile);
+ }
this.moduleResolutionEvent = null;
this.extensionResolutionEventsMap.clear();
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/ConfigCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/ConfigCommand.java
index 53e43ca..c0ddd7b 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/ConfigCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/ConfigCommand.java
@@ -332,6 +332,15 @@
public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult options) {
ImmutableSortedMap<BuildConfigurationKey, BuildConfigurationValue> configurations =
findConfigurations(env);
+ if (configurations.isEmpty()) {
+ String message =
+ "No configurations found. This can happen if the 'config' subcommand is used after "
+ + "files, including their metadata, have changed since the last invocation of "
+ + "another subcommand. Try running a 'build' or 'cquery' directly followed by "
+ + "'config'.";
+ env.getReporter().handle(Event.error(message));
+ return createFailureResult(message, Code.CONFIGURATION_NOT_FOUND);
+ }
try (PrintWriter writer =
new PrintWriter(
diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh
index 8417dc1..9a9cb52 100755
--- a/src/test/shell/integration/configured_query_test.sh
+++ b/src/test/shell/integration/configured_query_test.sh
@@ -60,10 +60,6 @@
add_to_bazelrc "build --package_path=%workspace%"
-# TODO: enable Bzlmod for this Test
-# https://github.com/bazelbuild/bazel/issues/19823
-disable_bzlmod
-
#### TESTS #############################################################
function test_basic_query() {
@@ -897,8 +893,8 @@
--starlark:expr="str(target.label) + '%foo'" > output \
2>"$TEST_log" || fail "Expected success"
- assert_contains "^@//$pkg:pylib%foo$" output
- assert_contains "^@//$pkg:pylibtwo%foo$" output
+ assert_contains "^@@\?//$pkg:pylib%foo$" output
+ assert_contains "^@@\?//$pkg:pylibtwo%foo$" output
bazel cquery "//$pkg:all" --output=starlark \
--noincompatible_unambiguous_label_stringification \
@@ -1294,7 +1290,7 @@
2>"$TEST_log" || fail "Expected success"
assert_contains "//$pkg:srcfile.txt:providers=.*FileProvider.*FilesToRunProvider.*LicensesProvider.*VisibilityProvider" \
output
- assert_contains "VisibilityProvider.label:@//$pkg:srcfile.txt" output
+ assert_contains "VisibilityProvider.label:@@\?//$pkg:srcfile.txt" output
}
function test_starlark_output_providers_starlark_provider() {
@@ -1394,6 +1390,7 @@
EOF
mkdir -p $dir/main
+ write_default_lockfile $dir/main/MODULE.bazel.lock
cat > $dir/main/WORKSPACE <<EOF
local_repository(name = "repo", path = "../repo")
EOF
