Make Label#toString return @@-prefixed labels

This change causes all labels in error messages, log output, Build Event Protocol, etc. to be prefixed with double-at (`@@`) if they are from non-main repos. This prevents cases where messages contain stuff like `the target @abc~1.0//:def is wrong` but the user can't do `bazel query @abc~1.0//:def` at all, since the actual target is `@@abc~1.0//:def`.

This required more test changes than expected as there have been many places in the codebase where we compare labels using some sort of string form (for example, toolchain types as automatic exec groups).

Fixes bazelbuild#18543.

RELNOTES[INC]: All labels in Bazel error messages, log output, Build Event Protocol, etc. are now prefixed with double-at (`@@`) instead of single-at (`@`) where applicable, to properly denote that they contain canonical repo names.

PiperOrigin-RevId: 581287175
Change-Id: I1261ab14067bcf5d44cd140e5528b5da5916dc87

src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java

@@ -348,7 +348,7 @@ private void propagateTransitiveValidationOutputGroups() {
       Label rdeLabel =
           ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironmentLabel();
       // only allow native and builtins to override transitive validation propagation
-      if (rdeLabel != null && !"@_builtins".equals(rdeLabel.getRepository().getNameWithAt())) {
+      if (rdeLabel != null && !rdeLabel.getRepository().getName().equals("_builtins")) {
         ruleContext.ruleError(rdeLabel + " cannot access the _transitive_validation private API");
         return;
       }

src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java

@@ -120,9 +120,10 @@ public final PackageContext getPackageContext() {
     return packageContext;
   }
 
-  /** Which .bzl file defines this transition? */
-  public String parentLabel() {
-    return parentLabel.getCanonicalForm();
+  /** Is this transition the same one specified by --experimental_exec_config? */
+  public boolean matchesExecConfigFlag(String starlarkExecConfig) {
+    return starlarkExecConfig.contains(parentLabel.getPackageName())
+        && starlarkExecConfig.contains(parentLabel.getName());
   }
 
   /**

src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java

@@ -144,10 +144,8 @@ private static BuildOptions maybeGetExecDefaults(
       BuildOptions fromOptions, StarlarkDefinedConfigTransition starlarkTransition) {
     if (starlarkTransition == null
         || fromOptions.get(CoreOptions.class).starlarkExecConfig == null
-        || !fromOptions
-            .get(CoreOptions.class)
-            .starlarkExecConfig
-            .startsWith(starlarkTransition.parentLabel())) {
+        || !starlarkTransition.matchesExecConfigFlag(
+            fromOptions.get(CoreOptions.class).starlarkExecConfig)) {
       // Not an exec transition: the baseline options are just the input options.
       return fromOptions;
     }
@@ -471,7 +469,7 @@ private static BuildOptions applyTransition(
     boolean isExecTransition =
         coreOptions.starlarkExecConfig != null
             && starlarkTransition != null
-            && coreOptions.starlarkExecConfig.startsWith(starlarkTransition.parentLabel());
+            && starlarkTransition.matchesExecConfigFlag(coreOptions.starlarkExecConfig);
 
     if (!isExecTransition
         && coreOptions.outputDirectoryNamingScheme.equals(

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -521,7 +521,7 @@ public static StarlarkRuleFunction createRule(
     // Verify the child against parent's allowlist
     if (parent != null
         && parent.getExtendableAllowlist() != null
-        && !bzlFile.getRepository().getNameWithAt().equals("@_builtins")) {
+        && !bzlFile.getRepository().getName().equals("_builtins")) {
       builder.addAllowlistChecker(EXTEND_RULE_ALLOWLIST_CHECKER);
       Attribute.Builder<Label> allowlistAttr =
           attr("$allowlist_extend_rule", LABEL)
@@ -705,7 +705,7 @@ public static StarlarkRuleFunction createRule(
       hasFunctionTransitionAllowlist = true;
     }
     if (hasStarlarkDefinedTransition) {
-      if (!bzlFile.getRepository().getNameWithAt().equals("@_builtins")) {
+      if (!bzlFile.getRepository().getName().equals("_builtins")) {
         if (!hasFunctionTransitionAllowlist) {
           // add the allowlist automatically
           builder.add(

src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java

@@ -71,7 +71,8 @@ private BazelCppSemantics(Language language) {
     this.language = language;
   }
 
-  private static final String CPP_TOOLCHAIN_TYPE = "@bazel_tools//tools/cpp:toolchain_type";
+  private static final String CPP_TOOLCHAIN_TYPE =
+      Label.parseCanonicalUnchecked("@bazel_tools//tools/cpp:toolchain_type").toString();
 
   @Override
   public String getCppToolchainType() {

src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java

@@ -79,10 +79,10 @@ public class BazelJavaSemantics implements JavaSemantics {
   private static final String BAZEL_TEST_RUNNER_MAIN_CLASS =
       "com.google.testing.junit.runner.BazelTestRunner";
 
-  private BazelJavaSemantics() {
-  }
+  private BazelJavaSemantics() {}
 
-  private static final String JAVA_TOOLCHAIN_TYPE = "@bazel_tools//tools/jdk:toolchain_type";
+  private static final String JAVA_TOOLCHAIN_TYPE =
+      Label.parseCanonicalUnchecked("@bazel_tools//tools/jdk:toolchain_type").toString();
   private static final Label JAVA_RUNITME_TOOLCHAIN_TYPE =
       Label.parseCanonicalUnchecked("@bazel_tools//tools/jdk:runtime_toolchain_type");
 

src/main/java/com/google/devtools/build/lib/cmdline/Label.java

@@ -412,8 +412,6 @@ public String getDisplayForm(RepositoryMapping mainRepositoryMapping) {
    *
    * <p>Unlike {@link #getDisplayForm}, this method elides the name part of the label if possible.
    *
-   * <p>Unlike {@link #toShorthandString}, this method respects {@link RepositoryMapping}.
-   *
    * @param mainRepositoryMapping the {@link RepositoryMapping} of the main repository
    */
   public String getShorthandDisplayForm(RepositoryMapping mainRepositoryMapping) {
@@ -443,27 +441,6 @@ public String getWorkspaceName() throws EvalException {
     return packageIdentifier.getRepository().getName();
   }
 
-  /**
-   * Renders this label in shorthand form.
-   *
-   * <p>Labels with canonical form {@code //foo/bar:bar} have the shorthand form {@code //foo/bar}.
-   * All other labels have identical shorthand and canonical forms.
-   *
-   * <p>Unlike {@link #getShorthandDisplayForm}, this method does not respect repository mapping.
-   */
-  public String toShorthandString() {
-    if (!getPackageFragment().getBaseName().equals(name)) {
-      return toString();
-    }
-    String repository;
-    if (packageIdentifier.getRepository().isMain()) {
-      repository = "";
-    } else {
-      repository = packageIdentifier.getRepository().getNameWithAt();
-    }
-    return repository + "//" + getPackageFragment();
-  }
-
   /**
    * Returns a label in the same package as this label with the given target name.
    *

src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java

@@ -195,7 +195,7 @@ public String getCanonicalForm() {
    * package.
    */
   public String getUnambiguousCanonicalForm() {
-    return String.format("@%s//%s", repository.getNameWithAt(), pkgName);
+    return repository.getNameWithAt() + "//" + pkgName;
   }
 
   /**
@@ -216,7 +216,7 @@ public String getUnambiguousCanonicalForm() {
    *           from the main module
    */
   public String getDisplayForm(RepositoryMapping mainRepositoryMapping) {
-    return String.format("%s//%s", repository.getDisplayForm(mainRepositoryMapping), pkgName);
+    return repository.getDisplayForm(mainRepositoryMapping) + "//" + pkgName;
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java

@@ -153,12 +153,12 @@ static void validate(String name) throws LabelSyntaxException {
     // Some special cases for more user-friendly error messages.
     if (name.equals(".") || name.equals("..")) {
       throw LabelParser.syntaxErrorf(
-          "invalid repository name '@%s': repo names are not allowed to be '@%s'", name, name);
+          "invalid repository name '%s': repo names are not allowed to be '%s'", name, name);
     }
 
     if (!VALID_REPO_NAME.matcher(name).matches()) {
       throw LabelParser.syntaxErrorf(
-          "invalid repository name '@%s': repo names may contain only A-Z, a-z, 0-9, '-', '_', '.'"
+          "invalid repository name '%s': repo names may contain only A-Z, a-z, 0-9, '-', '_', '.'"
               + " and '~' and must not start with '~'",
           StringUtilities.sanitizeControlChars(name));
     }
@@ -203,7 +203,7 @@ public String getOwnerRepoDisplayString() {
     if (ownerRepoIfNotVisible.isMain()) {
       return "main repository";
     } else {
-      return String.format("repository '%s'", ownerRepoIfNotVisible.getNameWithAt());
+      return String.format("repository '%s'", ownerRepoIfNotVisible);
     }
   }
 
@@ -212,20 +212,23 @@ public boolean isMain() {
     return equals(MAIN);
   }
 
-  /** Returns the repository name, with leading "{@literal @}". */
+  /**
+   * Returns the repository name, with two leading "{@literal @}"s, indicating that this is a
+   * canonical repo name.
+   */
+  // TODO(bazel-team): Rename to "getCanonicalForm".
   public String getNameWithAt() {
     if (!isVisible()) {
-      return String.format(
-          "@[unknown repo '%s' requested from %s]", name, ownerRepoIfNotVisible.getNameWithAt());
+      return String.format("@@[unknown repo '%s' requested from %s]", name, ownerRepoIfNotVisible);
     }
-    return '@' + name;
+    return "@@" + name;
   }
 
   /**
-   * Returns the repository name with leading "{@literal @}" except for the main repo, which is just
-   * the empty string.
+   * Returns the repository name with leading "{@literal @}"s except for the main repo, which is
+   * just the empty string.
    */
-  // TODO(bazel-team): Consider renaming to "getDefaultForm".
+  // TODO(bazel-team): Rename to "getDefaultForm".
   public String getCanonicalForm() {
     return isMain() ? "" : getNameWithAt();
   }
@@ -252,7 +255,7 @@ public String getDisplayForm(RepositoryMapping mainRepositoryMapping) {
     Preconditions.checkArgument(
         mainRepositoryMapping.ownerRepo() == null || mainRepositoryMapping.ownerRepo().isMain());
     if (!isVisible()) {
-      return '@' + getNameWithAt();
+      return getNameWithAt();
     }
     if (isMain()) {
       // Packages in the main repository can always use repo-relative form.
@@ -262,14 +265,14 @@ public String getDisplayForm(RepositoryMapping mainRepositoryMapping) {
       // If the main repository mapping is not using strict visibility, then Bzlmod is certainly
       // disabled, which means that canonical and apparent names can be used interchangeably from
       // the context of the main repository.
-      return getNameWithAt();
+      return '@' + getName();
     }
     // If possible, represent the repository with a non-canonical label using the apparent name the
     // main repository has for it, otherwise fall back to a canonical label.
     return mainRepositoryMapping
         .getInverse(this)
         .map(apparentName -> "@" + apparentName)
-        .orElse("@" + getNameWithAt());
+        .orElse(getNameWithAt());
   }
 
   /**
@@ -300,7 +303,7 @@ public PathFragment getRunfilesPath() {
         : PathFragment.create("..").getRelative(getName());
   }
 
-  /** Returns the repository name, with leading "{@literal @}". */
+  /** Same as {@link #getNameWithAt}. */
   @Override
   public String toString() {
     return getNameWithAt();

src/main/java/com/google/devtools/build/lib/packages/BuiltinRestriction.java

@@ -40,7 +40,7 @@ private BuiltinRestriction() {}
    */
   public static void failIfCalledOutsideBuiltins(StarlarkThread thread) throws EvalException {
     Label currentFile = BazelModuleContext.ofInnermostBzlOrThrow(thread).label();
-    if (!currentFile.getRepository().getNameWithAt().equals("@_builtins")) {
+    if (!currentFile.getRepository().getName().equals("_builtins")) {
       throw Starlark.errorf(
           "file '%s' cannot use private @_builtins API", currentFile.getCanonicalForm());
     }
@@ -103,7 +103,7 @@ public static void failIfModuleOutsideAllowlist(
   public static void failIfLabelOutsideAllowlist(
       Label label, RepositoryMapping repoMapping, Collection<AllowlistEntry> allowlist)
       throws EvalException {
-    if (label.getRepository().getNameWithAt().equals("@_builtins")) {
+    if (label.getRepository().getName().equals("_builtins")) {
       return;
     }
     if (allowlist.stream().noneMatch(e -> e.allows(label, repoMapping))) {

src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java

@@ -618,7 +618,8 @@ public String repositoryName(StarlarkThread thread) throws EvalException {
     BazelStarlarkContext.checkLoadingPhase(thread, "native.repository_name");
     PackageIdentifier packageId =
         PackageFactory.getContext(thread).getBuilder().getPackageIdentifier();
-    return packageId.getRepository().getNameWithAt();
+    // for legacy reasons, this is prefixed with a single '@'.
+    return '@' + packageId.getRepository().getName();
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/repository/RepositoryFetchProgress.java

@@ -24,7 +24,7 @@ public class RepositoryFetchProgress implements FetchProgress {
 
   /** Returns the unique identifying string for a repository fetching event. */
   public static String repositoryFetchContextString(RepositoryName repoName) {
-    return "repository " + repoName.getNameWithAt();
+    return "repository " + repoName;
   }
 
   public static RepositoryFetchProgress ongoing(RepositoryName repoName, String message) {

src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java

@@ -785,7 +785,7 @@ public static boolean isOldStarlarkApiWhiteListed(
 
     RuleClass ruleClass = rule.getRuleClassObject();
     Label label = ruleClass.getRuleDefinitionEnvironmentLabel();
-    if (label.getRepository().getNameWithAt().equals("@_builtins")) {
+    if (label.getRepository().getName().equals("_builtins")) {
       // always permit builtins
       return true;
     }

src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginsFlagAliasRule.java

@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.RuleClass;
 import javax.annotation.Nullable;
 
@@ -37,9 +38,10 @@
  */
 public final class JavaPluginsFlagAliasRule implements RuleDefinition {
 
-  private static final ImmutableSet<String> ALLOWLISTED_LABELS =
+  private static final ImmutableSet<Label> ALLOWLISTED_LABELS =
       ImmutableSet.of(
-          "//tools/jdk:java_plugins_flag_alias", "@bazel_tools//tools/jdk:java_plugins_flag_alias");
+          Label.parseCanonicalUnchecked("//tools/jdk:java_plugins_flag_alias"),
+          Label.parseCanonicalUnchecked("@bazel_tools//tools/jdk:java_plugins_flag_alias"));
 
   @Override
   public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
@@ -72,7 +74,7 @@ public static class JavaPluginsFlagAlias implements RuleConfiguredTargetFactory
     public ConfiguredTarget create(RuleContext ruleContext)
         throws InterruptedException, RuleErrorException,
             MutableActionGraph.ActionConflictException {
-      if (!ALLOWLISTED_LABELS.contains(ruleContext.getLabel().getCanonicalForm())) {
+      if (!ALLOWLISTED_LABELS.contains(ruleContext.getLabel())) {
         ruleContext.ruleError("Rule " + ruleContext.getLabel() + " cannot use private rule");
         return null;
       }

src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java

@@ -208,7 +208,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       throw new RepositoryFunctionException(
           new IOException(
               "to fix, run\n\tbazel fetch //...\nExternal repository "
-                  + repositoryName.getNameWithAt()
+                  + repositoryName
                   + " not found and fetching repositories is disabled."),
           Transience.TRANSIENT);
     }

src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadCycleReporter.java

@@ -139,7 +139,7 @@ public boolean maybeReportCycle(
       Function<Object, String> printer =
           input -> {
             if (input instanceof RepositoryDirectoryValue.Key) {
-              return ((RepositoryDirectoryValue.Key) input).argument().getNameWithAt();
+              return ((RepositoryDirectoryValue.Key) input).argument().toString();
             } else {
               throw new UnsupportedOperationException();
             }
@@ -172,15 +172,15 @@ public boolean maybeReportCycle(
         if (repo instanceof RepositoryDirectoryValue.Key) {
           message
               .append(" - ")
-              .append(((RepositoryDirectoryValue.Key) repo).argument().getNameWithAt())
+              .append(((RepositoryDirectoryValue.Key) repo).argument())
               .append("\n");
         }
       }
       SkyKey missingRepo = Iterables.getLast(repos);
       if (missingRepo instanceof RepositoryDirectoryValue.Key) {
         message
             .append("This could either mean you have to add the '")
-            .append(((RepositoryDirectoryValue.Key) missingRepo).argument().getNameWithAt())
+            .append(((RepositoryDirectoryValue.Key) missingRepo).argument())
             .append("' repository with a statement like `http_archive` in your WORKSPACE file")
             .append(" (note that transitive dependencies are not added automatically), or move")
             .append(" an existing definition earlier in your WORKSPACE file.");

src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoCycleReporter.java

@@ -104,7 +104,7 @@ public boolean maybeReportCycle(
           rawInput -> {
             SkyKey input = (SkyKey) rawInput;
             if (input instanceof RepositoryDirectoryValue.Key) {
-              return ((RepositoryDirectoryValue.Key) input).argument().getNameWithAt();
+              return ((RepositoryDirectoryValue.Key) input).argument().toString();
             } else if (input.argument() instanceof ModuleExtensionId) {
               ModuleExtensionId id = (ModuleExtensionId) input.argument();
               return String.format(

src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java

@@ -163,7 +163,7 @@ public PathFragment getPath() {
 
     @Override
     public String toString() {
-      return "SuccessfulLocalRepositoryLookupValue(" + repositoryName.getNameWithAt() + ")";
+      return "SuccessfulLocalRepositoryLookupValue(" + repositoryName + ")";
     }
 
     @Override

src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java

@@ -100,7 +100,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     RepositoryName repoName = packageKey.getRepository();
     if (!repoName.isVisible()) {
       return new PackageLookupValue.NoRepositoryPackageLookupValue(
-          repoName.getNameWithAt(),
+          repoName,
           String.format(
               "No repository visible as '@%s' from %s",
               repoName.getName(), repoName.getOwnerRepoDisplayString()));
@@ -157,7 +157,7 @@ public static String explainNoBuildFileValue(PackageIdentifier packageKey, Envir
       return "BUILD file not found in directory '"
           + packageKey.getPackageFragment()
           + "' of external repository "
-          + packageKey.getRepository().getNameWithAt()
+          + packageKey.getRepository()
           + ". "
           + educationalMessage;
     }
@@ -390,7 +390,7 @@ private PackageLookupValue computeExternalPackageLookupValue(
     }
     if (!repositoryValue.repositoryExists()) {
       return new PackageLookupValue.NoRepositoryPackageLookupValue(
-          id.getRepository().getNameWithAt(), repositoryValue.getErrorMsg());
+          id.getRepository(), repositoryValue.getErrorMsg());
     }
 
     // Check .bazelignore file after fetching the external repository.