Skip to content

Commit 0662df0

Browse files
Wyveraldcopybara-github
Wyverald
authored and
copybara-github
committed
aquery: interpret "//foo:bar" as "all configured targets with label //foo:bar"
cquery acquired this behavior 4 years ago in unknown commit, but it was never ported to aquery. This fixes some potentially surprising behavior when multiple configured targets corresponding to the same label are present (see added test cases). RELNOTES: aquery: `//foo:bar` now means "all configured targets with label `//foo:bar`" instead of "choose an arbitrary configured target with label `//foo:bar`". This is in line with cquery behavior. PiperOrigin-RevId: 620091100 Change-Id: Ib5c5ee33e35fe7ac30bc31f703b119dec40185b7
1 parent 70bb1da commit 0662df0

File tree

13 files changed

+139
-88
lines changed

13 files changed

+139
-88
lines changed

‎src/main/java/com/google/devtools/build/lib/buildtool/AqueryProcessor.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
package com.google.devtools.build.lib.buildtool;
1515

1616
import com.google.common.collect.ImmutableList;
17+
import com.google.common.collect.ImmutableMap;
1718
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
1819
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
1920
import com.google.devtools.build.lib.analysis.actions.TemplateExpansionException;
21+
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
2022
import com.google.devtools.build.lib.cmdline.TargetPattern;
2123
import com.google.devtools.build.lib.events.Event;
2224
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
@@ -45,11 +47,9 @@
4547
import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryOutputHandler;
4648
import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryOutputHandler.OutputType;
4749
import com.google.devtools.build.lib.skyframe.actiongraph.v2.InvalidAqueryOutputFormatException;
48-
import com.google.devtools.build.skyframe.SkyKey;
4950
import com.google.devtools.build.skyframe.WalkableGraph;
5051
import java.io.IOException;
5152
import java.io.PrintStream;
52-
import java.util.Collection;
5353
import java.util.Optional;
5454
import java.util.regex.Pattern;
5555
import java.util.regex.PatternSyntaxException;
@@ -140,7 +140,7 @@ protected PostAnalysisQueryEnvironment<ConfiguredTargetValue> getQueryEnvironmen
140140
BuildRequest request,
141141
CommandEnvironment env,
142142
TopLevelConfigurations topLevelConfigurations,
143-
Collection<SkyKey> transitiveConfigurationKeys,
143+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations,
144144
WalkableGraph walkableGraph) {
145145
ImmutableList<QueryFunction> extraFunctions =
146146
new ImmutableList.Builder<QueryFunction>()
@@ -158,6 +158,7 @@ protected PostAnalysisQueryEnvironment<ConfiguredTargetValue> getQueryEnvironmen
158158
env.getReporter(),
159159
extraFunctions,
160160
topLevelConfigurations,
161+
transitiveConfigurations,
161162
mainRepoTargetParser,
162163
env.getPackageManager().getPackagePath(),
163164
() -> walkableGraph,

‎src/main/java/com/google/devtools/build/lib/buildtool/CqueryProcessor.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
package com.google.devtools.build.lib.buildtool;
1515

1616
import com.google.common.collect.ImmutableList;
17+
import com.google.common.collect.ImmutableMap;
18+
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
1719
import com.google.devtools.build.lib.cmdline.TargetPattern;
1820
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
1921
import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment.TopLevelConfigurations;
@@ -23,9 +25,7 @@
2325
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
2426
import com.google.devtools.build.lib.query2.engine.QueryExpression;
2527
import com.google.devtools.build.lib.runtime.CommandEnvironment;
26-
import com.google.devtools.build.skyframe.SkyKey;
2728
import com.google.devtools.build.skyframe.WalkableGraph;
28-
import java.util.Collection;
2929
import net.starlark.java.eval.StarlarkSemantics;
3030

3131
/** Performs {@code cquery} processing. */
@@ -41,7 +41,7 @@ protected ConfiguredTargetQueryEnvironment getQueryEnvironment(
4141
BuildRequest request,
4242
CommandEnvironment env,
4343
TopLevelConfigurations configurations,
44-
Collection<SkyKey> transitiveConfigurationKeys,
44+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations,
4545
WalkableGraph walkableGraph)
4646
throws InterruptedException {
4747
ImmutableList<QueryFunction> extraFunctions =
@@ -58,7 +58,7 @@ protected ConfiguredTargetQueryEnvironment getQueryEnvironment(
5858
env.getReporter(),
5959
extraFunctions,
6060
configurations,
61-
transitiveConfigurationKeys,
61+
transitiveConfigurations,
6262
mainRepoTargetParser,
6363
env.getPackageManager().getPackagePath(),
6464
() -> walkableGraph,

‎src/main/java/com/google/devtools/build/lib/buildtool/PostAnalysisQueryProcessor.java‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.buildtool;
1515

16+
import static com.google.common.collect.ImmutableMap.toImmutableMap;
17+
18+
import com.google.common.collect.ImmutableMap;
1619
import com.google.devtools.build.lib.analysis.AnalysisResult;
1720
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
21+
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
1822
import com.google.devtools.build.lib.buildtool.BuildTool.ExitException;
1923
import com.google.devtools.build.lib.cmdline.TargetPattern;
2024
import com.google.devtools.build.lib.events.Event;
@@ -42,7 +46,9 @@
4246
import com.google.devtools.common.options.OptionsParsingException;
4347
import java.io.IOException;
4448
import java.util.Collection;
49+
import java.util.Comparator;
4550
import java.util.Set;
51+
import java.util.function.Function;
4652

4753
/**
4854
* Version of {@link BuildTool} that handles all work for queries based on results from the analysis
@@ -129,10 +135,21 @@ protected abstract PostAnalysisQueryEnvironment<T> getQueryEnvironment(
129135
BuildRequest request,
130136
CommandEnvironment env,
131137
TopLevelConfigurations topLevelConfigurations,
132-
Collection<SkyKey> transitiveConfigurationKeys,
138+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations,
133139
WalkableGraph walkableGraph)
134140
throws InterruptedException;
135141

142+
private static ImmutableMap<String, BuildConfigurationValue> getTransitiveConfigurations(
143+
Collection<SkyKey> transitiveConfigurationKeys, WalkableGraph graph)
144+
throws InterruptedException {
145+
// BuildConfigurationKey and BuildConfigurationValue should be 1:1
146+
// so merge function intentionally omitted
147+
return graph.getSuccessfulValues(transitiveConfigurationKeys).values().stream()
148+
.map(BuildConfigurationValue.class::cast)
149+
.sorted(Comparator.comparing(BuildConfigurationValue::checksum))
150+
.collect(toImmutableMap(BuildConfigurationValue::checksum, Function.identity()));
151+
}
152+
136153
private void doPostAnalysisQuery(
137154
BuildRequest request,
138155
CommandEnvironment env,
@@ -145,14 +162,12 @@ private void doPostAnalysisQuery(
145162
OptionsParsingException {
146163
WalkableGraph walkableGraph =
147164
SkyframeExecutorWrappingWalkableGraph.of(env.getSkyframeExecutor());
165+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations =
166+
getTransitiveConfigurations(transitiveConfigurationKeys, walkableGraph);
148167

149168
PostAnalysisQueryEnvironment<T> postAnalysisQueryEnvironment =
150169
getQueryEnvironment(
151-
request,
152-
env,
153-
topLevelConfigurations,
154-
transitiveConfigurationKeys,
155-
walkableGraph);
170+
request, env, topLevelConfigurations, transitiveConfigurations, walkableGraph);
156171

157172
Iterable<NamedThreadSafeOutputFormatterCallback<T>> callbacks =
158173
postAnalysisQueryEnvironment.getDefaultOutputFormatters(

‎src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,39 @@ public abstract class PostAnalysisQueryEnvironment<T> extends AbstractBlazeQuery
115115
private final Supplier<WalkableGraph> walkableGraphSupplier;
116116
protected WalkableGraph graph;
117117

118+
/**
119+
* Stores every configuration in the transitive closure of the build graph as a map from its
120+
* user-friendly hash to the configuration itself.
121+
*
122+
* <p>This is used to find configured targets in, e.g. {@code somepath} queries. Given {@code
123+
* somepath(//foo, //bar)}, cquery finds the configured targets for {@code //foo} and {@code
124+
* //bar} by creating a {@link ConfiguredTargetKey} from their labels and <i>some</i>
125+
* configuration, then querying the {@link WalkableGraph} to find the matching configured target.
126+
*
127+
* <p>Having this map lets cquery choose from all available configurations in the graph,
128+
* particularly including configurations that aren't the top-level.
129+
*
130+
* <p>This can also be used in cquery's {@code config} function to match against explicitly
131+
* specified configs. This, in particular, is where having user-friendly hashes is invaluable.
132+
*/
133+
protected final ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations;
134+
118135
protected RecursivePackageProviderBackedTargetPatternResolver resolver;
119136

120137
public PostAnalysisQueryEnvironment(
121138
boolean keepGoing,
122139
ExtendedEventHandler eventHandler,
123140
Iterable<QueryFunction> extraFunctions,
124141
TopLevelConfigurations topLevelConfigurations,
142+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations,
125143
TargetPattern.Parser mainRepoTargetParser,
126144
PathPackageLocator pkgPath,
127145
Supplier<WalkableGraph> walkableGraphSupplier,
128146
Set<Setting> settings,
129147
LabelPrinter labelPrinter) {
130148
super(keepGoing, true, Rule.ALL_LABELS, eventHandler, settings, extraFunctions, labelPrinter);
131149
this.topLevelConfigurations = topLevelConfigurations;
150+
this.transitiveConfigurations = transitiveConfigurations;
132151
this.mainRepoTargetParser = mainRepoTargetParser;
133152
this.pkgPath = pkgPath;
134153
this.walkableGraphSupplier = walkableGraphSupplier;

‎src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import com.google.common.base.Preconditions;
1717
import com.google.common.collect.ImmutableList;
18+
import com.google.common.collect.ImmutableMap;
1819
import com.google.common.collect.ImmutableSet;
1920
import com.google.common.util.concurrent.AsyncFunction;
2021
import com.google.common.util.concurrent.Futures;
@@ -79,6 +80,7 @@ public ActionGraphQueryEnvironment(
7980
ExtendedEventHandler eventHandler,
8081
Iterable<QueryFunction> extraFunctions,
8182
TopLevelConfigurations topLevelConfigurations,
83+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations,
8284
TargetPattern.Parser mainRepoTargetParser,
8385
PathPackageLocator pkgPath,
8486
Supplier<WalkableGraph> walkableGraphSupplier,
@@ -89,6 +91,7 @@ public ActionGraphQueryEnvironment(
8991
eventHandler,
9092
extraFunctions,
9193
topLevelConfigurations,
94+
transitiveConfigurations,
9295
mainRepoTargetParser,
9396
pkgPath,
9497
walkableGraphSupplier,
@@ -105,6 +108,7 @@ public ActionGraphQueryEnvironment(
105108
ExtendedEventHandler eventHandler,
106109
Iterable<QueryFunction> extraFunctions,
107110
TopLevelConfigurations topLevelConfigurations,
111+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations,
108112
TargetPattern.Parser mainRepoTargetParser,
109113
PathPackageLocator pkgPath,
110114
Supplier<WalkableGraph> walkableGraphSupplier,
@@ -115,6 +119,7 @@ public ActionGraphQueryEnvironment(
115119
eventHandler,
116120
extraFunctions,
117121
topLevelConfigurations,
122+
transitiveConfigurations,
118123
mainRepoTargetParser,
119124
pkgPath,
120125
walkableGraphSupplier,
@@ -313,11 +318,7 @@ public QueryTaskFuture<Void> getTargetsMatchingPattern(
313318
partialResult -> {
314319
List<ConfiguredTargetValue> transformedResult = new ArrayList<>();
315320
for (Target target : partialResult) {
316-
ConfiguredTargetValue configuredTargetValue =
317-
getConfiguredTargetValue(target.getLabel());
318-
if (configuredTargetValue != null) {
319-
transformedResult.add(configuredTargetValue);
320-
}
321+
transformedResult.addAll(getConfiguredTargetsForLabel(target.getLabel()));
321322
}
322323
callback.process(transformedResult);
323324
},
@@ -327,14 +328,27 @@ public QueryTaskFuture<Void> getTargetsMatchingPattern(
327328
MoreExecutors.directExecutor()));
328329
}
329330

330-
private ConfiguredTargetValue getConfiguredTargetValue(Label label) throws InterruptedException {
331-
// Try with target configuration.
332-
ConfiguredTargetValue configuredTargetValue = getTargetConfiguredTarget(label);
333-
if (configuredTargetValue != null) {
334-
return configuredTargetValue;
331+
/**
332+
* Returns all configured targets in Skyframe with the given label.
333+
*
334+
* <p>If there are no matches, returns an empty list.
335+
*/
336+
private ImmutableList<ConfiguredTargetValue> getConfiguredTargetsForLabel(Label label)
337+
throws InterruptedException {
338+
ImmutableList.Builder<ConfiguredTargetValue> ans = ImmutableList.builder();
339+
for (BuildConfigurationValue config : transitiveConfigurations.values()) {
340+
ConfiguredTargetValue configuredTargetValue =
341+
createConfiguredTargetValueFromKey(
342+
ConfiguredTargetKey.builder().setLabel(label).setConfiguration(config).build());
343+
if (configuredTargetValue != null) {
344+
ans.add(configuredTargetValue);
345+
}
346+
}
347+
ConfiguredTargetValue nullConfiguredTarget = getNullConfiguredTarget(label);
348+
if (nullConfiguredTarget != null) {
349+
ans.add(nullConfiguredTarget);
335350
}
336-
// Last chance: source file.
337-
return getNullConfiguredTarget(label);
351+
return ans.build();
338352
}
339353

340354
@Override

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

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.query2.cquery;
1515

16-
import static com.google.common.collect.ImmutableMap.toImmutableMap;
17-
1816
import com.google.common.base.Joiner;
1917
import com.google.common.base.Verify;
2018
import com.google.common.collect.ImmutableList;
@@ -62,12 +60,9 @@
6260
import com.google.devtools.build.skyframe.WalkableGraph;
6361
import java.io.OutputStream;
6462
import java.util.ArrayList;
65-
import java.util.Collection;
66-
import java.util.Comparator;
6763
import java.util.List;
6864
import java.util.Objects;
6965
import java.util.Set;
70-
import java.util.function.Function;
7166
import java.util.function.Supplier;
7267
import javax.annotation.Nullable;
7368
import net.starlark.java.eval.StarlarkSemantics;
@@ -94,23 +89,6 @@ public class ConfiguredTargetQueryEnvironment extends PostAnalysisQueryEnvironme
9489

9590
private final ConfiguredTargetAccessor accessor;
9691

97-
/**
98-
* F Stores every configuration in the transitive closure of the build graph as a map from its
99-
* user-friendly hash to the configuration itself.
100-
*
101-
* <p>This is used to find configured targets in, e.g. {@code somepath} queries. Given {@code
102-
* somepath(//foo, //bar)}, cquery finds the configured targets for {@code //foo} and {@code
103-
* //bar} by creating a {@link ConfiguredTargetKey} from their labels and <i>some</i>
104-
* configuration, then querying the {@link WalkableGraph} to find the matching configured target.
105-
*
106-
* <p>Having this map lets cquery choose from all available configurations in the graph,
107-
* particularly including configurations that aren't the top-level.
108-
*
109-
* <p>This can also be used in cquery's {@code config} function to match against explicitly
110-
* specified configs. This, in particular, is where having user-friendly hashes is invaluable.
111-
*/
112-
private final ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations;
113-
11492
@Override
11593
protected KeyExtractor<CqueryNode, ActionLookupKey> getConfiguredTargetKeyExtractor() {
11694
return configuredTargetKeyExtractor;
@@ -121,7 +99,7 @@ public ConfiguredTargetQueryEnvironment(
12199
ExtendedEventHandler eventHandler,
122100
Iterable<QueryFunction> extraFunctions,
123101
TopLevelConfigurations topLevelConfigurations,
124-
Collection<SkyKey> transitiveConfigurationKeys,
102+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations,
125103
TargetPattern.Parser mainRepoTargetParser,
126104
PathPackageLocator pkgPath,
127105
Supplier<WalkableGraph> walkableGraphSupplier,
@@ -134,15 +112,14 @@ public ConfiguredTargetQueryEnvironment(
134112
eventHandler,
135113
extraFunctions,
136114
topLevelConfigurations,
115+
transitiveConfigurations,
137116
mainRepoTargetParser,
138117
pkgPath,
139118
walkableGraphSupplier,
140119
settings,
141120
labelPrinter);
142121
this.accessor = new ConfiguredTargetAccessor(walkableGraphSupplier.get(), this);
143122
this.configuredTargetKeyExtractor = CqueryNode::getLookupKey;
144-
this.transitiveConfigurations =
145-
getTransitiveConfigurations(transitiveConfigurationKeys, walkableGraphSupplier.get());
146123
this.topLevelArtifactContext = topLevelArtifactContext;
147124
}
148125

@@ -151,7 +128,7 @@ public ConfiguredTargetQueryEnvironment(
151128
ExtendedEventHandler eventHandler,
152129
Iterable<QueryFunction> extraFunctions,
153130
TopLevelConfigurations topLevelConfigurations,
154-
Collection<SkyKey> transitiveConfigurationKeys,
131+
ImmutableMap<String, BuildConfigurationValue> transitiveConfigurations,
155132
TargetPattern.Parser mainRepoTargetParser,
156133
PathPackageLocator pkgPath,
157134
Supplier<WalkableGraph> walkableGraphSupplier,
@@ -164,7 +141,7 @@ public ConfiguredTargetQueryEnvironment(
164141
eventHandler,
165142
extraFunctions,
166143
topLevelConfigurations,
167-
transitiveConfigurationKeys,
144+
transitiveConfigurations,
168145
mainRepoTargetParser,
169146
pkgPath,
170147
walkableGraphSupplier,
@@ -185,17 +162,6 @@ private static ImmutableList<QueryFunction> getCqueryFunctions() {
185162
return ImmutableList.of(new ConfigFunction());
186163
}
187164

188-
private static ImmutableMap<String, BuildConfigurationValue> getTransitiveConfigurations(
189-
Collection<SkyKey> transitiveConfigurationKeys, WalkableGraph graph)
190-
throws InterruptedException {
191-
// BuildConfigurationKey and BuildConfigurationValue should be 1:1
192-
// so merge function intentionally omitted
193-
return graph.getSuccessfulValues(transitiveConfigurationKeys).values().stream()
194-
.map(BuildConfigurationValue.class::cast)
195-
.sorted(Comparator.comparing(BuildConfigurationValue::checksum))
196-
.collect(toImmutableMap(BuildConfigurationValue::checksum, Function.identity()));
197-
}
198-
199165
@Override
200166
public ImmutableList<NamedThreadSafeOutputFormatterCallback<CqueryNode>>
201167
getDefaultOutputFormatters(
@@ -318,8 +284,7 @@ public QueryTaskFuture<Void> getTargetsMatchingPattern(
318284
partialResult -> {
319285
List<CqueryNode> transformedResult = new ArrayList<>();
320286
for (Target target : partialResult) {
321-
transformedResult.addAll(
322-
getConfiguredTargetsForConfigFunction(target.getLabel()));
287+
transformedResult.addAll(getConfiguredTargetsForLabel(target.getLabel()));
323288
}
324289
callback.process(transformedResult);
325290
},
@@ -376,7 +341,7 @@ protected CqueryNode getValueFromKey(SkyKey key) throws InterruptedException {
376341
*
377342
* <p>If there are no matches, returns an empty list.
378343
*/
379-
private ImmutableList<CqueryNode> getConfiguredTargetsForConfigFunction(Label label)
344+
private ImmutableList<CqueryNode> getConfiguredTargetsForLabel(Label label)
380345
throws InterruptedException {
381346
ImmutableList.Builder<CqueryNode> ans = ImmutableList.builder();
382347
for (BuildConfigurationValue config : transitiveConfigurations.values()) {

0 commit comments

Comments
 (0)