Fix valid json when using jsonproto output in queries with new `--out…

…put=streamed_jsonproto` implementation. Closes #18701. PiperOrigin-RevId: 555417403 Change-Id: I30eb06f734188f8511884954f43c5f5b3c0091a3
bazelbuild · Aug 10, 2023 · 8e5a0f5 · 8e5a0f5
1 parent 7978458
commit 8e5a0f5
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 16 deletions.
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
@@ -37,9 +37,9 @@ public static ImmutableList<OutputFormatter> 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
@@ -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/...uery/output/JSONProtoOutputFormatter.java → ...put/StreamedJSONProtoOutputFormatter.java b/...uery/output/JSONProtoOutputFormatter.java → ...put/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<Target> 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
@@ -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"