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(