From d2343d90f19dc9a3f3a227e1c0c113fde60a7747 Mon Sep 17 00:00:00 2001 From: leba Date: Wed, 15 Jan 2020 01:34:21 -0800 Subject: [PATCH] Fix JSON format for aquery analysis_v2.proto. The JSON format doesn't work with the streamed output since by nature it needs to be grouped by message type. This CL adds a monolithic output handler for analysis_v2.proto that can be used for JSON format. RELNOTES: None PiperOrigin-RevId: 289815268 --- ...onGraphProtoV2OutputFormatterCallback.java | 30 ++++--- .../actiongraph/v2/ActionGraphDump.java | 28 +++--- .../actiongraph/v2/AqueryOutputHandler.java | 63 ++++++++++++++ .../skyframe/actiongraph/v2/BaseCache.java | 10 +-- .../actiongraph/v2/KnownArtifacts.java | 10 +-- .../v2/KnownAspectDescriptors.java | 8 +- .../actiongraph/v2/KnownConfigurations.java | 8 +- .../actiongraph/v2/KnownNestedSets.java | 8 +- .../actiongraph/v2/KnownPathFragments.java | 8 +- .../actiongraph/v2/KnownRuleClassStrings.java | 8 +- .../v2/KnownRuleConfiguredTargets.java | 8 +- .../v2/MonolithicOutputHandler.java | 85 +++++++++++++++++++ .../actiongraph/v2/StreamedOutputHandler.java | 74 +++++++--------- src/test/shell/integration/aquery_test.sh | 57 ++++++------- 14 files changed, 272 insertions(+), 133 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/AqueryOutputHandler.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/MonolithicOutputHandler.java diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoV2OutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoV2OutputFormatterCallback.java index 8d7962a5d160b5..5ec7728b53c89f 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoV2OutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoV2OutputFormatterCallback.java @@ -21,8 +21,10 @@ import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.skyframe.actiongraph.v2.ActionGraphDump; +import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryOutputHandler; +import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryOutputHandler.OutputType; +import com.google.devtools.build.lib.skyframe.actiongraph.v2.MonolithicOutputHandler; import com.google.devtools.build.lib.skyframe.actiongraph.v2.StreamedOutputHandler; -import com.google.devtools.build.lib.skyframe.actiongraph.v2.StreamedOutputHandler.OutputType; import com.google.protobuf.CodedOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -33,7 +35,7 @@ public class ActionGraphProtoV2OutputFormatterCallback extends AqueryThreadsafeC private final OutputType outputType; private final ActionGraphDump actionGraphDump; private final AqueryActionFilter actionFilters; - private StreamedOutputHandler streamedOutputHandler; + private AqueryOutputHandler aqueryOutputHandler; /** * Pseudo-arbitrarily chosen buffer size for output. Chosen to be large enough to fit a handful of @@ -52,20 +54,28 @@ public class ActionGraphProtoV2OutputFormatterCallback extends AqueryThreadsafeC super(eventHandler, options, out, skyframeExecutor, accessor); this.outputType = outputType; this.actionFilters = actionFilters; - if (out != null) { - this.streamedOutputHandler = - new StreamedOutputHandler( - this.outputType, - CodedOutputStream.newInstance(out, OUTPUT_BUFFER_SIZE), - this.printStream); + + switch (outputType) { + case BINARY: + case TEXT: + this.aqueryOutputHandler = + new StreamedOutputHandler( + this.outputType, + CodedOutputStream.newInstance(out, OUTPUT_BUFFER_SIZE), + this.printStream); + break; + case JSON: + this.aqueryOutputHandler = new MonolithicOutputHandler(this.printStream); + break; } + this.actionGraphDump = new ActionGraphDump( options.includeCommandline, options.includeArtifacts, this.actionFilters, options.includeParamFiles, - streamedOutputHandler); + aqueryOutputHandler); } @Override @@ -98,7 +108,7 @@ public void processOutput(Iterable partialResult) @Override public void close(boolean failFast) throws IOException { if (!failFast) { - streamedOutputHandler.close(); + aqueryOutputHandler.close(); } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java index ab2ac798fe275b..8530c5b49998bb 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java @@ -64,7 +64,7 @@ public class ActionGraphDump { private final boolean includeActionCmdLine; private final boolean includeArtifacts; private final boolean includeParamFiles; - private final StreamedOutputHandler streamedOutputHandler; + private final AqueryOutputHandler aqueryOutputHandler; private Map> paramFileNameToContentMap; @@ -73,14 +73,14 @@ public ActionGraphDump( boolean includeArtifacts, AqueryActionFilter actionFilters, boolean includeParamFiles, - StreamedOutputHandler streamedOutputHandler) { + AqueryOutputHandler aqueryOutputHandler) { this( /* actionGraphTargets= */ ImmutableList.of("..."), includeActionCmdLine, includeArtifacts, actionFilters, includeParamFiles, - streamedOutputHandler); + aqueryOutputHandler); } public ActionGraphDump( @@ -89,21 +89,21 @@ public ActionGraphDump( boolean includeArtifacts, AqueryActionFilter actionFilters, boolean includeParamFiles, - StreamedOutputHandler streamedOutputHandler) { + AqueryOutputHandler aqueryOutputHandler) { this.actionGraphTargets = ImmutableSet.copyOf(actionGraphTargets); this.includeActionCmdLine = includeActionCmdLine; this.includeArtifacts = includeArtifacts; this.actionFilters = actionFilters; this.includeParamFiles = includeParamFiles; - this.streamedOutputHandler = streamedOutputHandler; + this.aqueryOutputHandler = aqueryOutputHandler; - knownRuleClassStrings = new KnownRuleClassStrings(streamedOutputHandler); - knownArtifacts = new KnownArtifacts(streamedOutputHandler); - knownConfigurations = new KnownConfigurations(streamedOutputHandler); - knownNestedSets = new KnownNestedSets(streamedOutputHandler, knownArtifacts); - knownAspectDescriptors = new KnownAspectDescriptors(streamedOutputHandler); + knownRuleClassStrings = new KnownRuleClassStrings(aqueryOutputHandler); + knownArtifacts = new KnownArtifacts(aqueryOutputHandler); + knownConfigurations = new KnownConfigurations(aqueryOutputHandler); + knownNestedSets = new KnownNestedSets(aqueryOutputHandler, knownArtifacts); + knownAspectDescriptors = new KnownAspectDescriptors(aqueryOutputHandler); knownRuleConfiguredTargets = - new KnownRuleConfiguredTargets(streamedOutputHandler, knownRuleClassStrings); + new KnownRuleConfiguredTargets(aqueryOutputHandler, knownRuleClassStrings); } public ActionKeyContext getActionKeyContext() { @@ -232,11 +232,7 @@ private void dumpSingleAction(ConfiguredTarget configuredTarget, ActionAnalysisM } } - printToOutput(actionBuilder.build()); - } - - private void printToOutput(AnalysisProtosV2.Action actionProto) throws IOException { - streamedOutputHandler.printAction(actionProto); + aqueryOutputHandler.outputAction(actionBuilder.build()); } public void dumpAspect(AspectValue aspectValue, ConfiguredTargetValue configuredTargetValue) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/AqueryOutputHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/AqueryOutputHandler.java new file mode 100644 index 00000000000000..a6410b0ff4369f --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/AqueryOutputHandler.java @@ -0,0 +1,63 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe.actiongraph.v2; + +import com.google.devtools.build.lib.analysis.AnalysisProtosV2.Action; +import com.google.devtools.build.lib.analysis.AnalysisProtosV2.Artifact; +import com.google.devtools.build.lib.analysis.AnalysisProtosV2.AspectDescriptor; +import com.google.devtools.build.lib.analysis.AnalysisProtosV2.Configuration; +import com.google.devtools.build.lib.analysis.AnalysisProtosV2.DepSetOfFiles; +import com.google.devtools.build.lib.analysis.AnalysisProtosV2.PathFragment; +import com.google.devtools.build.lib.analysis.AnalysisProtosV2.RuleClass; +import com.google.devtools.build.lib.analysis.AnalysisProtosV2.Target; +import java.io.IOException; + +/** Outputs various messages of analysis_v2.proto. */ +public interface AqueryOutputHandler { + /** Defines the types of proto output this class can handle. */ + enum OutputType { + BINARY("proto"), + TEXT("textproto"), + JSON("jsonproto"); + + private final String formatName; + + OutputType(String formatName) { + this.formatName = formatName; + } + + public String formatName() { + return formatName; + } + } + + void outputArtifact(Artifact message) throws IOException; + + void outputAction(Action message) throws IOException; + + void outputTarget(Target message) throws IOException; + + void outputDepSetOfFiles(DepSetOfFiles message) throws IOException; + + void outputConfiguration(Configuration message) throws IOException; + + void outputAspectDescriptor(AspectDescriptor message) throws IOException; + + void outputRuleClass(RuleClass message) throws IOException; + + void outputPathFragment(PathFragment message) throws IOException; + + /** Called at the end of the query process. */ + void close() throws IOException; +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/BaseCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/BaseCache.java index 2b4ebb3231e965..e417ba16e0545f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/BaseCache.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/BaseCache.java @@ -22,10 +22,10 @@ */ abstract class BaseCache { private final Map cache = new HashMap<>(); - protected final StreamedOutputHandler streamedOutputHandler; + protected final AqueryOutputHandler aqueryOutputHandler; - BaseCache(StreamedOutputHandler streamedOutputHandler) { - this.streamedOutputHandler = streamedOutputHandler; + BaseCache(AqueryOutputHandler aqueryOutputHandler) { + this.aqueryOutputHandler = aqueryOutputHandler; } private int generateNextId() { @@ -55,12 +55,12 @@ int dataToIdAndStreamOutputProto(K data) throws IOException { id = generateNextId(); cache.put(key, id); P proto = createProto(data, id); - streamToOutput(proto); + toOutput(proto); } return id; } abstract P createProto(K key, int id) throws IOException; - abstract void streamToOutput(P proto) throws IOException; + abstract void toOutput(P proto) throws IOException; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownArtifacts.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownArtifacts.java index 0125b971c8da7d..d2d35e3cd9d077 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownArtifacts.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownArtifacts.java @@ -22,9 +22,9 @@ public class KnownArtifacts extends BaseCache { - KnownAspectDescriptors(StreamedOutputHandler streamedOutputHandler) { - super(streamedOutputHandler); + KnownAspectDescriptors(AqueryOutputHandler aqueryOutputHandler) { + super(aqueryOutputHandler); } @Override @@ -44,7 +44,7 @@ AnalysisProtosV2.AspectDescriptor createProto(AspectDescriptor aspectDescriptor, } @Override - void streamToOutput(AnalysisProtosV2.AspectDescriptor aspectDescriptorProto) throws IOException { - streamedOutputHandler.printAspectDescriptor(aspectDescriptorProto); + void toOutput(AnalysisProtosV2.AspectDescriptor aspectDescriptorProto) throws IOException { + aqueryOutputHandler.outputAspectDescriptor(aspectDescriptorProto); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownConfigurations.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownConfigurations.java index 2a48a5ac950837..0b2bc48655be50 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownConfigurations.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownConfigurations.java @@ -21,8 +21,8 @@ /** Cache for BuildConfigurations in the action graph. */ public class KnownConfigurations extends BaseCache { - KnownConfigurations(StreamedOutputHandler streamedOutputHandler) { - super(streamedOutputHandler); + KnownConfigurations(AqueryOutputHandler aqueryOutputHandler) { + super(aqueryOutputHandler); } @Override @@ -38,7 +38,7 @@ Configuration createProto(BuildEvent config, int id) throws IOException { } @Override - void streamToOutput(Configuration configurationProto) throws IOException { - streamedOutputHandler.printConfiguration(configurationProto); + void toOutput(Configuration configurationProto) throws IOException { + aqueryOutputHandler.outputConfiguration(configurationProto); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownNestedSets.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownNestedSets.java index 88362f51d6a991..990131c26aeb11 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownNestedSets.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownNestedSets.java @@ -22,8 +22,8 @@ public class KnownNestedSets extends BaseCache { private final KnownArtifacts knownArtifacts; - KnownNestedSets(StreamedOutputHandler streamedOutputHandler, KnownArtifacts knownArtifacts) { - super(streamedOutputHandler); + KnownNestedSets(AqueryOutputHandler aqueryOutputHandler, KnownArtifacts knownArtifacts) { + super(aqueryOutputHandler); this.knownArtifacts = knownArtifacts; } @@ -50,7 +50,7 @@ DepSetOfFiles createProto(Object nestedSetViewObject, int id) throws IOException } @Override - void streamToOutput(DepSetOfFiles depSetOfFilesProto) throws IOException { - streamedOutputHandler.printDepSetOfFiles(depSetOfFilesProto); + void toOutput(DepSetOfFiles depSetOfFilesProto) throws IOException { + aqueryOutputHandler.outputDepSetOfFiles(depSetOfFilesProto); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownPathFragments.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownPathFragments.java index bc04b28b5fa988..1377471c23fe86 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownPathFragments.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownPathFragments.java @@ -19,8 +19,8 @@ /** Cache for {@link PathFragment} in the action graph. */ public class KnownPathFragments extends BaseCache { - KnownPathFragments(StreamedOutputHandler streamedOutputHandler) { - super(streamedOutputHandler); + KnownPathFragments(AqueryOutputHandler aqueryOutputHandler) { + super(aqueryOutputHandler); } @Override @@ -40,8 +40,8 @@ AnalysisProtosV2.PathFragment createProto(PathFragment pathFragment, int id) thr } @Override - void streamToOutput(AnalysisProtosV2.PathFragment pathFragmentProto) throws IOException { - streamedOutputHandler.printPathFragment(pathFragmentProto); + void toOutput(AnalysisProtosV2.PathFragment pathFragmentProto) throws IOException { + aqueryOutputHandler.outputPathFragment(pathFragmentProto); } private static boolean hasParent(PathFragment pathFragment) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownRuleClassStrings.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownRuleClassStrings.java index 4a6a4f75c259d4..43fffc12571a3e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownRuleClassStrings.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownRuleClassStrings.java @@ -19,8 +19,8 @@ /** Cache for RuleClassStrings in the action graph. */ public class KnownRuleClassStrings extends BaseCache { - KnownRuleClassStrings(StreamedOutputHandler streamedOutputHandler) { - super(streamedOutputHandler); + KnownRuleClassStrings(AqueryOutputHandler aqueryOutputHandler) { + super(aqueryOutputHandler); } @Override @@ -29,7 +29,7 @@ RuleClass createProto(String ruleClassString, int id) throws IOException { } @Override - void streamToOutput(RuleClass ruleClassProto) throws IOException { - streamedOutputHandler.printRuleClass(ruleClassProto); + void toOutput(RuleClass ruleClassProto) throws IOException { + aqueryOutputHandler.outputRuleClass(ruleClassProto); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownRuleConfiguredTargets.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownRuleConfiguredTargets.java index 12d9808a293511..14675c25e393f3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownRuleConfiguredTargets.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/KnownRuleConfiguredTargets.java @@ -24,8 +24,8 @@ public class KnownRuleConfiguredTargets extends BaseCache "$pkg/BUILD" <<'EOF' -#genrule( -# name = "bar", -# srcs = ["dummy.txt"], -# outs = ["bar_out.txt"], -# cmd = "echo unused > $(OUTS)", -#) -#EOF -# bazel aquery --incompatible_proto_output_v2 --output=jsonproto "//$pkg:bar" > output 2> "$TEST_log" \ -# || fail "Expected success" -# cat output >> "$TEST_log" -# -# # Verify than ids come in integers instead of strings. -# assert_contains "\"id\": 1," output -# assert_not_contains "\"id\": \"1\"" output -# -# # Verify that paths are broken down to path fragments. -# assert_contains "\"pathFragment\": {" output -# -# # Verify that the appropriate action was included. -# assert_contains "\"label\": \"dummy.txt\"" output -# assert_contains "\"mnemonic\": \"Genrule\"" output -# assert_contains "\"mnemonic\": \".*-fastbuild\"" output -# assert_contains "echo unused" output -#} +function test_basic_aquery_jsonproto_v2() { + local pkg="${FUNCNAME[0]}" + mkdir -p "$pkg" || fail "mkdir -p $pkg" + cat > "$pkg/BUILD" <<'EOF' +genrule( + name = "bar", + srcs = ["dummy.txt"], + outs = ["bar_out.txt"], + cmd = "echo unused > $(OUTS)", +) +EOF + bazel aquery --incompatible_proto_output_v2 --output=jsonproto "//$pkg:bar" > output 2> "$TEST_log" \ + || fail "Expected success" + cat output >> "$TEST_log" + + # Verify than ids come in integers instead of strings. + assert_contains "\"id\": 1," output + assert_not_contains "\"id\": \"1\"" output + + # Verify that paths are broken down to path fragments. + assert_contains "\"pathFragments\": \[{" output + + # Verify that the appropriate action was included. + assert_contains "\"label\": \"dummy.txt\"" output + assert_contains "\"mnemonic\": \"Genrule\"" output + assert_contains "\"mnemonic\": \".*-fastbuild\"" output + assert_contains "echo unused" output +} run_suite "${PRODUCT_NAME} action graph query tests"