Automated rollback of commit 13112c0.

*** Reason for rollback ***

Causes [failures](https://buildkite.com/bazel/google-bazel-presubmit/builds/52620) in Presubmit of unknown commit, itself a Rollback of bazelbuild@78d0131

*** Original change description ***

Bzlmod: When evaluating extensions in the main repo, do not load WORKSPACE

(bazelbuild#13316)

See new comment in BzlLoadFunction.java for details. bazelbuild/bazel-central-registry#47 (comment) also has a bit more context.

PiperOrigin-RevId: 417668153

Author: Googler

src/main/java/com/google/devtools/build/lib/skyframe/BUILD

@@ -2207,7 +2207,6 @@ java_library(
         "//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",
     ],
 )

src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java

@@ -29,7 +29,6 @@
 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;
@@ -814,7 +813,6 @@ private static RepositoryMapping getRepositoryMapping(BzlLoadValue.Key key, Envi
     }
 
     Label enclosingFileLabel = key.getLabel();
-    RepositoryName repoName = enclosingFileLabel.getRepository();
 
     if (key instanceof BzlLoadValue.KeyForWorkspace) {
       // Still during workspace file evaluation
@@ -830,41 +828,30 @@ private static RepositoryMapping getRepositoryMapping(BzlLoadValue.Key key, Envi
         // 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(repoName, ImmutableMap.of()));
+            workspaceFileValue
+                .getRepositoryMapping()
+                .getOrDefault(enclosingFileLabel.getRepository(), 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) {
-      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();
-      }
+    if (key instanceof BzlLoadValue.KeyForBzlmod
+        && enclosingFileLabel.getRepository().getName().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;
     }
 
     // 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(repoName));
+        (RepositoryMappingValue)
+            env.getValue(RepositoryMappingValue.key(packageIdentifier.getRepository()));
     if (repositoryMappingValue == null) {
       return null;
     }

src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java

@@ -46,7 +46,7 @@ public class RepositoryMappingFunction implements SkyFunction {
   @Override
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws SkyFunctionException, InterruptedException {
-    RepositoryName repositoryName = ((RepositoryMappingValue.Key) skyKey).repoName();
+    RepositoryName repositoryName = (RepositoryName) skyKey.argument();
 
     BazelModuleResolutionValue bazelModuleResolutionValue = null;
     if (Preconditions.checkNotNull(RepositoryDelegatorFunction.ENABLE_BZLMOD.get(env))) {
@@ -56,10 +56,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
         return null;
       }
 
-      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.
+      // 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()) {
         SkyKey externalPackageKey = PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
         PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey);
         if (env.valuesMissing()) {

src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java

@@ -14,15 +14,14 @@
 
 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;
 
@@ -49,8 +48,6 @@
  * 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;
 
@@ -66,8 +63,7 @@ public RepositoryMapping getRepositoryMapping() {
 
   /** Returns the {@link Key} for {@link RepositoryMappingValue}s. */
   public static Key key(RepositoryName repositoryName) {
-    return RepositoryMappingValue.Key.create(
-        repositoryName, /*rootModuleShouldSeeWorkspaceRepos=*/ true);
+    return RepositoryMappingValue.Key.create(repositoryName);
   }
 
   /** Returns a {@link RepositoryMappingValue} for a workspace with the given name. */
@@ -94,25 +90,21 @@ public String toString() {
     return repositoryMapping.toString();
   }
 
-  /** {@link SkyKey} for {@link RepositoryMappingValue}. */
-  @AutoValue
-  abstract static class Key implements SkyKey {
+  /** {@link com.google.devtools.build.skyframe.SkyKey} for {@link RepositoryMappingValue}. */
+  @AutoCodec.VisibleForSerialization
+  @AutoCodec
+  static class Key extends AbstractSkyKey<RepositoryName> {
 
     private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
 
-    /** The name of the repo to grab mappings for. */
-    abstract RepositoryName repoName();
-
-    /**
-     * Whether the root module should see repos defined in WORKSPACE. This only takes effect when
-     * {@link #repoName} is the main repo.
-     */
-    abstract boolean rootModuleShouldSeeWorkspaceRepos();
+    private Key(RepositoryName arg) {
+      super(arg);
+    }
 
+    @AutoCodec.VisibleForSerialization
     @AutoCodec.Instantiator
-    static Key create(RepositoryName repoName, boolean rootModuleShouldSeeWorkspaceRepos) {
-      return interner.intern(
-          new AutoValue_RepositoryMappingValue_Key(repoName, rootModuleShouldSeeWorkspaceRepos));
+    static Key create(RepositoryName arg) {
+      return interner.intern(new Key(arg));
     }
 
     @Override

src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java

@@ -103,7 +103,8 @@ public RepositoryMappingValue withMappingForRootModule(
 
   @Test
   public void testSimpleMapping() throws Exception {
-    rewriteWorkspace(
+    scratch.overwriteFile(
+        "WORKSPACE",
         "workspace(name = 'good')",
         "local_repository(",
         "    name = 'a_remote_repo',",
@@ -315,7 +316,8 @@ public void testRepoNameMapping_multipleVersionOverride_lookup() throws Exceptio
 
   @Test
   public void testMultipleRepositoriesWithMapping() throws Exception {
-    rewriteWorkspace(
+    scratch.overwriteFile(
+        "WORKSPACE",
         "workspace(name = 'good')",
         "local_repository(",
         "    name = 'a_remote_repo',",
@@ -354,7 +356,8 @@ public void testMultipleRepositoriesWithMapping() throws Exception {
 
   @Test
   public void testRepositoryWithMultipleMappings() throws Exception {
-    rewriteWorkspace(
+    scratch.overwriteFile(
+        "WORKSPACE",
         "workspace(name = 'good')",
         "local_repository(",
         "    name = 'a_remote_repo',",
@@ -378,8 +381,9 @@ public void testRepositoryWithMultipleMappings() throws Exception {
   }
 
   @Test
-  public void testMixtureOfBothSystems_workspaceRepo() throws Exception {
-    rewriteWorkspace(
+  public void testMixtureOfBothSystems() throws Exception {
+    scratch.overwriteFile(
+        "WORKSPACE",
         "workspace(name = 'root')",
         "local_repository(",
         "    name = 'ws_repo',",
@@ -429,72 +433,6 @@ public void testMixtureOfBothSystems_workspaceRepo() throws Exception {
                     .build()));
   }
 
-  @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);