Change ConfiguredTargetQuery to use KeyedConfiguredTarget as a value.

This will allow us to keep the configured target key between lookup rounds.

Fixes #11993.

Closes #12547.

PiperOrigin-RevId: 344299744

Author: katre

src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java

@@ -14,11 +14,11 @@
 package com.google.devtools.build.lib.buildtool;
 
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment.TopLevelConfigurations;
 import com.google.devtools.build.lib.query2.cquery.ConfiguredTargetQueryEnvironment;
 import com.google.devtools.build.lib.query2.cquery.CqueryOptions;
+import com.google.devtools.build.lib.query2.cquery.KeyedConfiguredTarget;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
 import com.google.devtools.build.lib.query2.engine.QueryExpression;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -27,7 +27,7 @@
 import java.util.Collection;
 
 /** A version of {@link BuildTool} that handles all cquery work. */
-public final class CqueryBuildTool extends PostAnalysisQueryBuildTool<ConfiguredTarget> {
+public final class CqueryBuildTool extends PostAnalysisQueryBuildTool<KeyedConfiguredTarget> {
 
   public CqueryBuildTool(CommandEnvironment env, QueryExpression queryExpression) {
     super(env, queryExpression);

src/main/java/com/google/devtools/build/lib/query2/BUILD

@@ -110,6 +110,7 @@ java_library(
         "//src/main/protobuf:analysis_java_proto",
         "//src/main/protobuf:build_java_proto",
         "//src/main/protobuf:failure_details_java_proto",
+        "//third_party:auto_value",
         "//third_party:flogger",
         "//third_party:guava",
         "//third_party:jsr305",

src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java

@@ -15,7 +15,6 @@
 package com.google.devtools.build.lib.query2.cquery;
 
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.Attribute;
@@ -39,7 +38,7 @@ class BuildOutputFormatterCallback extends CqueryThreadsafeCallback {
       CqueryOptions options,
       OutputStream out,
       SkyframeExecutor skyframeExecutor,
-      TargetAccessor<ConfiguredTarget> accessor) {
+      TargetAccessor<KeyedConfiguredTarget> accessor) {
     super(eventHandler, options, out, skyframeExecutor, accessor);
   }
 
@@ -69,24 +68,21 @@ public PossibleAttributeValues getPossibleValues(Rule rule, Attribute attr) {
     }
   }
 
-  private ConfiguredAttributeMapper getAttributeMap(ConfiguredTarget ct)
+  private ConfiguredAttributeMapper getAttributeMap(KeyedConfiguredTarget kct)
       throws InterruptedException {
-    Rule associatedRule = accessor.getTargetFromConfiguredTarget(ct).getAssociatedRule();
+    Rule associatedRule = accessor.getTarget(kct).getAssociatedRule();
     if (associatedRule == null) {
       return null;
-    } else if (ct instanceof OutputFileConfiguredTarget) {
+    } else if (kct.getConfiguredTarget() instanceof OutputFileConfiguredTarget) {
       return ConfiguredAttributeMapper.of(
-          associatedRule,
-          accessor
-              .getGeneratingConfiguredTarget((OutputFileConfiguredTarget) ct)
-              .getConfigConditions());
+          associatedRule, accessor.getGeneratingConfiguredTarget(kct).getConfigConditions());
     } else {
-      return ConfiguredAttributeMapper.of(associatedRule, ct.getConfigConditions());
+      return ConfiguredAttributeMapper.of(associatedRule, kct.getConfigConditions());
     }
   }
 
   @Override
-  public void processOutput(Iterable<ConfiguredTarget> partialResult)
+  public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
       throws InterruptedException, IOException {
     BuildOutputFormatter.TargetOutputter outputter =
         new TargetOutputter(
@@ -97,8 +93,8 @@ public void processOutput(Iterable<ConfiguredTarget> partialResult)
             // and which path is chosen, which people may find even more informative.
             (rule, attr) -> false,
             System.lineSeparator());
-    for (ConfiguredTarget configuredTarget : partialResult) {
-      Target target = accessor.getTargetFromConfiguredTarget(configuredTarget);
+    for (KeyedConfiguredTarget configuredTarget : partialResult) {
+      Target target = accessor.getTarget(configuredTarget);
       outputter.output(target, new CqueryAttributeReader(getAttributeMap(configuredTarget)));
     }
   }

src/main/java/com/google/devtools/build/lib/query2/cquery/ConfigFunction.java

@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.query2.cquery;
 
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.query2.common.AbstractBlazeQueryEnvironment;
 import com.google.devtools.build.lib.query2.engine.Callback;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment;
@@ -60,7 +59,7 @@ public List<ArgumentType> getArgumentTypes() {
 
   /**
    * This function is only viable with ConfiguredTargetQueryEnvironment which extends {@link
-   * AbstractBlazeQueryEnvironment <ConfiguredTarget>}
+   * AbstractBlazeQueryEnvironment <KeyedConfiguredTarget>}
    */
   @Override
   @SuppressWarnings("unchecked")
@@ -84,8 +83,8 @@ public <T> QueryTaskFuture<Void> eval(
         ((ConfiguredTargetQueryEnvironment) env)
             .getConfiguredTargetsForConfigFunction(
                 targetExpression.toString(),
-                (ThreadSafeMutableSet<ConfiguredTarget>) targets.getIfSuccessful(),
+                (ThreadSafeMutableSet<KeyedConfiguredTarget>) targets.getIfSuccessful(),
                 configuration,
-                (Callback<ConfiguredTarget>) callback));
+                (Callback<KeyedConfiguredTarget>) callback));
   }
 }

src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java

@@ -56,7 +56,7 @@
  *
  * <p>Incomplete; we'll implement getVisibility when needed.
  */
-public class ConfiguredTargetAccessor implements TargetAccessor<ConfiguredTarget> {
+public class ConfiguredTargetAccessor implements TargetAccessor<KeyedConfiguredTarget> {
 
   private final WalkableGraph walkableGraph;
   private final ConfiguredTargetQueryEnvironment queryEnvironment;
@@ -68,62 +68,58 @@ public ConfiguredTargetAccessor(
   }
 
   @Override
-  public String getTargetKind(ConfiguredTarget target) {
-    Target actualTarget = getTargetFromConfiguredTarget(target);
+  public String getTargetKind(KeyedConfiguredTarget target) {
+    Target actualTarget = getTarget(target);
     return actualTarget.getTargetKind();
   }
 
   @Override
-  public String getLabel(ConfiguredTarget target) {
+  public String getLabel(KeyedConfiguredTarget target) {
     return target.getLabel().toString();
   }
 
   @Override
-  public String getPackage(ConfiguredTarget target) {
+  public String getPackage(KeyedConfiguredTarget target) {
     return target.getLabel().getPackageIdentifier().getPackageFragment().toString();
   }
 
   @Override
-  public boolean isRule(ConfiguredTarget target) {
-    Target actualTarget = getTargetFromConfiguredTarget(target);
+  public boolean isRule(KeyedConfiguredTarget target) {
+    Target actualTarget = getTarget(target);
     return actualTarget instanceof Rule;
   }
 
   @Override
-  public boolean isTestRule(ConfiguredTarget target) {
-    Target actualTarget = getTargetFromConfiguredTarget(target);
+  public boolean isTestRule(KeyedConfiguredTarget target) {
+    Target actualTarget = getTarget(target);
     return TargetUtils.isTestRule(actualTarget);
   }
 
   @Override
-  public boolean isTestSuite(ConfiguredTarget target) {
-    Target actualTarget = getTargetFromConfiguredTarget(target);
+  public boolean isTestSuite(KeyedConfiguredTarget target) {
+    Target actualTarget = getTarget(target);
     return TargetUtils.isTestSuiteRule(actualTarget);
   }
 
   @Override
-  public List<ConfiguredTarget> getPrerequisites(
+  public List<KeyedConfiguredTarget> getPrerequisites(
       QueryExpression caller,
-      ConfiguredTarget configuredTarget,
+      KeyedConfiguredTarget keyedConfiguredTarget,
       String attrName,
       String errorMsgPrefix)
       throws QueryException, InterruptedException {
-    ConfiguredTarget actualConfiguredTarget = configuredTarget.getActual();
+    // Process aliases.
+    KeyedConfiguredTarget actual = keyedConfiguredTarget.getActual();
 
     Preconditions.checkArgument(
-        isRule(actualConfiguredTarget),
-        "%s %s is not a rule configured target",
-        errorMsgPrefix,
-        getLabel(actualConfiguredTarget));
+        isRule(actual), "%s %s is not a rule configured target", errorMsgPrefix, getLabel(actual));
 
-    Multimap<Label, ConfiguredTarget> depsByLabel =
+    Multimap<Label, KeyedConfiguredTarget> depsByLabel =
         Multimaps.index(
-            queryEnvironment.getFwdDeps(ImmutableList.of(actualConfiguredTarget)),
-            ConfiguredTarget::getLabel);
+            queryEnvironment.getFwdDeps(ImmutableList.of(actual)), kct -> kct.getLabel());
 
-    Rule rule = (Rule) getTargetFromConfiguredTarget(actualConfiguredTarget);
-    ImmutableMap<Label, ConfigMatchingProvider> configConditions =
-        actualConfiguredTarget.getConfigConditions();
+    Rule rule = (Rule) getTarget(actual);
+    ImmutableMap<Label, ConfigMatchingProvider> configConditions = actual.getConfigConditions();
     ConfiguredAttributeMapper attributeMapper =
         ConfiguredAttributeMapper.of(rule, configConditions);
     if (!attributeMapper.has(attrName)) {
@@ -134,42 +130,42 @@ public List<ConfiguredTarget> getPrerequisites(
               errorMsgPrefix, rule.getRuleClass(), attrName),
           ConfigurableQuery.Code.ATTRIBUTE_MISSING);
     }
-    ImmutableList.Builder<ConfiguredTarget> toReturn = ImmutableList.builder();
+    ImmutableList.Builder<KeyedConfiguredTarget> toReturn = ImmutableList.builder();
     attributeMapper.visitLabels(attributeMapper.getAttributeDefinition(attrName)).stream()
         .forEach(depEdge -> toReturn.addAll(depsByLabel.get(depEdge.getLabel())));
     return toReturn.build();
   }
 
   @Override
-  public List<String> getStringListAttr(ConfiguredTarget target, String attrName) {
-    Target actualTarget = getTargetFromConfiguredTarget(target);
+  public List<String> getStringListAttr(KeyedConfiguredTarget target, String attrName) {
+    Target actualTarget = getTarget(target);
     return TargetUtils.getStringListAttr(actualTarget, attrName);
   }
 
   @Override
-  public String getStringAttr(ConfiguredTarget target, String attrName) {
-    Target actualTarget = getTargetFromConfiguredTarget(target);
+  public String getStringAttr(KeyedConfiguredTarget target, String attrName) {
+    Target actualTarget = getTarget(target);
     return TargetUtils.getStringAttr(actualTarget, attrName);
   }
 
   @Override
-  public Iterable<String> getAttrAsString(ConfiguredTarget target, String attrName) {
-    Target actualTarget = getTargetFromConfiguredTarget(target);
+  public Iterable<String> getAttrAsString(KeyedConfiguredTarget target, String attrName) {
+    Target actualTarget = getTarget(target);
     return TargetUtils.getAttrAsString(actualTarget, attrName);
   }
 
   @Override
-  public ImmutableSet<QueryVisibility<ConfiguredTarget>> getVisibility(
-      QueryExpression caller, ConfiguredTarget from) throws QueryException {
+  public ImmutableSet<QueryVisibility<KeyedConfiguredTarget>> getVisibility(
+      QueryExpression caller, KeyedConfiguredTarget from) throws QueryException {
     // TODO(bazel-team): implement this if needed.
     throw new QueryException(
         "visible() is not supported on configured targets",
         ConfigurableQuery.Code.VISIBLE_FUNCTION_NOT_SUPPORTED);
   }
 
-  public Target getTargetFromConfiguredTarget(ConfiguredTarget configuredTarget) {
+  public Target getTarget(KeyedConfiguredTarget keyedConfiguredTarget) {
     // Dereference any aliases that might be present.
-    Label label = configuredTarget.getOriginalLabel();
+    Label label = keyedConfiguredTarget.getConfiguredTarget().getOriginalLabel();
     try {
       return queryEnvironment.getTarget(label);
     } catch (InterruptedException e) {
@@ -180,14 +176,18 @@ public Target getTargetFromConfiguredTarget(ConfiguredTarget configuredTarget) {
   }
 
   /** Returns the rule that generates the given output file. */
-  public RuleConfiguredTarget getGeneratingConfiguredTarget(OutputFileConfiguredTarget oct)
+  RuleConfiguredTarget getGeneratingConfiguredTarget(KeyedConfiguredTarget kct)
       throws InterruptedException {
+    Preconditions.checkArgument(kct.getConfiguredTarget() instanceof OutputFileConfiguredTarget);
     return (RuleConfiguredTarget)
         ((ConfiguredTargetValue)
                 walkableGraph.getValue(
                     ConfiguredTargetKey.builder()
-                        .setLabel(oct.getGeneratingRule().getLabel())
-                        .setConfiguration(queryEnvironment.getConfiguration(oct))
+                        .setLabel(
+                            ((OutputFileConfiguredTarget) kct.getConfiguredTarget())
+                                .getGeneratingRule()
+                                .getLabel())
+                        .setConfiguration(queryEnvironment.getConfiguration(kct))
                         .build()))
             .getConfiguredTarget();
   }