Make sure that absolute packages don't go through repo mapping
https://github.com/bazelbuild/bazel/commit/103ea9d3f3a0d85a0d7c1ad98fd8dbb422505d95 refactored label parsing,
getting rid of the "default" repo name (using "main" instead). This meant that absolute packages such as
"//visibility" would now always map to the main repo. This is normally correct, except that this main repo would
then go through repo mapping, so if any non-main repo tried to use such labels, they would get a "non-visible"
error (with Bzlmod enabled only; otherwise, repo mappings allow fallback). This commit fixes it so that such
usages of the main repo don't go through repo mapping at all.
Fixes https://github.com/bazelbuild/bazel/issues/14403.
Closes #14412.
PiperOrigin-RevId: 416252184
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index d592862..fcc08a1 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -18,6 +18,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Interner;
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.actions.CommandLineItem;
@@ -53,6 +54,18 @@
@Immutable
@ThreadSafe
public final class Label implements Comparable<Label>, StarlarkValue, SkyKey, CommandLineItem {
+ /**
+ * Package names that aren't made relative to the current repository because they mean special
+ * things to Bazel.
+ */
+ private static final ImmutableSet<String> ABSOLUTE_PACKAGE_NAMES =
+ ImmutableSet.of(
+ // Used for select's `//conditions:default` label (not a target)
+ "conditions",
+ // Used for the public and private visibility labels (not targets)
+ "visibility",
+ // There is only one //external package
+ LabelConstants.EXTERNAL_PACKAGE_NAME.getPathString());
// Intern "__pkg__" and "__subpackages__" pseudo-targets, which appears in labels used for
// visibility specifications. This saves a couple tenths of a percent of RAM off the loading
@@ -83,6 +96,18 @@
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
}
+ /** Computes the repo name for the label, within the context of a current repo. */
+ private static RepositoryName computeRepoNameWithRepoContext(
+ Parts parts, RepositoryName currentRepo, RepositoryMapping repoMapping) {
+ if (parts.repo == null) {
+ // Certain package names when used without a "@" part are always absolutely in the main repo,
+ // disregarding the current repo and repo mappings.
+ return ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg) ? RepositoryName.MAIN : currentRepo;
+ }
+ // TODO(b/200024947): Make repo mapping take a string and return a RepositoryName.
+ return repoMapping.get(RepositoryName.createFromValidStrippedName(parts.repo));
+ }
+
// TODO(b/200024947): Make this public.
/**
* Parses a raw label string within the context of a current repo. It must be of the form {@code
@@ -94,11 +119,7 @@
throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
parts.checkPkgIsAbsolute();
- // TODO(b/200024947): Make repo mapping take a string and return a RepositoryName.
- RepositoryName repoName =
- parts.repo == null
- ? currentRepo
- : repoMapping.get(RepositoryName.createFromValidStrippedName(parts.repo));
+ RepositoryName repoName = computeRepoNameWithRepoContext(parts, currentRepo, repoMapping);
return createUnvalidated(
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
}
@@ -119,11 +140,8 @@
if (!parts.pkg.isEmpty()) {
parts.checkPkgIsAbsolute();
}
- // TODO(b/200024947): Make repo mapping take a string and return a RepositoryName.
RepositoryName repoName =
- parts.repo == null
- ? packageIdentifier.getRepository()
- : repoMapping.get(RepositoryName.createFromValidStrippedName(parts.repo));
+ computeRepoNameWithRepoContext(parts, packageIdentifier.getRepository(), repoMapping);
PathFragment pkgFragment =
parts.pkgIsAbsolute
? PathFragment.create(parts.pkg)
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java b/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java
index 00612b9..75afd2f 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java
@@ -14,26 +14,12 @@
package com.google.devtools.build.lib.cmdline;
-import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.util.StringUtilities;
import com.google.errorprone.annotations.FormatMethod;
import javax.annotation.Nullable;
/** Utilities to help parse labels. */
final class LabelParser {
- /**
- * Package names that aren't made relative to the current repository because they mean special
- * things to Bazel.
- */
- private static final ImmutableSet<String> ABSOLUTE_PACKAGE_NAMES =
- ImmutableSet.of(
- // Used for select's `//conditions:default` label (not a target)
- "conditions",
- // Used for the public and private visibility labels (not targets)
- "visibility",
- // There is only one //external package
- LabelConstants.EXTERNAL_PACKAGE_NAME.getPathString());
-
private LabelParser() {}
/**
@@ -67,12 +53,9 @@
private static Parts validateAndCreate(
@Nullable String repo, boolean pkgIsAbsolute, String pkg, String target, String raw)
throws LabelSyntaxException {
- return new Parts(
- validateAndProcessRepoName(repo, pkg),
- pkgIsAbsolute,
- validateAndProcessPackageName(pkg, target),
- validateAndProcessTargetName(pkg, target),
- raw);
+ validateRepoName(repo);
+ validatePackageName(pkg, target);
+ return new Parts(repo, pkgIsAbsolute, pkg, validateAndProcessTargetName(pkg, target), raw);
}
/**
@@ -135,30 +118,22 @@
}
@Nullable
- private static String validateAndProcessRepoName(@Nullable String repo, String pkg)
- throws LabelSyntaxException {
- if (repo == null && ABSOLUTE_PACKAGE_NAMES.contains(pkg)) {
- // These package names when used without a "@" part are always absolutely in the main repo.
- return "";
- }
+ private static void validateRepoName(@Nullable String repo) throws LabelSyntaxException {
if (repo == null) {
- return null;
+ return;
}
String error = RepositoryName.validate('@' + repo);
if (error != null) {
throw syntaxErrorf("invalid repository name '@%s': %s", repo, error);
}
- return repo;
}
- private static String validateAndProcessPackageName(String pkg, String target)
- throws LabelSyntaxException {
+ private static void validatePackageName(String pkg, String target) throws LabelSyntaxException {
String pkgError = LabelValidator.validatePackageName(pkg);
if (pkgError != null) {
throw syntaxErrorf(
"invalid package name '%s': %s%s", pkg, pkgError, perhapsYouMeantMessage(pkg, target));
}
- return pkg;
}
void checkPkgIsAbsolute() throws LabelSyntaxException {
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
index 1fe0dc0..f4b89c3 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
@@ -198,7 +198,9 @@
PackageIdentifier packageId = PackageIdentifier.create("@repo", PathFragment.create("foo"));
Label base = Label.create(packageId, "bar");
- Label relative = base.getRelativeWithRemapping("//conditions:default", ImmutableMap.of());
+ Label relative =
+ base.getRelativeWithRemapping(
+ "//conditions:default", RepositoryMapping.create(ImmutableMap.of(), "repo"));
PackageIdentifier expected = PackageIdentifier.createInMainRepo("conditions");
assertThat(relative.getRepository()).isEqualTo(expected.getRepository());
