From 8e5a0f5e4379c6bc37a18be9ef621bbfa970034b Mon Sep 17 00:00:00 2001 From: Chirag Ramani Date: Thu, 10 Aug 2023 02:10:00 -0700 Subject: [PATCH] Fix valid json when using jsonproto output in queries with new `--output=streamed_jsonproto` implementation. Closes #18701. PiperOrigin-RevId: 555417403 Change-Id: I30eb06f734188f8511884954f43c5f5b3c0091a3 --- .../query2/query/output/OutputFormatters.java | 2 +- .../lib/query2/query/output/QueryOptions.java | 4 +- ... => StreamedJSONProtoOutputFormatter.java} | 14 ++++--- .../shell/integration/bazel_query_test.sh | 38 +++++++++++++++---- 4 files changed, 42 insertions(+), 16 deletions(-) rename src/main/java/com/google/devtools/build/lib/query2/query/output/{JSONProtoOutputFormatter.java => StreamedJSONProtoOutputFormatter.java} (76%) diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/OutputFormatters.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/OutputFormatters.java index 41268a365a7977..af532a30bddec8 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/OutputFormatters.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/OutputFormatters.java @@ -37,9 +37,9 @@ public static ImmutableList getDefaultFormatters() { new PackageOutputFormatter(), new LocationOutputFormatter(), new GraphOutputFormatter(), - new JSONProtoOutputFormatter(), new XmlOutputFormatter(), new ProtoOutputFormatter(), + new StreamedJSONProtoOutputFormatter(), new StreamedProtoOutputFormatter()); } diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java index 87f036b6d987f1..867b1b7dadaafa 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java @@ -40,8 +40,8 @@ public OrderOutputConverter() { effectTags = {OptionEffectTag.TERMINAL_OUTPUT}, help = "The format in which the query results should be printed. Allowed values for query are:" - + " build, graph, jsonproto, label, label_kind, location, maxrank, minrank, package," - + " proto, xml.") + + " build, graph, streamed_jsonproto, label, label_kind, location, maxrank, minrank," + + " package, proto, xml.") public String outputFormat; @Option( diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/JSONProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/StreamedJSONProtoOutputFormatter.java similarity index 76% rename from src/main/java/com/google/devtools/build/lib/query2/query/output/JSONProtoOutputFormatter.java rename to src/main/java/com/google/devtools/build/lib/query2/query/output/StreamedJSONProtoOutputFormatter.java index 83ecb50354c1b2..d8509ded692fdf 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/JSONProtoOutputFormatter.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/StreamedJSONProtoOutputFormatter.java @@ -22,13 +22,13 @@ import java.nio.charset.StandardCharsets; /** - * An output formatter that outputs a protocol buffer json representation of a query result and - * outputs the json to the output print stream. + * An output formatter that prints a list of targets according to ndjson spec to the output print + * stream. */ -public class JSONProtoOutputFormatter extends ProtoOutputFormatter { +public class StreamedJSONProtoOutputFormatter extends ProtoOutputFormatter { @Override public String getName() { - return "jsonproto"; + return "streamed_jsonproto"; } private final JsonFormat.Printer jsonPrinter = JsonFormat.printer(); @@ -42,7 +42,11 @@ public void processOutput(Iterable partialResult) throws IOException, InterruptedException { for (Target target : partialResult) { out.write( - jsonPrinter.print(toTargetProtoBuffer(target)).getBytes(StandardCharsets.UTF_8)); + jsonPrinter + .omittingInsignificantWhitespace() + .print(toTargetProtoBuffer(target)) + .getBytes(StandardCharsets.UTF_8)); + out.write(System.lineSeparator().getBytes(StandardCharsets.UTF_8)); } } }; diff --git a/src/test/shell/integration/bazel_query_test.sh b/src/test/shell/integration/bazel_query_test.sh index 7689099aa9ceef..fc29bff0b63e94 100755 --- a/src/test/shell/integration/bazel_query_test.sh +++ b/src/test/shell/integration/bazel_query_test.sh @@ -668,7 +668,7 @@ genquery(name='q', opts = ["--output=blargh"],) EOF - local expected_error_msg="in genquery rule //starfruit:q: Invalid output format 'blargh'. Valid values are: label, label_kind, build, minrank, maxrank, package, location, graph, jsonproto, xml, proto" + local expected_error_msg="in genquery rule //starfruit:q: Invalid output format 'blargh'. Valid values are: label, label_kind, build, minrank, maxrank, package, location, graph, xml, proto, streamed_jsonproto, " bazel build //starfruit:q >& $TEST_log && fail "Expected failure" expect_log "$expected_error_msg" } @@ -1064,7 +1064,7 @@ EOF expect_log "//${package}:hint" } -function test_basic_query_jsonproto() { +function test_basic_query_streamed_jsonproto() { local pkg="${FUNCNAME[0]}" mkdir -p "$pkg" || fail "mkdir -p $pkg" cat > "$pkg/BUILD" <<'EOF' @@ -1074,17 +1074,39 @@ genrule( outs = ["bar_out.txt"], cmd = "echo unused > $(OUTS)", ) +genrule( + name = "foo", + srcs = ["dummy.txt"], + outs = ["foo_out.txt"], + cmd = "echo unused > $(OUTS)", +) EOF - bazel query --output=jsonproto --noimplicit_deps "//$pkg:bar" > output 2> "$TEST_log" \ + bazel query --output=streamed_jsonproto --noimplicit_deps "//$pkg/..." > output 2> "$TEST_log" \ || fail "Expected success" cat output >> "$TEST_log" # Verify that the appropriate attributes were included. - assert_contains "\"ruleClass\": \"genrule\"" output - assert_contains "\"name\": \"//$pkg:bar\"" output - assert_contains "\"ruleInput\": \[\"//$pkg:dummy.txt\"\]" output - assert_contains "\"ruleOutput\": \[\"//$pkg:bar_out.txt\"\]" output - assert_contains "echo unused" output + + foo_line_number=$(grep -n "foo" output | cut -d':' -f1) + bar_line_number=$(grep -n "bar" output | cut -d':' -f1) + + foo_ndjson_line=$(sed -n "${foo_line_number}p" output) + bar_ndjson_line=$(sed -n "${bar_line_number}p" output) + + echo "$foo_ndjson_line" > foo_ndjson_file + echo "$bar_ndjson_line" > bar_ndjson_file + + assert_contains "\"ruleClass\":\"genrule\"" foo_ndjson_file + assert_contains "\"name\":\"//$pkg:foo\"" foo_ndjson_file + assert_contains "\"ruleInput\":\[\"//$pkg:dummy.txt\"\]" foo_ndjson_file + assert_contains "\"ruleOutput\":\[\"//$pkg:foo_out.txt\"\]" foo_ndjson_file + assert_contains "echo unused" foo_ndjson_file + + assert_contains "\"ruleClass\":\"genrule\"" bar_ndjson_file + assert_contains "\"name\":\"//$pkg:bar\"" bar_ndjson_file + assert_contains "\"ruleInput\":\[\"//$pkg:dummy.txt\"\]" bar_ndjson_file + assert_contains "\"ruleOutput\":\[\"//$pkg:bar_out.txt\"\]" bar_ndjson_file + assert_contains "echo unused" bar_ndjson_file } run_suite "${PRODUCT_NAME} query tests"