Fix query command suggestion in error message with repo mappings

The package label in the query command suggested when a specified target
isn't found in a package looked like `@rules_cc~1.0.0//pkg`, which
doesn't work with Bzlmod as the repo name is canonical, but the label
isn't.

This is fixed by introducing a new function `getDisplayForm` on
`PackageIdentifier` that prints the most readable label representation
of a package from the context of the main repository given a repository
mapping (ideally that of the main repository).

This function will be used in a follow-up PR to decanonicalize labels in
{a,c,}query output when possible.

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

@@ -21,6 +21,7 @@
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Objects;
+import java.util.Optional;
 import javax.annotation.concurrent.Immutable;
 
 /**
@@ -177,6 +178,50 @@ public String getCanonicalForm() {
     return repository.getCanonicalForm() + "//" + getPackageFragment();
   }
 
+  /**
+   * Returns an absolutely unambiguous canonical form for this package in label form. Parsing this
+   * string in any environment, even when subject to repository mapping, should identify the same
+   * package.
+   */
+  public String getUnambiguousCanonicalForm() {
+    return String.format("@@%s//%s", getRepository().getName(), getPackageFragment());
+  }
+
+  /**
+   * Returns a label representation for this package that is suitable for display. The returned
+   * string is as simple as possible while referencing the current package when parsed in the
+   * context of the main repository.
+   *
+   * @param mainRepositoryMapping the {@link RepositoryMapping} of the main repository
+   * @return <dl><dt><code>//some/pkg</code></dt>
+   * <dd>if this package lives in the main repository</dd>
+   * <dt><code>@protobuf//some/pkg</code></dt>
+   * <dd>if this package lives in a repository with "protobuf" as <code>name</code> of a repository
+   * in WORKSPACE or as local name of a Bzlmod dependency of the main module</dd>
+   * <dt><code>@@protobuf~3.19.2//some/pkg</code></dt>
+   * <dd>only with Bzlmod if the current package belongs to a repository that is not visible from
+   * the main module</dd>
+   */
+  public String getDisplayForm(RepositoryMapping mainRepositoryMapping) {
+    Preconditions.checkArgument(
+        mainRepositoryMapping.ownerRepo() == null || mainRepositoryMapping.ownerRepo().isMain());
+    if (repository.isMain()) {
+      // Packages in the main repository can always use repo-relative form.
+      return "//" + getPackageFragment();
+    }
+    if (mainRepositoryMapping.ownerRepo() == null) {
+      // If the main repository mapping is not using strict visibility, then Bzlmod is certainly
+      // disabled, which means that canonical and local names can be used interchangeably from the
+      // context of the main repository.
+      return repository.getNameWithAt() + "//" + getPackageFragment();
+    }
+    // If possible, represent the repository with a non-canonical label using the local name the
+    // main repository has for it, otherwise fall back to a canonical label.
+    return mainRepositoryMapping.getInverse(repository)
+        .map(localName -> "@" + localName + "//" + getPackageFragment())
+        .orElseGet(this::getUnambiguousCanonicalForm);
+  }
+
   /**
    * Returns the package path, possibly qualified with a repository name.
    *

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

@@ -19,6 +19,8 @@
 import com.google.common.collect.ImmutableMap;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
 import javax.annotation.Nullable;
 
 /**
@@ -93,4 +95,14 @@ public RepositoryName get(String preMappingName) {
       return RepositoryName.createUnvalidated(preMappingName).toNonVisible(ownerRepo());
     }
   }
+
+  /**
+   * Returns the first apparent name in this mapping that maps to the given canonical name, if any.
+   */
+  public Optional<String> getInverse(RepositoryName postMappingName) {
+    return repositoryMapping().entrySet().stream()
+        .filter(e -> e.getValue().equals(postMappingName))
+        .map(Entry::getKey)
+        .findFirst();
+  }
 }

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

@@ -248,6 +248,13 @@ public enum ConfigSettingVisibilityPolicy {
    */
   private RepositoryMapping repositoryMapping;
 
+  /**
+   * The repository mapping of the main repository. This is only used internally to obtain
+   * user-friendly apparent names from canonical repository names in error message arising from this
+   * package.
+   */
+  private RepositoryMapping mainRepositoryMapping;
+
   private Set<Label> defaultCompatibleWith = ImmutableSet.of();
   private Set<Label> defaultRestrictedTo = ImmutableSet.of();
 
@@ -478,6 +485,7 @@ private void finishInit(Builder builder) {
     this.registeredExecutionPlatforms = ImmutableList.copyOf(builder.registeredExecutionPlatforms);
     this.registeredToolchains = ImmutableList.copyOf(builder.registeredToolchains);
     this.repositoryMapping = Preconditions.checkNotNull(builder.repositoryMapping);
+    this.mainRepositoryMapping = Preconditions.checkNotNull(builder.mainRepositoryMapping);
     ImmutableMap.Builder<RepositoryName, ImmutableMap<String, RepositoryName>>
         repositoryMappingsBuilder = ImmutableMap.builder();
     if (!builder.externalPackageRepositoryMappings.isEmpty() && !builder.isRepoRulePackage()) {
@@ -731,7 +739,7 @@ private String getAlternateTargetSuggestion(String targetName) {
       String blazeQuerySuggestion =
           String.format(
               "Tip: use `query %s:*` to see all the targets in that package",
-              packageIdentifier.getCanonicalForm());
+              packageIdentifier.getDisplayForm(mainRepositoryMapping));
       return String.format(
           " (%s)", Joiner.on(" ").skipNulls().join(targetSuggestion, blazeQuerySuggestion));
     }
@@ -880,6 +888,7 @@ public static Builder newExternalPackageBuilder(
             LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
             workspaceName,
             starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
+            mainRepoMapping,
             mainRepoMapping)
         .setFilename(workspacePath);
   }
@@ -894,7 +903,11 @@ public static Builder newExternalPackageBuilderForBzlmod(
             basePackageId,
             DUMMY_WORKSPACE_NAME_FOR_BZLMOD_PACKAGES,
             starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
-            repoMapping)
+            repoMapping,
+            // This mapping is *not* the main repository's mapping, but since it is only used to
+            // construct a query command in an error message and the package built here can't be
+            // seen by query, the particular value does not matter.
+            RepositoryMapping.ALWAYS_FALLBACK)
         .setFilename(moduleFilePath);
   }
 
@@ -974,6 +987,11 @@ public boolean recordLoadedModules() {
      * workspace.
      */
     private final RepositoryMapping repositoryMapping;
+    /**
+     * The repository mapping of the main repository. This is only used to resolve user-friendly
+     * apparent names from canonical repository names in error message arising from this package.
+     */
+    private final RepositoryMapping mainRepositoryMapping;
     /** Converts label literals to Label objects within this package. */
     private final LabelConverter labelConverter;
 
@@ -1098,10 +1116,12 @@ public T intern(T sample) {
         PackageIdentifier id,
         String workspaceName,
         boolean noImplicitFileExport,
-        RepositoryMapping repositoryMapping) {
+        RepositoryMapping repositoryMapping,
+        RepositoryMapping mainRepositoryMapping) {
       this.pkg = new Package(id, workspaceName, packageSettings.succinctTargetNotFoundErrors());
       this.noImplicitFileExport = noImplicitFileExport;
       this.repositoryMapping = repositoryMapping;
+      this.mainRepositoryMapping = mainRepositoryMapping;
       this.labelConverter = new LabelConverter(id, repositoryMapping);
       if (pkg.getName().startsWith("javatests/")) {
         setDefaultTestonly(true);

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

@@ -465,13 +465,15 @@ public Package.Builder newPackageBuilder(
       PackageIdentifier packageId,
       String workspaceName,
       StarlarkSemantics starlarkSemantics,
-      RepositoryMapping repositoryMapping) {
+      RepositoryMapping repositoryMapping,
+      RepositoryMapping mainRepositoryMapping) {
     return new Package.Builder(
         packageSettings,
         packageId,
         workspaceName,
         starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
-        repositoryMapping);
+        repositoryMapping,
+        mainRepositoryMapping);
   }
 
   /** Returns a new {@link NonSkyframeGlobber}. */

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

@@ -33,6 +33,7 @@
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 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.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
@@ -1215,6 +1216,9 @@ private LoadedPackage loadPackage(
     RepositoryMappingValue repositoryMappingValue =
         (RepositoryMappingValue)
             env.getValue(RepositoryMappingValue.key(packageId.getRepository()));
+    RepositoryMappingValue mainRepositoryMappingValue = (RepositoryMappingValue) env.getValue(
+        RepositoryMappingValue.key(
+            RepositoryName.MAIN));
     RootedPath buildFileRootedPath = packageLookupValue.getRootedPath(packageId);
     FileValue buildFileValue = getBuildFileValue(env, buildFileRootedPath);
     RuleVisibility defaultVisibility = PrecomputedValue.DEFAULT_VISIBILITY.get(env);
@@ -1346,7 +1350,8 @@ private LoadedPackage loadPackage(
                   packageId,
                   workspaceName,
                   starlarkBuiltinsValue.starlarkSemantics,
-                  repositoryMapping)
+                  repositoryMapping,
+                  mainRepositoryMappingValue.getRepositoryMapping())
               .setFilename(buildFileRootedPath)
               .setDefaultVisibility(defaultVisibility)
               // "defaultVisibility" comes from the command line.

src/test/java/com/google/devtools/build/lib/cmdline/BUILD

@@ -23,6 +23,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline:query_exception_marker_interface",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
+        "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/net/starlark/java/eval",
         "//src/test/java/com/google/devtools/build/lib/testutil:TestThread",

src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java

@@ -17,6 +17,7 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Map;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -93,4 +94,30 @@ public void testRunfilesDir() throws Exception {
     assertThat(PackageIdentifier.create("", PathFragment.create("bar/baz")).getRunfilesPath())
         .isEqualTo(PathFragment.create("bar/baz"));
   }
+
+  @Test
+  public void testDisplayFormInMainRepository() throws Exception {
+    PackageIdentifier pkg = PackageIdentifier.create(RepositoryName.MAIN,
+        PathFragment.create("some/pkg"));
+
+    assertThat(pkg.getDisplayForm(RepositoryMapping.ALWAYS_FALLBACK)).isEqualTo("//some/pkg");
+    assertThat(pkg.getDisplayForm(
+        RepositoryMapping.create(Map.of("foo", RepositoryName.create("bar")),
+            RepositoryName.MAIN))).isEqualTo("//some/pkg");
+  }
+
+  @Test
+  public void testDisplayFormInExternalRepository() throws Exception {
+    RepositoryName repo = RepositoryName.create("canonical");
+    PackageIdentifier pkg = PackageIdentifier.create(repo, PathFragment.create("some/pkg"));
+
+    assertThat(pkg.getDisplayForm(RepositoryMapping.ALWAYS_FALLBACK)).isEqualTo(
+        "@canonical//some/pkg");
+    assertThat(pkg.getDisplayForm(
+        RepositoryMapping.create(Map.of("local", repo),
+            RepositoryName.MAIN))).isEqualTo("@local//some/pkg");
+    assertThat(pkg.getDisplayForm(
+        RepositoryMapping.create(Map.of("local", RepositoryName.create("other_repo")),
+            RepositoryName.MAIN))).isEqualTo("@@canonical//some/pkg");
+  }
 }

src/test/java/com/google/devtools/build/lib/packages/PackageTest.java

@@ -164,6 +164,7 @@ private Package.Builder pkgBuilder(String name) {
             PackageIdentifier.createInMainRepo(name),
             "workspace",
             /*noImplicitFileExport=*/ true,
+            RepositoryMapping.ALWAYS_FALLBACK,
             RepositoryMapping.ALWAYS_FALLBACK);
     result.setFilename(
         RootedPath.toRootedPath(

src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java

@@ -265,6 +265,7 @@ private Package.Builder createDummyPackageBuilder() {
             PackageIdentifier.createInMainRepo(TEST_PACKAGE_NAME),
             "TESTING",
             StarlarkSemantics.DEFAULT,
+            RepositoryMapping.ALWAYS_FALLBACK,
             RepositoryMapping.ALWAYS_FALLBACK)
         .setFilename(RootedPath.toRootedPath(root, testBuildfilePath));
   }

src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java

@@ -58,7 +58,8 @@ public final class RuleFactoryTest extends PackageLoadingTestCase {
   private Package.Builder newBuilder(PackageIdentifier id, Path filename) {
     return packageFactory
         .newPackageBuilder(
-            id, "TESTING", StarlarkSemantics.DEFAULT, RepositoryMapping.ALWAYS_FALLBACK)
+            id, "TESTING", StarlarkSemantics.DEFAULT, RepositoryMapping.ALWAYS_FALLBACK,
+            RepositoryMapping.ALWAYS_FALLBACK)
         .setFilename(RootedPath.toRootedPath(root, filename));
   }