From 010ac99ed02882d391c9d7120f6076489f616530 Mon Sep 17 00:00:00 2001
From: "bazel.build machine account"
<15028808+bazel-io@users.noreply.github.com>
Date: Thu, 14 Mar 2024 19:22:32 +0100
Subject: [PATCH] [7.1.1] Fix two `bazel mod tidy` crashes (#21700)
* Fixes a crash when using a non-registry override with a specified
version:
```
Caused by: java.lang.NullPointerException: null value in entry: foo=null
at com.google.common.collect.CollectPreconditions.checkEntryNotNull(CollectPreconditions.java:33)
at com.google.common.collect.ImmutableMapEntry.<init>(ImmutableMapEntry.java:54)
at com.google.common.collect.ImmutableMap.entryOf(ImmutableMap.java:345)
at com.google.common.collect.ImmutableMap$Builder.put(ImmutableMap.java:454)
at com.google.devtools.build.lib.bazel.bzlmod.Module.getRepoMappingWithBazelDepsOnly(Module.java:67)
at com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphFunction.getExtensionUsagesById(BazelDepGraphFunction.java:233)
at com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphFunction.compute(BazelDepGraphFunction.java:126)
at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:464)
```
* Fixes a crash for root modules with no extension usages:
```
Caused by: java.lang.NullPointerException: Cannot invoke "java.util.Map.keySet()" because the return value of "com.google.common.collect.ImmutableMap.get(Object)" is null
at com.google.devtools.build.lib.bazel.bzlmod.BazelModTidyFunction.compute(BazelModTidyFunction.java:85)
at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:464)
```
Fixes #21651
Closes #21686.
Commit
https://github.com/bazelbuild/bazel/commit/77369dc895597c012794c5ada78705a90a61c2cb
PiperOrigin-RevId: 615826860
Change-Id: I22be3fd53d0dc97aec92afe3dc51a9d6b7e60c98
Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
---
.../lib/bazel/bzlmod/BazelLockFileValue.java | 9 ++-
.../bazel/bzlmod/BazelModTidyFunction.java | 8 ++-
.../build/lib/bazel/bzlmod/Discovery.java | 34 +++------
.../build/lib/bazel/bzlmod/InterimModule.java | 22 ++++++
src/test/py/bazel/bzlmod/mod_command_test.py | 70 +++++++++++++++++++
5 files changed, 116 insertions(+), 27 deletions(-)
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java
index 3c85644a59a..17c92325e67 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java
@@ -132,7 +132,14 @@ public abstract class BazelLockFileValue implements SkyValue, Postable {
newDepGraph.putAll(getModuleDepGraph());
newDepGraph.put(
ModuleKey.ROOT,
- toModule(value.getModule(), /* override= */ null, /* remoteRepoSpec= */ null));
+ toModule(
+ value
+ .getModule()
+ .withDepSpecsTransformed(
+ InterimModule.applyOverrides(
+ value.getOverrides(), value.getModule().getName())),
+ /* override= */ null,
+ /* remoteRepoSpec= */ null));
return toBuilder()
.setModuleFileHash(value.getModuleFileHash())
.setModuleDepGraph(newDepGraph.buildKeepingLast())
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java
index a1eac4d6473..6cdf50a1374 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java
@@ -21,6 +21,7 @@ import static com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction.IGNO
import static com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction.MODULE_OVERRIDES;
import static com.google.devtools.build.lib.skyframe.PrecomputedValue.STARLARK_SEMANTICS;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
@@ -81,7 +82,12 @@ public class BazelModTidyFunction implements SkyFunction {
}
ImmutableSet<SkyKey> extensionsUsedByRootModule =
- depGraphValue.getExtensionUsagesTable().columnMap().get(ModuleKey.ROOT).keySet().stream()
+ depGraphValue
+ .getExtensionUsagesTable()
+ .columnMap()
+ .getOrDefault(ModuleKey.ROOT, ImmutableMap.of())
+ .keySet()
+ .stream()
.map(SingleExtensionEvalValue::key)
.collect(toImmutableSet());
SkyframeLookupResult result = env.getValuesAndExceptions(extensionsUsedByRootModule);
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java
index 32f7d0a020c..5da2540e923 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java
@@ -54,7 +54,10 @@ final class Discovery {
String rootModuleName = root.getModule().getName();
ImmutableMap<String, ModuleOverride> overrides = root.getOverrides();
Map<ModuleKey, InterimModule> depGraph = new HashMap<>();
- depGraph.put(ModuleKey.ROOT, rewriteDepSpecs(root.getModule(), overrides, rootModuleName));
+ depGraph.put(
+ ModuleKey.ROOT,
+ root.getModule()
+ .withDepSpecsTransformed(InterimModule.applyOverrides(overrides, rootModuleName)));
Queue<ModuleKey> unexpanded = new ArrayDeque<>();
Map<ModuleKey, ModuleKey> predecessors = new HashMap<>();
unexpanded.add(ModuleKey.ROOT);
@@ -101,7 +104,11 @@ final class Discovery {
depGraph.put(depKey, null);
} else {
depGraph.put(
- depKey, rewriteDepSpecs(moduleFileValue.getModule(), overrides, rootModuleName));
+ depKey,
+ moduleFileValue
+ .getModule()
+ .withDepSpecsTransformed(
+ InterimModule.applyOverrides(overrides, rootModuleName)));
unexpanded.add(depKey);
}
}
@@ -111,27 +118,4 @@ final class Discovery {
}
return ImmutableMap.copyOf(depGraph);
}
-
- private static InterimModule rewriteDepSpecs(
- InterimModule module, ImmutableMap<String, ModuleOverride> overrides, String rootModuleName) {
- return module.withDepSpecsTransformed(
- depSpec -> {
- if (rootModuleName.equals(depSpec.getName())) {
- return DepSpec.fromModuleKey(ModuleKey.ROOT);
- }
-
- Version newVersion = depSpec.getVersion();
- @Nullable ModuleOverride override = overrides.get(depSpec.getName());
- if (override instanceof NonRegistryOverride) {
- newVersion = Version.EMPTY;
- } else if (override instanceof SingleVersionOverride) {
- Version overrideVersion = ((SingleVersionOverride) override).getVersion();
- if (!overrideVersion.isEmpty()) {
- newVersion = overrideVersion;
- }
- }
-
- return DepSpec.create(depSpec.getName(), newVersion, depSpec.getMaxCompatibilityLevel());
- });
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java
index cc70b04467e..21aa20ed58b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java
@@ -260,4 +260,26 @@ public abstract class InterimModule extends ModuleBase {
.setAttributes(AttributeValues.create(attrBuilder.buildOrThrow()))
.build();
}
+
+ static UnaryOperator<DepSpec> applyOverrides(
+ ImmutableMap<String, ModuleOverride> overrides, String rootModuleName) {
+ return depSpec -> {
+ if (rootModuleName.equals(depSpec.getName())) {
+ return DepSpec.fromModuleKey(ModuleKey.ROOT);
+ }
+
+ Version newVersion = depSpec.getVersion();
+ @Nullable ModuleOverride override = overrides.get(depSpec.getName());
+ if (override instanceof NonRegistryOverride) {
+ newVersion = Version.EMPTY;
+ } else if (override instanceof SingleVersionOverride) {
+ Version overrideVersion = ((SingleVersionOverride) override).getVersion();
+ if (!overrideVersion.isEmpty()) {
+ newVersion = overrideVersion;
+ }
+ }
+
+ return DepSpec.create(depSpec.getName(), newVersion, depSpec.getMaxCompatibilityLevel());
+ };
+ }
}
diff --git a/src/test/py/bazel/bzlmod/mod_command_test.py b/src/test/py/bazel/bzlmod/mod_command_test.py
index 3f6853dee97..fa68bee94a0 100644
--- a/src/test/py/bazel/bzlmod/mod_command_test.py
+++ b/src/test/py/bazel/bzlmod/mod_command_test.py
@@ -800,6 +800,76 @@ class ModCommandTest(test_base.TestBase):
module_file.read().split('\n'),
)
+ def testModTidyWithNonRegistryOverride(self):
+ self.ScratchFile(
+ 'MODULE.bazel',
+ [
+ 'bazel_dep(name = "foo", version = "1.2.3")',
+ 'local_path_override(module_name = "foo", path = "foo")',
+ 'ext = use_extension("//:extension.bzl", "ext")',
+ 'use_repo(ext, "dep")',
+ ],
+ )
+ self.ScratchFile('BUILD.bazel')
+ self.ScratchFile(
+ 'extension.bzl',
+ [
+ 'def _ext_impl(ctx):',
+ ' pass',
+ '',
+ 'ext = module_extension(implementation=_ext_impl)',
+ ],
+ )
+ self.ScratchFile(
+ 'foo/MODULE.bazel', ['module(name = "foo", version = "1.2.3")']
+ )
+
+ # Verify that bazel mod tidy doesn't fail without the lockfile.
+ self.RunBazel(['mod', 'tidy'])
+
+ with open('MODULE.bazel', 'r') as module_file:
+ self.assertEqual(
+ [
+ 'bazel_dep(name = "foo", version = "1.2.3")',
+ 'local_path_override(',
+ ' module_name = "foo",',
+ ' path = "foo",',
+ ')',
+ '',
+ 'ext = use_extension("//:extension.bzl", "ext")',
+ 'use_repo(ext, "dep")',
+ # This newline is from ScratchFile.
+ '',
+ ],
+ module_file.read().split('\n'),
+ )
+
+ # Verify that bazel mod tidy doesn't fail with the lockfile.
+ self.RunBazel(['mod', 'tidy'])
+
+ def testModTidyWithoutUsages(self):
+ self.ScratchFile(
+ 'MODULE.bazel',
+ [
+ 'module( name = "foo", version = "1.2.3")',
+ ],
+ )
+
+ self.RunBazel(['mod', 'tidy'])
+
+ with open('MODULE.bazel', 'r') as module_file:
+ self.assertEqual(
+ [
+ 'module(',
+ ' name = "foo",',
+ ' version = "1.2.3",',
+ ')',
+ # This newline is from ScratchFile.
+ '',
+ ],
+ module_file.read().split('\n'),
+ )
+
def testModTidyFailsOnExtensionFailure(self):
self.ScratchFile(
'MODULE.bazel',
--
GitLab