Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change ConfiguredTargetQuery to use KeyedConfiguredTarget as a value. #12547

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
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
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));
}
}
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we assert that this KeyedConfiguredTarget contains an OutputFileConfiguredTarget if we're going to cast on 186?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

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