[PATCH 0/2] routines to generate JSON data

[PATCH 0/2] routines to generate JSON data

[git]

From: Jeff Hostetler <jeffhost@microsoft.com>

This patch series adds a set of utility routines to compose data in JSON
format into a "struct strbuf".  The resulting string can then be output
by commands wanting to support a JSON output format.

This is a stand alone patch.  Nothing currently uses these routines.  I'm
currently working on a series to log "telemetry" data (as we discussed
briefly during Ævar's "Performance Misc" session [1] in Barcelona last
week).  And I want emit the data in JSON rather than a fixed column/field
format.  The JSON routines here are independent of that, so it made sense
to submit the JSON part by itself.

Back when we added porcelain=v2 format to status, we talked about adding a
JSON format.  I think the routines in this patch would let us easily do
that, if someone were interested.  (Extending status is not on my radar
right now, however.)

Documentation for the new API is given in json-writer.h at the bottom of
the first patch.

I wasn't sure how to unit test the API from a shell script, so I added a
helper command that does most of the work in the second patch.

[1] https://public-inbox.org/git/20180313004940.GG61720@google.com/T/


Jeff Hostetler (2):
  json_writer: new routines to create data in JSON format
  json-writer: unit test

 Makefile                    |   2 +
 json-writer.c               | 224 ++++++++++++++++++++++++++++++++++++++++++++
 json-writer.h               | 120 ++++++++++++++++++++++++
 t/helper/test-json-writer.c | 146 +++++++++++++++++++++++++++++
 t/t0019-json-writer.sh      |  10 ++
 5 files changed, 502 insertions(+)
 create mode 100644 json-writer.c
 create mode 100644 json-writer.h
 create mode 100644 t/helper/test-json-writer.c
 create mode 100755 t/t0019-json-writer.sh

-- 
2.9.3

[PATCH 1/2] json_writer: new routines to create data in JSON format

[git]

From: Jeff Hostetler <jeffhost@microsoft.com>

Add basic routines to generate data in JSON format.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 Makefile      |   1 +
 json-writer.c | 224 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 json-writer.h | 120 +++++++++++++++++++++++++++++++
 3 files changed, 345 insertions(+)
 create mode 100644 json-writer.c
 create mode 100644 json-writer.h

diff --git a/Makefile b/Makefile
index 1a9b23b..9000369 100644
--- a/Makefile
+++ b/Makefile
@@ -815,6 +815,7 @@ LIB_OBJS += hashmap.o
 LIB_OBJS += help.o
 LIB_OBJS += hex.o
 LIB_OBJS += ident.o
+LIB_OBJS += json-writer.o
 LIB_OBJS += kwset.o
 LIB_OBJS += levenshtein.o
 LIB_OBJS += line-log.o
diff --git a/json-writer.c b/json-writer.c
new file mode 100644
index 0000000..755ff80
--- /dev/null
+++ b/json-writer.c
@@ -0,0 +1,224 @@
+#include "cache.h"
+#include "json-writer.h"
+
+/*
+ * Append JSON-quoted version of the given string to 'out'.
+ */
+static void jw_append_quoted_string(struct strbuf *out, const char *in)
+{
+	strbuf_addch(out, '"');
+	for (/**/; *in; in++) {
+		unsigned char c = (unsigned char)*in;
+		if (c == '"')
+			strbuf_add(out, "\\\"", 2);
+		else if (c == '\\')
+			strbuf_add(out, "\\\\", 2);
+		else if (c == '\n')
+			strbuf_add(out, "\\n", 2);
+		else if (c == '\r')
+			strbuf_add(out, "\\r", 2);
+		else if (c == '\t')
+			strbuf_add(out, "\\t", 2);
+		else if (c == '\f')
+			strbuf_add(out, "\\f", 2);
+		else if (c == '\b')
+			strbuf_add(out, "\\b", 2);
+		else if (c < 0x20)
+			strbuf_addf(out, "\\u%04x", c);
+		else
+			strbuf_addch(out, c);
+	}
+	strbuf_addch(out, '"');
+}
+
+void jw_object_begin(struct strbuf *out)
+{
+	strbuf_reset(out);
+	strbuf_addch(out, '{');
+}
+
+void jw_object_append_string(struct strbuf *out, const char *key,
+			     const char *value)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addch(out, ':');
+	jw_append_quoted_string(out, value);
+}
+
+void jw_object_append_int(struct strbuf *out, const char *key, int value)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addf(out, ":%d", value);
+}
+
+void jw_object_append_uint64(struct strbuf *out, const char *key, uint64_t value)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addf(out, ":%"PRIuMAX, value);
+}
+
+void jw_object_append_double(struct strbuf *out, const char *key, double value)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addf(out, ":%f", value);
+}
+
+void jw_object_append_true(struct strbuf *out, const char *key)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addstr(out, ":true");
+}
+
+void jw_object_append_false(struct strbuf *out, const char *key)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addstr(out, ":false");
+}
+
+void jw_object_append_null(struct strbuf *out, const char *key)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addstr(out, ":null");
+}
+
+void jw_object_append_object(struct strbuf *out, const char *key,
+			     const char *value)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addch(out, ':');
+	strbuf_addstr(out, value);
+}
+
+void jw_object_append_array(struct strbuf *out, const char *key,
+			    const char *value)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+
+	jw_append_quoted_string(out, key);
+	strbuf_addch(out, ':');
+	strbuf_addstr(out, value);
+}
+
+void jw_object_end(struct strbuf *out)
+{
+	strbuf_addch(out, '}');
+}
+
+void jw_array_begin(struct strbuf *out)
+{
+	strbuf_reset(out);
+	strbuf_addch(out, '[');
+}
+
+void jw_array_append_string(struct strbuf *out, const char *elem)
+{
+	struct strbuf qe = STRBUF_INIT;
+
+	jw_append_quoted_string(&qe, elem);
+
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addstr(out, qe.buf);
+
+	strbuf_release(&qe);
+}
+
+void jw_array_append_int(struct strbuf *out, int elem)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addf(out, "%d", elem);
+}
+
+void jw_array_append_uint64(struct strbuf *out, uint64_t elem)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addf(out, "%"PRIuMAX, elem);
+}
+
+void jw_array_append_double(struct strbuf *out, double elem)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addf(out, "%f", elem);
+}
+
+void jw_array_append_true(struct strbuf *out)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addstr(out, "true");
+}
+
+void jw_array_append_false(struct strbuf *out)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addstr(out, "false");
+}
+
+void jw_array_append_null(struct strbuf *out)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addstr(out, "null");
+}
+
+void jw_array_append_object(struct strbuf *out, const char *obj)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addstr(out, obj);
+}
+
+void jw_array_append_array(struct strbuf *out, const char *array)
+{
+	if (out->len > 1)
+		strbuf_addch(out, ',');
+	strbuf_addstr(out, array);
+}
+
+void jw_array_append_argc_argv(struct strbuf *out, int argc, const char **argv)
+{
+	int k;
+
+	for (k = 0; k < argc; k++)
+		jw_array_append_string(out, argv[k]);
+}
+
+void jw_array_append_argv(struct strbuf *out, const char **argv)
+{
+	while (*argv)
+		jw_array_append_string(out, *argv++);
+}
+
+void jw_array_end(struct strbuf *out)
+{
+	strbuf_addch(out, ']');
+}
diff --git a/json-writer.h b/json-writer.h
new file mode 100644
index 0000000..0a60ab3
--- /dev/null
+++ b/json-writer.h
@@ -0,0 +1,120 @@
+#ifndef JSON_WRITER_H
+#define JSON_WRITER_H
+
+/*
+ * JSON data structures are defined at:
+ *      http://json.org/
+ *      http://www.ietf.org/rfc/rfc7159.txt
+ *
+ * The JSON-writer API allows one to build JSON data structures using a
+ * "struct strbuf" buffer.  This is intended as a simple API to build
+ * output strings; it is not intended to be a general object model for
+ * JSON data.
+ *
+ * All string values (both keys and string r-values) are properly quoted
+ * and escaped if they contain special characters.
+ *
+ * These routines create compact JSON data (with no unnecessary whitespace,
+ * newlines, or indenting).
+ *
+ * Both "JSON objects" (aka sets of k/v pairs) and "JSON array" can be
+ * constructed using a 'begin append* end' model.
+ *
+ * Example JSON Object Usage:
+ *
+ *      struct strbuf obj1 = STRBUF_INIT;
+ *
+ *      jw_object_begin(&obj1);
+ *          jw_object_append_string(&obj1, "a", "abc");
+ *          jw_object_append_int(&obj1, "b", 42);
+ *          jw_object_append_true(&obj1, "c");
+ *      jw_object_end(&obj1);
+ *
+ *      printf("%s\n", obj1.buf);
+ *
+ * Should yield:   
+ *
+ *      {"a":"abc","b":42,"c":true}
+ *
+ * Example JSON Array Usage:
+ *
+ *      struct strbuf arr1 = STRBUF_INIT;
+ *
+ *      jw_array_begin(&arr1);
+ *          jw_array_append_string(&arr1, "abc");
+ *          jw_array_append_int(&arr1, 42);
+ *          jw_array_append_true(&arr1);
+ *      jw_array_end(&arr1);
+ *
+ *      printf("%s\n", arr1.buf);
+ *
+ * Should yield:
+ *
+ *      ["abc",42,true]
+ *
+ * Nested JSON structures are also supported.  These should be composed bottom
+ * up using multiple strbuf variables.
+ *
+ * Example Nested Usage (using the above "obj1" and "arr1" variables):
+ *
+ *       struct strbuf obj2 = STRBUF_INIT;
+ *
+ *       jw_object_begin(&obj2);
+ *           jw_object_append_object(&obj2, "obj1", obj1.buf);
+ *           jw_object_append_array(&obj2, "arr1", arr1.buf);
+ *       jw_object_end(&obj2);
+ *
+ *       printf("%s\n", obj2.buf);
+ *
+ * Should yield:
+ *
+ *       {"obj1":{"a":"abc","b":42,"c":true},"arr1":["abc",42,true]}
+ *
+ * And:
+ *
+ *       struct strbuf arr2 = STRBUF_INIT;
+ *
+ *       jw_array_begin(&arr2);
+ *           jw_array_append_object(&arr2, obj1.buf);
+ *           jw_array_append_array(&arr2, arr1.buf);
+ *       jw_array_end(&arr2);
+ *
+ *       printf("%s\n", arr2.buf);
+ *
+ * Should yield:
+ *
+ *       [{"a":"abc","b":42,"c":true},["abc",42,true]]
+ *
+ */
+
+void jw_object_begin(struct strbuf *out);
+void jw_object_append_string(struct strbuf *out, const char *key,
+			     const char *value);
+void jw_object_append_int(struct strbuf *out, const char *key, int value);
+void jw_object_append_uint64(struct strbuf *out, const char *key,
+			     uint64_t value);
+void jw_object_append_double(struct strbuf *out, const char *key, double value);
+void jw_object_append_true(struct strbuf *out, const char *key);
+void jw_object_append_false(struct strbuf *out, const char *key);
+void jw_object_append_null(struct strbuf *out, const char *key);
+void jw_object_append_object(struct strbuf *out, const char *key,
+			     const char *value);
+void jw_object_append_array(struct strbuf *out, const char *key,
+			    const char *value);
+void jw_object_end(struct strbuf *out);
+
+void jw_array_begin(struct strbuf *out);
+void jw_array_append_string(struct strbuf *out, const char *elem);
+void jw_array_append_int(struct strbuf *out, int elem);
+void jw_array_append_uint64(struct strbuf *out, uint64_t elem);
+void jw_array_append_double(struct strbuf *out, double elem);
+void jw_array_append_true(struct strbuf *out);
+void jw_array_append_false(struct strbuf *out);
+void jw_array_append_null(struct strbuf *out);
+void jw_array_append_object(struct strbuf *out, const char *obj);
+void jw_array_append_array(struct strbuf *out, const char *array);
+void jw_array_append_argc_argv(struct strbuf *out, int argc, const char **argv);
+void jw_array_append_argv(struct strbuf *out, const char **argv);
+void jw_array_end(struct strbuf *out);
+
+#endif /* JSON_WRITER_H */
-- 
2.9.3

[PATCH 2/2] json-writer: unit test

[git]

From: Jeff Hostetler <jeffhost@microsoft.com>

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 Makefile                    |   1 +
 t/helper/test-json-writer.c | 146 ++++++++++++++++++++++++++++++++++++++++++++
 t/t0019-json-writer.sh      |  10 +++
 3 files changed, 157 insertions(+)
 create mode 100644 t/helper/test-json-writer.c
 create mode 100755 t/t0019-json-writer.sh

diff --git a/Makefile b/Makefile
index 9000369..57f58e6 100644
--- a/Makefile
+++ b/Makefile
@@ -662,6 +662,7 @@ TEST_PROGRAMS_NEED_X += test-fake-ssh
 TEST_PROGRAMS_NEED_X += test-genrandom
 TEST_PROGRAMS_NEED_X += test-hashmap
 TEST_PROGRAMS_NEED_X += test-index-version
+TEST_PROGRAMS_NEED_X += test-json-writer
 TEST_PROGRAMS_NEED_X += test-lazy-init-name-hash
 TEST_PROGRAMS_NEED_X += test-line-buffer
 TEST_PROGRAMS_NEED_X += test-match-trees
diff --git a/t/helper/test-json-writer.c b/t/helper/test-json-writer.c
new file mode 100644
index 0000000..bb43efb
--- /dev/null
+++ b/t/helper/test-json-writer.c
@@ -0,0 +1,146 @@
+#include "cache.h"
+#include "json-writer.h"
+
+const char *expect_obj1 = "{\"a\":\"abc\",\"b\":42,\"c\":true}";
+const char *expect_obj2 = "{\"a\":-1,\"b\":2147483647,\"c\":0}";
+const char *expect_obj3 = "{\"a\":0,\"b\":4294967295,\"c\":18446744073709551615}";
+const char *expect_obj4 = "{\"t\":true,\"f\":false,\"n\":null}";
+const char *expect_obj5 = "{\"abc\\tdef\":\"abc\\\\def\"}";
+
+struct strbuf obj1 = STRBUF_INIT;
+struct strbuf obj2 = STRBUF_INIT;
+struct strbuf obj3 = STRBUF_INIT;
+struct strbuf obj4 = STRBUF_INIT;
+struct strbuf obj5 = STRBUF_INIT;
+
+void make_obj1(void)
+{
+	jw_object_begin(&obj1);
+	jw_object_append_string(&obj1, "a", "abc");
+	jw_object_append_int(&obj1, "b", 42);
+	jw_object_append_true(&obj1, "c");
+	jw_object_end(&obj1);
+}
+
+void make_obj2(void)
+{
+	jw_object_begin(&obj2);
+	jw_object_append_int(&obj2, "a", -1);
+	jw_object_append_int(&obj2, "b", 0x7fffffff);
+	jw_object_append_int(&obj2, "c", 0);
+	jw_object_end(&obj2);
+}
+
+void make_obj3(void)
+{
+	jw_object_begin(&obj3);
+	jw_object_append_uint64(&obj3, "a", 0);
+	jw_object_append_uint64(&obj3, "b", 0xffffffff);
+	jw_object_append_uint64(&obj3, "c", 0xffffffffffffffff);
+	jw_object_end(&obj3);
+}
+
+void make_obj4(void)
+{
+	jw_object_begin(&obj4);
+	jw_object_append_true(&obj4, "t");
+	jw_object_append_false(&obj4, "f");
+	jw_object_append_null(&obj4, "n");
+	jw_object_end(&obj4);
+}
+
+void make_obj5(void)
+{
+	jw_object_begin(&obj5);
+	jw_object_append_string(&obj5, "abc" "\x09" "def", "abc" "\\" "def");
+	jw_object_end(&obj5);
+}
+
+const char *expect_arr1 = "[\"abc\",42,true]";
+const char *expect_arr2 = "[-1,2147483647,0]";
+const char *expect_arr3 = "[0,4294967295,18446744073709551615]";
+const char *expect_arr4 = "[true,false,null]";
+
+struct strbuf arr1 = STRBUF_INIT;
+struct strbuf arr2 = STRBUF_INIT;
+struct strbuf arr3 = STRBUF_INIT;
+struct strbuf arr4 = STRBUF_INIT;
+
+void make_arr1(void)
+{
+	jw_array_begin(&arr1);
+	jw_array_append_string(&arr1, "abc");
+	jw_array_append_int(&arr1, 42);
+	jw_array_append_true(&arr1);
+	jw_array_end(&arr1);
+}
+
+void make_arr2(void)
+{
+	jw_array_begin(&arr2);
+	jw_array_append_int(&arr2, -1);
+	jw_array_append_int(&arr2, 0x7fffffff);
+	jw_array_append_int(&arr2, 0);
+	jw_array_end(&arr2);
+}
+
+void make_arr3(void)
+{
+	jw_array_begin(&arr3);
+	jw_array_append_uint64(&arr3, 0);
+	jw_array_append_uint64(&arr3, 0xffffffff);
+	jw_array_append_uint64(&arr3, 0xffffffffffffffff);
+	jw_array_end(&arr3);
+}
+
+void make_arr4(void)
+{
+	jw_array_begin(&arr4);
+	jw_array_append_true(&arr4);
+	jw_array_append_false(&arr4);
+	jw_array_append_null(&arr4);
+	jw_array_end(&arr4);
+}
+
+char *expect_nest1 =
+	"{\"obj1\":{\"a\":\"abc\",\"b\":42,\"c\":true},\"arr1\":[\"abc\",42,true]}";
+
+struct strbuf nest1 = STRBUF_INIT;
+
+void make_nest1(void)
+{
+	jw_object_begin(&nest1);
+	jw_object_append_object(&nest1, "obj1", obj1.buf);
+	jw_object_append_array(&nest1, "arr1", arr1.buf);
+	jw_object_end(&nest1);
+}
+
+void cmp(const char *test, const struct strbuf *buf, const char *exp)
+{
+	if (!strcmp(buf->buf, exp))
+		return;
+
+	printf("error[%s]: observed '%s' expected '%s'\n",
+	       test, buf->buf, exp);
+	exit(1);
+}
+
+#define t(v) do { make_##v(); cmp(#v, &v, expect_##v); } while (0)
+
+int cmd_main(int argc, const char **argv)
+{
+	t(obj1);
+	t(obj2);
+	t(obj3);
+	t(obj4);
+	t(obj5);
+
+	t(arr1);
+	t(arr2);
+	t(arr3);
+	t(arr4);
+
+	t(nest1);
+
+	return 0;
+}
diff --git a/t/t0019-json-writer.sh b/t/t0019-json-writer.sh
new file mode 100755
index 0000000..7b86087
--- /dev/null
+++ b/t/t0019-json-writer.sh
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+test_description='test json-writer JSON generation'
+. ./test-lib.sh
+
+test_expect_success 'unit test of json-writer routines' '
+	test-json-writer
+'
+
+test_done
-- 
2.9.3

Re: [PATCH 0/2] routines to generate JSON data

[Jeff King]

On Fri, Mar 16, 2018 at 07:40:55PM +0000, git@jeffhostetler.com wrote:

> This patch series adds a set of utility routines to compose data in JSON
> format into a "struct strbuf".  The resulting string can then be output
> by commands wanting to support a JSON output format.
> 
> This is a stand alone patch.  Nothing currently uses these routines.  I'm
> currently working on a series to log "telemetry" data (as we discussed
> briefly during Ævar's "Performance Misc" session [1] in Barcelona last
> week).  And I want emit the data in JSON rather than a fixed column/field
> format.  The JSON routines here are independent of that, so it made sense
> to submit the JSON part by itself.
> 
> Back when we added porcelain=v2 format to status, we talked about adding a
> JSON format.  I think the routines in this patch would let us easily do
> that, if someone were interested.  (Extending status is not on my radar
> right now, however.)

I really like the idea of being able to send our machine-readable output
in some "standard" syntax for which people may already have parsers. But
one big hangup with JSON is that it assumes all strings are UTF-8. That
may be OK for telemetry data, but it would probably lead to problems for
something like status porcelain, since Git's view of paths is just a
string of bytes (not to mention possible uses elsewhere like author
names, subject lines, etc).

Before we commit to a standardized format, I think we need to work out
a solution there (because I'd much rather not go down this road for
telemetry data only to find that we cannot use the same standardized
format in other parts of Git).

Some possible solutions I can think of:

  1. Ignore the UTF-8 requirement, making a JSON-like output (which I
     think is what your patches do). I'm not sure what problems this
     might cause on the parsing side.

  2. Specially encode non-UTF-8 bits. I'm not familiar enough with JSON
     to know the options here, but my understanding is that numeric
     escapes are just for inserting unicode code points. _Can_ you
     actually transport arbitrary binary data across JSON without
     base64-encoding it (yech)?

  3. Some other similar format. YAML comes to mind. Last time I looked
     (quite a while ago), it seemed insanely complex, but I think you
     could implement only a reasonable subset. OTOH, I think the tools
     ecosystem for parsing JSON (e.g., jq) is much better.

> Documentation for the new API is given in json-writer.h at the bottom of
> the first patch.

The API generally looks pleasant, but the nesting surprised me.  E.g.,
I'd have expected:

  jw_array_begin(out);
  jw_array_begin(out);
  jw_array_append_int(out, 42);
  jw_array_end(out);
  jw_array_end(out);

to result in an array containing an array containing an integer. But
array_begin() actually resets the strbuf, so you can't build up nested
items like this internally. Ditto for objects within objects. You have
to use two separate strbufs and copy the data an extra time.

To make the above work, I think you'd have to store a little more state.
E.g., the "array_append" functions check "out->len" to see if they need
to add a separating comma. That wouldn't work if we might be part of a
nested array. So I think you'd need a context struct like:

  struct json_writer {
    int first_item;
    struct strbuf out;
  };
  #define JSON_WRITER_INIT { 1, STRBUF_INIT }

to store the state and the output. As a bonus, you could also use it to
store some other sanity checks (e.g., keep a "depth" counter and BUG()
when somebody tries to access the finished strbuf with a hanging-open
object or array).

> I wasn't sure how to unit test the API from a shell script, so I added a
> helper command that does most of the work in the second patch.

In general I'd really prefer to keep the shell script as the driver for
the tests, and have t/helper programs just be conduits. E.g., something
like:

  cat >expect <<-\EOF &&
  {"key": "value", "foo": 42}
  EOF
  test-json-writer >actual \
    object_begin \
    object_append_string key value \
    object_append_int foo 42 \
    object_end &&
  test_cmp expect actual

It's a bit tedious (though fairly mechanical) to expose the API in this
way, but it makes it much easier to debug, modify, or add tests later
on (for example, I had to modify the C program to find out that my
append example above wouldn't work).

-Peff

Re: [PATCH 0/2] routines to generate JSON data

[Ævar Arnfjörð Bjarmason]

On Fri, Mar 16 2018, Jeff King jotted:

> I really like the idea of being able to send our machine-readable output
> in some "standard" syntax for which people may already have parsers. But
> one big hangup with JSON is that it assumes all strings are UTF-8.

FWIW It's not UTF-8 but "Unicode characters", i.e. any Unicode encoding
is valid, not that it changes anything you're pointing out, but people
on Win32 could use UTF-16 as-is if their filenames were in that format.

I'm just going to use UTF-8 synonymously with "Unicode encoding" for the
rest of this mail...

> Some possible solutions I can think of:
>
>   1. Ignore the UTF-8 requirement, making a JSON-like output (which I
>      think is what your patches do). I'm not sure what problems this
>      might cause on the parsing side.

Maybe some JSON parsers are more permissive, but they'll commonly just
die on non-Unicode (usually UTF-8) input, e.g.:

    $ (echo -n '{"str ": "'; head -c 3 /dev/urandom ; echo -n '"}') | perl -0666 -MJSON::XS -wE 'say decode_json(<>)->{str}'
    malformed UTF-8 character in JSON string, at character offset 10 (before "\x{fffd}e\x{fffd}"}") at -e line 1, <> chunk 1.

>   2. Specially encode non-UTF-8 bits. I'm not familiar enough with JSON
>      to know the options here, but my understanding is that numeric
>      escapes are just for inserting unicode code points. _Can_ you
>      actually transport arbitrary binary data across JSON without
>      base64-encoding it (yech)?

There's no way to transfer binary data in JSON without it being shoved
into a UTF-8 encoding, so you'd need to know on the other side that
such-and-such a field has binary in it, i.e. you'll need to invent your
own schema.

E.g.:

    head -c 10 /dev/urandom | perl -MDevel::Peek -MJSON::XS -wE 'my $in = <STDIN>; my $roundtrip = decode_json(encode_json({str => $in}))->{str}; utf8::decode($roundtrip) if $ARGV[0]; say Dump [$in, $roundtrip]' 0

You can tweak that trailing "0" to "1" to toggle the ad-hoc schema,
i.e. after we decode the JSON we go and manually UTF-8 decode it to get
back at the same binary data, otherwise we end up with an UTF-8 escaped
version of what we put in.

>   3. Some other similar format. YAML comes to mind. Last time I looked
>      (quite a while ago), it seemed insanely complex, but I think you
>      could implement only a reasonable subset. OTOH, I think the tools
>      ecosystem for parsing JSON (e.g., jq) is much better.

The lack of fast schema-less formats that supported arrays, hashes
etc. and didn't suck when it came to mixed binary/UTF-8 led us to
implementing our own at work: https://github.com/Sereal/Sereal

I think for git's use-case we're probably best off with JSON. It's going
to work almost all of the time, and when it doesn't it's going to be on
someone's weird non-UTF-8 repo, and those people are probably used to
dealing with crap because of that anyway and can just manually decode
their thing after it gets double-encoded.

That sucks, but given that we'll be using this either for just ASCII
(telemetry) or UTF-8 most of the time, and that realistically other
formats either suck more or aren't nearly as ubiquitous...

Re: [PATCH 0/2] routines to generate JSON data

[Jeff King]

On Sat, Mar 17, 2018 at 12:00:26AM +0100, Ævar Arnfjörð Bjarmason wrote:

> 
> On Fri, Mar 16 2018, Jeff King jotted:
> 
> > I really like the idea of being able to send our machine-readable output
> > in some "standard" syntax for which people may already have parsers. But
> > one big hangup with JSON is that it assumes all strings are UTF-8.
> 
> FWIW It's not UTF-8 but "Unicode characters", i.e. any Unicode encoding
> is valid, not that it changes anything you're pointing out, but people
> on Win32 could use UTF-16 as-is if their filenames were in that format.

But AIUI, non-UTF8 has to come as "\u" escapes, right? That at least
gives us an "out" for exotic characters, but I don't think we can just
blindly dump pathnames into quoted strings, can we?

> > Some possible solutions I can think of:
> >
> >   1. Ignore the UTF-8 requirement, making a JSON-like output (which I
> >      think is what your patches do). I'm not sure what problems this
> >      might cause on the parsing side.
> 
> Maybe some JSON parsers are more permissive, but they'll commonly just
> die on non-Unicode (usually UTF-8) input, e.g.:
> 
>     $ (echo -n '{"str ": "'; head -c 3 /dev/urandom ; echo -n '"}') | perl -0666 -MJSON::XS -wE 'say decode_json(<>)->{str}'
>     malformed UTF-8 character in JSON string, at character offset 10 (before "\x{fffd}e\x{fffd}"}") at -e line 1, <> chunk 1.

OK, that's about what I expected.

> >   2. Specially encode non-UTF-8 bits. I'm not familiar enough with JSON
> >      to know the options here, but my understanding is that numeric
> >      escapes are just for inserting unicode code points. _Can_ you
> >      actually transport arbitrary binary data across JSON without
> >      base64-encoding it (yech)?
> 
> There's no way to transfer binary data in JSON without it being shoved
> into a UTF-8 encoding, so you'd need to know on the other side that
> such-and-such a field has binary in it, i.e. you'll need to invent your
> own schema.

Yuck. That's what I was afraid of. Is there any kind of standard scheme
here? It seems like we lose all of the benefits of JSON if the receiver
has to know whether and when to de-base64 (or whatever) our data.

> I think for git's use-case we're probably best off with JSON. It's going
> to work almost all of the time, and when it doesn't it's going to be on
> someone's weird non-UTF-8 repo, and those people are probably used to
> dealing with crap because of that anyway and can just manually decode
> their thing after it gets double-encoded.

That sounds a bit hand-wavy. While I agree that anybody using non-utf8
at this point is slightly insane, Git _does_ actually work with
arbitrary encodings in things like pathnames. It just seems kind of lame
to settle on a new universal encoding format for output that's actually
less capable than the current output.

> That sucks, but given that we'll be using this either for just ASCII
> (telemetry) or UTF-8 most of the time, and that realistically other
> formats either suck more or aren't nearly as ubiquitous...

I'd hoped to be able to output something like "git status" in JSON,
which is inherently going to deal with user paths.

-Peff

Re: [PATCH 0/2] routines to generate JSON data

[Jacob Keller]

On Fri, Mar 16, 2018 at 2:18 PM, Jeff King <peff@peff.net> wrote:
>   3. Some other similar format. YAML comes to mind. Last time I looked
>      (quite a while ago), it seemed insanely complex, but I think you
>      could implement only a reasonable subset. OTOH, I think the tools
>      ecosystem for parsing JSON (e.g., jq) is much better.
>

I would personally avoid YAML. It's "easier" for humans to read/parse,
but honestly JSON is already simple enough and anyone who writes C or
javascript can likely parse and hand-write JSON anyways. YAML lacks
built-in parsers for most languages, where as many scripting languages
already have JSON parsing built in, or have more easily attainable
libraries available. In contrast, the YAML libraries are much more
complex and less likely to be available.

That's just my own experience at $dayjob though.

Thanks,
Jake

Re: [PATCH 0/2] routines to generate JSON data

[Jeff Hostetler]

On 3/17/2018 3:38 AM, Jacob Keller wrote:
> On Fri, Mar 16, 2018 at 2:18 PM, Jeff King <peff@peff.net> wrote:
>>    3. Some other similar format. YAML comes to mind. Last time I looked
>>       (quite a while ago), it seemed insanely complex, but I think you
>>       could implement only a reasonable subset. OTOH, I think the tools
>>       ecosystem for parsing JSON (e.g., jq) is much better.
>>
> 
> I would personally avoid YAML. It's "easier" for humans to read/parse,
> but honestly JSON is already simple enough and anyone who writes C or
> javascript can likely parse and hand-write JSON anyways. YAML lacks
> built-in parsers for most languages, where as many scripting languages
> already have JSON parsing built in, or have more easily attainable
> libraries available. In contrast, the YAML libraries are much more
> complex and less likely to be available.
> 
> That's just my own experience at $dayjob though.

Agreed.  I just looked at the spec for it and I think it would be
harder for us to be assured we are generating valid output with
leading whitespace being significant (without a lot more inspection
of the strings being passed down to us).

Jeff

Re: [PATCH 0/2] routines to generate JSON data

[Jeff Hostetler]

On 3/16/2018 5:18 PM, Jeff King wrote:
> On Fri, Mar 16, 2018 at 07:40:55PM +0000, git@jeffhostetler.com wrote:
> 
[...]
> I really like the idea of being able to send our machine-readable output
> in some "standard" syntax for which people may already have parsers. But
> one big hangup with JSON is that it assumes all strings are UTF-8. That
> may be OK for telemetry data, but it would probably lead to problems for
> something like status porcelain, since Git's view of paths is just a
> string of bytes (not to mention possible uses elsewhere like author
> names, subject lines, etc).
[...]

I'll come back to the UTF-8/YAML questions in a separate response.


>> Documentation for the new API is given in json-writer.h at the bottom of
>> the first patch.
> 
> The API generally looks pleasant, but the nesting surprised me.  E.g.,
> I'd have expected:
> 
>    jw_array_begin(out);
>    jw_array_begin(out);
>    jw_array_append_int(out, 42);
>    jw_array_end(out);
>    jw_array_end(out);
> 
> to result in an array containing an array containing an integer. But
> array_begin() actually resets the strbuf, so you can't build up nested
> items like this internally. Ditto for objects within objects. You have
> to use two separate strbufs and copy the data an extra time.
> 
> To make the above work, I think you'd have to store a little more state.
> E.g., the "array_append" functions check "out->len" to see if they need
> to add a separating comma. That wouldn't work if we might be part of a
> nested array. So I think you'd need a context struct like:
> 
>    struct json_writer {
>      int first_item;
>      struct strbuf out;
>    };
>    #define JSON_WRITER_INIT { 1, STRBUF_INIT }
> 
> to store the state and the output. As a bonus, you could also use it to
> store some other sanity checks (e.g., keep a "depth" counter and BUG()
> when somebody tries to access the finished strbuf with a hanging-open
> object or array).

Yeah, I thought about that, but I think it gets more complex than that.
I'd need a stack of "first_item" values.  Or maybe the _begin() needs to
increment a depth and set first_item and the _end() needs to always
unset first_item.  I'll look at this gain.

The thing I liked about the bottom-up construction is that it is easier
to collect multiple sets in parallel and combine them during the final
roll-up.  With the in-line nesting, you're tempted to try to construct
the resulting JSON in a single series and that may not fit what the code
is trying to do.  For example, if I wanted to collect an array of error
messages as they are generated and an array of argv arrays and any alias
expansions, then put together a final JSON string containing them and
the final exit code, I'd need to build it in parts.  I can build these
parts in pieces of JSON and combine them at the end -- or build up other
similar data structures (string arrays, lists, or whatever) and then
have a JSON conversion step.  But we can make it work both ways, I just
wanted to keep it simpler.


>> I wasn't sure how to unit test the API from a shell script, so I added a
>> helper command that does most of the work in the second patch.
> 
> In general I'd really prefer to keep the shell script as the driver for
> the tests, and have t/helper programs just be conduits. E.g., something
> like:
> 
>    cat >expect <<-\EOF &&
>    {"key": "value", "foo": 42}
>    EOF
>    test-json-writer >actual \
>      object_begin \
>      object_append_string key value \
>      object_append_int foo 42 \
>      object_end &&
>    test_cmp expect actual
> 
> It's a bit tedious (though fairly mechanical) to expose the API in this
> way, but it makes it much easier to debug, modify, or add tests later
> on (for example, I had to modify the C program to find out that my
> append example above wouldn't work).

Yeah, I wasn't sure if such a simple api required exposing all that
machinery to the shell or not.  And the api is fairly self-contained
and not depending on a lot of disk/repo setup or anything, so my tests
would be essentially static WRT everything else.

With my t0019 script you should have been able use -x -v to see what
was failing.

> 
> -Peff
> 

thanks for the quick review
Jeff

Re: [PATCH 0/2] routines to generate JSON data

[Jeff King]

On Mon, Mar 19, 2018 at 06:19:26AM -0400, Jeff Hostetler wrote:

> > To make the above work, I think you'd have to store a little more state.
> > E.g., the "array_append" functions check "out->len" to see if they need
> > to add a separating comma. That wouldn't work if we might be part of a
> > nested array. So I think you'd need a context struct like:
> > 
> >    struct json_writer {
> >      int first_item;
> >      struct strbuf out;
> >    };
> >    #define JSON_WRITER_INIT { 1, STRBUF_INIT }
> > 
> > to store the state and the output. As a bonus, you could also use it to
> > store some other sanity checks (e.g., keep a "depth" counter and BUG()
> > when somebody tries to access the finished strbuf with a hanging-open
> > object or array).
> 
> Yeah, I thought about that, but I think it gets more complex than that.
> I'd need a stack of "first_item" values.  Or maybe the _begin() needs to
> increment a depth and set first_item and the _end() needs to always
> unset first_item.  I'll look at this gain.

I think you may be able to get by with just unsetting first_item for any
"end". Because as you "pop" to whatever data structure is holding
whatever has ended, you know it's no longer the first item (whatever
just ended was there before it).

I admit I haven't thought too hard on it, though, so maybe I'm missing
something.

> The thing I liked about the bottom-up construction is that it is easier
> to collect multiple sets in parallel and combine them during the final
> roll-up.  With the in-line nesting, you're tempted to try to construct
> the resulting JSON in a single series and that may not fit what the code
> is trying to do.  For example, if I wanted to collect an array of error
> messages as they are generated and an array of argv arrays and any alias
> expansions, then put together a final JSON string containing them and
> the final exit code, I'd need to build it in parts.  I can build these
> parts in pieces of JSON and combine them at the end -- or build up other
> similar data structures (string arrays, lists, or whatever) and then
> have a JSON conversion step.  But we can make it work both ways, I just
> wanted to keep it simpler.

Yeah, I agree that kind of bottom-up construction would be nice for some
cases. I'm mostly worried about inefficiency copying the strings over
and over as we build up the final output.  Maybe that's premature
worrying, though.

If the first_item thing isn't too painful, then it might be nice to have
both approaches available.

> > In general I'd really prefer to keep the shell script as the driver for
> > the tests, and have t/helper programs just be conduits. E.g., something
> > like:
> > 
> >    cat >expect <<-\EOF &&
> >    {"key": "value", "foo": 42}
> >    EOF
> >    test-json-writer >actual \
> >      object_begin \
> >      object_append_string key value \
> >      object_append_int foo 42 \
> >      object_end &&
> >    test_cmp expect actual
> > 
> > It's a bit tedious (though fairly mechanical) to expose the API in this
> > way, but it makes it much easier to debug, modify, or add tests later
> > on (for example, I had to modify the C program to find out that my
> > append example above wouldn't work).
> 
> Yeah, I wasn't sure if such a simple api required exposing all that
> machinery to the shell or not.  And the api is fairly self-contained
> and not depending on a lot of disk/repo setup or anything, so my tests
> would be essentially static WRT everything else.
> 
> With my t0019 script you should have been able use -x -v to see what
> was failing.

I was able to run the test-helper directly. The tricky thing is that I
had to write new C code to test my theory about how the API worked.
Admittedly that's not something most people would do regularly, but I
often seem to end up doing that kind of probing and debugging. Many
times I've found the more generic t/helper programs useful.

I also wonder if various parts of the system embrace JSON, if we'd want
to have a tool for generating it as part of other tests (e.g., to create
"expect" files).

-Peff

Re: [PATCH 0/2] routines to generate JSON data

[Jeff Hostetler]

On 3/20/2018 1:42 AM, Jeff King wrote:
> On Mon, Mar 19, 2018 at 06:19:26AM -0400, Jeff Hostetler wrote:
> 
>>> To make the above work, I think you'd have to store a little more state.
>>> E.g., the "array_append" functions check "out->len" to see if they need
>>> to add a separating comma. That wouldn't work if we might be part of a
>>> nested array. So I think you'd need a context struct like:
>>>
>>>     struct json_writer {
>>>       int first_item;
>>>       struct strbuf out;
>>>     };
>>>     #define JSON_WRITER_INIT { 1, STRBUF_INIT }
>>>
>>> to store the state and the output. As a bonus, you could also use it to
>>> store some other sanity checks (e.g., keep a "depth" counter and BUG()
>>> when somebody tries to access the finished strbuf with a hanging-open
>>> object or array).
>>
>> Yeah, I thought about that, but I think it gets more complex than that.
>> I'd need a stack of "first_item" values.  Or maybe the _begin() needs to
>> increment a depth and set first_item and the _end() needs to always
>> unset first_item.  I'll look at this gain.
> 
> I think you may be able to get by with just unsetting first_item for any
> "end". Because as you "pop" to whatever data structure is holding
> whatever has ended, you know it's no longer the first item (whatever
> just ended was there before it).
> 
> I admit I haven't thought too hard on it, though, so maybe I'm missing
> something.

I'll take a look.  Thanks.

  
>> The thing I liked about the bottom-up construction is that it is easier
>> to collect multiple sets in parallel and combine them during the final
>> roll-up.  With the in-line nesting, you're tempted to try to construct
>> the resulting JSON in a single series and that may not fit what the code
>> is trying to do.  For example, if I wanted to collect an array of error
>> messages as they are generated and an array of argv arrays and any alias
>> expansions, then put together a final JSON string containing them and
>> the final exit code, I'd need to build it in parts.  I can build these
>> parts in pieces of JSON and combine them at the end -- or build up other
>> similar data structures (string arrays, lists, or whatever) and then
>> have a JSON conversion step.  But we can make it work both ways, I just
>> wanted to keep it simpler.
> 
> Yeah, I agree that kind of bottom-up construction would be nice for some
> cases. I'm mostly worried about inefficiency copying the strings over
> and over as we build up the final output.  Maybe that's premature
> worrying, though.
> 
> If the first_item thing isn't too painful, then it might be nice to have
> both approaches available.

True.

  
>>> In general I'd really prefer to keep the shell script as the driver for
>>> the tests, and have t/helper programs just be conduits. E.g., something
>>> like:
>>>
>>>     cat >expect <<-\EOF &&
>>>     {"key": "value", "foo": 42}
>>>     EOF
>>>     test-json-writer >actual \
>>>       object_begin \
>>>       object_append_string key value \
>>>       object_append_int foo 42 \
>>>       object_end &&
>>>     test_cmp expect actual
>>>
>>> It's a bit tedious (though fairly mechanical) to expose the API in this
>>> way, but it makes it much easier to debug, modify, or add tests later
>>> on (for example, I had to modify the C program to find out that my
>>> append example above wouldn't work).
>>
>> Yeah, I wasn't sure if such a simple api required exposing all that
>> machinery to the shell or not.  And the api is fairly self-contained
>> and not depending on a lot of disk/repo setup or anything, so my tests
>> would be essentially static WRT everything else.
>>
>> With my t0019 script you should have been able use -x -v to see what
>> was failing.
> 
> I was able to run the test-helper directly. The tricky thing is that I
> had to write new C code to test my theory about how the API worked.
> Admittedly that's not something most people would do regularly, but I
> often seem to end up doing that kind of probing and debugging. Many
> times I've found the more generic t/helper programs useful.
> 
> I also wonder if various parts of the system embrace JSON, if we'd want
> to have a tool for generating it as part of other tests (e.g., to create
> "expect" files).

Ok, let me see what I can come up with.

Thanks
Jeff