From 34d98234324da83e93ba0d5ef5702880d5ac7c5c Mon Sep 17 00:00:00 2001 From: John Cater Date: Wed, 25 Nov 2020 12:37:09 -0800 Subject: [PATCH] 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 --- .../build/lib/buildtool/CqueryBuildTool.java | 4 +- .../google/devtools/build/lib/query2/BUILD | 1 + .../cquery/BuildOutputFormatterCallback.java | 22 ++--- .../lib/query2/cquery/ConfigFunction.java | 7 +- .../cquery/ConfiguredTargetAccessor.java | 76 +++++++------- .../ConfiguredTargetQueryEnvironment.java | 99 +++++++++---------- .../cquery/CqueryThreadsafeCallback.java | 5 +- .../cquery/GraphOutputFormatterCallback.java | 37 +++---- .../query2/cquery/KeyedConfiguredTarget.java | 81 +++++++++++++++ ...dConfigurationOutputFormatterCallback.java | 17 ++-- .../cquery/ProtoOutputFormatterCallback.java | 19 ++-- .../StarlarkOutputFormatterCallback.java | 11 ++- .../TransitionsOutputFormatterCallback.java | 26 ++--- .../lib/skyframe/ConfiguredTargetKey.java | 9 +- .../devtools/build/lib/query2/cquery/BUILD | 1 + .../BuildOutputFormatterCallbackTest.java | 3 +- .../cquery/ConfiguredTargetQueryHelper.java | 7 +- .../ConfiguredTargetQuerySemanticsTest.java | 63 ++++++------ .../cquery/ConfiguredTargetQueryTest.java | 16 +-- .../GraphOutputFormatterCallbackTest.java | 3 +- .../ProtoOutputFormatterCallbackTest.java | 5 +- .../TransitionsOutputFormatterTest.java | 3 +- .../shell/bazel/toolchain_transition_test.sh | 29 +++++- 23 files changed, 326 insertions(+), 218 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/query2/cquery/KeyedConfiguredTarget.java diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java index 432b625ee67701..9f9815c4c30fa9 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java +++ b/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 { +public final class CqueryBuildTool extends PostAnalysisQueryBuildTool { public CqueryBuildTool(CommandEnvironment env, QueryExpression queryExpression) { super(env, queryExpression); diff --git a/src/main/java/com/google/devtools/build/lib/query2/BUILD b/src/main/java/com/google/devtools/build/lib/query2/BUILD index da72f2930491c6..735cbfe850cf35 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/BUILD +++ b/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", diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java index dc695036dfe5ce..b03f6f141674f5 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java +++ b/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 accessor) { + TargetAccessor 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 partialResult) + public void processOutput(Iterable partialResult) throws InterruptedException, IOException { BuildOutputFormatter.TargetOutputter outputter = new TargetOutputter( @@ -97,8 +93,8 @@ public void processOutput(Iterable 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))); } } diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfigFunction.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfigFunction.java index 2272ddd023d1d2..3cc69d08418f73 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfigFunction.java +++ b/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 getArgumentTypes() { /** * This function is only viable with ConfiguredTargetQueryEnvironment which extends {@link - * AbstractBlazeQueryEnvironment } + * AbstractBlazeQueryEnvironment } */ @Override @SuppressWarnings("unchecked") @@ -84,8 +83,8 @@ public QueryTaskFuture eval( ((ConfiguredTargetQueryEnvironment) env) .getConfiguredTargetsForConfigFunction( targetExpression.toString(), - (ThreadSafeMutableSet) targets.getIfSuccessful(), + (ThreadSafeMutableSet) targets.getIfSuccessful(), configuration, - (Callback) callback)); + (Callback) callback)); } } diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java index 877d459972271c..262455e8c56ebe 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java @@ -56,7 +56,7 @@ * *

Incomplete; we'll implement getVisibility when needed. */ -public class ConfiguredTargetAccessor implements TargetAccessor { +public class ConfiguredTargetAccessor implements TargetAccessor { 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 getPrerequisites( + public List 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 depsByLabel = + Multimap depsByLabel = Multimaps.index( - queryEnvironment.getFwdDeps(ImmutableList.of(actualConfiguredTarget)), - ConfiguredTarget::getLabel); + queryEnvironment.getFwdDeps(ImmutableList.of(actual)), kct -> kct.getLabel()); - Rule rule = (Rule) getTargetFromConfiguredTarget(actualConfiguredTarget); - ImmutableMap configConditions = - actualConfiguredTarget.getConfigConditions(); + Rule rule = (Rule) getTarget(actual); + ImmutableMap configConditions = actual.getConfigConditions(); ConfiguredAttributeMapper attributeMapper = ConfiguredAttributeMapper.of(rule, configConditions); if (!attributeMapper.has(attrName)) { @@ -134,42 +130,42 @@ public List getPrerequisites( errorMsgPrefix, rule.getRuleClass(), attrName), ConfigurableQuery.Code.ATTRIBUTE_MISSING); } - ImmutableList.Builder toReturn = ImmutableList.builder(); + ImmutableList.Builder toReturn = ImmutableList.builder(); attributeMapper.visitLabels(attributeMapper.getAttributeDefinition(attrName)).stream() .forEach(depEdge -> toReturn.addAll(depsByLabel.get(depEdge.getLabel()))); return toReturn.build(); } @Override - public List getStringListAttr(ConfiguredTarget target, String attrName) { - Target actualTarget = getTargetFromConfiguredTarget(target); + public List 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 getAttrAsString(ConfiguredTarget target, String attrName) { - Target actualTarget = getTargetFromConfiguredTarget(target); + public Iterable getAttrAsString(KeyedConfiguredTarget target, String attrName) { + Target actualTarget = getTarget(target); return TargetUtils.getAttrAsString(actualTarget, attrName); } @Override - public ImmutableSet> getVisibility( - QueryExpression caller, ConfiguredTarget from) throws QueryException { + public ImmutableSet> 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(); } diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java index 595a9d10bc06fe..a179f96a067ab8 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java @@ -73,7 +73,7 @@ * comments on {@link PostAnalysisQueryEnvironment#targetifyValues} and b/163052263 for details. */ public class ConfiguredTargetQueryEnvironment - extends PostAnalysisQueryEnvironment { + extends PostAnalysisQueryEnvironment { /** Common query functions and cquery specific functions. */ public static final ImmutableList FUNCTIONS = populateFunctions(); /** Cquery specific functions. */ @@ -81,7 +81,8 @@ public class ConfiguredTargetQueryEnvironment private CqueryOptions cqueryOptions; - private final KeyExtractor configuredTargetKeyExtractor; + private final KeyExtractor + configuredTargetKeyExtractor; private final ConfiguredTargetAccessor accessor; @@ -103,7 +104,8 @@ public class ConfiguredTargetQueryEnvironment private final ImmutableMap transitiveConfigurations; @Override - protected KeyExtractor getConfiguredTargetKeyExtractor() { + protected KeyExtractor + getConfiguredTargetKeyExtractor() { return configuredTargetKeyExtractor; } @@ -130,12 +132,7 @@ public ConfiguredTargetQueryEnvironment( walkableGraphSupplier, settings); this.accessor = new ConfiguredTargetAccessor(walkableGraphSupplier.get(), this); - this.configuredTargetKeyExtractor = - element -> - ConfiguredTargetKey.builder() - .setConfiguredTarget(element) - .setConfigurationKey(element.getConfigurationKey()) - .build(); + this.configuredTargetKeyExtractor = KeyedConfiguredTarget::getConfiguredTargetKey; this.transitiveConfigurations = getTransitiveConfigurations(transitiveConfigurationKeys, walkableGraphSupplier.get()); } @@ -201,9 +198,9 @@ private static ImmutableMap getTransitiveConfigurati } @Override - public ImmutableList> + public ImmutableList> getDefaultOutputFormatters( - TargetAccessor accessor, + TargetAccessor accessor, ExtendedEventHandler eventHandler, OutputStream out, SkyframeExecutor skyframeExecutor, @@ -258,7 +255,7 @@ private static ImmutableMap getTransitiveConfigurati out, skyframeExecutor, accessor, - ct -> getFwdDeps(ImmutableList.of(ct))), + kct -> getFwdDeps(ImmutableList.of(kct))), new StarlarkOutputFormatterCallback( eventHandler, cqueryOptions, out, skyframeExecutor, accessor)); } @@ -275,7 +272,7 @@ public ConfiguredTargetAccessor getAccessor() { @Override public QueryTaskFuture getTargetsMatchingPattern( - QueryExpression owner, String pattern, Callback callback) { + QueryExpression owner, String pattern, Callback callback) { TargetPattern patternToEval; try { patternToEval = getPattern(pattern); @@ -302,7 +299,7 @@ public QueryTaskFuture getTargetsMatchingPattern( /* excludedSubdirectories= */ ImmutableSet.of(), (Callback) partialResult -> { - List transformedResult = new ArrayList<>(); + List transformedResult = new ArrayList<>(); for (Target target : partialResult) { transformedResult.addAll( getConfiguredTargetsForConfigFunction(target.getLabel())); @@ -323,7 +320,7 @@ public QueryTaskFuture getTargetsMatchingPattern( * null. */ @Nullable - private ConfiguredTarget getConfiguredTarget(Label label, BuildConfiguration configuration) + private KeyedConfiguredTarget getConfiguredTarget(Label label, BuildConfiguration configuration) throws InterruptedException { return getValueFromKey( ConfiguredTargetKey.builder().setLabel(label).setConfiguration(configuration).build()); @@ -331,9 +328,11 @@ private ConfiguredTarget getConfiguredTarget(Label label, BuildConfiguration con @Override @Nullable - protected ConfiguredTarget getValueFromKey(SkyKey key) throws InterruptedException { + protected KeyedConfiguredTarget getValueFromKey(SkyKey key) throws InterruptedException { ConfiguredTargetValue value = getConfiguredTargetValue(key); - return value == null ? null : value.getConfiguredTarget(); + return value == null + ? null + : KeyedConfiguredTarget.create((ConfiguredTargetKey) key, value.getConfiguredTarget()); } /** @@ -341,16 +340,16 @@ protected ConfiguredTarget getValueFromKey(SkyKey key) throws InterruptedExcepti * *

If there are no matches, returns an empty list. */ - private List getConfiguredTargetsForConfigFunction(Label label) + private List getConfiguredTargetsForConfigFunction(Label label) throws InterruptedException { - ImmutableList.Builder ans = ImmutableList.builder(); + ImmutableList.Builder ans = ImmutableList.builder(); for (BuildConfiguration config : transitiveConfigurations.values()) { - ConfiguredTarget ct = getConfiguredTarget(label, config); - if (ct != null) { - ans.add(ct); + KeyedConfiguredTarget kct = getConfiguredTarget(label, config); + if (kct != null) { + ans.add(kct); } } - ConfiguredTarget nullConfiguredTarget = getNullConfiguredTarget(label); + KeyedConfiguredTarget nullConfiguredTarget = getNullConfiguredTarget(label); if (nullConfiguredTarget != null) { ans.add(nullConfiguredTarget); } @@ -372,28 +371,28 @@ private List getConfiguredTargetsForConfigFunction(Label label */ QueryTaskCallable getConfiguredTargetsForConfigFunction( String pattern, - ThreadSafeMutableSet targets, + ThreadSafeMutableSet targets, String configPrefix, - Callback callback) { + Callback callback) { // There's no technical reason other callers beside ConfigFunction can't call this. But they'd // need to adjust the error messaging below to not make it config()-specific. Please don't just // remove that line: the counter-priority is making error messages as clear, precise, and // actionable as possible. return () -> { - List transformedResult = new ArrayList<>(); + List transformedResult = new ArrayList<>(); boolean userFriendlyConfigName = true; - for (ConfiguredTarget target : targets) { + for (KeyedConfiguredTarget target : targets) { Label label = getCorrectLabel(target); - ConfiguredTarget configuredTarget; + KeyedConfiguredTarget keyedConfiguredTarget; switch (configPrefix) { case "host": - configuredTarget = getHostConfiguredTarget(label); + keyedConfiguredTarget = getHostConfiguredTarget(label); break; case "target": - configuredTarget = getTargetConfiguredTarget(label); + keyedConfiguredTarget = getTargetConfiguredTarget(label); break; case "null": - configuredTarget = getNullConfiguredTarget(label); + keyedConfiguredTarget = getNullConfiguredTarget(label); break; default: ImmutableList matchingConfigs = @@ -401,7 +400,7 @@ QueryTaskCallable getConfiguredTargetsForConfigFunction( .filter(fullConfig -> fullConfig.startsWith(configPrefix)) .collect(ImmutableList.toImmutableList()); if (matchingConfigs.size() == 1) { - configuredTarget = + keyedConfiguredTarget = getConfiguredTarget( label, Verify.verifyNotNull(transitiveConfigurations.get(matchingConfigs.get(0)))); @@ -435,8 +434,8 @@ QueryTaskCallable getConfiguredTargetsForConfigFunction( ConfigurableQuery.Code.INCORRECT_CONFIG_ARGUMENT_ERROR); } } - if (configuredTarget != null) { - transformedResult.add(configuredTarget); + if (keyedConfiguredTarget != null) { + transformedResult.add(keyedConfiguredTarget); } } if (transformedResult.isEmpty()) { @@ -459,25 +458,26 @@ QueryTaskCallable getConfiguredTargetsForConfigFunction( * the "actual" target instead of the alias target. Grr. */ @Override - public Label getCorrectLabel(ConfiguredTarget target) { + public Label getCorrectLabel(KeyedConfiguredTarget target) { // Dereference any aliases that might be present. - return target.getOriginalLabel(); + return target.getConfiguredTarget().getOriginalLabel(); } @Nullable @Override - protected ConfiguredTarget getHostConfiguredTarget(Label label) throws InterruptedException { + protected KeyedConfiguredTarget getHostConfiguredTarget(Label label) throws InterruptedException { return getConfiguredTarget(label, hostConfiguration); } @Nullable @Override - protected ConfiguredTarget getTargetConfiguredTarget(Label label) throws InterruptedException { + protected KeyedConfiguredTarget getTargetConfiguredTarget(Label label) + throws InterruptedException { if (topLevelConfigurations.isTopLevelTarget(label)) { return getConfiguredTarget( label, topLevelConfigurations.getConfigurationForTopLevelTarget(label)); } else { - ConfiguredTarget toReturn; + KeyedConfiguredTarget toReturn; for (BuildConfiguration configuration : topLevelConfigurations.getConfigurations()) { toReturn = getConfiguredTarget(label, configuration); if (toReturn != null) { @@ -490,22 +490,22 @@ protected ConfiguredTarget getTargetConfiguredTarget(Label label) throws Interru @Nullable @Override - protected ConfiguredTarget getNullConfiguredTarget(Label label) throws InterruptedException { + protected KeyedConfiguredTarget getNullConfiguredTarget(Label label) throws InterruptedException { return getConfiguredTarget(label, null); } @Nullable @Override - protected RuleConfiguredTarget getRuleConfiguredTarget(ConfiguredTarget configuredTarget) { - if (configuredTarget instanceof RuleConfiguredTarget) { - return (RuleConfiguredTarget) configuredTarget; + protected RuleConfiguredTarget getRuleConfiguredTarget(KeyedConfiguredTarget configuredTarget) { + if (configuredTarget.getConfiguredTarget() instanceof RuleConfiguredTarget) { + return (RuleConfiguredTarget) configuredTarget.getConfiguredTarget(); } return null; } @Nullable @Override - protected BuildConfiguration getConfiguration(ConfiguredTarget target) { + protected BuildConfiguration getConfiguration(KeyedConfiguredTarget target) { try { return target.getConfigurationKey() == null ? null @@ -517,18 +517,15 @@ protected BuildConfiguration getConfiguration(ConfiguredTarget target) { } @Override - protected ConfiguredTargetKey getSkyKey(ConfiguredTarget target) { - return ConfiguredTargetKey.builder() - .setConfiguredTarget(target) - .setConfiguration(getConfiguration(target)) - .build(); + protected ConfiguredTargetKey getSkyKey(KeyedConfiguredTarget target) { + return target.getConfiguredTargetKey(); } @Override - public ThreadSafeMutableSet createThreadSafeMutableSet() { + public ThreadSafeMutableSet createThreadSafeMutableSet() { return new ThreadSafeMutableKeyExtractorBackedSetImpl<>( configuredTargetKeyExtractor, - ConfiguredTarget.class, + KeyedConfiguredTarget.class, SkyQueryEnvironment.DEFAULT_THREAD_COUNT); } } diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryThreadsafeCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryThreadsafeCallback.java index 0dd3b399edf5fb..9cb83bb7e9d88c 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryThreadsafeCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryThreadsafeCallback.java @@ -15,7 +15,6 @@ import com.google.common.annotations.VisibleForTesting; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.query2.NamedThreadSafeOutputFormatterCallback; @@ -42,7 +41,7 @@ * on completeness, should output full configuration checksums. */ public abstract class CqueryThreadsafeCallback - extends NamedThreadSafeOutputFormatterCallback { + extends NamedThreadSafeOutputFormatterCallback { protected final ExtendedEventHandler eventHandler; protected final CqueryOptions options; @@ -63,7 +62,7 @@ public abstract class CqueryThreadsafeCallback CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor) { + TargetAccessor accessor) { this.eventHandler = eventHandler; this.options = options; if (out != null) { 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 index fc453645e98255..25bdf03a986266 100644 --- 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 @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.query2.cquery; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.graph.Digraph; @@ -37,15 +36,16 @@ public String getName() { /** Interface for finding a configured target's direct dependencies. */ @FunctionalInterface public interface DepsRetriever { - Iterable getDirectDeps(ConfiguredTarget taget) throws InterruptedException; + Iterable getDirectDeps(KeyedConfiguredTarget target) + throws InterruptedException; } private final DepsRetriever depsRetriever; - private final GraphOutputWriter.NodeReader nodeReader = - new NodeReader() { + private final GraphOutputWriter.NodeReader nodeReader = + new NodeReader() { - private final Comparator configuredTargetOrdering = + private final Comparator configuredTargetOrdering = (ct1, ct2) -> { // Order graph output first by target label, then by configuration hash. Label label1 = ct1.getLabel(); @@ -56,16 +56,16 @@ public interface DepsRetriever { }; @Override - public String getLabel(Node node) { + public String getLabel(Node node) { // Node payloads are ConfiguredTargets. Output node labels are target labels + config // hashes. - ConfiguredTarget ct = node.getLabel(); + KeyedConfiguredTarget kct = node.getLabel(); return String.format( - "%s (%s)", ct.getLabel(), shortId(getConfiguration(ct.getConfigurationKey()))); + "%s (%s)", kct.getLabel(), shortId(getConfiguration(kct.getConfigurationKey()))); } @Override - public Comparator comparator() { + public Comparator comparator() { return configuredTargetOrdering; } }; @@ -75,30 +75,31 @@ public Comparator comparator() { CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, DepsRetriever depsRetriever) { super(eventHandler, options, out, skyframeExecutor, accessor); this.depsRetriever = depsRetriever; } @Override - public void processOutput(Iterable partialResult) throws InterruptedException { + public void processOutput(Iterable partialResult) + throws InterruptedException { // Transform the cquery-backed graph into a Digraph to make it suitable for GraphOutputWriter. // Note that this involves an extra iteration over the entire query result subgraph. We could // conceptually merge transformation and output writing into the same iteration if needed. - Digraph graph = new Digraph<>(); - ImmutableSet allNodes = ImmutableSet.copyOf(partialResult); - for (ConfiguredTarget configuredTarget : partialResult) { - Node node = graph.createNode(configuredTarget); - for (ConfiguredTarget dep : depsRetriever.getDirectDeps(configuredTarget)) { + Digraph graph = new Digraph<>(); + ImmutableSet allNodes = ImmutableSet.copyOf(partialResult); + for (KeyedConfiguredTarget configuredTarget : partialResult) { + Node node = graph.createNode(configuredTarget); + for (KeyedConfiguredTarget dep : depsRetriever.getDirectDeps(configuredTarget)) { if (allNodes.contains(dep)) { - Node depNode = graph.createNode(dep); + Node depNode = graph.createNode(dep); graph.addEdge(node, depNode); } } } - GraphOutputWriter graphWriter = + GraphOutputWriter graphWriter = new GraphOutputWriter<>( nodeReader, options.getLineTerminator(), diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/KeyedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/KeyedConfiguredTarget.java new file mode 100644 index 00000000000000..203a112544a516 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/KeyedConfiguredTarget.java @@ -0,0 +1,81 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.query2.cquery; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.skyframe.BuildConfigurationValue; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; +import javax.annotation.Nullable; + +/** + * A representation of a ConfiguredTargetKey and the ConfiguredTarget that it points to, for use in + * configured target queries. + */ +@AutoValue +public abstract class KeyedConfiguredTarget { + + /** Returns a new KeyedConfiguredTarget for the given data. */ + public static KeyedConfiguredTarget create( + ConfiguredTargetKey configuredTargetKey, ConfiguredTarget configuredTarget) { + return new AutoValue_KeyedConfiguredTarget(configuredTargetKey, configuredTarget); + } + + /** Returns the key for this KeyedConfiguredTarget. */ + @Nullable + public abstract ConfiguredTargetKey getConfiguredTargetKey(); + + /** Returns the ConfiguredTarget for this KeyedConfiguredTarget. */ + public abstract ConfiguredTarget getConfiguredTarget(); + + /** Returns the original (pre-alias) label for the underlying ConfiguredTarget. */ + public Label getLabel() { + return getConfiguredTarget().getOriginalLabel(); + } + + /** Returns the configuration key used for this KeyedConfiguredTarget. */ + @Nullable + public BuildConfigurationValue.Key getConfigurationKey() { + return getConfiguredTarget().getConfigurationKey(); + } + + /** Returns the configuration checksum in use for this KeyedConfiguredTarget. */ + @Nullable + public String getConfigurationChecksum() { + return getConfigurationKey() == null + ? null + : getConfigurationKey().getOptionsDiff().getChecksum(); + } + + /** + * The configuration conditions that trigger this configured target's configurable attributes. For + * targets that do not support configurable attributes, this will be an empty map. + */ + public ImmutableMap getConfigConditions() { + return getConfiguredTarget().getConfigConditions(); + } + + /** Returns a KeyedConfiguredTarget instance that resolves aliases. */ + public KeyedConfiguredTarget getActual() { + ConfiguredTarget actual = getConfiguredTarget().getActual(); + ConfiguredTargetKey actualKey = + this.getConfiguredTargetKey() == null + ? null + : this.getConfiguredTargetKey().toBuilder().setLabel(actual.getLabel()).build(); + return KeyedConfiguredTarget.create(actualKey, actual); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java index 731ea5c3df8e84..40c2d3f506bddf 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.query2.cquery; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider; import com.google.devtools.build.lib.analysis.config.CoreOptions.IncludeConfigFragmentsEnum; import com.google.devtools.build.lib.events.ExtendedEventHandler; @@ -31,7 +30,7 @@ public LabelAndConfigurationOutputFormatterCallback( CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, boolean showKind) { super(eventHandler, options, out, skyframeExecutor, accessor); this.showKind = showKind; @@ -43,23 +42,25 @@ public String getName() { } @Override - public void processOutput(Iterable partialResult) { - for (ConfiguredTarget configuredTarget : partialResult) { + public void processOutput(Iterable partialResult) { + for (KeyedConfiguredTarget keyedConfiguredTarget : partialResult) { StringBuilder output = new StringBuilder(); if (showKind) { - Target actualTarget = accessor.getTargetFromConfiguredTarget(configuredTarget); + Target actualTarget = accessor.getTarget(keyedConfiguredTarget); output = output.append(actualTarget.getTargetKind()).append(" "); } output = output - .append(configuredTarget.getOriginalLabel()) + .append(keyedConfiguredTarget.getLabel()) .append(" (") - .append(shortId(getConfiguration(configuredTarget.getConfigurationKey()))) + .append(shortId(getConfiguration(keyedConfiguredTarget.getConfigurationKey()))) .append(")"); if (options.showRequiredConfigFragments != IncludeConfigFragmentsEnum.OFF) { RequiredConfigFragmentsProvider configFragmentsProvider = - configuredTarget.getProvider(RequiredConfigFragmentsProvider.class); + keyedConfiguredTarget + .getConfiguredTarget() + .getProvider(RequiredConfigFragmentsProvider.class); String requiredFragmentsOutput = configFragmentsProvider != null ? String.join(", ", configFragmentsProvider.getRequiredConfigFragments()) diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java index c5e7c20df60d0d..733f4d782382b4 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java @@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Ordering; import com.google.devtools.build.lib.analysis.AnalysisProtos; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.ExtendedEventHandler; @@ -66,14 +65,14 @@ public String formatName() { private AnalysisProtos.CqueryResult.Builder protoResult; - private ConfiguredTarget currentTarget; + private KeyedConfiguredTarget currentTarget; ProtoOutputFormatterCallback( ExtendedEventHandler eventHandler, CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, AspectResolver resolver, OutputType outputType) { super(eventHandler, options, out, skyframeExecutor, accessor); @@ -132,10 +131,11 @@ public AnalysisProtos.CqueryResult getProtoResult() { } @Override - public void processOutput(Iterable partialResult) throws InterruptedException { + public void processOutput(Iterable partialResult) + throws InterruptedException { ConfiguredProtoOutputFormatter formatter = new ConfiguredProtoOutputFormatter(); formatter.setOptions(options, resolver, skyframeExecutor.getHashFunction()); - for (ConfiguredTarget configuredTarget : partialResult) { + for (KeyedConfiguredTarget keyedConfiguredTarget : partialResult) { AnalysisProtos.ConfiguredTarget.Builder builder = AnalysisProtos.ConfiguredTarget.newBuilder(); @@ -143,12 +143,11 @@ public void processOutput(Iterable partialResult) throws Inter // for all its work with targets, ProtoOuputFormatterCallbackTest doesn't test any of the // logic in this next line. If this were to change (i.e. we manipulate targets any further), // we will want to add relevant tests. - currentTarget = configuredTarget; - builder.setTarget( - formatter.toTargetProtoBuffer(accessor.getTargetFromConfiguredTarget(configuredTarget))); + currentTarget = keyedConfiguredTarget; + builder.setTarget(formatter.toTargetProtoBuffer(accessor.getTarget(keyedConfiguredTarget))); if (options.protoIncludeConfigurations) { - String checksum = configuredTarget.getConfigurationChecksum(); + String checksum = keyedConfiguredTarget.getConfigurationChecksum(); builder.setConfiguration( AnalysisProtos.Configuration.newBuilder().setChecksum(String.valueOf(checksum))); } @@ -166,7 +165,7 @@ protected void addAttributes( // because this method is only triggered in ProtoOutputFormatter.toTargetProtoBuffer when // the target in currentTarget is an instanceof Rule. ImmutableMap configConditions = - currentTarget.getConfigConditions(); + currentTarget.getConfiguredTarget().getConfigConditions(); ConfiguredAttributeMapper attributeMapper = ConfiguredAttributeMapper.of(rule, configConditions); for (Attribute attr : sortAttributes(rule.getAttributes())) { diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/StarlarkOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/StarlarkOutputFormatterCallback.java index c18c97593f5fda..f611bf48d1e542 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/StarlarkOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/StarlarkOutputFormatterCallback.java @@ -140,7 +140,7 @@ public Object providers(ConfiguredTarget target) { CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor) + TargetAccessor accessor) throws QueryException, InterruptedException { super(eventHandler, options, out, skyframeExecutor, accessor); @@ -226,15 +226,18 @@ public String getName() { } @Override - public void processOutput(Iterable partialResult) throws InterruptedException { - for (ConfiguredTarget target : partialResult) { + public void processOutput(Iterable partialResult) + throws InterruptedException { + for (KeyedConfiguredTarget target : partialResult) { try { StarlarkThread thread = new StarlarkThread(Mutability.create("cquery evaluation"), StarlarkSemantics.DEFAULT); thread.setMaxExecutionSteps(500_000L); // Invoke formatFn with `target` argument. - Object result = Starlark.fastcall(thread, this.formatFn, new Object[] {target}, NO_ARGS); + Object result = + Starlark.fastcall( + thread, this.formatFn, new Object[] {target.getConfiguredTarget()}, NO_ARGS); addResult(Starlark.str(result)); } catch (EvalException ex) { diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java index 2a954b8aa63769..af2c55571a7248 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java @@ -81,7 +81,7 @@ public String getName() { CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, BuildConfiguration hostConfiguration, @Nullable TransitionFactory trimmingTransitionFactory) { super(eventHandler, options, out, skyframeExecutor, accessor); @@ -91,7 +91,8 @@ public String getName() { } @Override - public void processOutput(Iterable partialResult) throws InterruptedException { + public void processOutput(Iterable partialResult) + throws InterruptedException { CqueryOptions.Transitions verbosity = options.transitions; if (verbosity.equals(CqueryOptions.Transitions.NONE)) { eventHandler.handle( @@ -100,22 +101,21 @@ public void processOutput(Iterable partialResult) throws Inter + " flag explicitly to 'lite' or 'full'")); return; } - partialResult.forEach( - ct -> - partialResultMap.put( - ct.getOriginalLabel(), accessor.getTargetFromConfiguredTarget(ct))); - for (ConfiguredTarget configuredTarget : partialResult) { - Target target = partialResultMap.get(configuredTarget.getOriginalLabel()); - BuildConfiguration config = getConfiguration(configuredTarget.getConfigurationKey()); + partialResult.forEach(kct -> partialResultMap.put(kct.getLabel(), accessor.getTarget(kct))); + for (KeyedConfiguredTarget keyedConfiguredTarget : partialResult) { + Target target = partialResultMap.get(keyedConfiguredTarget.getLabel()); + BuildConfiguration config = getConfiguration(keyedConfiguredTarget.getConfigurationKey()); addResult( - getRuleClassTransition(configuredTarget, target) - + String.format("%s (%s)", configuredTarget.getOriginalLabel(), shortId(config))); - if (!(configuredTarget instanceof RuleConfiguredTarget)) { + getRuleClassTransition(keyedConfiguredTarget.getConfiguredTarget(), target) + + String.format( + "%s (%s)", + keyedConfiguredTarget.getConfiguredTarget().getOriginalLabel(), shortId(config))); + if (!(keyedConfiguredTarget.getConfiguredTarget() instanceof RuleConfiguredTarget)) { continue; } OrderedSetMultimap deps; ImmutableMap configConditions = - configuredTarget.getConfigConditions(); + keyedConfiguredTarget.getConfigConditions(); // Get a ToolchainContext to use for dependency resolution. ToolchainCollection toolchainContexts = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java index 6cc4c230438225..2a2ab222ebf2ef 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java @@ -56,6 +56,13 @@ static ConfiguredTargetKey create( return interner.intern(new ConfiguredTargetKey(label, configurationKey)); } + public Builder toBuilder() { + return builder() + .setConfigurationKey(getConfigurationKey()) + .setLabel(getLabel()) + .setToolchainContextKey(getToolchainContextKey()); + } + @Override public final Label getLabel() { return label; @@ -67,7 +74,7 @@ public final SkyFunctionName functionName() { } @Nullable - final BuildConfigurationValue.Key getConfigurationKey() { + public final BuildConfigurationValue.Key getConfigurationKey() { return configurationKey; } diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD index 87c52f7acbb762..829ef858bc6603 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD @@ -69,6 +69,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/query2", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//src/main/java/com/google/devtools/build/lib/util:filetype", "//src/main/java/com/google/devtools/build/lib/vfs", diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallbackTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallbackTest.java index d0b2e28a44d87a..be0bdc652aa91c 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallbackTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallbackTest.java @@ -19,7 +19,6 @@ import static com.google.devtools.build.lib.packages.BuildType.OUTPUT; import com.google.common.eventbus.EventBus; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.util.MockRule; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; @@ -74,7 +73,7 @@ private List getOutput(String queryExpression) throws Exception { Set targetPatternSet = new LinkedHashSet<>(); expression.collectTargetPatterns(targetPatternSet); helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); - PostAnalysisQueryEnvironment env = + PostAnalysisQueryEnvironment env = ((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet); ByteArrayOutputStream output = new ByteArrayOutputStream(); diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryHelper.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryHelper.java index 2a43f1448554eb..c806135ab6b4b4 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryHelper.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryHelper.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.analysis.util.AnalysisTestCase; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment.TopLevelConfigurations; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction; @@ -31,7 +30,7 @@ * AnalysisTestCase} must be run manually. @BeforeClass and @AfterClass are completely ignored for * now. */ -public class ConfiguredTargetQueryHelper extends PostAnalysisQueryHelper { +public class ConfiguredTargetQueryHelper extends PostAnalysisQueryHelper { @Override protected ConfiguredTargetQueryEnvironment getPostAnalysisQueryEnvironment( WalkableGraph walkableGraph, @@ -54,7 +53,7 @@ protected ConfiguredTargetQueryEnvironment getPostAnalysisQueryEnvironment( } @Override - public String getLabel(ConfiguredTarget target) { - return target.getLabel().toString(); + public String getLabel(KeyedConfiguredTarget target) { + return target.getConfiguredTarget().getLabel().toString(); } } diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java index e4b78705b774f8..807c092445663a 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java @@ -23,7 +23,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.BuildOptionsView; @@ -120,9 +119,9 @@ public void testLabelFunction_getCorrectlyConfiguredDeps() throws Exception { setUpLabelsFunctionTests(); // Test that this retrieves the correctly configured version(s) of the dep(s). - ConfiguredTarget patchDep = + KeyedConfiguredTarget patchDep = Iterables.getOnlyElement(eval("labels('patch_dep', //test:my_rule)")); - ConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); + KeyedConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); String targetConfiguration = myRule.getConfigurationChecksum(); assertThat(patchDep.getConfigurationChecksum()).doesNotMatch(targetConfiguration); } @@ -131,13 +130,13 @@ public void testLabelFunction_getCorrectlyConfiguredDeps() throws Exception { public void testLabelsFunction_splitTransitionAttribute() throws Exception { setUpLabelsFunctionTests(); - ConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); + KeyedConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); String targetConfiguration = myRule.getConfigurationChecksum(); - Collection splitDeps = eval("labels('split_dep', //test:my_rule)"); + Collection splitDeps = eval("labels('split_dep', //test:my_rule)"); assertThat(splitDeps).hasSize(2); - for (ConfiguredTarget ct : splitDeps) { - assertThat(ct.getConfigurationChecksum()).doesNotMatch(targetConfiguration); + for (KeyedConfiguredTarget kct : splitDeps) { + assertThat(kct.getConfigurationChecksum()).doesNotMatch(targetConfiguration); } } @@ -145,14 +144,14 @@ public void testLabelsFunction_splitTransitionAttribute() throws Exception { public void testLabelsFunction_labelListAttribute() throws Exception { setUpLabelsFunctionTests(); - ConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); + KeyedConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); String targetConfiguration = myRule.getConfigurationChecksum(); // Test that this works for label_lists as well. - Set deps = eval("labels('patch_dep_list', //test:my_rule)"); + Set deps = eval("labels('patch_dep_list', //test:my_rule)"); assertThat(deps).hasSize(2); - for (ConfiguredTarget ct : deps) { - assertThat(ct.getConfigurationChecksum()).doesNotMatch(targetConfiguration); + for (KeyedConfiguredTarget kct : deps) { + assertThat(kct.getConfigurationChecksum()).doesNotMatch(targetConfiguration); } } @@ -193,7 +192,7 @@ public void testGetPrerequisitesFromAliasReturnsActualPrerequisites() throws Exc "rule_with_dep(name = 'actual', dep = ':dep')", "rule_with_dep(name = 'dep')"); - ConfiguredTarget dep = Iterables.getOnlyElement(eval("labels('dep', '//test:alias')")); + KeyedConfiguredTarget dep = Iterables.getOnlyElement(eval("labels('dep', '//test:alias')")); assertThat(dep.getLabel()).isEqualTo(Label.parseAbsoluteUnchecked("//test:dep")); } @@ -221,11 +220,12 @@ public void testAlias_filtering() throws Exception { "alias(name = 'other_impl_dep', actual = 'impl_dep')", "simple_rule(name='impl_dep')"); - ConfiguredTarget other = Iterables.getOnlyElement(eval("//test:other_my_rule")); - ConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); - // Note: {@link AliasConfiguredTarget#getLabel} returns the label of the "actual" value not the - // label of the alias. - assertThat(other.getLabel()).isEqualTo(myRule.getLabel()); + KeyedConfiguredTarget other = Iterables.getOnlyElement(eval("//test:other_my_rule")); + KeyedConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); + // Note: {@link KeyedConfiguredTarget#getLabel} returns the label of the "actual" value not the + // label of the alias, so we need to check the underlying label. + assertThat(other.getConfiguredTarget().getLabel()) + .isEqualTo(myRule.getConfiguredTarget().getLabel()); // Regression test for b/73496081 in which alias-ed configured targets were skipping filtering. helper.setQuerySettings(Setting.ONLY_TARGET_DEPS, Setting.NO_IMPLICIT_DEPS); @@ -246,7 +246,7 @@ public void testTopLevelTransition() throws Exception { writeFile("test/BUILD", "rule_class_transition(name='rule_class')"); - Set ruleClass = eval("//test:rule_class"); + Set ruleClass = eval("//test:rule_class"); TestOptions testOptions = getConfiguration(Iterables.getOnlyElement(ruleClass)).getOptions().get(TestOptions.class); assertThat(testOptions.testArguments).containsExactly("SET BY PATCH"); @@ -405,10 +405,10 @@ public void testConfig_configHash() throws Exception { // setting --universe_scope we ensure only the transitioned version exists. helper.setUniverseScope("//test:buildme"); helper.setQuerySettings(Setting.ONLY_TARGET_DEPS, Setting.NO_IMPLICIT_DEPS); - Set result = eval("deps(//test:buildme, 1)"); + Set result = eval("deps(//test:buildme, 1)"); assertThat(result).hasSize(2); - ImmutableList stableOrderList = ImmutableList.copyOf(result); + ImmutableList stableOrderList = ImmutableList.copyOf(result); int myDepIndex = stableOrderList.get(0).getLabel().toString().equals("//test:mydep") ? 0 : 1; BuildConfiguration myDepConfig = getConfiguration(stableOrderList.get(myDepIndex)); BuildConfiguration stringFlagConfig = getConfiguration(stableOrderList.get(1 - myDepIndex)); @@ -432,11 +432,12 @@ public void testConfig_configHashPrefix() throws Exception { createConfigRulesAndBuild(); writeFile("mytest/BUILD", "simple_rule(name = 'mytarget')"); - Set result = eval("//mytest:mytarget"); + Set result = eval("//mytest:mytarget"); String configHash = getConfiguration(Iterables.getOnlyElement(result)).checksum(); String hashPrefix = configHash.substring(0, configHash.length() / 2); - Set resultFromPrefix = eval("config(//mytest:mytarget," + hashPrefix + ")"); + Set resultFromPrefix = + eval("config(//mytest:mytarget," + hashPrefix + ")"); assertThat(resultFromPrefix).containsExactlyElementsIn(result); } @@ -445,7 +446,7 @@ public void testConfig_configHashUnknownPrefix() throws Exception { createConfigRulesAndBuild(); writeFile("mytest/BUILD", "simple_rule(name = 'mytarget')"); - Set result = eval("//mytest:mytarget"); + Set result = eval("//mytest:mytarget"); String configHash = getConfiguration(Iterables.getOnlyElement(result)).checksum(); String rightPrefix = configHash.substring(0, configHash.length() / 2); char lastChar = rightPrefix.charAt(rightPrefix.length() - 1); @@ -496,12 +497,12 @@ public void testRecursiveTargetPatternNeverThrowsError() throws Exception { public void testMultipleTopLevelConfigurations_nullConfigs() throws Exception { writeFile("test/BUILD", "java_library(name='my_java',", " srcs = ['foo.java'],", ")"); - Set result = eval("//test:my_java+//test:foo.java"); + Set result = eval("//test:my_java+//test:foo.java"); assertThat(result).hasSize(2); - Iterator resultIterator = result.iterator(); - ConfiguredTarget first = resultIterator.next(); + Iterator resultIterator = result.iterator(); + KeyedConfiguredTarget first = resultIterator.next(); if (first.getLabel().toString().equals("//test:foo.java")) { assertThat(getConfiguration(first)).isNull(); assertThat(getConfiguration(resultIterator.next())).isNotNull(); @@ -531,8 +532,8 @@ public void testSomePath_depInCustomConfiguration() throws Exception { // cases cquery prefers the top-level configured one, which won't produce a match since that's // not the one down this dependency path. helper.setUniverseScope("//test:buildme"); - Set result = eval("somepath(//test:buildme, //test:mydep)"); - assertThat(result.stream().map(ct -> ct.getLabel().toString()).collect(Collectors.toList())) + Set result = eval("somepath(//test:buildme, //test:mydep)"); + assertThat(result.stream().map(kct -> kct.getLabel().toString()).collect(Collectors.toList())) .contains("//test:mydep"); } @@ -577,7 +578,7 @@ public void testQueryHandlesDroppingFragments() throws Exception { "simple_rule(name='foo', deps = [':bar'])", "simple_rule(name='bar')"); - Set result = + Set result = eval("somepath(//test:top, filter(//test:bar, deps(//test:top)))"); assertThat(result).isNotEmpty(); } @@ -597,7 +598,7 @@ public void testLabelExpressionsMatchesAllConfiguredTargetsWithLabel() throws Ex "simple_rule(name = 'simple')"); helper.setUniverseScope("//test:transitioner,//test:simple"); - Set result = eval("//test:simple"); + Set result = eval("//test:simple"); assertThat(result.size()).isEqualTo(2); } @@ -619,7 +620,7 @@ public void testConfigFunctionRefinesMultipleMatches() throws Exception { "simple_rule(name = 'simple')"); helper.setUniverseScope("//test:transitioner,//test:simple"); - Set result = eval("config(//test:simple, target)"); + Set result = eval("config(//test:simple, target)"); assertThat(result.size()).isEqualTo(1); } } diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryTest.java index 152dc1c9db3bb2..ae26a15aaee383 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryTest.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.BuildOptionsView; @@ -39,10 +38,11 @@ /** Tests for {@link ConfiguredTargetQueryEnvironment}. */ @RunWith(JUnit4.class) -public abstract class ConfiguredTargetQueryTest extends PostAnalysisQueryTest { +public abstract class ConfiguredTargetQueryTest + extends PostAnalysisQueryTest { @Override - protected QueryHelper createQueryHelper() { + protected QueryHelper createQueryHelper() { if (helper != null) { getHelper().cleanUp(); } @@ -62,10 +62,10 @@ public HashMap getDefaultFunctions() { } @Override - protected final BuildConfiguration getConfiguration(ConfiguredTarget ct) { + protected final BuildConfiguration getConfiguration(KeyedConfiguredTarget kct) { return getHelper() .getSkyframeExecutor() - .getConfiguration(getHelper().getReporter(), ct.getConfigurationKey()); + .getConfiguration(getHelper().getReporter(), kct.getConfigurationKey()); } /** SplitTransition on --test_arg */ @@ -98,12 +98,12 @@ public Map split(BuildOptionsView options, EventHandler ev public void testMultipleTopLevelConfigurations_nullConfigs() throws Exception { writeFile("test/BUILD", "java_library(name='my_java',", " srcs = ['foo.java'],", ")"); - Set result = eval("//test:my_java+//test:foo.java"); + Set result = eval("//test:my_java+//test:foo.java"); assertThat(result).hasSize(2); - Iterator resultIterator = result.iterator(); - ConfiguredTarget first = resultIterator.next(); + Iterator resultIterator = result.iterator(); + KeyedConfiguredTarget first = resultIterator.next(); if (first.getLabel().toString().equals("//test:foo.java")) { assertThat(getConfiguration(first)).isNull(); assertThat(getConfiguration(resultIterator.next())).isNotNull(); diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java index 974b6871b3124b..0d96445e0d9a20 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java @@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.eventbus.EventBus; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment; @@ -70,7 +69,7 @@ private List getOutput(String queryExpression) throws Exception { Set targetPatternSet = new LinkedHashSet<>(); expression.collectTargetPatterns(targetPatternSet); helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); - PostAnalysisQueryEnvironment env = + PostAnalysisQueryEnvironment env = ((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet); ByteArrayOutputStream output = new ByteArrayOutputStream(); diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallbackTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallbackTest.java index 94f179b736d99a..fb44dfc87ea8b1 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallbackTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallbackTest.java @@ -20,7 +20,6 @@ import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.analysis.AnalysisProtos; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.TransitionFactories; import com.google.devtools.build.lib.analysis.util.MockRule; import com.google.devtools.build.lib.events.Event; @@ -144,7 +143,7 @@ public void testConfigurationHash() throws Exception { // Assert checksum from proto is proper checksum. AnalysisProtos.ConfiguredTarget myRuleProto = Iterables.getOnlyElement(getOutput("//test:my_rule").getResultsList()); - ConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); + KeyedConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); assertThat(myRuleProto.getConfiguration().getChecksum()) .isEqualTo(myRule.getConfigurationChecksum()); @@ -216,7 +215,7 @@ private AnalysisProtos.CqueryResult getOutput(String queryExpression) throws Exc Set targetPatternSet = new LinkedHashSet<>(); expression.collectTargetPatterns(targetPatternSet); helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); - PostAnalysisQueryEnvironment env = + PostAnalysisQueryEnvironment env = ((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet); ProtoOutputFormatterCallback callback = diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterTest.java index 9c61391d060614..be67fbb8ce8d46 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterTest.java @@ -20,7 +20,6 @@ import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.TransitionFactories; import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory; @@ -212,7 +211,7 @@ private List getOutput(String queryExpression, CqueryOptions.Transitions Set targetPatternSet = new LinkedHashSet<>(); expression.collectTargetPatterns(targetPatternSet); helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); - PostAnalysisQueryEnvironment env = + PostAnalysisQueryEnvironment env = ((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet); options.transitions = verbosity; // TODO(juliexxia): Test late-bound attributes. diff --git a/src/test/shell/bazel/toolchain_transition_test.sh b/src/test/shell/bazel/toolchain_transition_test.sh index c80f79225c0521..9464737e5ce6aa 100755 --- a/src/test/shell/bazel/toolchain_transition_test.sh +++ b/src/test/shell/bazel/toolchain_transition_test.sh @@ -15,7 +15,6 @@ # limitations under the License. # # Test the toolchain transition. -# # Load the test setup defined in the parent directory CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -355,4 +354,32 @@ EOF expect_log 'extra_lib: message: extra_lib foo, target_dep: target, tool_dep: exec-alpha' } +# Regression test for https://github.com/bazelbuild/bazel/issues/11993 +# This was causing cquery to not correctly generate ConfiguredTargetKeys for +# toolchains, leading to the message "Targets were missing from graph" +function test_toolchain_transition_cquery() { + write_constraints + write_platforms + write_toolchains + write_rule + + cat >BUILD.bazel < $TEST_log || fail "Build failed" + + expect_not_log "Targets were missing from graph" +} + run_suite "toolchain transition tests"