Bzlmod: Proper repo mappings for @_builtins
@_builtins is not a real repo and does not currently have repo mappings. However, things in there still sometimes use things from other repos (eg. @platforms). This isn't a problem pre-Bzlmod since @platforms is just called @platforms, but in Bzlmod it's called @platforms.0.0.4, so things break.
This CL changes it so that when Bzlmod is enabled, @_builtins adopts the repo mapping of @bazel_tools, the built-in module. Note that the repo mapping of @bazel_tools does not have a dependency on the //external package, so there is still no circular SkyValue dependency.
Fixes https://github.com/bazelbuild/bazel/issues/15317.
PiperOrigin-RevId: 450665054
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java
index 9447925..4b5b255 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java
@@ -179,7 +179,8 @@
throw Starlark.errorf("the module() directive can only be called once");
}
moduleCalled = true;
- // TODO(wyv): add validation logic for name (alphanumerical) & others in the future
+ // TODO(wyv): add validation logic for name (alphanumerical, start with a letter) & others in
+ // the future
Version parsedVersion;
try {
parsedVersion = Version.parse(version);
@@ -250,7 +251,8 @@
if (repoName.isEmpty()) {
repoName = name;
}
- // TODO(wyv): add validation logic for name (alphanumerical) and repoName (RepositoryName?)
+ // TODO(wyv): add validation logic for name (alphanumerical, start with a letter) and repoName
+ // (RepositoryName?, start with a letter)
Version parsedVersion;
try {
parsedVersion = Version.parse(version);
@@ -341,6 +343,7 @@
void addImport(String localRepoName, String exportedName, Location location)
throws EvalException {
+ // TODO(wyv): validate both repo names (RepositoryName.validate; starts with a letter)
addRepoNameUsage(localRepoName, "by a use_repo() call", location);
if (imports.containsValue(exportedName)) {
String collisionRepoName = imports.inverse().get(exportedName);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index dbdb4b8..5d381a6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2129,6 +2129,7 @@
deps = [
":package_value",
":repository_mapping_value",
+ ":starlark_builtins_value",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
index b7865e5..ebd03a6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
@@ -44,6 +44,7 @@
import com.google.devtools.build.lib.packages.StarlarkExportable;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.StarlarkLoading;
import com.google.devtools.build.lib.server.FailureDetails.StarlarkLoading.Code;
@@ -806,12 +807,14 @@
private static RepositoryMapping getRepositoryMapping(BzlLoadValue.Key key, Environment env)
throws InterruptedException {
- if (key.isBuiltins()) {
- // Builtins .bzls never have a repo mapping defined for them, so return without requesting a
- // RepositoryMappingValue. (NB: In addition to being a slight optimization, this avoids
- // adding a reverse dependency on the special //external package, which helps avoid tickling
- // some peculiarities of the Google-internal Skyframe implementation; see b/182293526 for
- // details.)
+ if (key.isBuiltins() && !RepositoryDelegatorFunction.ENABLE_BZLMOD.get(env)) {
+ // Without Bzlmod, builtins .bzls never have a repo mapping defined for them, so return
+ // without requesting a RepositoryMappingValue. (NB: In addition to being a slight
+ // optimization, this avoids adding a reverse dependency on the special //external package,
+ // which helps avoid tickling some peculiarities of the Google-internal Skyframe
+ // implementation; see b/182293526 for details.)
+ // Otherwise, builtins .bzls should use the repo mapping of @bazel_tools, and *do* request a
+ // normal RepositoryMappingValue (see logic in RepositoryMappingFunction).
return RepositoryMapping.ALWAYS_FALLBACK;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java
index 4296154..58512bc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java
@@ -50,6 +50,24 @@
BazelModuleResolutionValue bazelModuleResolutionValue = null;
if (Preconditions.checkNotNull(RepositoryDelegatorFunction.ENABLE_BZLMOD.get(env))) {
+ if (StarlarkBuiltinsValue.isBuiltinsRepo(repositoryName)) {
+ // Builtins .bzl files should use the repo mapping of @bazel_tools, to get access to repos
+ // such as @platforms.
+ RepositoryMappingValue bazelToolsMapping =
+ (RepositoryMappingValue)
+ env.getValue(RepositoryMappingValue.key(RepositoryName.BAZEL_TOOLS));
+ if (bazelToolsMapping == null) {
+ return null;
+ }
+ // We need to make sure that @_builtins maps to @_builtins too.
+ return RepositoryMappingValue.withMapping(
+ RepositoryMapping.create(
+ ImmutableMap.of(
+ StarlarkBuiltinsValue.BUILTINS_REPO, StarlarkBuiltinsValue.BUILTINS_REPO),
+ StarlarkBuiltinsValue.BUILTINS_NAME)
+ .withAdditionalMappings(bazelToolsMapping.getRepositoryMapping()));
+ }
+
bazelModuleResolutionValue =
(BazelModuleResolutionValue) env.getValue(BazelModuleResolutionValue.KEY);
if (env.valuesMissing()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
index 9726844..0b7efe8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
@@ -52,7 +52,8 @@
/**
* A Skyframe function that evaluates the {@code @_builtins} pseudo-repository and reports the
- * values exported by {@link #EXPORTS_ENTRYPOINT}.
+ * values exported by {@link #EXPORTS_ENTRYPOINT}. The {@code @_builtins} pseudo-repository shares a
+ * repo mapping with the {@code @bazel_tools} repository.
*
* <p>The process of "builtins injection" refers to evaluating this Skyfunction and applying its
* result to {@link BzlLoadFunction}'s computation. See also the <a
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java
index 2464de5..2ba7271 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java
@@ -583,6 +583,39 @@
}
@Test
+ public void builtinsRepo() throws Exception {
+ scratch.overwriteFile(
+ "MODULE.bazel",
+ // Add an explicit dependency on @bazel_tools since we're not using built-in modules in this
+ // test
+ "bazel_dep(name='bazel_tools',version='1.0')");
+ registry
+ .addModule(
+ createModuleKey("bazel_tools", "1.0"),
+ "module(name='bazel_tools',version='1.0')",
+ "bazel_dep(name='foo', version='1.0')")
+ .addModule(createModuleKey("foo", "1.0"), "module(name='foo', version='1.0')");
+
+ RepositoryName name = RepositoryName.create("_builtins");
+ SkyKey skyKey = RepositoryMappingValue.key(name);
+ EvaluationResult<RepositoryMappingValue> result = eval(skyKey);
+
+ assertThat(result.hasError()).isFalse();
+ assertThatEvaluationResult(result)
+ .hasEntryThat(skyKey)
+ .isEqualTo(
+ withMapping(
+ ImmutableMap.of(
+ RepositoryName.create("bazel_tools"),
+ RepositoryName.create("bazel_tools"), // bazel_tools is a well-known module
+ RepositoryName.create("foo"),
+ RepositoryName.create("foo.1.0"),
+ RepositoryName.create("_builtins"),
+ RepositoryName.create("_builtins")),
+ name));
+ }
+
+ @Test
public void testEqualsAndHashCode() throws Exception {
new EqualsTester()
.addEqualityGroup(
