Automated rollback of commit 7d09b4a15985052670244c277e4357557b4d0039.
*** Reason for rollback ***
Now that unknown commit has been submitted, we can roll this forward with a small fix.
*** Original change description ***
Automated rollback of commit 13112c027c0064cf0a29256e80560cb6630af94d.
*** Reason for rollback ***
Causes [failures](https://buildkite.com/bazel/google-bazel-presubmit/builds/52620) in Presubmit of unknown commit, itself a Rollback of https://github.com/bazelbuild/bazel/commit/78d01316b22667e9d1758472c91dfee35cc189bd
*** Original change description ***
Bzlmod: When evaluating extensions in the main repo, do not load WORKSPACE
(https://github.com/bazelbuild/bazel/issues/13316)
See new comment in BzlLoadFunction.java for details. https://github.com/bazelbuild/bazel-central-registry/pull/47#issuecomment-998883652 also has a bit more context.
***
PiperOrigin-RevId: 417820112
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 dd2b6cf..a3aa4cc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2207,6 +2207,7 @@
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+ "//third_party:auto_value",
"//third_party:guava",
],
)
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 3de8e89..0c276e8 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
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
@@ -813,6 +814,7 @@
}
Label enclosingFileLabel = key.getLabel();
+ RepositoryName repoName = enclosingFileLabel.getRepository();
if (key instanceof BzlLoadValue.KeyForWorkspace) {
// Still during workspace file evaluation
@@ -828,30 +830,41 @@
// Note: we know for sure that the requested WorkspaceFileValue is fully computed so we do
// not need to check if it is null
return RepositoryMapping.createAllowingFallback(
- workspaceFileValue
- .getRepositoryMapping()
- .getOrDefault(enclosingFileLabel.getRepository(), ImmutableMap.of()));
+ workspaceFileValue.getRepositoryMapping().getOrDefault(repoName, ImmutableMap.of()));
// NOTE(wyv): this means that, in the WORKSPACE file, we can't load from a repo generated by
// bzlmod. If that's a problem, we should "fall back" to the bzlmod case below.
}
}
- if (key instanceof BzlLoadValue.KeyForBzlmod
- && enclosingFileLabel.getRepository().strippedName().equals("bazel_tools")) {
- // Special case: we're only here to get the @bazel_tools repo (for example, for http_archive).
- // This repo shouldn't have visibility into anything else (during repo generation), so we just
- // return an empty repo mapping.
- // TODO(wyv): disallow fallback.
- return RepositoryMapping.ALWAYS_FALLBACK;
+ if (key instanceof BzlLoadValue.KeyForBzlmod) {
+ if (repoName.equals(RepositoryName.BAZEL_TOOLS)) {
+ // Special case: we're only here to get the @bazel_tools repo (for example, for
+ // http_archive). This repo shouldn't have visibility into anything else (during repo
+ // generation), so we just return an empty repo mapping.
+ // TODO(wyv): disallow fallback.
+ return RepositoryMapping.ALWAYS_FALLBACK;
+ }
+ if (repoName.isMain()) {
+ // Special case: when we try to run an extension in the main repo, we need to grab the repo
+ // mapping for the main repo, which normally would include all WORKSPACE repos. This is
+ // problematic if the reason we're running an extension at all is that we're trying to do a
+ // `load` in WORKSPACE. So we specifically say that, to run an extension in the main repo,
+ // we ask for a repo mapping *without* WORKSPACE repos.
+ RepositoryMappingValue repositoryMappingValue =
+ (RepositoryMappingValue)
+ env.getValue(RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS);
+ if (repositoryMappingValue == null) {
+ return null;
+ }
+ return repositoryMappingValue.getRepositoryMapping();
+ }
}
// This is either a .bzl loaded from BUILD files, or a .bzl loaded for bzlmod (in which case the
// .bzl file *has* to be from a Bazel module anyway). So we can just use the full repo mapping
// from RepositoryMappingFunction.
- PackageIdentifier packageIdentifier = enclosingFileLabel.getPackageIdentifier();
RepositoryMappingValue repositoryMappingValue =
- (RepositoryMappingValue)
- env.getValue(RepositoryMappingValue.key(packageIdentifier.getRepository()));
+ (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(repoName));
if (repositoryMappingValue == null) {
return null;
}
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 594a7a2..e2da4dd 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
@@ -46,7 +46,7 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws SkyFunctionException, InterruptedException {
- RepositoryName repositoryName = (RepositoryName) skyKey.argument();
+ RepositoryName repositoryName = ((RepositoryMappingValue.Key) skyKey).repoName();
BazelModuleResolutionValue bazelModuleResolutionValue = null;
if (Preconditions.checkNotNull(RepositoryDelegatorFunction.ENABLE_BZLMOD.get(env))) {
@@ -56,9 +56,10 @@
return null;
}
- // The root module should be able to see repos defined in WORKSPACE. Therefore, we find all
- // workspace repos and add them as extra visible repos in root module's repo mappings.
- if (repositoryName.isMain()) {
+ if (repositoryName.isMain()
+ && ((RepositoryMappingValue.Key) skyKey).rootModuleShouldSeeWorkspaceRepos()) {
+ // The root module should be able to see repos defined in WORKSPACE. Therefore, we find all
+ // workspace repos and add them as extra visible repos in root module's repo mappings.
SkyKey externalPackageKey = PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey);
if (env.valuesMissing()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java
index 6ab436e..6d45291 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java
@@ -14,14 +14,15 @@
package com.google.devtools.build.lib.skyframe;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.Interner;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.skyframe.AbstractSkyKey;
import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Objects;
@@ -48,6 +49,8 @@
* packages. If the mappings are changed then the external packages need to be reloaded.
*/
public class RepositoryMappingValue implements SkyValue {
+ public static final Key KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS =
+ Key.create(RepositoryName.MAIN, /*rootModuleShouldSeeWorkspaceRepos=*/ false);
private final RepositoryMapping repositoryMapping;
@@ -63,7 +66,8 @@
/** Returns the {@link Key} for {@link RepositoryMappingValue}s. */
public static Key key(RepositoryName repositoryName) {
- return RepositoryMappingValue.Key.create(repositoryName);
+ return RepositoryMappingValue.Key.create(
+ repositoryName, /*rootModuleShouldSeeWorkspaceRepos=*/ true);
}
/** Returns a {@link RepositoryMappingValue} for a workspace with the given name. */
@@ -90,21 +94,25 @@
return repositoryMapping.toString();
}
- /** {@link com.google.devtools.build.skyframe.SkyKey} for {@link RepositoryMappingValue}. */
- @AutoCodec.VisibleForSerialization
- @AutoCodec
- static class Key extends AbstractSkyKey<RepositoryName> {
+ /** {@link SkyKey} for {@link RepositoryMappingValue}. */
+ @AutoValue
+ abstract static class Key implements SkyKey {
private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
- private Key(RepositoryName arg) {
- super(arg);
- }
+ /** The name of the repo to grab mappings for. */
+ abstract RepositoryName repoName();
- @AutoCodec.VisibleForSerialization
+ /**
+ * Whether the root module should see repos defined in WORKSPACE. This only takes effect when
+ * {@link #repoName} is the main repo.
+ */
+ abstract boolean rootModuleShouldSeeWorkspaceRepos();
+
@AutoCodec.Instantiator
- static Key create(RepositoryName arg) {
- return interner.intern(new Key(arg));
+ static Key create(RepositoryName repoName, boolean rootModuleShouldSeeWorkspaceRepos) {
+ return interner.intern(
+ new AutoValue_RepositoryMappingValue_Key(repoName, rootModuleShouldSeeWorkspaceRepos));
}
@Override
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 b6030c7..0ec34a1 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
@@ -103,8 +103,7 @@
@Test
public void testSimpleMapping() throws Exception {
- scratch.overwriteFile(
- "WORKSPACE",
+ rewriteWorkspace(
"workspace(name = 'good')",
"local_repository(",
" name = 'a_remote_repo',",
@@ -316,8 +315,7 @@
@Test
public void testMultipleRepositoriesWithMapping() throws Exception {
- scratch.overwriteFile(
- "WORKSPACE",
+ rewriteWorkspace(
"workspace(name = 'good')",
"local_repository(",
" name = 'a_remote_repo',",
@@ -356,8 +354,7 @@
@Test
public void testRepositoryWithMultipleMappings() throws Exception {
- scratch.overwriteFile(
- "WORKSPACE",
+ rewriteWorkspace(
"workspace(name = 'good')",
"local_repository(",
" name = 'a_remote_repo',",
@@ -381,9 +378,8 @@
}
@Test
- public void testMixtureOfBothSystems() throws Exception {
- scratch.overwriteFile(
- "WORKSPACE",
+ public void testMixtureOfBothSystems_workspaceRepo() throws Exception {
+ rewriteWorkspace(
"workspace(name = 'root')",
"local_repository(",
" name = 'ws_repo',",
@@ -434,6 +430,72 @@
}
@Test
+ public void testMixtureOfBothSystems_mainRepo() throws Exception {
+ rewriteWorkspace(
+ "workspace(name = 'root')",
+ "local_repository(",
+ " name = 'ws_repo',",
+ " path = '/ws_repo',",
+ ")");
+ scratch.overwriteFile(
+ "MODULE.bazel", "module(name='A',version='0.1')", "bazel_dep(name='B',version='1.0')");
+ registry
+ .addModule(
+ createModuleKey("B", "1.0"),
+ "module(name='B', version='1.0');bazel_dep(name='C', version='2.0')")
+ .addModule(createModuleKey("C", "2.0"), "module(name='C', version='2.0')");
+
+ SkyKey skyKey = RepositoryMappingValue.key(RepositoryName.MAIN);
+ assertThatEvaluationResult(eval(skyKey))
+ .hasEntryThat(skyKey)
+ .isEqualTo(
+ withMappingForRootModule(
+ ImmutableMap.of(
+ RepositoryName.MAIN,
+ RepositoryName.MAIN,
+ RepositoryName.create("@A"),
+ RepositoryName.MAIN,
+ RepositoryName.create("@B"),
+ RepositoryName.create("@B.1.0"),
+ RepositoryName.create("@root"),
+ RepositoryName.create("@root"),
+ RepositoryName.create("@ws_repo"),
+ RepositoryName.create("@ws_repo")),
+ RepositoryName.MAIN));
+ }
+
+ @Test
+ public void testMixtureOfBothSystems_mainRepo_shouldNotSeeWorkspaceRepos() throws Exception {
+ rewriteWorkspace(
+ "workspace(name = 'root')",
+ "local_repository(",
+ " name = 'ws_repo',",
+ " path = '/ws_repo',",
+ ")");
+ scratch.overwriteFile(
+ "MODULE.bazel", "module(name='A',version='0.1')", "bazel_dep(name='B',version='1.0')");
+ registry
+ .addModule(
+ createModuleKey("B", "1.0"),
+ "module(name='B', version='1.0');bazel_dep(name='C', version='2.0')")
+ .addModule(createModuleKey("C", "2.0"), "module(name='C', version='2.0')");
+
+ SkyKey skyKey = RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS;
+ assertThatEvaluationResult(eval(skyKey))
+ .hasEntryThat(skyKey)
+ .isEqualTo(
+ withMapping(
+ ImmutableMap.of(
+ RepositoryName.MAIN,
+ RepositoryName.MAIN,
+ RepositoryName.create("@A"),
+ RepositoryName.MAIN,
+ RepositoryName.create("@B"),
+ RepositoryName.create("@B.1.0")),
+ RepositoryName.MAIN));
+ }
+
+ @Test
public void testErrorWithMapping() throws Exception {
reporter.removeHandler(failFastHandler);
scratch.overwriteFile(
