Skip to content

Commit

Permalink
Change ConfiguredTargetQuery to use KeyedConfiguredTarget as a value.
Browse files Browse the repository at this point in the history 
This will allow us to keep the configured target key between lookup rounds.

Fixes #11993.

Closes #12547.

PiperOrigin-RevId: 344299744
  • Loading branch information
@katre @philwo
katre authored and philwo committed Mar 15, 2021
1 parent 46963d2 commit ed01f42
Show file tree
Hide file tree
Showing 23 changed files with 326 additions and 218 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/query2/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,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",
Expand Down
22 changes: 9 additions & 13 deletions src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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(
Expand All @@ -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)));
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/com/google/devtools/build/lib/query2/cquery/ConfigFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand All @@ -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));
}
}
76 changes: 38 additions & 38 deletions src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)) {
Expand All @@ -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) {
Expand All @@ -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();
}
Expand Down
Loading

0 comments on commit ed01f42

Please sign in to comment.