Skip to content

Commit 7d09b4a

Browse files
Googlercopybara-github
Googler
authored and
copybara-github
committed
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
1 parent fafe16d commit 7d09b4a

5 files changed

Lines changed: 38 additions & 123 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2207,7 +2207,6 @@ java_library(
22072207
"//src/main/java/com/google/devtools/build/lib/concurrent",
22082208
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
22092209
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
2210-
"//third_party:auto_value",
22112210
"//third_party:guava",
22122211
],
22132212
)

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

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
3030
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
3131
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
32-
import com.google.devtools.build.lib.cmdline.RepositoryName;
3332
import com.google.devtools.build.lib.concurrent.BlazeInterners;
3433
import com.google.devtools.build.lib.events.Event;
3534
import com.google.devtools.build.lib.events.EventHandler;
@@ -814,7 +813,6 @@ private static RepositoryMapping getRepositoryMapping(BzlLoadValue.Key key, Envi
814813
}
815814

816815
Label enclosingFileLabel = key.getLabel();
817-
RepositoryName repoName = enclosingFileLabel.getRepository();
818816

819817
if (key instanceof BzlLoadValue.KeyForWorkspace) {
820818
// Still during workspace file evaluation
@@ -830,41 +828,30 @@ private static RepositoryMapping getRepositoryMapping(BzlLoadValue.Key key, Envi
830828
// Note: we know for sure that the requested WorkspaceFileValue is fully computed so we do
831829
// not need to check if it is null
832830
return RepositoryMapping.createAllowingFallback(
833-
workspaceFileValue.getRepositoryMapping().getOrDefault(repoName, ImmutableMap.of()));
831+
workspaceFileValue
832+
.getRepositoryMapping()
833+
.getOrDefault(enclosingFileLabel.getRepository(), ImmutableMap.of()));
834834
// NOTE(wyv): this means that, in the WORKSPACE file, we can't load from a repo generated by
835835
// bzlmod. If that's a problem, we should "fall back" to the bzlmod case below.
836836
}
837837
}
838838

839-
if (key instanceof BzlLoadValue.KeyForBzlmod) {
840-
if (repoName.equals(RepositoryName.BAZEL_TOOLS)) {
841-
// Special case: we're only here to get the @bazel_tools repo (for example, for
842-
// http_archive). This repo shouldn't have visibility into anything else (during repo
843-
// generation), so we just return an empty repo mapping.
844-
// TODO(wyv): disallow fallback.
845-
return RepositoryMapping.ALWAYS_FALLBACK;
846-
}
847-
if (repoName.isMain()) {
848-
// Special case: when we try to run an extension in the main repo, we need to grab the repo
849-
// mapping for the main repo, which normally would include all WORKSPACE repos. This is
850-
// problematic if the reason we're running an extension at all is that we're trying to do a
851-
// `load` in WORKSPACE. So we specifically say that, to run an extension in the main repo,
852-
// we ask for a repo mapping *without* WORKSPACE repos.
853-
RepositoryMappingValue repositoryMappingValue =
854-
(RepositoryMappingValue)
855-
env.getValue(RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS);
856-
if (repositoryMappingValue == null) {
857-
return null;
858-
}
859-
return repositoryMappingValue.getRepositoryMapping();
860-
}
839+
if (key instanceof BzlLoadValue.KeyForBzlmod
840+
&& enclosingFileLabel.getRepository().getName().equals("bazel_tools")) {
841+
// Special case: we're only here to get the @bazel_tools repo (for example, for http_archive).
842+
// This repo shouldn't have visibility into anything else (during repo generation), so we just
843+
// return an empty repo mapping.
844+
// TODO(wyv): disallow fallback.
845+
return RepositoryMapping.ALWAYS_FALLBACK;
861846
}
862847

863848
// This is either a .bzl loaded from BUILD files, or a .bzl loaded for bzlmod (in which case the
864849
// .bzl file *has* to be from a Bazel module anyway). So we can just use the full repo mapping
865850
// from RepositoryMappingFunction.
851+
PackageIdentifier packageIdentifier = enclosingFileLabel.getPackageIdentifier();
866852
RepositoryMappingValue repositoryMappingValue =
867-
(RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(repoName));
853+
(RepositoryMappingValue)
854+
env.getValue(RepositoryMappingValue.key(packageIdentifier.getRepository()));
868855
if (repositoryMappingValue == null) {
869856
return null;
870857
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class RepositoryMappingFunction implements SkyFunction {
4646
@Override
4747
public SkyValue compute(SkyKey skyKey, Environment env)
4848
throws SkyFunctionException, InterruptedException {
49-
RepositoryName repositoryName = ((RepositoryMappingValue.Key) skyKey).repoName();
49+
RepositoryName repositoryName = (RepositoryName) skyKey.argument();
5050

5151
BazelModuleResolutionValue bazelModuleResolutionValue = null;
5252
if (Preconditions.checkNotNull(RepositoryDelegatorFunction.ENABLE_BZLMOD.get(env))) {
@@ -56,10 +56,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
5656
return null;
5757
}
5858

59-
if (repositoryName.isMain()
60-
&& ((RepositoryMappingValue.Key) skyKey).rootModuleShouldSeeWorkspaceRepos()) {
61-
// The root module should be able to see repos defined in WORKSPACE. Therefore, we find all
62-
// workspace repos and add them as extra visible repos in root module's repo mappings.
59+
// The root module should be able to see repos defined in WORKSPACE. Therefore, we find all
60+
// workspace repos and add them as extra visible repos in root module's repo mappings.
61+
if (repositoryName.isMain()) {
6362
SkyKey externalPackageKey = PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
6463
PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey);
6564
if (env.valuesMissing()) {

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

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,14 @@
1414

1515
package com.google.devtools.build.lib.skyframe;
1616

17-
import com.google.auto.value.AutoValue;
1817
import com.google.common.base.Preconditions;
1918
import com.google.common.collect.Interner;
2019
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
2120
import com.google.devtools.build.lib.cmdline.RepositoryName;
2221
import com.google.devtools.build.lib.concurrent.BlazeInterners;
2322
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
23+
import com.google.devtools.build.skyframe.AbstractSkyKey;
2424
import com.google.devtools.build.skyframe.SkyFunctionName;
25-
import com.google.devtools.build.skyframe.SkyKey;
2625
import com.google.devtools.build.skyframe.SkyValue;
2726
import java.util.Objects;
2827

@@ -49,8 +48,6 @@
4948
* packages. If the mappings are changed then the external packages need to be reloaded.
5049
*/
5150
public class RepositoryMappingValue implements SkyValue {
52-
public static final Key KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS =
53-
Key.create(RepositoryName.MAIN, /*rootModuleShouldSeeWorkspaceRepos=*/ false);
5451

5552
private final RepositoryMapping repositoryMapping;
5653

@@ -66,8 +63,7 @@ public RepositoryMapping getRepositoryMapping() {
6663

6764
/** Returns the {@link Key} for {@link RepositoryMappingValue}s. */
6865
public static Key key(RepositoryName repositoryName) {
69-
return RepositoryMappingValue.Key.create(
70-
repositoryName, /*rootModuleShouldSeeWorkspaceRepos=*/ true);
66+
return RepositoryMappingValue.Key.create(repositoryName);
7167
}
7268

7369
/** Returns a {@link RepositoryMappingValue} for a workspace with the given name. */
@@ -94,25 +90,21 @@ public String toString() {
9490
return repositoryMapping.toString();
9591
}
9692

97-
/** {@link SkyKey} for {@link RepositoryMappingValue}. */
98-
@AutoValue
99-
abstract static class Key implements SkyKey {
93+
/** {@link com.google.devtools.build.skyframe.SkyKey} for {@link RepositoryMappingValue}. */
94+
@AutoCodec.VisibleForSerialization
95+
@AutoCodec
96+
static class Key extends AbstractSkyKey<RepositoryName> {
10097

10198
private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
10299

103-
/** The name of the repo to grab mappings for. */
104-
abstract RepositoryName repoName();
105-
106-
/**
107-
* Whether the root module should see repos defined in WORKSPACE. This only takes effect when
108-
* {@link #repoName} is the main repo.
109-
*/
110-
abstract boolean rootModuleShouldSeeWorkspaceRepos();
100+
private Key(RepositoryName arg) {
101+
super(arg);
102+
}
111103

104+
@AutoCodec.VisibleForSerialization
112105
@AutoCodec.Instantiator
113-
static Key create(RepositoryName repoName, boolean rootModuleShouldSeeWorkspaceRepos) {
114-
return interner.intern(
115-
new AutoValue_RepositoryMappingValue_Key(repoName, rootModuleShouldSeeWorkspaceRepos));
106+
static Key create(RepositoryName arg) {
107+
return interner.intern(new Key(arg));
116108
}
117109

118110
@Override

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

Lines changed: 9 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,8 @@ public RepositoryMappingValue withMappingForRootModule(
103103

104104
@Test
105105
public void testSimpleMapping() throws Exception {
106-
rewriteWorkspace(
106+
scratch.overwriteFile(
107+
"WORKSPACE",
107108
"workspace(name = 'good')",
108109
"local_repository(",
109110
" name = 'a_remote_repo',",
@@ -315,7 +316,8 @@ public void testRepoNameMapping_multipleVersionOverride_lookup() throws Exceptio
315316

316317
@Test
317318
public void testMultipleRepositoriesWithMapping() throws Exception {
318-
rewriteWorkspace(
319+
scratch.overwriteFile(
320+
"WORKSPACE",
319321
"workspace(name = 'good')",
320322
"local_repository(",
321323
" name = 'a_remote_repo',",
@@ -354,7 +356,8 @@ public void testMultipleRepositoriesWithMapping() throws Exception {
354356

355357
@Test
356358
public void testRepositoryWithMultipleMappings() throws Exception {
357-
rewriteWorkspace(
359+
scratch.overwriteFile(
360+
"WORKSPACE",
358361
"workspace(name = 'good')",
359362
"local_repository(",
360363
" name = 'a_remote_repo',",
@@ -378,8 +381,9 @@ public void testRepositoryWithMultipleMappings() throws Exception {
378381
}
379382

380383
@Test
381-
public void testMixtureOfBothSystems_workspaceRepo() throws Exception {
382-
rewriteWorkspace(
384+
public void testMixtureOfBothSystems() throws Exception {
385+
scratch.overwriteFile(
386+
"WORKSPACE",
383387
"workspace(name = 'root')",
384388
"local_repository(",
385389
" name = 'ws_repo',",
@@ -429,72 +433,6 @@ public void testMixtureOfBothSystems_workspaceRepo() throws Exception {
429433
.build()));
430434
}
431435

432-
@Test
433-
public void testMixtureOfBothSystems_mainRepo() throws Exception {
434-
rewriteWorkspace(
435-
"workspace(name = 'root')",
436-
"local_repository(",
437-
" name = 'ws_repo',",
438-
" path = '/ws_repo',",
439-
")");
440-
scratch.overwriteFile(
441-
"MODULE.bazel", "module(name='A',version='0.1')", "bazel_dep(name='B',version='1.0')");
442-
registry
443-
.addModule(
444-
createModuleKey("B", "1.0"),
445-
"module(name='B', version='1.0');bazel_dep(name='C', version='2.0')")
446-
.addModule(createModuleKey("C", "2.0"), "module(name='C', version='2.0')");
447-
448-
SkyKey skyKey = RepositoryMappingValue.key(RepositoryName.MAIN);
449-
assertThatEvaluationResult(eval(skyKey))
450-
.hasEntryThat(skyKey)
451-
.isEqualTo(
452-
withMappingForRootModule(
453-
ImmutableMap.of(
454-
RepositoryName.MAIN,
455-
RepositoryName.MAIN,
456-
RepositoryName.create("A"),
457-
RepositoryName.MAIN,
458-
RepositoryName.create("B"),
459-
RepositoryName.create("B.1.0"),
460-
RepositoryName.create("root"),
461-
RepositoryName.create("root"),
462-
RepositoryName.create("ws_repo"),
463-
RepositoryName.create("ws_repo")),
464-
RepositoryName.MAIN));
465-
}
466-
467-
@Test
468-
public void testMixtureOfBothSystems_mainRepo_shouldNotSeeWorkspaceRepos() throws Exception {
469-
rewriteWorkspace(
470-
"workspace(name = 'root')",
471-
"local_repository(",
472-
" name = 'ws_repo',",
473-
" path = '/ws_repo',",
474-
")");
475-
scratch.overwriteFile(
476-
"MODULE.bazel", "module(name='A',version='0.1')", "bazel_dep(name='B',version='1.0')");
477-
registry
478-
.addModule(
479-
createModuleKey("B", "1.0"),
480-
"module(name='B', version='1.0');bazel_dep(name='C', version='2.0')")
481-
.addModule(createModuleKey("C", "2.0"), "module(name='C', version='2.0')");
482-
483-
SkyKey skyKey = RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS;
484-
assertThatEvaluationResult(eval(skyKey))
485-
.hasEntryThat(skyKey)
486-
.isEqualTo(
487-
withMapping(
488-
ImmutableMap.of(
489-
RepositoryName.MAIN,
490-
RepositoryName.MAIN,
491-
RepositoryName.create("A"),
492-
RepositoryName.MAIN,
493-
RepositoryName.create("B"),
494-
RepositoryName.create("B.1.0")),
495-
RepositoryName.MAIN));
496-
}
497-
498436
@Test
499437
public void testErrorWithMapping() throws Exception {
500438
reporter.removeHandler(failFastHandler);

0 commit comments

Comments
 (0)