From 81e1333c2e4e0f7db7cf3e38be750fe8cc58420c Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 3 Jan 2024 14:48:04 -0800 Subject: [PATCH] explcitly traverse aspects in cquery This CL adds aspect nodes as explicit targets in cquery deps by making ConfiguredTargetQueryEnvironment generic over a new interface, CqueryNode, rather than ConfiguredTarget. CqueryNode is implemented by AspectKey as well as ConfiguredTarget so that both can be traversed in a deps query. PiperOrigin-RevId: 595509373 Change-Id: I8a637cd3ed640907d2b1501bcd2b4a4507d183e7 --- .../java/com/google/devtools/build/lib/BUILD | 1 + .../google/devtools/build/lib/analysis/BUILD | 2 + .../build/lib/analysis/ConfiguredTarget.java | 14 +- .../lib/analysis/TopLevelArtifactHelper.java | 3 +- .../build/lib/buildtool/CqueryProcessor.java | 4 +- .../google/devtools/build/lib/query2/BUILD | 2 + .../query2/PostAnalysisQueryEnvironment.java | 57 +++- .../aquery/ActionGraphQueryEnvironment.java | 11 +- .../aquery/ConfiguredTargetValueAccessor.java | 7 +- .../devtools/build/lib/query2/common/BUILD | 17 + .../lib/query2/common/CommonQueryOptions.java | 13 + .../build/lib/query2/common/CqueryNode.java | 110 +++++++ .../cquery/BuildOutputFormatterCallback.java | 11 +- .../lib/query2/cquery/ConfigFunction.java | 6 +- .../cquery/ConfiguredTargetAccessor.java | 43 ++- .../ConfiguredTargetQueryEnvironment.java | 98 +++--- .../cquery/CqueryThreadsafeCallback.java | 7 +- .../cquery/CqueryTransitionResolver.java | 6 +- .../cquery/FilesOutputFormatterCallback.java | 16 +- .../cquery/GraphOutputFormatterCallback.java | 36 +-- ...dConfigurationOutputFormatterCallback.java | 17 +- .../cquery/ProtoOutputFormatterCallback.java | 10 +- .../StarlarkOutputFormatterCallback.java | 14 +- .../TransitionsOutputFormatterCallback.java | 10 +- .../lib/query2/engine/QueryEnvironment.java | 5 +- .../com/google/devtools/build/lib/rules/BUILD | 1 - .../build/lib/skyframe/AspectKeyCreator.java | 39 ++- .../google/devtools/build/lib/skyframe/BUILD | 4 +- .../lib/skyframe/ConfiguredTargetKey.java | 4 +- .../google/devtools/build/lib/buildtool/BUILD | 1 - .../devtools/build/lib/query2/cquery/BUILD | 19 +- .../BuildOutputFormatterCallbackTest.java | 4 +- .../cquery/ConfiguredTargetQueryHelper.java | 6 +- .../ConfiguredTargetQuerySemanticsTest.java | 290 ++++++++++++++++-- .../cquery/ConfiguredTargetQueryTest.java | 14 +- .../FilesOutputFormatterCallbackTest.java | 4 +- .../GraphOutputFormatterCallbackTest.java | 4 +- .../ProtoOutputFormatterCallbackTest.java | 14 +- .../TransitionsOutputFormatterTest.java | 4 +- 39 files changed, 691 insertions(+), 237 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/query2/common/CqueryNode.java diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index a18652803fce18..3a656e1d0fda99 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -385,6 +385,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/profiler/memory:allocationtracker", "//src/main/java/com/google/devtools/build/lib/query2", "//src/main/java/com/google/devtools/build/lib/query2:aquery-utils", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//src/main/java/com/google/devtools/build/lib/query2/query/output", "//src/main/java/com/google/devtools/build/lib/runtime/events", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index ae85e8bd2925e4..f95ee8742aa167 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -405,6 +405,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/profiler:google-auto-profiler-utils", "//src/main/java/com/google/devtools/build/lib/profiler/memory:current_rule_tracker", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator", "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data", @@ -688,6 +689,7 @@ java_library( ":transitive_info_collection", "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/skyframe/config", "//src/main/java/net/starlark/java/eval", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java index 3db2decc9ae279..af72a3058b8f38 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; import javax.annotation.Nullable; import net.starlark.java.eval.Dict; @@ -35,7 +36,7 @@ * their direct dependencies, only the corresponding {@link TransitiveInfoCollection}s. Also, {@link * ConfiguredTarget} objects should not be accessible from the action graph. */ -public interface ConfiguredTarget extends TransitiveInfoCollection, Structure { +public interface ConfiguredTarget extends TransitiveInfoCollection, Structure, CqueryNode { /** All ConfiguredTargets have a "label" field. */ String LABEL_FIELD = "label"; @@ -44,6 +45,7 @@ public interface ConfiguredTarget extends TransitiveInfoCollection, Structure { String FILES_FIELD = "files"; /** Returns a key that may be used to lookup this {@link ConfiguredTarget}. */ + @Override ActionLookupKey getLookupKey(); @Override @@ -51,6 +53,7 @@ default Label getLabel() { return getLookupKey().getLabel(); } + @Override @Nullable default String getConfigurationChecksum() { return getConfigurationKey() == null ? null : getConfigurationKey().getOptions().checksum(); @@ -65,8 +68,9 @@ default String getConfigurationChecksum() { * com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget} for * which it is always null. * - *

If this changes, {@link AspectResolver#aspecMatchesConfiguredTarget} should be updated. + *

If this changes, {@link AspectResolver#aspectMatchesConfiguredTarget} should be updated. */ + @Override @Nullable default BuildConfigurationKey getConfigurationKey() { return getLookupKey().getConfigurationKey(); @@ -89,6 +93,7 @@ default BuildConfigurationKey getConfigurationKey() { * If the configured target is an alias, return the actual target, otherwise return the current * target. This follows alias chains. */ + @Override default ConfiguredTarget getActual() { return this; } @@ -98,6 +103,7 @@ default ConfiguredTarget getActual() { * label. This is not the same as {@code getActual().getLabel()}, because it does not follow alias * chains. */ + @Override default Label getOriginalLabel() { return getLabel(); } @@ -106,10 +112,12 @@ default Label getOriginalLabel() { * 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. */ + @Override default ImmutableMap getConfigConditions() { return ImmutableMap.of(); } + @Override default boolean isRuleConfiguredTarget() { return false; } @@ -119,6 +127,7 @@ default boolean isRuleConfiguredTarget() { * *

Unwrapping is recursive if there are multiple layers. */ + @Override default ConfiguredTarget unwrapIfMerged() { return this; } @@ -128,6 +137,7 @@ default ConfiguredTarget unwrapIfMerged() { * * @return a map of provider names to their values, or null if there are no providers */ + @Override @Nullable default Dict getProvidersDictForQuery() { return null; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java index cbe7eb522cbd7b..9560b21a856fc2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.profiler.AutoProfiler; import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils; +import com.google.devtools.build.lib.query2.common.CqueryNode; import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; @@ -243,7 +244,7 @@ static ArtifactsToBuild getAllArtifactsToBuild( * *

Always returns false for hidden rules and source file targets. */ - public static boolean shouldConsiderForDisplay(ConfiguredTarget configuredTarget) { + public static boolean shouldConsiderForDisplay(CqueryNode configuredTarget) { // TODO(bazel-team): this is quite ugly. Add a marker provider for this check. if (configuredTarget instanceof InputFileConfiguredTarget) { // Suppress display of source files (because we do no work to build them). diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/CqueryProcessor.java b/src/main/java/com/google/devtools/build/lib/buildtool/CqueryProcessor.java index d2a607991afd3b..fa0ab52e861629 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/CqueryProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/CqueryProcessor.java @@ -14,10 +14,10 @@ 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.cmdline.TargetPattern; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment.TopLevelConfigurations; +import com.google.devtools.build.lib.query2.common.CqueryNode; 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.engine.QueryEnvironment.QueryFunction; @@ -29,7 +29,7 @@ import net.starlark.java.eval.StarlarkSemantics; /** Performs {@code cquery} processing. */ -public final class CqueryProcessor extends PostAnalysisQueryProcessor { +public final class CqueryProcessor extends PostAnalysisQueryProcessor { public CqueryProcessor( QueryExpression queryExpression, TargetPattern.Parser mainRepoTargetParser) { 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 08a14e51737d10..a9ccbcb641d9af 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/BUILD +++ b/src/main/java/com/google/devtools/build/lib/query2/BUILD @@ -27,6 +27,7 @@ java_library( ), deps = [ "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", @@ -75,6 +76,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/query2/common:QueryTransitivePackagePreloader", "//src/main/java/com/google/devtools/build/lib/query2/common:UniverseSkyKey", "//src/main/java/com/google/devtools/build/lib/query2/common:abstract-blaze-query-env", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/common:options", "//src/main/java/com/google/devtools/build/lib/query2/common:universe-scope", "//src/main/java/com/google/devtools/build/lib/query2/compat:fake-load-target", diff --git a/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java index 74b8498c814216..0b9c1af92a7717 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java @@ -24,7 +24,9 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.analysis.AliasProvider; +import com.google.devtools.build.lib.analysis.AspectValue; import com.google.devtools.build.lib.analysis.ConfiguredTargetValue; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; @@ -74,6 +76,7 @@ import com.google.devtools.build.lib.supplier.InterruptibleSupplier; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.WalkableGraph; import java.io.IOException; import java.io.OutputStream; @@ -144,7 +147,7 @@ public PostAnalysisQueryEnvironment( public abstract String getOutputFormat(); - protected abstract KeyExtractor getConfiguredTargetKeyExtractor(); + protected abstract KeyExtractor getConfiguredTargetKeyExtractor(); @Override public QueryEvalResult evaluateQuery( @@ -217,12 +220,18 @@ public T getOrCreate(T target) { protected abstract T getNullConfiguredTarget(Label label) throws InterruptedException; @Nullable - public ConfiguredTargetValue getConfiguredTargetValue(SkyKey key) throws InterruptedException { - return (ConfiguredTargetValue) walkableGraphSupplier.get().getValue(key); + public SkyValue getConfiguredTargetValue(SkyKey key) throws InterruptedException { + return walkableGraphSupplier.get().getValue(key); + } + + @Nullable + public AspectValue getAspectValue(SkyKey key) throws InterruptedException { + return (AspectValue) walkableGraphSupplier.get().getValue(key); } private boolean isAliasConfiguredTarget(ConfiguredTargetKey key) throws InterruptedException { - return AliasProvider.isAlias(getConfiguredTargetValue(key).getConfiguredTarget()); + return AliasProvider.isAlias( + ((ConfiguredTargetValue) getConfiguredTargetValue(key)).getConfiguredTarget()); } public InterruptibleSupplier> @@ -362,7 +371,7 @@ private Set unwindReverseDependencyDelegationLayersIfFound( if (!rdep.functionName().equals(SkyFunctions.CONFIGURED_TARGET)) { continue; } - ConfiguredTargetKey actualParentKey = getConfiguredTargetKey(getValueFromKey(rdep)); + var actualParentKey = getConfiguredTargetKey(getValueFromKey(rdep)); if (actualParentKey.equals(child)) { // The parent has the same value as the child because it is delegating. foundDelegatingRdep = true; @@ -386,7 +395,7 @@ private void unwindReverseDependencyDelegationLayers( output.add(rdep); continue; } - ConfiguredTargetKey actualParentKey = getConfiguredTargetKey(getValueFromKey(rdep)); + var actualParentKey = getConfiguredTargetKey(getValueFromKey(rdep)); if (!actualParentKey.equals(child)) { output.add(rdep); continue; @@ -479,19 +488,24 @@ private ImmutableList> targetifyValues( } } + boolean explicitAspects = + settings.containsAll(ImmutableSet.of(Setting.INCLUDE_ASPECTS, Setting.EXPLICIT_ASPECTS)); + ImmutableList.Builder> values = ImmutableList.builder(); - // TODO(bazel-team): An even better approach would be to treat aspects and toolchains as + // TODO(bazel-team): The end-goal approach is to treat aspects and toolchains as // first-class query nodes just like targets. In other words, let query expressions reference // them (they also have identifying labels) and make the graph connections between targets, // aspects, and toolchains explicit. That would permit more detailed queries and eliminate the - // per-key-type special casing below. The challenge is to generalize all query code that - // currently assumes its inputs are configured targets. Toolchains may have additional caveats: - // see b/148550864. + // per-key-type special casing below. + // This is being experimentally implemented in phases. Currently support for aspects has been + // implemented behind the --experimental_explicit_aspects flag. + // See https://github.com/bazelbuild/bazel/issues/16310 for details. for (SkyKey key : dependencies) { if (knownCtDeps.contains(key)) { continue; } - if (key.functionName().equals(SkyFunctions.CONFIGURED_TARGET)) { + if (key.functionName().equals(SkyFunctions.CONFIGURED_TARGET) + || (explicitAspects && key.functionName().equals(SkyFunctions.ASPECT))) { T dependency = getValueFromKey(key); Preconditions.checkState( dependency != null, @@ -501,17 +515,24 @@ private ImmutableList> targetifyValues( + " configurability team.", key); - boolean implicit = + boolean implicitConfiguredTarget = // Check both the original guess key and the second correct key. In the case of the // target platform, Util.findImplicitDeps also uses the original guess key. implicitDeps == null || implicitDeps.contains(key) || implicitDeps.contains(getConfiguredTargetKey(dependency)); + + boolean implicit = + !(key.argument() instanceof ConfiguredTargetKey) || implicitConfiguredTarget; + values.add(new ClassifiedDependency<>(dependency, implicit)); knownCtDeps.add(key); } else if (settings.contains(Setting.INCLUDE_ASPECTS) - && key.functionName().equals(SkyFunctions.ASPECT) - && !resolvedAspectClasses.contains(((AspectKey) key).getAspectClass())) { + && key.functionName().equals(SkyFunctions.ASPECT)) { + Preconditions.checkState(!settings.contains(Setting.EXPLICIT_ASPECTS)); + if (resolvedAspectClasses.contains(((AspectKey) key).getAspectClass())) { + continue; + } // When an aspect is attached to an alias configured target, it bypasses standard dependency // resolution and just Skyframe-loads the same aspect for the alias' referent. That means // the original aspect's attribute deps aren't Skyframe-resolved through AspectFunction's @@ -550,8 +571,8 @@ private Map>> targetifyValues( targetifyValues( fromTargetsByKey.get(fromKey), entry.getValue(), - /*knownCtDeps=*/ new HashSet<>(), - /*resolvedAspectClasses=*/ new HashSet<>())); + /* knownCtDeps= */ new HashSet<>(), + /* resolvedAspectClasses= */ new HashSet<>())); } return result; } @@ -586,7 +607,7 @@ private static ImmutableList getDependencies( @Nullable protected abstract BuildConfigurationValue getConfiguration(T target); - protected abstract ConfiguredTargetKey getConfiguredTargetKey(T target); + protected abstract ActionLookupKey getConfiguredTargetKey(T target); @Override public ThreadSafeMutableSet getTransitiveClosure( @@ -649,11 +670,13 @@ public static class TopLevelConfigurations { /** A map of non-null configured top-level targets sorted by configuration checksum. */ private final ImmutableMap nonNulls; + /** * {@code nonNulls} may often have many duplicate values in its value set so we store a sorted * set of all the non-null configurations here. */ private final ImmutableSortedSet nonNullConfigs; + /** A list of null configured top-level targets. */ private final ImmutableList

If this changes, {@link AspectResolver#aspectMatchesConfiguredTarget} should be updated. + */ + @Nullable + default BuildConfigurationKey getConfigurationKey() { + return getLookupKey().getConfigurationKey(); + } + + /** + * If the configured target is an alias, return the actual target, otherwise return the current + * target. This follows alias chains. + */ + default CqueryNode getActual() { + return this; + } + + /** + * If the configured target is an alias, return the original label, otherwise return the current + * label. This is not the same as {@code getActual().getLabel()}, because it does not follow alias + * chains. + */ + default Label getOriginalLabel() { + return getLabel(); + } + + /** + * 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. + */ + default ImmutableMap getConfigConditions() { + return ImmutableMap.of(); + } + + default boolean isRuleConfiguredTarget() { + return false; + } + + /** + * The base configured target if it has been merged with aspects otherwise the current value. + * + *

Unwrapping is recursive if there are multiple layers. + */ + default CqueryNode unwrapIfMerged() { + return this; + } + + /** + * This is only intended to be called from the query dialects of Starlark. + * + * @return a map of provider names to their values, or null if there are no providers + */ + @Nullable + default Dict getProvidersDictForQuery() { + return null; + } +} 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 6ee85ab00d9de7..5b4bc337b73979 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; @@ -23,6 +22,7 @@ import com.google.devtools.build.lib.packages.LabelPrinter; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; import com.google.devtools.build.lib.query2.query.output.BuildOutputFormatter; import com.google.devtools.build.lib.query2.query.output.BuildOutputFormatter.AttributeReader; @@ -42,7 +42,7 @@ class BuildOutputFormatterCallback extends CqueryThreadsafeCallback { CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, LabelPrinter labelPrinter) { super(eventHandler, options, out, skyframeExecutor, accessor, /* uniquifyResults= */ false); this.labelPrinter = labelPrinter; @@ -73,8 +73,7 @@ public Iterable getPossibleValues(Rule rule, Attribute attr) { } @Nullable - private ConfiguredAttributeMapper getAttributeMap(ConfiguredTarget kct) - throws InterruptedException { + private ConfiguredAttributeMapper getAttributeMap(CqueryNode kct) throws InterruptedException { Rule associatedRule = accessor.getTarget(kct).getAssociatedRule(); if (associatedRule == null) { return null; @@ -94,7 +93,7 @@ private ConfiguredAttributeMapper getAttributeMap(ConfiguredTarget kct) } @Override - public void processOutput(Iterable partialResult) + public void processOutput(Iterable partialResult) throws InterruptedException, IOException { BuildOutputFormatter.TargetOutputter outputter = new TargetOutputter( @@ -106,7 +105,7 @@ public void processOutput(Iterable partialResult) (rule, attr) -> false, System.lineSeparator(), labelPrinter); - for (ConfiguredTarget configuredTarget : partialResult) { + for (CqueryNode 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 1b105be912fa65..a31868af7a2d40 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,8 +14,8 @@ 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.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.Callback; import com.google.devtools.build.lib.query2.engine.QueryEnvironment; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Argument; @@ -59,7 +59,7 @@ public List getArgumentTypes() { /** * This function is only viable with ConfiguredTargetQueryEnvironment which extends {@link - * AbstractBlazeQueryEnvironment }. + * AbstractBlazeQueryEnvironment }. */ @Override @SuppressWarnings("unchecked") @@ -85,6 +85,6 @@ public QueryTaskFuture eval( targetExpression.toString(), targetsFuture, 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 c25a6e6ed24b18..6df105f18161b8 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 @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetNotFoundException; import com.google.devtools.build.lib.query2.engine.QueryException; @@ -47,7 +48,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; @@ -74,56 +75,55 @@ public ConfiguredTargetAccessor( } @Override - public String getTargetKind(ConfiguredTarget target) { + public String getTargetKind(CqueryNode target) { Target actualTarget = getTarget(target); return actualTarget.getTargetKind(); } @Override - public String getLabel(ConfiguredTarget target) { + public String getLabel(CqueryNode target) { return target.getOriginalLabel().toString(); } @Override - public String getPackage(ConfiguredTarget target) { + public String getPackage(CqueryNode target) { return target.getOriginalLabel().getPackageIdentifier().getPackageFragment().toString(); } @Override - public boolean isRule(ConfiguredTarget target) { + public boolean isRule(CqueryNode target) { Target actualTarget = getTarget(target); return actualTarget instanceof Rule; } @Override - public boolean isTestRule(ConfiguredTarget target) { + public boolean isTestRule(CqueryNode target) { Target actualTarget = getTarget(target); return TargetUtils.isTestRule(actualTarget); } @Override - public boolean isTestSuite(ConfiguredTarget target) { + public boolean isTestSuite(CqueryNode target) { Target actualTarget = getTarget(target); return TargetUtils.isTestSuiteRule(actualTarget); } @Override - public List getPrerequisites( + public List getPrerequisites( QueryExpression caller, - ConfiguredTarget keyedConfiguredTarget, + CqueryNode keyedConfiguredTarget, String attrName, String errorMsgPrefix) throws QueryException, InterruptedException { // Process aliases. - ConfiguredTarget actual = keyedConfiguredTarget.getActual(); + CqueryNode actual = keyedConfiguredTarget.getActual(); Preconditions.checkArgument( isRule(actual), "%s %s is not a rule configured target", errorMsgPrefix, getLabel(actual)); - ImmutableListMultimap depsByLabel = + ImmutableListMultimap depsByLabel = Multimaps.index( - queryEnvironment.getFwdDeps(ImmutableList.of(actual)), - ConfiguredTarget::getOriginalLabel); + queryEnvironment.getFwdDeps(ImmutableList.of(actual)), CqueryNode::getOriginalLabel); Rule rule = (Rule) getTarget(actual); ImmutableMap configConditions = actual.getConfigConditions(); @@ -141,39 +141,39 @@ public List getPrerequisites( errorMsgPrefix, rule.getRuleClass(), attrName), ConfigurableQuery.Code.ATTRIBUTE_MISSING); } - ImmutableList.Builder toReturn = ImmutableList.builder(); + ImmutableList.Builder toReturn = ImmutableList.builder(); attributeMapper.visitLabels(attrName, label -> toReturn.addAll(depsByLabel.get(label))); return toReturn.build(); } @Override - public List getStringListAttr(ConfiguredTarget target, String attrName) { + public List getStringListAttr(CqueryNode target, String attrName) { Target actualTarget = getTarget(target); return TargetUtils.getStringListAttr(actualTarget, attrName); } @Override - public String getStringAttr(ConfiguredTarget target, String attrName) { + public String getStringAttr(CqueryNode target, String attrName) { Target actualTarget = getTarget(target); return TargetUtils.getStringAttr(actualTarget, attrName); } @Override - public Iterable getAttrAsString(ConfiguredTarget target, String attrName) { + public Iterable getAttrAsString(CqueryNode 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, CqueryNode 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 getTarget(ConfiguredTarget configuredTarget) { + public Target getTarget(CqueryNode configuredTarget) { // Dereference any aliases that might be present. Label label = configuredTarget.getOriginalLabel(); try { @@ -190,8 +190,7 @@ SkyFunction.LookupEnvironment getLookupEnvironment() { } /** Returns the rule that generates the given output file. */ - RuleConfiguredTarget getGeneratingConfiguredTarget(ConfiguredTarget kct) - throws InterruptedException { + RuleConfiguredTarget getGeneratingConfiguredTarget(CqueryNode kct) throws InterruptedException { Preconditions.checkArgument(kct instanceof OutputFileConfiguredTarget); return (RuleConfiguredTarget) ((ConfiguredTargetValue) 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 45284c8ccd34ae..b14ebb2ef8c0e9 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 @@ -23,6 +23,8 @@ import com.google.common.util.concurrent.AsyncFunction; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.MoreExecutors; +import com.google.devtools.build.lib.actions.ActionLookupKey; +import com.google.devtools.build.lib.analysis.AspectValue; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.ConfiguredTargetValue; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; @@ -40,6 +42,7 @@ import com.google.devtools.build.lib.query2.NamedThreadSafeOutputFormatterCallback; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment; import com.google.devtools.build.lib.query2.SkyQueryEnvironment; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.cquery.ProtoOutputFormatterCallback.OutputType; import com.google.devtools.build.lib.query2.engine.Callback; import com.google.devtools.build.lib.query2.engine.KeyExtractor; @@ -50,10 +53,12 @@ import com.google.devtools.build.lib.query2.query.aspectresolvers.AspectResolver; import com.google.devtools.build.lib.rules.AliasConfiguredTarget; import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery; +import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.WalkableGraph; import java.io.OutputStream; import java.util.ArrayList; @@ -71,11 +76,11 @@ * {@link QueryEnvironment} that runs queries over the configured target (analysis) graph. * *

Aspects are partially supported. Their dependencies appear as implicit dependencies on the - * targets they're connected to, but the aspects themselves aren't visible as query nodes. See - * comments on {@link PostAnalysisQueryEnvironment#targetifyValues} and b/163052263 for details. + * targets they're connected to. When using the --experimental_explicit_aspects flag, the aspects + * themselves are visible as query nodes. See https://github.com/bazelbuild/bazel/issues/16310 for + * details. */ -public class ConfiguredTargetQueryEnvironment - extends PostAnalysisQueryEnvironment { +public class ConfiguredTargetQueryEnvironment extends PostAnalysisQueryEnvironment { /** Common query functions and cquery specific functions. */ public static final ImmutableList FUNCTIONS = populateFunctions(); /** Cquery specific functions. */ @@ -85,12 +90,12 @@ public class ConfiguredTargetQueryEnvironment private final TopLevelArtifactContext topLevelArtifactContext; - private final KeyExtractor configuredTargetKeyExtractor; + private final KeyExtractor configuredTargetKeyExtractor; private final ConfiguredTargetAccessor accessor; /** - * Stores every configuration in the transitive closure of the build graph as a map from its + * F Stores every configuration in the transitive closure of the build graph as a map from its * user-friendly hash to the configuration itself. * *

This is used to find configured targets in, e.g. {@code somepath} queries. Given {@code @@ -107,7 +112,7 @@ public class ConfiguredTargetQueryEnvironment private final ImmutableMap transitiveConfigurations; @Override - protected KeyExtractor getConfiguredTargetKeyExtractor() { + protected KeyExtractor getConfiguredTargetKeyExtractor() { return configuredTargetKeyExtractor; } @@ -135,7 +140,7 @@ public ConfiguredTargetQueryEnvironment( settings, labelPrinter); this.accessor = new ConfiguredTargetAccessor(walkableGraphSupplier.get(), this); - this.configuredTargetKeyExtractor = ConfiguredTargetKey::fromConfiguredTarget; + this.configuredTargetKeyExtractor = CqueryNode::getLookupKey; this.transitiveConfigurations = getTransitiveConfigurations(transitiveConfigurationKeys, walkableGraphSupplier.get()); this.topLevelArtifactContext = topLevelArtifactContext; @@ -192,9 +197,9 @@ private static ImmutableMap getTransitiveConfig } @Override - public ImmutableList> + public ImmutableList> getDefaultOutputFormatters( - TargetAccessor accessor, + TargetAccessor accessor, ExtendedEventHandler eventHandler, OutputStream out, SkyframeExecutor skyframeExecutor, @@ -285,7 +290,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); @@ -311,7 +316,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())); @@ -325,14 +330,13 @@ public QueryTaskFuture getTargetsMatchingPattern( } /** - * Returns the {@link ConfiguredTarget} for the given label and configuration if it exists, else - * null. + * Returns the {@link CqueryNode} for the given label and configuration if it exists, else null. */ @Nullable - private ConfiguredTarget getConfiguredTarget( + private CqueryNode getConfiguredTarget( Label label, @Nullable BuildConfigurationValue configuration) throws InterruptedException { BuildConfigurationKey configurationKey = configuration == null ? null : configuration.getKey(); - ConfiguredTarget target = + CqueryNode target = getValueFromKey( ConfiguredTargetKey.builder() .setLabel(label) @@ -346,11 +350,25 @@ private ConfiguredTarget getConfiguredTarget( return target; } + /** + * Returns the {@link CqueryNode} for the given key if its value is a supported instance of + * CqueryNode. This function can only receive keys of node types that the calling logic can + * support. For example, if the caller does not support handling of AspectKey types of + * CqueryNodes, then this function should not be called with an AspectKey key. + */ @Override @Nullable - protected ConfiguredTarget getValueFromKey(SkyKey key) throws InterruptedException { - ConfiguredTargetValue value = getConfiguredTargetValue(key); - return value == null ? null : value.getConfiguredTarget(); + protected CqueryNode getValueFromKey(SkyKey key) throws InterruptedException { + SkyValue value = getConfiguredTargetValue(key); + if (value == null) { + return null; + } else if (value instanceof ConfiguredTargetValue) { + return ((ConfiguredTargetValue) value).getConfiguredTarget(); + } else if (value instanceof AspectValue && key instanceof AspectKey) { + return (AspectKey) key; + } else { + throw new IllegalStateException("unknown value type for CqueryNode"); + } } /** @@ -358,16 +376,16 @@ protected ConfiguredTarget getValueFromKey(SkyKey key) throws InterruptedExcepti * *

If there are no matches, returns an empty list. */ - private ImmutableList getConfiguredTargetsForConfigFunction(Label label) + private ImmutableList getConfiguredTargetsForConfigFunction(Label label) throws InterruptedException { - ImmutableList.Builder ans = ImmutableList.builder(); + ImmutableList.Builder ans = ImmutableList.builder(); for (BuildConfigurationValue config : transitiveConfigurations.values()) { - ConfiguredTarget kct = getConfiguredTarget(label, config); + CqueryNode kct = getConfiguredTarget(label, config); if (kct != null) { ans.add(kct); } } - ConfiguredTarget nullConfiguredTarget = getNullConfiguredTarget(label); + CqueryNode nullConfiguredTarget = getNullConfiguredTarget(label); if (nullConfiguredTarget != null) { ans.add(nullConfiguredTarget); } @@ -392,19 +410,19 @@ QueryTaskCallable getConfiguredTargetsForConfigFunction( String pattern, QueryTaskFuture> targetsFuture, 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 () -> { - ThreadSafeMutableSet targets = - (ThreadSafeMutableSet) targetsFuture.getIfSuccessful(); - List transformedResult = new ArrayList<>(); + ThreadSafeMutableSet targets = + (ThreadSafeMutableSet) targetsFuture.getIfSuccessful(); + List transformedResult = new ArrayList<>(); boolean userFriendlyConfigName = true; - for (ConfiguredTarget target : targets) { + for (CqueryNode target : targets) { Label label = getCorrectLabel(target); - ConfiguredTarget keyedConfiguredTarget; + CqueryNode keyedConfiguredTarget; switch (configPrefix) { case "host": throw new QueryException( @@ -479,19 +497,19 @@ QueryTaskCallable getConfiguredTargetsForConfigFunction( * the "actual" target instead of the alias target. Grr. */ @Override - public Label getCorrectLabel(ConfiguredTarget target) { + public Label getCorrectLabel(CqueryNode target) { // Dereference any aliases that might be present. return target.getOriginalLabel(); } @Nullable @Override - protected ConfiguredTarget getTargetConfiguredTarget(Label label) throws InterruptedException { + protected CqueryNode getTargetConfiguredTarget(Label label) throws InterruptedException { if (topLevelConfigurations.isTopLevelTarget(label)) { return getConfiguredTarget( label, topLevelConfigurations.getConfigurationForTopLevelTarget(label)); } else { - ConfiguredTarget toReturn; + CqueryNode toReturn; for (BuildConfigurationValue configuration : topLevelConfigurations.getConfigurations()) { toReturn = getConfiguredTarget(label, configuration); if (toReturn != null) { @@ -504,13 +522,13 @@ protected ConfiguredTarget getTargetConfiguredTarget(Label label) throws Interru @Nullable @Override - protected ConfiguredTarget getNullConfiguredTarget(Label label) throws InterruptedException { + protected CqueryNode getNullConfiguredTarget(Label label) throws InterruptedException { return getConfiguredTarget(label, null); } @Nullable @Override - protected RuleConfiguredTarget getRuleConfiguredTarget(ConfiguredTarget configuredTarget) { + protected RuleConfiguredTarget getRuleConfiguredTarget(CqueryNode configuredTarget) { if (configuredTarget instanceof RuleConfiguredTarget) { return (RuleConfiguredTarget) configuredTarget; } @@ -519,7 +537,7 @@ protected RuleConfiguredTarget getRuleConfiguredTarget(ConfiguredTarget configur @Nullable @Override - protected BuildConfigurationValue getConfiguration(ConfiguredTarget target) { + protected BuildConfigurationValue getConfiguration(CqueryNode target) { try { return target.getConfigurationKey() == null ? null @@ -530,15 +548,13 @@ protected BuildConfigurationValue getConfiguration(ConfiguredTarget target) { } @Override - protected ConfiguredTargetKey getConfiguredTargetKey(ConfiguredTarget target) { - return ConfiguredTargetKey.fromConfiguredTarget(target); + protected ActionLookupKey getConfiguredTargetKey(CqueryNode target) { + return target.getLookupKey(); } @Override - public ThreadSafeMutableSet createThreadSafeMutableSet() { + public ThreadSafeMutableSet createThreadSafeMutableSet() { return new ThreadSafeMutableKeyExtractorBackedSetImpl<>( - configuredTargetKeyExtractor, - ConfiguredTarget.class, - SkyQueryEnvironment.DEFAULT_THREAD_COUNT); + configuredTargetKeyExtractor, CqueryNode.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 178eee97380270..73f92b29df00cd 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,10 +15,10 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.query2.NamedThreadSafeOutputFormatterCallback; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; @@ -42,7 +42,7 @@ * focused on completeness, should output full configuration checksums. */ public abstract class CqueryThreadsafeCallback - extends NamedThreadSafeOutputFormatterCallback { + extends NamedThreadSafeOutputFormatterCallback { protected final ExtendedEventHandler eventHandler; protected final CqueryOptions options; @@ -64,7 +64,7 @@ public abstract class CqueryThreadsafeCallback CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, boolean uniquifyResults) { this.eventHandler = eventHandler; this.options = options; @@ -119,4 +119,3 @@ protected static String shortId(@Nullable BuildConfigurationValue config) { return config == null ? "null" : config.shortId(); } } - diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryTransitionResolver.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryTransitionResolver.java index 16ae3e2e455be8..f9e9acfd83333e 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryTransitionResolver.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryTransitionResolver.java @@ -24,7 +24,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimaps; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.DependencyKind; import com.google.devtools.build.lib.analysis.DependencyKind.NonAttributeDependencyKind; import com.google.devtools.build.lib.analysis.DependencyKind.ToolchainDependencyKind; @@ -43,6 +42,7 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.skyframe.ConfiguredTargetEvaluationExceptions.ReportedException; import com.google.devtools.build.lib.skyframe.ConfiguredTargetEvaluationExceptions.UnreportedException; @@ -126,7 +126,7 @@ public CqueryTransitionResolver( * @see ResolvedTransition for more details. * @param configuredTarget the configured target whose dependencies are being looked up. */ - public ImmutableSet dependencies(ConfiguredTarget configuredTarget) + public ImmutableSet dependencies(CqueryNode configuredTarget) throws EvaluateException, InterruptedException { if (!(configuredTarget instanceof RuleConfiguredTarget)) { return ImmutableSet.of(); @@ -250,7 +250,7 @@ private static String getDependencyName(DependencyKind kind) { } @Nullable - private ConfigurationTransition getRuleTransition(ConfiguredTarget configuredTarget) { + private ConfigurationTransition getRuleTransition(CqueryNode configuredTarget) { if (configuredTarget instanceof RuleConfiguredTarget) { return computeTransition( accessor.getTarget(configuredTarget).getAssociatedRule(), diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallback.java index f873120c8417c1..eafe8007bb152c 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallback.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper; import com.google.devtools.build.lib.analysis.configuredtargets.InputFileConfiguredTarget; import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import java.io.IOException; @@ -36,7 +37,7 @@ public class FilesOutputFormatterCallback extends CqueryThreadsafeCallback { CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, TopLevelArtifactContext topLevelArtifactContext) { // Different targets may provide the same artifact, so we deduplicate the collection of all // results at the end. @@ -50,14 +51,17 @@ public String getName() { } @Override - public void processOutput(Iterable partialResult) + public void processOutput(Iterable partialResult) throws IOException, InterruptedException { - for (ConfiguredTarget target : partialResult) { - if (!TopLevelArtifactHelper.shouldConsiderForDisplay(target) - && !(target instanceof InputFileConfiguredTarget)) { + for (CqueryNode target : partialResult) { + if (!(target instanceof ConfiguredTarget) + || (!TopLevelArtifactHelper.shouldConsiderForDisplay(target) + && !(target instanceof InputFileConfiguredTarget))) { continue; } - TopLevelArtifactHelper.getAllArtifactsToBuild(target, topLevelArtifactContext) + + var cf = (ConfiguredTarget) target; + TopLevelArtifactHelper.getAllArtifactsToBuild(cf, topLevelArtifactContext) .getImportantArtifacts() .toList() .stream() 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 d42edca19c95d9..a03dfcff227a14 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,12 +15,12 @@ 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; import com.google.devtools.build.lib.graph.Node; import com.google.devtools.build.lib.packages.LabelPrinter; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; import com.google.devtools.build.lib.query2.query.output.GraphOutputWriter; import com.google.devtools.build.lib.query2.query.output.GraphOutputWriter.NodeReader; @@ -38,15 +38,15 @@ public String getName() { /** Interface for finding a configured target's direct dependencies. */ @FunctionalInterface public interface DepsRetriever { - Iterable getDirectDeps(ConfiguredTarget target) throws InterruptedException; + Iterable getDirectDeps(CqueryNode 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.getOriginalLabel(); @@ -66,18 +66,18 @@ public interface DepsRetriever { }; @Override - public String getLabel(Node node, LabelPrinter labelPrinter) { + public String getLabel(Node node, LabelPrinter labelPrinter) { // Node payloads are ConfiguredTargets. Output node labels are target labels + config // hashes. - ConfiguredTarget kct = node.getLabel(); + CqueryNode kct = node.getLabel(); return String.format( "%s (%s)", - labelPrinter.toString(kct.getOriginalLabel()), + kct.getDescription(labelPrinter), shortId(getConfiguration(kct.getConfigurationKey()))); } @Override - public Comparator comparator() { + public Comparator comparator() { return configuredTargetOrdering; } }; @@ -89,7 +89,7 @@ public Comparator comparator() { CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, DepsRetriever depsRetriever, LabelPrinter labelPrinter) { super(eventHandler, options, out, skyframeExecutor, accessor, /* uniquifyResults= */ false); @@ -98,23 +98,23 @@ public Comparator comparator() { } @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 (CqueryNode configuredTarget : partialResult) { + Node node = graph.createNode(configuredTarget); + for (CqueryNode 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/LabelAndConfigurationOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java index 3aa922b3d8b49f..b9a411ef0c2e5b 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 @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.LabelPrinter; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.util.ClassName; @@ -37,7 +38,7 @@ public class LabelAndConfigurationOutputFormatterCallback extends CqueryThreadsa CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, boolean showKind, LabelPrinter labelPrinter) { super(eventHandler, options, out, skyframeExecutor, accessor, /* uniquifyResults= */ false); @@ -51,8 +52,8 @@ public String getName() { } @Override - public void processOutput(Iterable partialResult) { - for (ConfiguredTarget keyedConfiguredTarget : partialResult) { + public void processOutput(Iterable partialResult) { + for (CqueryNode keyedConfiguredTarget : partialResult) { StringBuilder output = new StringBuilder(); if (showKind) { Target actualTarget = accessor.getTarget(keyedConfiguredTarget); @@ -60,7 +61,7 @@ public void processOutput(Iterable partialResult) { } output = output - .append(labelPrinter.toString(keyedConfiguredTarget.getOriginalLabel())) + .append(keyedConfiguredTarget.getDescription(labelPrinter)) .append(" (") .append(shortId(getConfiguration(keyedConfiguredTarget.getConfigurationKey()))) .append(")"); @@ -74,9 +75,13 @@ public void processOutput(Iterable partialResult) { } private static ImmutableSortedSet requiredFragmentStrings( - ConfiguredTarget keyedConfiguredTarget) { + CqueryNode keyedConfiguredTarget) { + if (!(keyedConfiguredTarget instanceof ConfiguredTarget)) { + return ImmutableSortedSet.of(); + } + RequiredConfigFragmentsProvider requiredFragments = - keyedConfiguredTarget + ((ConfiguredTarget) keyedConfiguredTarget) .getProvider(RequiredConfigFragmentsProvider.class); if (requiredFragments == null) { return ImmutableSortedSet.of(); 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 4d1b1ed927566d..df4b277a9abf7b 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 @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.AnalysisProtosV2.Configuration; import com.google.devtools.build.lib.analysis.AnalysisProtosV2.CqueryResult; import com.google.devtools.build.lib.analysis.AnalysisProtosV2.CqueryResultOrBuilder; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; @@ -38,6 +37,7 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.cquery.CqueryOptions.Transitions; import com.google.devtools.build.lib.query2.cquery.CqueryTransitionResolver.EvaluateException; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; @@ -132,14 +132,14 @@ public ImmutableList getConfigurations() { private final Map partialResultMap; private final LabelPrinter labelPrinter; - private ConfiguredTarget currentTarget; + private CqueryNode currentTarget; ProtoOutputFormatterCallback( ExtendedEventHandler eventHandler, CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, AspectResolver resolver, OutputType outputType, RuleClassProvider ruleClassProvider, @@ -238,7 +238,7 @@ public String getName() { } @Override - public void processOutput(Iterable partialResult) + public void processOutput(Iterable partialResult) throws InterruptedException, IOException { partialResult.forEach( kct -> partialResultMap.put(kct.getOriginalLabel(), accessor.getTarget(kct))); @@ -253,7 +253,7 @@ public void processOutput(Iterable partialResult) ConfiguredProtoOutputFormatter formatter = new ConfiguredProtoOutputFormatter(); formatter.setOptions(options, resolver, skyframeExecutor.getDigestFunction().getHashFunction()); - for (ConfiguredTarget keyedConfiguredTarget : partialResult) { + for (CqueryNode keyedConfiguredTarget : partialResult) { AnalysisProtosV2.ConfiguredTarget.Builder builder = AnalysisProtosV2.ConfiguredTarget.newBuilder(); 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 a4ecf8707b5263..e744685c556ec5 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 @@ -17,7 +17,6 @@ import static com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition.COMMAND_LINE_OPTION_PREFIX; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.FragmentOptions; @@ -25,6 +24,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; import com.google.devtools.build.lib.query2.engine.QueryException; import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery; @@ -64,7 +64,7 @@ private class CqueryDialectGlobals { parameters = { @Param(name = "target"), }) - public Object buildOptions(ConfiguredTarget target) { + public Object buildOptions(CqueryNode target) { BuildConfigurationValue config = getConfiguration(target.getConfigurationKey()); if (config == null) { @@ -118,7 +118,7 @@ public Object buildOptions(ConfiguredTarget target) { parameters = { @Param(name = "target"), }) - public Object providers(ConfiguredTarget target) { + public Object providers(CqueryNode target) { Dict ret = target.getProvidersDictForQuery(); if (ret == null) { return Starlark.NONE; @@ -129,7 +129,7 @@ public Object providers(ConfiguredTarget target) { private static final Object[] NO_ARGS = new Object[0]; - // Starlark function with single required parameter "target", a ConfiguredTarget query result. + // Starlark function with single required parameter "target", a CqueryNode query result. private final StarlarkFunction formatFn; private final StarlarkSemantics starlarkSemantics; @@ -138,7 +138,7 @@ public Object providers(ConfiguredTarget target) { CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, StarlarkSemantics starlarkSemantics) throws QueryException, InterruptedException { super(eventHandler, options, out, skyframeExecutor, accessor, /* uniquifyResults= */ false); @@ -227,8 +227,8 @@ public String getName() { } @Override - public void processOutput(Iterable partialResult) throws InterruptedException { - for (ConfiguredTarget target : partialResult) { + public void processOutput(Iterable partialResult) throws InterruptedException { + for (CqueryNode target : partialResult) { try { StarlarkThread thread = new StarlarkThread(Mutability.create("cquery evaluation"), starlarkSemantics); 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 f3aa928a9ea57e..d55d1188bcb922 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 @@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.OptionsDiff; @@ -32,6 +31,7 @@ import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.RuleTransitionData; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.cquery.CqueryTransitionResolver.EvaluateException; import com.google.devtools.build.lib.query2.cquery.CqueryTransitionResolver.ResolvedTransition; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; @@ -63,7 +63,7 @@ public String getName() { CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor, - TargetAccessor accessor, + TargetAccessor accessor, RuleClassProvider ruleClassProvider, LabelPrinter labelPrinter) { super(eventHandler, options, out, skyframeExecutor, accessor, /* uniquifyResults= */ false); @@ -74,7 +74,7 @@ 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( @@ -85,7 +85,7 @@ public void processOutput(Iterable partialResult) throws Inter } partialResult.forEach( kct -> partialResultMap.put(kct.getOriginalLabel(), accessor.getTarget(kct))); - for (ConfiguredTarget keyedConfiguredTarget : partialResult) { + for (CqueryNode keyedConfiguredTarget : partialResult) { Target target = partialResultMap.get(keyedConfiguredTarget.getOriginalLabel()); BuildConfigurationValue config = getConfiguration(keyedConfiguredTarget.getConfigurationKey()); @@ -130,7 +130,7 @@ public void processOutput(Iterable partialResult) throws Inter } } - private static String getRuleClassTransition(ConfiguredTarget ct, Target target) { + private static String getRuleClassTransition(CqueryNode ct, Target target) { String output = ""; if (ct instanceof RuleConfiguredTarget) { TransitionFactory factory = diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java index 87b6e1e8c6a1ea..6ae3a13f962a71 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java @@ -647,7 +647,10 @@ enum Setting { NO_NODEP_DEPS, /** Include aspect-generated output. No-op for query, which always follows aspects. */ - INCLUDE_ASPECTS; + INCLUDE_ASPECTS, + + /** Include configured aspect targets in cquery output. */ + EXPLICIT_ASPECTS; } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/BUILD b/src/main/java/com/google/devtools/build/lib/rules/BUILD index 854929cbb53584..5964a715a522c1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/BUILD @@ -256,7 +256,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization", - "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", "//src/main/java/net/starlark/java/eval", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectKeyCreator.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectKeyCreator.java index 215fc406991b86..3f4d7fb2926835 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectKeyCreator.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectKeyCreator.java @@ -24,6 +24,8 @@ import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.AspectParameters; +import com.google.devtools.build.lib.packages.LabelPrinter; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -95,7 +97,7 @@ public final int hashCode() { * aspects and its {@code baseKeys} list will be empty. */ @AutoCodec - public abstract static class AspectKey extends AspectBaseKey { + public abstract static class AspectKey extends AspectBaseKey implements CqueryNode { private static final SkyKeyInterner interner = SkyKey.newInterner(); private final AspectDescriptor aspectDescriptor; @@ -145,6 +147,11 @@ static AspectKey createAspectKey( public abstract String getDescription(); + @Override + public String getDescription(LabelPrinter labelPrinter) { + return getDescription(); + } + @Override public SkyFunctionName functionName() { return SkyFunctions.ASPECT; @@ -165,6 +172,16 @@ public Label getLabel() { return getBaseConfiguredTargetKey().getLabel(); } + @Override + public SkyKeyInterner getSkyKeyInterner() { + return interner; + } + + @Override + public ActionLookupKey getLookupKey() { + return this; + } + public AspectClass getAspectClass() { return aspectDescriptor.getAspectClass(); } @@ -215,11 +232,15 @@ public String prettyPrint() { getLabel(), aspectDescriptor.getAspectClass().getName(), baseKeysString); } - @Override - public String toString() { + public String getAspectLabel() { return (getBaseKeys().isEmpty() ? getLabel() : getBaseKeys().toString()) + "#" - + aspectDescriptor + + aspectDescriptor; + } + + @Override + public String toString() { + return getAspectLabel() + " " + getBaseConfiguredTargetKey() + " " @@ -239,11 +260,6 @@ AspectKey withLabel(Label label) { aspectDescriptor); } - @Override - public SkyKeyInterner getSkyKeyInterner() { - return interner; - } - static class SimpleAspectKey extends AspectKey { SimpleAspectKey( ConfiguredTargetKey baseConfiguredTargetKey, @@ -282,7 +298,10 @@ public ImmutableList getBaseKeys() { @Override public String getDescription() { - return String.format("%s on top of %s", getAspectClass().getName(), baseKeys); + return String.format( + "%s on top of %s", + getAspectClass().getName(), + baseKeys.stream().map(AspectKey::getDescription).collect(toImmutableList())); } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 46a3c216e3b882..cb7fbf1a75f971 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -749,6 +749,8 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages:label_printer", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/skyframe/config", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", @@ -1079,8 +1081,8 @@ java_library( ":sky_functions", "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/skyframe/config", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/util:hash_codes", 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 9515df13bb1205..3f6cf2a9a6fa0e 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 @@ -20,9 +20,9 @@ import com.google.common.base.MoreObjects; import com.google.devtools.build.lib.actions.ActionLookupKey; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; import com.google.devtools.build.lib.skyframe.serialization.AsyncDeserializationContext; import com.google.devtools.build.lib.skyframe.serialization.DeferredObjectCodec; @@ -236,7 +236,7 @@ public static Builder builder() { } /** Returns the {@link ConfiguredTargetKey} that owns {@code configuredTarget}. */ - public static ConfiguredTargetKey fromConfiguredTarget(ConfiguredTarget configuredTarget) { + public static ConfiguredTargetKey fromConfiguredTarget(CqueryNode configuredTarget) { // If configuredTarget is a MergedConfiguredTarget unwraps it first. MergedConfiguredTarget is // ephemeral and does not have a directly corresponding entry in Skyframe. // diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD index 9c797dcc1318b5..e3c9f2d8a86af0 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD +++ b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD @@ -477,7 +477,6 @@ java_test( "//src/main/protobuf:invocation_policy_java_proto", "//src/test/java/com/google/devtools/build/lib/buildtool/util", "//src/test/java/com/google/devtools/build/lib/events:testutil", - "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", "//src/test/java/com/google/devtools/build/lib/testutil:TestUtils", "//src/test/java/com/google/devtools/build/skyframe:testutil", "//third_party:guava", 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 9e1d90b6515857..d3bd09d275d5a0 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 @@ -19,11 +19,11 @@ java_test( deps = [ ":configured_target_query_helper", ":configured_target_query_test", - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//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/packages:label_printer", "//src/main/java/com/google/devtools/build/lib/query2", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//src/main/java/com/google/devtools/build/lib/query2/query/aspectresolvers", "//src/main/java/com/google/devtools/build/lib/util:filetype", @@ -42,11 +42,10 @@ java_test( deps = [ ":configured_target_query_helper", ":configured_target_query_test", - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", - "//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:label_printer", "//src/main/java/com/google/devtools/build/lib/query2", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//third_party:guava", "//third_party:junit4", @@ -67,11 +66,12 @@ java_test( "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition", - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration", "//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/packages:label_printer", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//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", @@ -89,9 +89,9 @@ java_library( testonly = 1, srcs = ["ConfiguredTargetQueryHelper.java"], deps = [ - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/packages:label_printer", "//src/main/java/com/google/devtools/build/lib/query2", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", @@ -111,9 +111,9 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/split_transition", - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/query2", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//src/test/java/com/google/devtools/build/lib/analysis/util", "//src/test/java/com/google/devtools/build/lib/query2/testutil", @@ -133,11 +133,11 @@ java_test( ":configured_target_query_test", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory", - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//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/packages:label_printer", "//src/main/java/com/google/devtools/build/lib/query2", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//src/main/java/com/google/devtools/build/lib/query2/query/aspectresolvers", "//src/main/java/com/google/devtools/build/lib/util:filetype", @@ -164,12 +164,11 @@ java_test( "//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory", - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", - "//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/packages:label_printer", "//src/main/java/com/google/devtools/build/lib/query2", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//src/main/java/com/google/devtools/build/lib/util:filetype", "//src/test/java/com/google/devtools/build/lib/analysis/util", @@ -188,10 +187,10 @@ java_test( ":configured_target_query_helper", ":configured_target_query_test", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", - "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/query2", + "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//third_party:guava", "//third_party:junit4", 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 b5fb358560b7f8..fc3672a33eb77f 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,12 +19,12 @@ 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; import com.google.devtools.build.lib.packages.LabelPrinter; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting; import com.google.devtools.build.lib.query2.engine.QueryExpression; import com.google.devtools.build.lib.query2.engine.QueryParser; @@ -75,7 +75,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 b1be0899732587..9fe9d68a291531 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,10 +14,10 @@ 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.packages.LabelPrinter; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment.TopLevelConfigurations; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction; import com.google.devtools.build.lib.query2.testutil.AbstractQueryTest.QueryHelper; import com.google.devtools.build.lib.query2.testutil.PostAnalysisQueryHelper; @@ -32,7 +32,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, @@ -56,7 +56,7 @@ protected ConfiguredTargetQueryEnvironment getPostAnalysisQueryEnvironment( } @Override - public String getLabel(ConfiguredTarget target) { + public String getLabel(CqueryNode target) { return target.getOriginalLabel().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 2fac0a8be019d7..6f25d6b643f91e 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 @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.query2.cquery; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; @@ -23,7 +24,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.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.BuildOptionsView; @@ -38,6 +38,8 @@ import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.BuildType; +import com.google.devtools.build.lib.packages.LabelPrinter; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting; import com.google.devtools.build.lib.query2.engine.QueryException; import com.google.devtools.build.lib.server.FailureDetails; @@ -121,9 +123,8 @@ public void testLabelFunction_getCorrectlyConfiguredDeps() throws Exception { setUpLabelsFunctionTests(); // Test that this retrieves the correctly configured version(s) of the dep(s). - ConfiguredTarget patchDep = - Iterables.getOnlyElement(eval("labels('patch_dep', //test:my_rule)")); - ConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); + CqueryNode patchDep = Iterables.getOnlyElement(eval("labels('patch_dep', //test:my_rule)")); + CqueryNode myRule = Iterables.getOnlyElement(eval("//test:my_rule")); String targetConfiguration = myRule.getConfigurationChecksum(); assertThat(patchDep.getConfigurationChecksum()).doesNotMatch(targetConfiguration); } @@ -132,12 +133,12 @@ public void testLabelFunction_getCorrectlyConfiguredDeps() throws Exception { public void testLabelsFunction_splitTransitionAttribute() throws Exception { setUpLabelsFunctionTests(); - ConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); + CqueryNode myRule = Iterables.getOnlyElement(eval("//test:my_rule")); String targetConfiguration = myRule.getConfigurationChecksum(); - Set splitDeps = eval("labels('split_dep', //test:my_rule)"); + Set splitDeps = eval("labels('split_dep', //test:my_rule)"); assertThat(splitDeps).hasSize(2); - for (ConfiguredTarget kct : splitDeps) { + for (CqueryNode kct : splitDeps) { assertThat(kct.getConfigurationChecksum()).doesNotMatch(targetConfiguration); } } @@ -146,13 +147,13 @@ public void testLabelsFunction_splitTransitionAttribute() throws Exception { public void testLabelsFunction_labelListAttribute() throws Exception { setUpLabelsFunctionTests(); - ConfiguredTarget myRule = Iterables.getOnlyElement(eval("//test:my_rule")); + CqueryNode 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 kct : deps) { + for (CqueryNode kct : deps) { assertThat(kct.getConfigurationChecksum()).doesNotMatch(targetConfiguration); } } @@ -195,7 +196,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')")); + CqueryNode dep = Iterables.getOnlyElement(eval("labels('dep', '//test:alias')")); assertThat(dep.getLabel()).isEqualTo(Label.parseCanonicalUnchecked("//test:dep")); } @@ -223,8 +224,8 @@ 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")); + CqueryNode other = Iterables.getOnlyElement(eval("//test:other_my_rule")); + CqueryNode myRule = Iterables.getOnlyElement(eval("//test:my_rule")); // Note: {@link ConfiguredTarget#getLabel} returns the label of the "actual" value not the // label of the alias, so we need to check the underlying label. assertThat(other.getLabel()).isEqualTo(myRule.getLabel()); @@ -249,7 +250,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"); DummyTestOptions testOptions = getConfiguration(Iterables.getOnlyElement(ruleClass)) .getOptions() @@ -373,10 +374,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; BuildConfigurationValue myDepConfig = getConfiguration(stableOrderList.get(myDepIndex)); BuildConfigurationValue stringFlagConfig = @@ -401,11 +402,11 @@ 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); } @@ -414,7 +415,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); @@ -437,7 +438,7 @@ public void testConfig_exprArgumentFailure() throws Exception { EvalThrowsResult evalThrowsResult = evalThrows( "config(filter(\"??not-a-valid-regex\", //test:foo.java), null)", - /*unconditionallyThrows=*/ true); + /* unconditionallyThrows= */ true); assertThat(evalThrowsResult.getMessage()) .startsWith("illegal 'filter' pattern regexp '??not-a-valid-regex'"); assertThat(evalThrowsResult.getFailureDetail().hasQuery()).isTrue(); @@ -500,12 +501,12 @@ public void testRecursiveTargetPatternOutsideOfScopeFailsGracefully() throws Exc 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(); + CqueryNode first = resultIterator.next(); if (first.getLabel().toString().equals("//test:foo.java")) { assertThat(getConfiguration(first)).isNull(); assertThat(getConfiguration(resultIterator.next())).isNotNull(); @@ -535,7 +536,7 @@ 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)"); + Set result = eval("somepath(//test:buildme, //test:mydep)"); assertThat(result.stream().map(kct -> kct.getLabel().toString()).collect(Collectors.toList())) .contains("//test:mydep"); } @@ -581,8 +582,7 @@ public void testQueryHandlesDroppingFragments() throws Exception { "simple_rule(name='foo', deps = [':bar'])", "simple_rule(name='bar')"); - Set result = - eval("somepath(//test:top, filter(//test:bar, deps(//test:top)))"); + Set result = eval("somepath(//test:top, filter(//test:bar, deps(//test:top)))"); assertThat(result).isNotEmpty(); } @@ -601,7 +601,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); } @@ -623,7 +623,241 @@ 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); } + + @Test + public void testAspectDepsAppearInCqueryDeps() throws Exception { + writeFile( + "donut/test.bzl", + "TestAspectInfo = provider('TestAspectInfo', fields = ['info'])", + "def _test_aspect_impl(target, ctx):", + " return [", + " TestAspectInfo(", + " info = depset([target.label]),", + " ),", + " ]", + "", + "_test_aspect = aspect(", + " implementation = _test_aspect_impl,", + " attr_aspects = ['deps'],", + " attrs = {", + " '_test_attr': attr.label(", + " allow_files = True,", + " default = Label('//donut:test_filegroup'),", + " ),", + " },", + " provides = [TestAspectInfo],", + ")", + "def _test_impl(ctx):", + " pass", + "test_rule = rule(", + " _test_impl,", + " attrs = {", + " 'deps': attr.label_list(", + " aspects = [_test_aspect],", + " ),", + " },", + ")"); + writeFile( + "donut/BUILD", + "load(':test.bzl', 'test_rule')", + "filegroup(", + " name = 'test_filegroup',", + " srcs = ['test.bzl'],", + ")", + "test_rule(", + " name = 'test_rule_dep',", + ")", + "test_rule(", + " name = 'test_rule',", + " deps = [':test_rule_dep'],", + ")"); + + helper.setQuerySettings(Setting.INCLUDE_ASPECTS, Setting.EXPLICIT_ASPECTS); + var result = + eval("filter(//donut, deps(//donut:test_rule))").stream() + .map(cf -> cf.getDescription(LabelPrinter.legacy())) + .collect(ImmutableList.toImmutableList()); + assertThat(result) + .containsExactly( + "//donut:test_rule", + "//donut:test_rule_dep", + "//donut:test.bzl%_test_aspect of //donut:test_rule_dep", + "//donut:test.bzl", + "//donut:test_filegroup"); + } + + @Test + public void testAspectOnAspectDepsAppearInCqueryDeps() throws Exception { + writeFile( + "donut/test.bzl", + "TestAspectInfo = provider('TestAspectInfo', fields = ['info'])", + "TestAspectOnAspectInfo = provider('TestAspectOnAspectInfo', fields = ['info'])", + "def _test_aspect_impl(target, ctx):", + " return [", + " TestAspectInfo(", + " info = depset([target.label]),", + " ),", + " ]", + "_test_aspect = aspect(", + " implementation = _test_aspect_impl,", + " attr_aspects = ['deps'],", + " attrs = {", + " '_test_attr': attr.label(", + " allow_files = True,", + " default = Label('//donut:test_aspect_filegroup'),", + " ),", + " },", + " provides = [TestAspectInfo],", + ")", + "def _test_aspect_on_aspect_impl(target, ctx):", + " return [", + " TestAspectOnAspectInfo(", + " info = depset(", + " direct = [target.label],", + " transitive = [target[TestAspectInfo].info],", + " ),", + " ),", + " ]", + "_test_aspect_on_aspect = aspect(", + " implementation = _test_aspect_on_aspect_impl,", + " attr_aspects = ['deps'],", + " attrs = {", + " '_test_attr': attr.label(", + " allow_files = True,", + " default = Label('//donut:test_aspect_on_aspect_filegroup'),", + " ),", + " },", + " required_aspect_providers = [TestAspectInfo],", + " provides = [TestAspectOnAspectInfo],", + ")", + "def _test_impl(ctx):", + " pass", + "test_rule = rule(", + " _test_impl,", + " attrs = {", + " 'deps': attr.label_list(", + " aspects = [_test_aspect],", + " ),", + " },", + ")", + "def _test_aspect_on_aspect_rule_impl(ctx):", + " pass", + "test_aspect_on_aspect_rule = rule(", + " _test_aspect_on_aspect_rule_impl,", + " attrs = {", + " 'deps': attr.label_list(", + " aspects = [_test_aspect, _test_aspect_on_aspect],", + " ),", + " },", + ")"); + writeFile("donut/test_aspect.file"); + writeFile("donut/test_aspect_on_aspect.file"); + writeFile( + "donut/BUILD", + "load(':test.bzl', 'test_rule', 'test_aspect_on_aspect_rule')", + "filegroup(", + " name = 'test_aspect_filegroup',", + " srcs = ['test_aspect.file'],", + ")", + "filegroup(", + " name = 'test_aspect_on_aspect_filegroup',", + " srcs = ['test_aspect_on_aspect.file'],", + ")", + "test_rule(", + " name = 'test_rule_dep',", + ")", + "test_rule(", + " name = 'test_rule',", + " deps = [':test_rule_dep'],", + ")", + "test_aspect_on_aspect_rule(", + " name = 'test_aspect_on_aspect_rule',", + " deps = ['test_rule'],", + ")"); + + helper.setUniverseScope("//donut/..."); + helper.setQuerySettings(Setting.INCLUDE_ASPECTS, Setting.EXPLICIT_ASPECTS); + var result = + eval("filter(//donut, deps(//donut:test_aspect_on_aspect_rule))").stream() + .map(cf -> cf.getDescription(LabelPrinter.legacy())) + .collect(toImmutableList()); + assertThat(result) + .containsExactly( + "//donut:test.bzl%_test_aspect_on_aspect on top of" + + " [//donut:test.bzl%_test_aspect of //donut:test_rule_dep]", + "//donut:test.bzl%_test_aspect_on_aspect on top of" + + " [//donut:test.bzl%_test_aspect of //donut:test_rule]", + "//donut:test_rule_dep", + "//donut:test_rule", + "//donut:test.bzl%_test_aspect of //donut:test_rule_dep", + "//donut:test.bzl%_test_aspect of //donut:test_rule", + "//donut:test_aspect_on_aspect_rule", + "//donut:test_aspect.file", + "//donut:test_aspect_on_aspect_filegroup", + "//donut:test_aspect_on_aspect.file", + "//donut:test_aspect_filegroup"); + } + + @Test + public void testAspectDepsAppearInCqueryRdeps() throws Exception { + writeFile( + "donut/test.bzl", + "TestAspectInfo = provider('TestAspectInfo', fields = ['info'])", + "def _test_aspect_impl(target, ctx):", + " return [", + " TestAspectInfo(", + " info = depset([target.label]),", + " ),", + " ]", + "", + "_test_aspect = aspect(", + " implementation = _test_aspect_impl,", + " attr_aspects = ['deps'],", + " attrs = {", + " '_test_attr': attr.label(", + " allow_files = True,", + " default = Label('//donut:test_filegroup'),", + " ),", + " },", + " provides = [TestAspectInfo],", + ")", + "def _test_impl(ctx):", + " pass", + "test_rule = rule(", + " _test_impl,", + " attrs = {", + " 'deps': attr.label_list(", + " aspects = [_test_aspect],", + " ),", + " },", + ")"); + writeFile( + "donut/BUILD", + "load(':test.bzl', 'test_rule')", + "filegroup(", + " name = 'test_filegroup',", + " srcs = ['test.bzl'],", + ")", + "test_rule(", + " name = 'test_rule_dep',", + ")", + "test_rule(", + " name = 'test_rule',", + " deps = [':test_rule_dep'],", + ")"); + + helper.setQuerySettings(Setting.INCLUDE_ASPECTS, Setting.EXPLICIT_ASPECTS); + var result = + eval("rdeps(//donut/..., //donut:test_filegroup)").stream() + .map(cf -> cf.getDescription(LabelPrinter.legacy())) + .collect(toImmutableList()); + assertThat(result) + .containsExactly( + "//donut:test_filegroup", + "//donut:test_rule", + "//donut:test.bzl%_test_aspect of //donut:test_rule_dep"); + } } 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 75176014fb2645..ed8e1af4932b0f 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.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.BuildOptionsView; @@ -26,6 +25,7 @@ import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition; import com.google.devtools.build.lib.analysis.util.DummyTestFragment.DummyTestOptions; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction; import com.google.devtools.build.lib.query2.testutil.PostAnalysisQueryTest; import java.util.HashMap; @@ -38,10 +38,10 @@ /** 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() { return new ConfiguredTargetQueryHelper(); } @@ -57,7 +57,7 @@ public HashMap getDefaultFunctions() { } @Override - protected final BuildConfigurationValue getConfiguration(ConfiguredTarget kct) { + protected final BuildConfigurationValue getConfiguration(CqueryNode kct) { return getHelper() .getSkyframeExecutor() .getConfiguration(getHelper().getReporter(), kct.getConfigurationKey()); @@ -93,12 +93,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(); + CqueryNode 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/FilesOutputFormatterCallbackTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallbackTest.java index 89c17b687f9f99..a2b0a929371635 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallbackTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallbackTest.java @@ -18,13 +18,13 @@ 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.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.OutputGroupInfo.ValidationMode; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryExpression; import com.google.devtools.build.lib.query2.engine.QueryParser; import java.io.ByteArrayOutputStream; @@ -112,7 +112,7 @@ private List getOutput(String queryExpression, List outputGroups QueryExpression expression = QueryParser.parse(queryExpression, getDefaultFunctions()); Set targetPatternSet = new LinkedHashSet<>(); expression.collectTargetPatterns(targetPatternSet); - 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/GraphOutputFormatterCallbackTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java index d6b2c38aec4b57..8c1ebc9e2c77e8 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,11 +17,11 @@ 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.packages.LabelPrinter; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting; import com.google.devtools.build.lib.query2.engine.QueryExpression; import com.google.devtools.build.lib.query2.engine.QueryParser; @@ -71,7 +71,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 4d685137abecd6..9fe3f5bbc55c03 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 @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.analysis.AnalysisProtosV2; import com.google.devtools.build.lib.analysis.AnalysisProtosV2.Configuration; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory; import com.google.devtools.build.lib.analysis.util.MockRule; import com.google.devtools.build.lib.events.Event; @@ -34,6 +33,7 @@ import com.google.devtools.build.lib.packages.LabelPrinter; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.cquery.CqueryOptions.Transitions; import com.google.devtools.build.lib.query2.cquery.ProtoOutputFormatterCallback.OutputType; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting; @@ -189,9 +189,9 @@ public void testConfigurations() throws Exception { AnalysisProtosV2.ConfiguredTarget parentRuleProto = getRuleProtoByName(resultsList, "//test:parent_rule"); - Set keyedTargets = eval("deps(//test:parent_rule)"); + Set keyedTargets = eval("deps(//test:parent_rule)"); - ConfiguredTarget parentRule = getKeyedTargetByLabel(keyedTargets, "//test:parent_rule"); + CqueryNode parentRule = getKeyedTargetByLabel(keyedTargets, "//test:parent_rule"); assertThat(parentRuleProto.getConfiguration().getChecksum()) .isEqualTo(parentRule.getConfigurationChecksum()); @@ -211,7 +211,7 @@ public void testConfigurations() throws Exception { AnalysisProtosV2.ConfiguredTarget transitionRuleProto = getRuleProtoByName(resultsList, "//test:transition_rule"); - ConfiguredTarget transitionRule = getKeyedTargetByLabel(keyedTargets, "//test:transition_rule"); + CqueryNode transitionRule = getKeyedTargetByLabel(keyedTargets, "//test:transition_rule"); assertThat(transitionRuleProto.getConfiguration().getChecksum()) .isEqualTo(transitionRule.getConfigurationChecksum()); @@ -227,7 +227,7 @@ public void testConfigurations() throws Exception { assertThat(depRuleConfiguration.getMnemonic()).matches("k8-opt-exec-.*"); assertThat(depRuleConfiguration.getIsTool()).isTrue(); - ConfiguredTarget depRule = getKeyedTargetByLabel(keyedTargets, "//test:dep"); + CqueryNode depRule = getKeyedTargetByLabel(keyedTargets, "//test:dep"); assertThat(depRuleProto.getConfiguration().getChecksum()) .isEqualTo(depRule.getConfigurationChecksum()); @@ -263,7 +263,7 @@ public void testConfigurations() throws Exception { .containsExactly(patchedConfiguredRuleInput, depConfiguredRuleInput); } - private ConfiguredTarget getKeyedTargetByLabel(Set keyedTargets, String label) { + private CqueryNode getKeyedTargetByLabel(Set keyedTargets, String label) { return Iterables.getOnlyElement( keyedTargets.stream() .filter(t -> label.equals(t.getLabel().getCanonicalForm())) @@ -486,7 +486,7 @@ private InputStream queryAndGetInputStream( Set targetPatternSet = new LinkedHashSet<>(); expression.collectTargetPatterns(targetPatternSet); helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); - PostAnalysisQueryEnvironment env = + PostAnalysisQueryEnvironment env = ((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet); ByteArrayOutputStream out = new ByteArrayOutputStream(); 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 6876b3530df418..1f2982fe2626e2 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; @@ -30,6 +29,7 @@ import com.google.devtools.build.lib.packages.LabelPrinter; import com.google.devtools.build.lib.packages.RuleTransitionData; import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment; +import com.google.devtools.build.lib.query2.common.CqueryNode; import com.google.devtools.build.lib.query2.cquery.CqueryOptions.Transitions; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting; import com.google.devtools.build.lib.query2.engine.QueryExpression; @@ -235,7 +235,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(blaze-configurability): Test late-bound attributes.