Use the proper main repo mapping where appropriate
Use the main repo mapping (instead of ALWAYS_FALLBACK) to a) process .bzl load labels in the WORKSPACE file, and b) to process any labels passed to repo rules in either the WORKSPACE file or WORKSPACE-loaded macros.
This change only has a noticeable effect for when the workspace name (specified using the `workspace` directive in the WORKSPACE file) is used in labels as `@workspace_name//some:thing`. Before this change, such a label would actually point to `@workspace_name//some:thing`, which is a different target than `@//some:thing`, even though they're backed by the same source. This is because we insert an implicit `local_repository(name='workspace_name',path='.')` clause into the WORKSPACE file (see also https://github.com/bazelbuild/bazel/issues/15657 for a similar issue when Bzlmod is enabled).
This quirk can cause many subtle bugs, including toolchains being missing because `@workspace_name//:toolchain_type` and `@//:toolchain_type` are in fact distinct toolchain types!
Closes #15666.
Fixes #15667.
PiperOrigin-RevId: 454644054
Change-Id: Icd3742cfdf85eb5cf05281dd043b02ddc6a4e3c1
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java
index 6ea74d6..a6495ef 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.cmdline.LabelConstants;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.Package;
@@ -62,6 +63,7 @@
Root.fromPath(directories.getWorkspace()),
LabelConstants.MODULE_DOT_BAZEL_FILE_NAME),
"dummy_name",
+ RepositoryMapping.ALWAYS_FALLBACK,
semantics);
BuildLangTypedAttributeValuesMap attributeValues =
new BuildLangTypedAttributeValuesMap(attributes);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 4682436..aca053d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -843,13 +843,14 @@
PackageSettings helper,
RootedPath workspacePath,
String workspaceName,
+ RepositoryMapping mainRepoMapping,
StarlarkSemantics starlarkSemantics) {
return new Builder(
helper,
LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
workspaceName,
starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
- RepositoryMapping.ALWAYS_FALLBACK)
+ mainRepoMapping)
.setFilename(workspacePath);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index a1a9746..98ff211 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -445,9 +445,12 @@
@VisibleForTesting // exposed to WorkspaceFileFunction and BzlmodRepoRuleFunction
public Package.Builder newExternalPackageBuilder(
- RootedPath workspacePath, String workspaceName, StarlarkSemantics starlarkSemantics) {
+ RootedPath workspacePath,
+ String workspaceName,
+ RepositoryMapping mainRepoMapping,
+ StarlarkSemantics starlarkSemantics) {
return Package.newExternalPackageBuilder(
- packageSettings, workspacePath, workspaceName, starlarkSemantics);
+ packageSettings, workspacePath, workspaceName, mainRepoMapping, starlarkSemantics);
}
// This function is public only for the benefit of skyframe.PackageFunction,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
index 3ea17ab..09f8678 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.cmdline.LabelConstants;
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.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -238,22 +239,6 @@
// -- start of historical WorkspaceFileFunction --
// TODO(adonovan): reorganize and simplify.
- Package.Builder builder =
- packageFactory.newExternalPackageBuilder(
- workspaceFile, ruleClassProvider.getRunfilesPrefix(), starlarkSemantics);
-
- if (chunks.isEmpty()) {
- return new WorkspaceFileValue(
- buildAndReportEvents(builder, env),
- /* loadedModules = */ ImmutableMap.<String, Module>of(),
- /* loadToChunkMap = */ ImmutableMap.<String, Integer>of(),
- /* bindings = */ ImmutableMap.<String, Object>of(),
- workspaceFile,
- /* idx = */ 0, // first fragment
- /* hasNext = */ false,
- ImmutableMap.of());
- }
-
// Get the state at the end of the previous chunk.
WorkspaceFileValue prevValue = null;
if (key.getIndex() > 0) {
@@ -267,6 +252,32 @@
return prevValue;
}
}
+ RepositoryMapping repoMapping;
+ if (prevValue == null) {
+ repoMapping = RepositoryMapping.ALWAYS_FALLBACK;
+ } else {
+ repoMapping =
+ RepositoryMapping.createAllowingFallback(
+ prevValue
+ .getRepositoryMapping()
+ .getOrDefault(RepositoryName.MAIN, ImmutableMap.of()));
+ }
+
+ Package.Builder builder =
+ packageFactory.newExternalPackageBuilder(
+ workspaceFile, ruleClassProvider.getRunfilesPrefix(), repoMapping, starlarkSemantics);
+
+ if (chunks.isEmpty()) {
+ return new WorkspaceFileValue(
+ buildAndReportEvents(builder, env),
+ /* loadedModules = */ ImmutableMap.<String, Module>of(),
+ /* loadToChunkMap = */ ImmutableMap.<String, Integer>of(),
+ /* bindings = */ ImmutableMap.<String, Object>of(),
+ workspaceFile,
+ /* idx = */ 0, // first fragment
+ /* hasNext = */ false,
+ ImmutableMap.of());
+ }
List<StarlarkFile> chunk = chunks.get(key.getIndex());
@@ -274,8 +285,7 @@
ImmutableList<Pair<String, Location>> programLoads =
BzlLoadFunction.getLoadsFromStarlarkFiles(chunk);
ImmutableList<Label> loadLabels =
- BzlLoadFunction.getLoadLabels(
- env.getListener(), programLoads, rootPackage, RepositoryMapping.ALWAYS_FALLBACK);
+ BzlLoadFunction.getLoadLabels(env.getListener(), programLoads, rootPackage, repoMapping);
if (loadLabels == null) {
NoSuchPackageException e =
PackageFunction.PackageFunctionException.builder()
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD
index ac60a64..247b531 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD
@@ -22,6 +22,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
+ "//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
@@ -68,6 +69,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
+ "//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java
index 73c22c9..219ef73 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java
@@ -26,6 +26,7 @@
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.io.CharStreams;
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Package;
@@ -139,6 +140,7 @@
DefaultPackageSettings.INSTANCE,
RootedPath.toRootedPath(root, workspaceFile),
"runfiles",
+ RepositoryMapping.ALWAYS_FALLBACK,
starlarkSemantics);
ExtendedEventHandler listener = Mockito.mock(ExtendedEventHandler.class);
Rule rule =
diff --git a/src/test/java/com/google/devtools/build/lib/packages/BUILD b/src/test/java/com/google/devtools/build/lib/packages/BUILD
index a154539..5958cb7 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/packages/BUILD
@@ -20,6 +20,7 @@
name = "PackageTestsUtil",
srcs = ["WorkspaceFactoryTestHelper.java"],
deps = [
+ "//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/vfs",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
index df85e78..71957a7 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
@@ -123,7 +123,10 @@
Path myPkgPath = scratch.resolve("/workspace/WORKSPACE");
Package.Builder pkgBuilder =
packageFactory.newExternalPackageBuilder(
- RootedPath.toRootedPath(root, myPkgPath), "TESTING", StarlarkSemantics.DEFAULT);
+ RootedPath.toRootedPath(root, myPkgPath),
+ "TESTING",
+ RepositoryMapping.ALWAYS_FALLBACK,
+ StarlarkSemantics.DEFAULT);
Map<String, Object> attributeValues = new HashMap<>();
attributeValues.put("name", "foo");
diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java
index 31d98d0..1ed39ed 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java
@@ -20,6 +20,7 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.Package.Builder.DefaultPackageSettings;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -35,6 +36,7 @@
/** Parses a WORKSPACE file with the given content. */
// TODO(adonovan): delete this junk class.
final class WorkspaceFactoryTestHelper {
+
private final Root root;
private Package.Builder builder;
private StarlarkSemantics starlarkSemantics;
@@ -67,6 +69,7 @@
DefaultPackageSettings.INSTANCE,
RootedPath.toRootedPath(root, workspaceFilePath),
"",
+ RepositoryMapping.ALWAYS_FALLBACK,
StarlarkSemantics.DEFAULT);
WorkspaceFactory factory =
new WorkspaceFactory(
