[6.0.0]Decanonicalize labels emitted by {a,c,}query if possible (baze…

…lbuild#16638) Uses the newly added `PackageIdentifier#getDisplayForm` to turn labels in the output of query, aquery, and cquery into the most concise representation that allows them to be resolved from the context of the main repository. Closes bazelbuild#16483. PiperOrigin-RevId: 485584469 Change-Id: I9037bf128713af75b6741eca42b25e3beeb112f7 Co-authored-by: kshyanashree <[email protected]>
krisukox · Nov 2, 2022 · 47b1cad · 47b1cad
1 parent fa9b1f3
commit 47b1cad
Show file tree

Hide file tree

Showing 34 changed files with 280 additions and 75 deletions.
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
@@ -419,9 +419,19 @@ public String getCanonicalForm() {
    * Label.parse*(x.getUnambiguousCanonicalForm(), ...).equals(x)}).
    */
   public String getUnambiguousCanonicalForm() {
-    return String.format(
-        "@@%s//%s:%s",
-        packageIdentifier.getRepository().getName(), packageIdentifier.getPackageFragment(), name);
+    return packageIdentifier.getUnambiguousCanonicalForm() + ":" + name;
+  }
+
+  /**
+   * Returns a label string that is suitable for display, i.e., it resolves to this label when
+   * parsed in the context of the main repository and has a repository part that is as simple as
+   * possible.
+   *
+   * @param mainRepositoryMapping the {@link RepositoryMapping} of the main repository
+   * @return analogous to {@link PackageIdentifier#getDisplayForm(RepositoryMapping)}
+   */
+  public String getDisplayForm(RepositoryMapping mainRepositoryMapping) {
+    return packageIdentifier.getDisplayForm(mainRepositoryMapping) + ":" + name;
   }
 
   /** Return the name of the repository label refers to without the leading `at` symbol. */

diff --git a/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java
@@ -28,6 +28,7 @@
 import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
@@ -251,6 +252,11 @@ protected TargetPattern getPattern(String pattern) throws TargetParsingException
     return mainRepoTargetParser.parse(pattern);
   }
 
+  @Override
+  public RepositoryMapping getMainRepoMapping() {
+    return mainRepoTargetParser.getRepoMapping();
+  }
+
   public ThreadSafeMutableSet<T> getFwdDeps(Iterable<T> targets) throws InterruptedException {
     Map<SkyKey, T> targetsByKey = Maps.newHashMapWithExpectedSize(Iterables.size(targets));
     for (T target : targets) {

diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -42,6 +42,7 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.cmdline.ParallelVisitor.VisitTaskStatusCallback;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.cmdline.TargetPattern;
@@ -958,6 +959,12 @@ public TargetAccessor<Target> getAccessor() {
     return accessor;
   }
 
+  @Override
+  @ThreadSafe
+  public RepositoryMapping getMainRepoMapping() {
+    return mainRepoTargetParser.getRepoMapping();
+  }
+
   @ThreadSafe
   private Package getPackage(PackageIdentifier packageIdentifier)
       throws InterruptedException, QueryException, NoSuchPackageException {

diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java
@@ -170,7 +170,13 @@ public ConfiguredTargetValueAccessor getAccessor() {
             StreamedOutputHandler.OutputType.JSON,
             actionFilters),
         new ActionGraphTextOutputFormatterCallback(
-            eventHandler, aqueryOptions, out, skyframeExecutor, accessor, actionFilters),
+            eventHandler,
+            aqueryOptions,
+            out,
+            skyframeExecutor,
+            accessor,
+            actionFilters,
+            getMainRepoMapping()),
         new ActionGraphSummaryOutputFormatterCallback(
             eventHandler, aqueryOptions, out, skyframeExecutor, accessor, actionFilters));
   }

diff --git a/...a/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java b/...a/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java
@@ -38,6 +38,7 @@
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
@@ -62,6 +63,7 @@ class ActionGraphTextOutputFormatterCallback extends AqueryThreadsafeCallback {
 
   private final ActionKeyContext actionKeyContext = new ActionKeyContext();
   private final AqueryActionFilter actionFilters;
+  private final RepositoryMapping mainRepoMapping;
   private Map<String, String> paramFileNameToContentMap;
 
   ActionGraphTextOutputFormatterCallback(
@@ -70,9 +72,11 @@ class ActionGraphTextOutputFormatterCallback extends AqueryThreadsafeCallback {
       OutputStream out,
       SkyframeExecutor skyframeExecutor,
       TargetAccessor<KeyedConfiguredTargetValue> accessor,
-      AqueryActionFilter actionFilters) {
+      AqueryActionFilter actionFilters,
+      RepositoryMapping mainRepoMapping) {
     super(eventHandler, options, out, skyframeExecutor, accessor);
     this.actionFilters = actionFilters;
+    this.mainRepoMapping = mainRepoMapping;
   }
 
   @Override
@@ -145,15 +149,15 @@ private void writeAction(ActionAnalysisMetadata action, PrintStream printStream)
 
       stringBuilder
           .append("  Target: ")
-          .append(actionOwner.getLabel())
+          .append(actionOwner.getLabel().getDisplayForm(mainRepoMapping))
           .append('\n')
           .append("  Configuration: ")
           .append(configProto.getMnemonic())
           .append('\n');
       if (actionOwner.getExecutionPlatform() != null) {
         stringBuilder
             .append("  Execution platform: ")
-            .append(actionOwner.getExecutionPlatform().label().toString())
+            .append(actionOwner.getExecutionPlatform().label().getDisplayForm(mainRepoMapping))
             .append("\n");
       }
 

diff --git a/...in/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java b/...in/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
@@ -205,17 +205,30 @@ private static ImmutableMap<String, BuildConfigurationValue> getTransitiveConfig
         cqueryOptions.aspectDeps.createResolver(packageManager, eventHandler);
     return ImmutableList.of(
         new LabelAndConfigurationOutputFormatterCallback(
-            eventHandler, cqueryOptions, out, skyframeExecutor, accessor, true),
+            eventHandler,
+            cqueryOptions,
+            out,
+            skyframeExecutor,
+            accessor,
+            true,
+            getMainRepoMapping()),
         new LabelAndConfigurationOutputFormatterCallback(
-            eventHandler, cqueryOptions, out, skyframeExecutor, accessor, false),
+            eventHandler,
+            cqueryOptions,
+            out,
+            skyframeExecutor,
+            accessor,
+            false,
+            getMainRepoMapping()),
         new TransitionsOutputFormatterCallback(
             eventHandler,
             cqueryOptions,
             out,
             skyframeExecutor,
             accessor,
             hostConfiguration,
-            trimmingTransitionFactory),
+            trimmingTransitionFactory,
+            getMainRepoMapping()),
         new ProtoOutputFormatterCallback(
             eventHandler,
             cqueryOptions,
@@ -251,7 +264,8 @@ private static ImmutableMap<String, BuildConfigurationValue> getTransitiveConfig
             out,
             skyframeExecutor,
             accessor,
-            kct -> getFwdDeps(ImmutableList.of(kct))),
+            kct -> getFwdDeps(ImmutableList.of(kct)),
+            getMainRepoMapping()),
         new StarlarkOutputFormatterCallback(
             eventHandler, cqueryOptions, out, skyframeExecutor, accessor),
         new FilesOutputFormatterCallback(

diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallback.java
@@ -16,6 +16,7 @@
 
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.graph.Digraph;
 import com.google.devtools.build.lib.graph.Node;
@@ -65,12 +66,15 @@ Iterable<KeyedConfiguredTarget> getDirectDeps(KeyedConfiguredTarget target)
             };
 
         @Override
-        public String getLabel(Node<KeyedConfiguredTarget> node) {
+        public String getLabel(
+            Node<KeyedConfiguredTarget> node, RepositoryMapping mainRepositoryMapping) {
           // Node payloads are ConfiguredTargets. Output node labels are target labels + config
           // hashes.
           KeyedConfiguredTarget kct = node.getLabel();
           return String.format(
-              "%s (%s)", kct.getLabel(), shortId(getConfiguration(kct.getConfigurationKey())));
+              "%s (%s)",
+              kct.getLabel().getDisplayForm(mainRepositoryMapping),
+              shortId(getConfiguration(kct.getConfigurationKey())));
         }
 
         @Override
@@ -79,15 +83,19 @@ public Comparator<KeyedConfiguredTarget> comparator() {
         }
       };
 
+  private final RepositoryMapping mainRepoMapping;
+
   GraphOutputFormatterCallback(
       ExtendedEventHandler eventHandler,
       CqueryOptions options,
       OutputStream out,
       SkyframeExecutor skyframeExecutor,
       TargetAccessor<KeyedConfiguredTarget> accessor,
-      DepsRetriever depsRetriever) {
+      DepsRetriever depsRetriever,
+      RepositoryMapping mainRepoMapping) {
     super(eventHandler, options, out, skyframeExecutor, accessor, /*uniquifyResults=*/ false);
     this.depsRetriever = depsRetriever;
+    this.mainRepoMapping = mainRepoMapping;
   }
 
   @Override
@@ -117,7 +125,8 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
             // select() conditions don't matter for cquery because cquery operates post-analysis
             // phase, when select()s have been resolved and removed from the graph.
             /*maxConditionalEdges=*/ 0,
-            options.graphFactored);
+            options.graphFactored,
+            mainRepoMapping);
     graphWriter.write(graph, /*conditionalEdges=*/ null, outputStream);
   }
 }
diff --git a/...google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java b/...google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java
@@ -18,6 +18,7 @@
 import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
 import com.google.devtools.build.lib.analysis.config.CoreOptions.IncludeConfigFragmentsEnum;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
@@ -28,16 +29,19 @@
 /** Default Output callback for cquery. Prints a label and configuration pair per result. */
 public class LabelAndConfigurationOutputFormatterCallback extends CqueryThreadsafeCallback {
   private final boolean showKind;
+  private final RepositoryMapping mainRepoMapping;
 
   LabelAndConfigurationOutputFormatterCallback(
       ExtendedEventHandler eventHandler,
       CqueryOptions options,
       OutputStream out,
       SkyframeExecutor skyframeExecutor,
       TargetAccessor<KeyedConfiguredTarget> accessor,
-      boolean showKind) {
+      boolean showKind,
+      RepositoryMapping mainRepoMapping) {
     super(eventHandler, options, out, skyframeExecutor, accessor, /*uniquifyResults=*/ false);
     this.showKind = showKind;
+    this.mainRepoMapping = mainRepoMapping;
   }
 
   @Override
@@ -55,7 +59,7 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult) {
       }
       output =
           output
-              .append(keyedConfiguredTarget.getLabel())
+              .append(keyedConfiguredTarget.getLabel().getDisplayForm(mainRepoMapping))
               .append(" (")
               .append(shortId(getConfiguration(keyedConfiguredTarget.getConfigurationKey())))
               .append(")");

diff --git a/.../java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java b/.../java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.RuleTransitionData;
@@ -48,6 +49,7 @@ class TransitionsOutputFormatterCallback extends CqueryThreadsafeCallback {
 
   private final HashMap<Label, Target> partialResultMap;
   @Nullable private final TransitionFactory<RuleTransitionData> trimmingTransitionFactory;
+  private final RepositoryMapping mainRepoMapping;
 
   @Override
   public String getName() {
@@ -65,11 +67,13 @@ public String getName() {
       SkyframeExecutor skyframeExecutor,
       TargetAccessor<KeyedConfiguredTarget> accessor,
       BuildConfigurationValue hostConfiguration,
-      @Nullable TransitionFactory<RuleTransitionData> trimmingTransitionFactory) {
+      @Nullable TransitionFactory<RuleTransitionData> trimmingTransitionFactory,
+      RepositoryMapping mainRepoMapping) {
     super(eventHandler, options, out, skyframeExecutor, accessor, /*uniquifyResults=*/ false);
     this.hostConfiguration = hostConfiguration;
     this.trimmingTransitionFactory = trimmingTransitionFactory;
     this.partialResultMap = Maps.newHashMap();
+    this.mainRepoMapping = mainRepoMapping;
   }
 
   @Override
@@ -92,7 +96,11 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
           getRuleClassTransition(keyedConfiguredTarget.getConfiguredTarget(), target)
               + String.format(
                   "%s (%s)",
-                  keyedConfiguredTarget.getConfiguredTarget().getOriginalLabel(), shortId(config)));
+                  keyedConfiguredTarget
+                      .getConfiguredTarget()
+                      .getOriginalLabel()
+                      .getDisplayForm(mainRepoMapping),
+                  shortId(config)));
       KnownTargetsDependencyResolver knownTargetsDependencyResolver =
           new KnownTargetsDependencyResolver(partialResultMap);
       ImmutableSet<ResolvedTransition> dependencies;
@@ -117,7 +125,7 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
             "  "
                 .concat(dep.attributeName())
                 .concat("#")
-                .concat(dep.label().toString())
+                .concat(dep.label().getDisplayForm(mainRepoMapping))
                 .concat("#")
                 .concat(dep.transitionName())
                 .concat(" -> ")

diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
@@ -19,6 +19,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.util.DetailedExitCode;
@@ -536,6 +537,13 @@ ThreadSafeMutableSet<T> getBuildFiles(
    */
   TargetAccessor<T> getAccessor();
 
+  /**
+   * Returns the {@link RepositoryMapping} of the main repository so that output formatters can
+   * resolve canonical repository names in labels back to the more readable local names used by the
+   * main repository.
+   */
+  RepositoryMapping getMainRepoMapping();
+
   /**
    * Whether the given setting is enabled. The code should default to return {@code false} for all
    * unknown settings. The enum is used rather than a method for each setting so that adding more

diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java
@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.cmdline.TargetPattern.Parser;
@@ -500,6 +501,11 @@ public TargetAccessor<Target> getAccessor() {
     return accessor;
   }
 
+  @Override
+  public RepositoryMapping getMainRepoMapping() {
+    return mainRepoTargetParser.getRepoMapping();
+  }
+
   /** Given a set of target nodes, returns the targets. */
   private ThreadSafeMutableSet<Target> getTargetsFromNodes(Iterable<Node<Target>> input) {
     ThreadSafeMutableSet<Target> result = createThreadSafeMutableSet();

diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
@@ -17,6 +17,7 @@
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.cmdline.TargetPattern.Parser;
@@ -507,4 +508,9 @@ protected void preloadOrThrow(QueryExpression caller, Collection<String> pattern
   public TargetAccessor<Target> getAccessor() {
     return accessor;
   }
+
+  @Override
+  public RepositoryMapping getMainRepoMapping() {
+    return mainRepoTargetParser.getRepoMapping();
+  }
 }