diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
index 82ea54975275ab..527cccc8099550 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
+++ b/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,65 @@ 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 mainOrOtherRepoMapping the {@link RepositoryMapping} of the main repository or, if that
+ * is not available to the caller, the {@link RepositoryMapping} of
+ * the current package. The former will yield more readable output.
+ * @return
- if the main repository's mapping is provided:
+ *
+ * //some/pkg
+ * - if this package lives in the main repository
+ * @protobuf//some/pkg
+ * - if this package lives in a repository with "protobuf" as
name
of a repository
+ * in WORKSPACE or as local name of a Bzlmod dependency of the main module
+ * @@protobuf~3.19.2//some/pkg
+ * - only with Bzlmod if the current package belongs to a repository that is not visible from
+ * the main module
+ *
+ *
- if another repository's mapping is provided:
+ *
+ * //some/pkg
+ * - if this package lives in the main repository
+ * @protobuf//some/pkg
+ * - if this package lives in a repository with "protobuf" as
name
of a repository
+ * in WORKSPACE or as local name of a Bzlmod dependency of the main module
+ * @@protobuf~3.19.2//some/pkg
+ * - if this package lives in a repository managed by Bzlmod
+ */
+ public String getDisplayForm(RepositoryMapping mainOrOtherRepoMapping) {
+ if (repository.isMain()) {
+ // Packages in the main repository can always use repo-relative form.
+ return "//" + getPackageFragment();
+ }
+ if (mainOrOtherRepoMapping.ownerRepo() == null) {
+ // Not using strict visibility, meaning that canonical and local names can be used
+ // interchangeably.
+ 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.
+ Optional maybeLocalName = Optional.empty();
+ if (mainOrOtherRepoMapping.ownerRepo().isMain()) {
+ maybeLocalName = mainOrOtherRepoMapping.repositoryMapping()
+ .inverse().get(repository).stream().findFirst()
+ .map(localName -> "@" + localName + "//" + getPackageFragment());
+ }
+ return maybeLocalName.orElseGet(this::getUnambiguousCanonicalForm);
+ }
+
/**
* Returns the package path, possibly qualified with a repository name.
*
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java
index e504ff95f39d8c..b44fbed9639d54 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java
@@ -14,11 +14,16 @@
package com.google.devtools.build.lib.cmdline;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+
import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import java.util.HashMap;
import java.util.Map;
+import java.util.Map.Entry;
import javax.annotation.Nullable;
/**
@@ -35,7 +40,10 @@ public abstract class RepositoryMapping {
// Always fallback to the requested name
public static final RepositoryMapping ALWAYS_FALLBACK = createAllowingFallback(ImmutableMap.of());
- abstract ImmutableMap repositoryMapping();
+ // A given String maps to exactly one RepositoryName, but a RepositoryName may have multiple
+ // Strings (that is, local repository names), mapping to it. In other words, the entries would
+ // form a valid ImmutableMap, but not necessarily a valid ImmutableBiMap.
+ abstract ImmutableListMultimap repositoryMapping();
/**
* The owner repo of this repository mapping. It is for providing useful debug information when
@@ -48,14 +56,19 @@ public abstract class RepositoryMapping {
public static RepositoryMapping create(
Map repositoryMapping, RepositoryName ownerRepo) {
return new AutoValue_RepositoryMapping(
- ImmutableMap.copyOf(Preconditions.checkNotNull(repositoryMapping)),
+ ImmutableListMultimap.builder()
+ .putAll(Preconditions.checkNotNull(repositoryMapping).entrySet())
+ .build(),
Preconditions.checkNotNull(ownerRepo));
}
public static RepositoryMapping createAllowingFallback(
Map repositoryMapping) {
return new AutoValue_RepositoryMapping(
- ImmutableMap.copyOf(Preconditions.checkNotNull(repositoryMapping)), null);
+ ImmutableListMultimap.builder()
+ .putAll(Preconditions.checkNotNull(repositoryMapping).entrySet())
+ .build(),
+ null);
}
/**
@@ -64,8 +77,12 @@ public static RepositoryMapping createAllowingFallback(
*/
public RepositoryMapping withAdditionalMappings(Map additionalMappings) {
HashMap allMappings = new HashMap<>(additionalMappings);
- allMappings.putAll(repositoryMapping());
- return new AutoValue_RepositoryMapping(ImmutableMap.copyOf(allMappings), ownerRepo());
+ repositoryMapping().entries().forEach(e -> allMappings.put(e.getKey(), e.getValue()));
+ return new AutoValue_RepositoryMapping(
+ ImmutableListMultimap.builder()
+ .putAll(allMappings.entrySet())
+ .build(),
+ ownerRepo());
}
/**
@@ -73,8 +90,9 @@ public RepositoryMapping withAdditionalMappings(Map addi
* additional mappings. If there are conflicts, existing mappings will take precedence. The owner
* repo of the given additional mappings is ignored.
*/
- public RepositoryMapping withAdditionalMappings(RepositoryMapping additionalMappings) {
- return withAdditionalMappings(additionalMappings.repositoryMapping());
+ public RepositoryMapping withAdditionalMappings(RepositoryMapping baseMapping) {
+ return withAdditionalMappings(baseMapping.repositoryMapping().entries().stream()
+ .collect(toImmutableMap(Entry::getKey, Entry::getValue)));
}
/**
@@ -82,7 +100,8 @@ public RepositoryMapping withAdditionalMappings(RepositoryMapping additionalMapp
* provided apparent repo name is assumed to be valid.
*/
public RepositoryName get(String preMappingName) {
- RepositoryName canonicalRepoName = repositoryMapping().get(preMappingName);
+ RepositoryName canonicalRepoName = Iterables.getOnlyElement(
+ repositoryMapping().get(preMappingName), null);
if (canonicalRepoName != null) {
return canonicalRepoName;
}
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 a73a3d38f20d15..36e82787a0f2c4 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
@@ -731,7 +731,9 @@ private String getAlternateTargetSuggestion(String targetName) {
String blazeQuerySuggestion =
String.format(
"Tip: use `query %s:*` to see all the targets in that package",
- packageIdentifier.getCanonicalForm());
+ // TODO: Make the resulting package label more readable by passing in the main
+ // repository's mapping.
+ packageIdentifier.getDisplayForm(repositoryMapping));
return String.format(
" (%s)", Joiner.on(" ").skipNulls().join(targetSuggestion, blazeQuerySuggestion));
}
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/BUILD b/src/test/java/com/google/devtools/build/lib/cmdline/BUILD
index 64a00ce611277b..de284de794d087 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/BUILD
+++ b/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",
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
index d7813329480cca..33f514fb1bf176 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
+++ b/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,38 @@ public void testRunfilesDir() throws Exception {
assertThat(PackageIdentifier.create("", PathFragment.create("bar/baz")).getRunfilesPath())
.isEqualTo(PathFragment.create("bar/baz"));
}
+
+ @Test
+ public void testDisplayFormInMainRepository() throws Exception {
+ RepositoryName repo = RepositoryName.MAIN;
+ PathFragment pkgFragment = PathFragment.create("some/pkg");
+ PackageIdentifier pkg = PackageIdentifier.create(repo, pkgFragment);
+
+ 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");
+ assertThat(pkg.getDisplayForm(
+ RepositoryMapping.create(Map.of("foo", RepositoryName.create("bar")),
+ RepositoryName.create("not_main")))).isEqualTo("//some/pkg");
+ }
+
+ @Test
+ public void testDisplayFormInExternalRepository() throws Exception {
+ PathFragment pkgFragment = PathFragment.create("some/pkg");
+ RepositoryName repo = RepositoryName.create("canonical");
+ PackageIdentifier pkg = PackageIdentifier.create(repo, pkgFragment);
+
+ 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");
+ assertThat(pkg.getDisplayForm(
+ RepositoryMapping.create(Map.of("local", repo),
+ RepositoryName.create("not_main")))).isEqualTo("@@canonical//some/pkg");
+ }
}