Automated rollback of commit 607d0f7335f95aa0ee236ba3c18ce2a232370cdb.
*** Reason for rollback ***
It changes the output format of --output=jsonproto to ndjson (i.e. which prints out ConfiguredTarget(s) and Configuration(s) separated by a newline) instead of being wrapped by a CqueryResult. Caught by https://github.com/bazelbuild/rules_go/issues/3597.
*** Original change description ***
Add new output format for cquery `--output=streamed_proto`.
* The current state of the output formats of cquery has a few forms now:
- (UNCHANGED) `cquery --output=proto|jsonproto|textproto --proto:include_configurations`: A single CqueryResult of the specified `--output` format.
- (NEW) `cquery --output=streamed_proto --proto:include_configurations`: Multiple length-delimited `CqueryResult` protos each containing a single `ConfiguredTarget` or `Configuration`.
- (UNCHANGED) `cquery --out...
***
PiperOrigin-RevId: 543767534
Change-Id: Ib63ed4b5bc2b8e4823a4f935c1f7a32a22a80bcc
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 b0eb759..a799e04 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
@@ -53,7 +53,6 @@
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.WalkableGraph;
-import com.google.protobuf.CodedOutputStream;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Collection;
@@ -78,12 +77,6 @@
/** Cquery specific functions. */
public static final ImmutableList<QueryFunction> CQUERY_FUNCTIONS = getCqueryFunctions();
- /**
- * Pseudo-arbitrarily chosen buffer size for output. Chosen to be large enough to fit a handful of
- * messages without needing to flush to the underlying output, which may not be buffered.
- */
- private static final int OUTPUT_BUFFER_SIZE = 16384;
-
private CqueryOptions cqueryOptions;
private final TopLevelArtifactContext topLevelArtifactContext;
@@ -231,7 +224,6 @@
eventHandler,
cqueryOptions,
out,
- CodedOutputStream.newInstance(out, OUTPUT_BUFFER_SIZE),
skyframeExecutor,
accessor,
aspectResolver,
@@ -241,17 +233,6 @@
eventHandler,
cqueryOptions,
out,
- CodedOutputStream.newInstance(out, OUTPUT_BUFFER_SIZE),
- skyframeExecutor,
- accessor,
- aspectResolver,
- OutputType.DELIMITED_BINARY,
- ruleClassProvider),
- new ProtoOutputFormatterCallback(
- eventHandler,
- cqueryOptions,
- out,
- CodedOutputStream.newInstance(out, OUTPUT_BUFFER_SIZE),
skyframeExecutor,
accessor,
aspectResolver,
@@ -261,7 +242,6 @@
eventHandler,
cqueryOptions,
out,
- CodedOutputStream.newInstance(out, OUTPUT_BUFFER_SIZE),
skyframeExecutor,
accessor,
aspectResolver,
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
index 7876039..0d85d0d 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
@@ -45,9 +45,8 @@
effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
help =
"The format in which the cquery results should be printed. Allowed values for cquery "
- + "are: label, label_kind, textproto, transitions, proto, streamed_proto, jsonproto. "
- + "If you select 'transitions', you also have to specify the "
- + "--transitions=(lite|full) option.")
+ + "are: label, label_kind, textproto, transitions, proto, jsonproto. If you select "
+ + "'transitions', you also have to specify the --transitions=(lite|full) option.")
public String outputFormat;
@Option(
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 4fe5f64..c22c7ba 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
@@ -15,7 +15,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
-import com.google.common.base.Preconditions;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
@@ -23,7 +23,6 @@
import com.google.devtools.build.lib.actions.BuildConfigurationEvent;
import com.google.devtools.build.lib.analysis.AnalysisProtosV2;
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.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
@@ -40,13 +39,11 @@
import com.google.devtools.build.lib.query2.cquery.CqueryTransitionResolver.EvaluateException;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
import com.google.devtools.build.lib.query2.proto.proto2api.Build;
-import com.google.devtools.build.lib.query2.proto.proto2api.Build.QueryResult;
import com.google.devtools.build.lib.query2.query.aspectresolvers.AspectResolver;
import com.google.devtools.build.lib.query2.query.output.ProtoOutputFormatter;
import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
-import com.google.protobuf.CodedOutputStream;
import com.google.protobuf.Message;
import com.google.protobuf.TextFormat;
import com.google.protobuf.util.JsonFormat;
@@ -63,7 +60,6 @@
/** Defines the types of proto output this class can handle. */
public enum OutputType {
BINARY("proto"),
- DELIMITED_BINARY("streamed_proto"),
TEXT("textproto"),
JSON("jsonproto");
@@ -106,7 +102,6 @@
}
}
- private final CodedOutputStream codedOut;
private final OutputType outputType;
private final AspectResolver resolver;
private final SkyframeExecutor skyframeExecutor;
@@ -114,6 +109,8 @@
private final JsonFormat.Printer jsonPrinter = JsonFormat.printer();
private final RuleClassProvider ruleClassProvider;
+ private AnalysisProtosV2.CqueryResult.Builder protoResult;
+
private final Map<Label, Target> partialResultMap;
private ConfiguredTarget currentTarget;
@@ -121,14 +118,12 @@
ExtendedEventHandler eventHandler,
CqueryOptions options,
OutputStream out,
- CodedOutputStream codedOut,
SkyframeExecutor skyframeExecutor,
TargetAccessor<ConfiguredTarget> accessor,
AspectResolver resolver,
OutputType outputType,
RuleClassProvider ruleClassProvider) {
super(eventHandler, options, out, skyframeExecutor, accessor, /*uniquifyResults=*/ false);
- this.codedOut = codedOut;
this.outputType = outputType;
this.skyframeExecutor = skyframeExecutor;
this.resolver = resolver;
@@ -137,40 +132,31 @@
}
@Override
+ public void start() {
+ protoResult = AnalysisProtosV2.CqueryResult.newBuilder();
+ }
+
+ @Override
public void close(boolean failFast) throws IOException {
- if (failFast || printStream == null) {
- return;
- }
- if (options.protoIncludeConfigurations) {
- for (Configuration configuration : configurationCache.getConfigurations()) {
- if (outputType == OutputType.DELIMITED_BINARY) {
- // For streamed protos, we wrap each Configuration in its own CqueryResult that will be
- // written length delimited to the stream.
- writeData(
- AnalysisProtosV2.CqueryResult.newBuilder().addConfigurations(configuration).build());
- } else {
- writeData(configuration, CqueryResult.CONFIGURATIONS_FIELD_NUMBER);
- }
+ if (!failFast && printStream != null) {
+ if (options.protoIncludeConfigurations) {
+ writeData(getProtoResult());
+ } else {
+ // Documentation promises that setting this flag to false means we convert directly
+ // to the build.proto format. This is hard to test in integration testing due to the way
+ // proto output is turned readable (codex). So change the following code with caution.
+ Build.QueryResult.Builder queryResult = Build.QueryResult.newBuilder();
+ protoResult.getResultsList().forEach(ct -> queryResult.addTarget(ct.getTarget()));
+ writeData(queryResult.build());
}
+ printStream.flush();
}
- codedOut.flush();
- outputStream.flush();
- printStream.flush();
}
private void writeData(Message message) throws IOException {
- writeData(message, 0);
- }
-
- private void writeData(Message message, int fieldNumber) throws IOException {
switch (outputType) {
case BINARY:
- Preconditions.checkState(
- fieldNumber != 0, "Cannot have fieldNumber of 0 when outputType is BINARY");
- codedOut.writeMessage(fieldNumber, message);
- break;
- case DELIMITED_BINARY:
- message.writeDelimitedTo(outputStream);
+ message.writeTo(outputStream);
break;
case TEXT:
TextFormat.printer().print(message, printStream);
@@ -189,9 +175,14 @@
return outputType.formatName();
}
+ @VisibleForTesting
+ public AnalysisProtosV2.CqueryResult getProtoResult() {
+ protoResult.addAllConfigurations(configurationCache.getConfigurations());
+ return protoResult.build();
+ }
+
@Override
- public void processOutput(Iterable<ConfiguredTarget> partialResult)
- throws InterruptedException, IOException {
+ public void processOutput(Iterable<ConfiguredTarget> partialResult) throws InterruptedException {
partialResult.forEach(
kct -> partialResultMap.put(kct.getOriginalLabel(), accessor.getTarget(kct)));
@@ -248,7 +239,6 @@
throw new InterruptedException(e.getMessage());
}
}
-
builder.setTarget(targetBuilder);
if (options.protoIncludeConfigurations) {
@@ -269,33 +259,7 @@
}
}
- // There are a few cases that affect the shape of the output:
- // 1. --output=proto|textproto|jsonproto --proto:include_configurations =>
- // Writes a single CqueryResult containing all the ConfiguredTarget(s) and
- // Configuration(s) in the specified output format.
- // 2. --output=streamed_proto --proto:include_configurations =>
- // Writes multiple length delimited CqueryResult protos, each containing a single
- // ConfiguredTarget or Configuration.
- // 3. --output=proto|textproto|jsonproto --noproto:include_configurations =>
- // Writes a single QueryResult containing all the corresponding Target(s) in the
- // specified output format.
- // 4.--output=streamed_proto --noproto:include_configurations =>
- // Writes multiple length delimited Target protos.
- if (options.protoIncludeConfigurations) {
- if (outputType == OutputType.DELIMITED_BINARY) {
- // Case 2.
- writeData(AnalysisProtosV2.CqueryResult.newBuilder().addResults(builder).build());
- } else {
- // Case 1.
- writeData(builder.build(), CqueryResult.RESULTS_FIELD_NUMBER);
- }
- } else {
- // Case 3 & 4.
- // Documentation promises that setting this flag to false means we convert directly
- // to the build.proto format. This is hard to test in integration testing due to the way
- // proto output is turned readable (codex). So change the following code with caution.
- writeData(builder.build().getTarget(), QueryResult.TARGET_FIELD_NUMBER);
- }
+ protoResult.addResults(builder.build());
}
}
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 54e9b61..095797e 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
@@ -137,14 +137,12 @@
"//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",
- "//src/main/java/com/google/devtools/build/lib/util:pair",
"//src/main/protobuf:analysis_v2_java_proto",
"//src/main/protobuf:build_java_proto",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
- "//third_party/protobuf:protobuf_java",
],
)
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 6f11ee3..5819474 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
@@ -18,7 +18,6 @@
import static com.google.devtools.build.lib.packages.Attribute.attr;
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.analysis.AnalysisProtosV2;
@@ -41,13 +40,6 @@
import com.google.devtools.build.lib.query2.proto.proto2api.Build.ConfiguredRuleInput;
import com.google.devtools.build.lib.query2.query.aspectresolvers.AspectResolver.Mode;
import com.google.devtools.build.lib.util.FileTypeSet;
-import com.google.devtools.build.lib.util.Pair;
-import com.google.protobuf.CodedInputStream;
-import com.google.protobuf.CodedOutputStream;
-import com.google.protobuf.ExtensionRegistry;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.InputStream;
import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
@@ -325,103 +317,31 @@
.containsAtLeast("//test:my_alias_rule", "//test:config1", "//test:target1");
}
- @Test
- public void testStreamedProtoAndProtoOutputsAreEquivalent() throws Exception {
- MockRule depsRule =
- () ->
- MockRule.define(
- "my_rule",
- (builder, env) ->
- builder.add(attr("deps", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE)));
- ConfiguredRuleClassProvider ruleClassProvider = setRuleClassProviders(depsRule).build();
- helper.useRuleClassProvider(ruleClassProvider);
-
- writeFile(
- "test/BUILD",
- "my_rule(name = 'my_rule',",
- " deps = select({",
- " ':garfield': ['lasagna.java', 'naps.java'],",
- " '//conditions:default': ['mondays.java']",
- " })",
- ")",
- "config_setting(",
- " name = 'garfield',",
- " values = {'foo': 'cat'}",
- ")");
- getHelper().useConfiguration("--foo=cat");
- AnalysisProtosV2.CqueryResult protoOutput = getOutput("//test:my_rule", ruleClassProvider);
- ImmutableList<AnalysisProtosV2.CqueryResult> streamedProtoOutput =
- getStreamedOutput("//test:my_rule", ruleClassProvider);
- AnalysisProtosV2.CqueryResult.Builder combinedStreamedProtoBuilder =
- AnalysisProtosV2.CqueryResult.newBuilder();
- for (AnalysisProtosV2.CqueryResult result : streamedProtoOutput) {
- if (!result.getResultsList().isEmpty()) {
- combinedStreamedProtoBuilder.addAllResults(result.getResultsList());
- }
- if (!result.getConfigurationsList().isEmpty()) {
- combinedStreamedProtoBuilder.addAllConfigurations(result.getConfigurationsList());
- }
- }
- assertThat(protoOutput)
- .ignoringRepeatedFieldOrder()
- .isEqualTo(combinedStreamedProtoBuilder.build());
- }
-
private MockRule getSimpleRule() {
return () -> MockRule.define("simple_rule");
}
private AnalysisProtosV2.CqueryResult getOutput(
String queryExpression, RuleClassProvider ruleClassProvider) throws Exception {
- CodedInputStream codedIn =
- getInputStreamsWithData(queryExpression, ruleClassProvider, OutputType.BINARY).getSecond();
- return AnalysisProtosV2.CqueryResult.parser()
- .parseFrom(codedIn, ExtensionRegistry.getEmptyRegistry());
- }
-
- private ImmutableList<AnalysisProtosV2.CqueryResult> getStreamedOutput(
- String queryExpression, RuleClassProvider ruleClassProvider) throws Exception {
- InputStream in =
- getInputStreamsWithData(queryExpression, ruleClassProvider, OutputType.DELIMITED_BINARY)
- .getFirst();
- ImmutableList.Builder<AnalysisProtosV2.CqueryResult> builder = new ImmutableList.Builder<>();
- AnalysisProtosV2.CqueryResult result;
- while ((result =
- AnalysisProtosV2.CqueryResult.parser()
- .parseDelimitedFrom(in, ExtensionRegistry.getEmptyRegistry()))
- != null) {
- builder.add(result);
- }
- return builder.build();
- }
-
- private Pair<InputStream, CodedInputStream> getInputStreamsWithData(
- String queryExpression, RuleClassProvider ruleClassProvider, OutputType outputType)
- throws Exception {
QueryExpression expression = QueryParser.parse(queryExpression, getDefaultFunctions());
Set<String> targetPatternSet = new LinkedHashSet<>();
expression.collectTargetPatterns(targetPatternSet);
helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS);
PostAnalysisQueryEnvironment<ConfiguredTarget> env =
((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet);
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- CodedOutputStream codedOut = CodedOutputStream.newInstance(out);
+
ProtoOutputFormatterCallback callback =
new ProtoOutputFormatterCallback(
reporter,
options,
- out,
- codedOut,
+ /* out= */ null,
getHelper().getSkyframeExecutor(),
env.getAccessor(),
options.aspectDeps.createResolver(
getHelper().getPackageManager(), NullEventHandler.INSTANCE),
- outputType,
+ OutputType.BINARY,
ruleClassProvider);
env.evaluateQuery(expression, callback);
- codedOut.flush();
- ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
- CodedInputStream codedIn = CodedInputStream.newInstance(in);
- return new Pair<>(in, codedIn);
+ return callback.getProtoResult();
}
}
