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: 417668153
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 a3aa4cc..dd2b6cf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2207,7 +2207,6 @@
"//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 0c276e8..3537eff 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,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 @@
}
Label enclosingFileLabel = key.getLabel();
- RepositoryName repoName = enclosingFileLabel.getRepository();
if (key instanceof BzlLoadValue.KeyForWorkspace) {
// Still during workspace file evaluation
@@ -830,41 +828,30 @@
// 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;
}
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 57a19d5..81dc320 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 = ((RepositoryMappingValue.Key) skyKey).repoName();
+ RepositoryName repositoryName = (RepositoryName) skyKey.argument();
BazelModuleResolutionValue bazelModuleResolutionValue = null;
if (Preconditions.checkNotNull(RepositoryDelegatorFunction.ENABLE_BZLMOD.get(env))) {
@@ -56,10 +56,9 @@
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()) {
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 6d45291..6ab436e 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,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 @@
/** 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 @@
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();
+ private Key(RepositoryName arg) {
+ super(arg);
+ }
- /**
- * 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.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
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 49a04f5..5c426d2 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,7 +103,8 @@
@Test
public void testSimpleMapping() throws Exception {
- rewriteWorkspace(
+ scratch.overwriteFile(
+ "WORKSPACE",
"workspace(name = 'good')",
"local_repository(",
" name = 'a_remote_repo',",
@@ -315,7 +316,8 @@
@Test
public void testMultipleRepositoriesWithMapping() throws Exception {
- rewriteWorkspace(
+ scratch.overwriteFile(
+ "WORKSPACE",
"workspace(name = 'good')",
"local_repository(",
" name = 'a_remote_repo',",
@@ -354,7 +356,8 @@
@Test
public void testRepositoryWithMultipleMappings() throws Exception {
- rewriteWorkspace(
+ scratch.overwriteFile(
+ "WORKSPACE",
"workspace(name = 'good')",
"local_repository(",
" name = 'a_remote_repo',",
@@ -378,8 +381,9 @@
}
@Test
- public void testMixtureOfBothSystems_workspaceRepo() throws Exception {
- rewriteWorkspace(
+ public void testMixtureOfBothSystems() throws Exception {
+ scratch.overwriteFile(
+ "WORKSPACE",
"workspace(name = 'root')",
"local_repository(",
" name = 'ws_repo',",
@@ -430,72 +434,6 @@
}
@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(
