git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC PATCH 02/13] bundle-uri client: add "bundle-uri" parsing + tests

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Aug 5, 2021, 15:07 UTC
Message-ID
<RFC-patch-02.13-70c2209c10-20210805T150534Z-avarab@gmail.com>
In-Reply-To
<RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com>

Add a "test-tool bundle-uri parse" which parses the format defined in the newly specified "bundle-uri" command.

As note in the "bundle-uri" section in protocol-v2.txt we haven't specified any key-values yet, just URI lines, but we need to make sure our client doesn't die if this optional data is provided by the server. Let's add a bundle_uri_parse_line() to do that, in subsequent commits the actual client code in {connect,transport}.c will make use of it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile                    |  1 +
 bundle-uri.c                | 80 ++++++++++++++++++++++++++++++
 bundle-uri.h                | 16 ++++++
 t/helper/test-bundle-uri.c  | 80 ++++++++++++++++++++++++++++++
 t/helper/test-tool.c        |  1 +
 t/helper/test-tool.h        |  1 +
 t/t5750-bundle-uri-parse.sh | 98 +++++++++++++++++++++++++++++++++++++
 7 files changed, 277 insertions(+)
 create mode 100644 t/helper/test-bundle-uri.c
 create mode 100755 t/t5750-bundle-uri-parse.sh
diff --git a/Makefile b/Makefile
index 877c6c47b6..6d3612b962 100644
--- a/Makefile
+++ b/Makefile
@@ -699,6 +699,7 @@ PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 TEST_BUILTINS_OBJS += test-advise.o
 TEST_BUILTINS_OBJS += test-bitmap.o
 TEST_BUILTINS_OBJS += test-bloom.o
+TEST_BUILTINS_OBJS += test-bundle-uri.o
 TEST_BUILTINS_OBJS += test-chmtime.o
 TEST_BUILTINS_OBJS += test-config.o
 TEST_BUILTINS_OBJS += test-crontab.o
diff --git a/bundle-uri.c b/bundle-uri.c
index 2d93e8b003..d48bb78012 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -63,3 +63,83 @@ int bundle_uri_command(struct repository *r,
 
 	return 0;
 }
+
+/**
+ * General API for {transport,connect}.c etc.
+ */
+int bundle_uri_parse_line(struct string_list *bundle_uri, const char *line)
+{
+	int i;
+	struct string_list uri = STRING_LIST_INIT_DUP;
+	struct string_list_item *item = NULL;
+	int err = 0;
+
+	/*
+	 * Right now we don't understand anything beyond the first SP,
+	 * but let's be tolerant and ignore any future unknown
+	 * fields. See the "MUST" note about "bundle-feature-key" in
+	 * technical/protocol-v2.txt
+	 */
+	if (string_list_split(&uri, line, ' ', -1) < 1)
+		return error(_("bundle-uri line not in SP-delimited format: %s"), line);
+
+	for (i = 0; i < uri.nr; i++) {
+		struct string_list kv = STRING_LIST_INIT_DUP;
+		struct string_list_item *kv_item = NULL;
+		const char *arg = uri.items[i].string;
+		int fields;
+
+		/*
+		 * The "string" for each list item is the parsed URI
+		 * at the start of the line
+		 */
+		if (i == 0) {
+			item = string_list_append(bundle_uri, arg);
+			continue;
+		}
+
+		/*
+		 * Anything else on the line is keys or key-value
+		 * pairs separated by "=".
+		 *
+		 * Let's parse the format, even if we don't understand
+		 * any of the keys or values yet.
+		 */
+		assert(item);
+		arg = uri.items[i].string;
+		if (i == 1) {
+			item->util = xcalloc(1, sizeof(struct string_list));
+			string_list_init(item->util, 1);
+		}
+
+		fields = string_list_split(&kv, arg, '=', 2);
+		if (fields < 1 || fields > 2) {
+			err = error("expected `k` or `k=v` in column %d of bundle-uri line '%s', got '%s'",
+				     i, line, arg);
+			string_list_clear(&kv, 0);
+			continue;
+		}
+
+		kv_item = string_list_append(item->util, kv.items[0].string);
+		if (kv.nr == 2)
+			kv_item->util = xstrdup(kv.items[1].string);
+
+		string_list_clear(&kv, 0);
+	}
+	string_list_clear(&uri, 0);
+	return err;
+}
+
+static void bundle_uri_string_list_clear_cb(void *util, const char *string)
+{
+	struct string_list *fields = util;
+	if (!fields)
+		return;
+	string_list_clear(fields, 1);
+	free(fields);
+}
+
+void bundle_uri_string_list_clear(struct string_list *bundle_uri)
+{
+	string_list_clear_func(bundle_uri, bundle_uri_string_list_clear_cb);
+}
diff --git a/bundle-uri.h b/bundle-uri.h
index 6a40efeb39..2d271801b8 100644
--- a/bundle-uri.h
+++ b/bundle-uri.h
@@ -4,6 +4,7 @@
 struct repository;
 struct packet_reader;
 struct packet_writer;
+struct string_list;
 
 /**
  * serve.[ch] API.
@@ -11,4 +12,19 @@ struct packet_writer;
 int bundle_uri_advertise(struct repository *r, struct strbuf *value);
 int bundle_uri_command(struct repository *r, struct packet_reader *request);
 
+/**
+ * General API for {transport,connect}.c etc.
+ */
+
+/**
+ * bundle_uri_parse_line() returns 0 when a valid bundle-uri has been
+ * added to `bundle_uri`, <0 on error.
+ */
+int bundle_uri_parse_line(struct string_list *bundle_uri, const char *line);
+
+/**
+ * Clear the `bundle_uri` list. Just a very thin wrapper on
+ * string_list_clear().
+ */
+void bundle_uri_string_list_clear(struct string_list *bundle_uri);
 #endif /* BUNDLE_URI_H */
diff --git a/t/helper/test-bundle-uri.c b/t/helper/test-bundle-uri.c
new file mode 100644
index 0000000000..014da1b6ab
--- /dev/null
+++ b/t/helper/test-bundle-uri.c
@@ -0,0 +1,80 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "bundle-uri.h"
+#include "strbuf.h"
+#include "string-list.h"
+
+static int cmd__bundle_uri_parse(int argc, const char **argv)
+{
+	const char *usage[] = {
+		"test-tool bundle-uri parse <in",
+		NULL
+	};
+	struct option options[] = {
+		OPT_END(),
+	};
+	struct strbuf sb = STRBUF_INIT;
+	struct string_list list = STRING_LIST_INIT_DUP;
+	int err = 0;
+	struct string_list_item *item;
+
+	argc = parse_options(argc, argv, NULL, options, usage, 0);
+	if (argc)
+		goto usage;
+
+	while (strbuf_getline(&sb, stdin) != EOF) {
+		if (bundle_uri_parse_line(&list, sb.buf) < 0)
+			err = error("bad line: %s", sb.buf);
+	}
+	for_each_string_list_item(item, &list) {
+		struct string_list_item *kv_item;
+		struct string_list *kv = item->util;
+
+		fprintf(stdout, "%s", item->string);
+		if (!kv) {
+			fprintf(stdout, "\n");
+			continue;
+		}
+		for_each_string_list_item(kv_item, kv) {
+			const char *k = kv_item->string;
+			const char *v = kv_item->util;
+
+			if (v)
+				fprintf(stdout, " [kv: %s => %s]", k, v);
+			else
+				fprintf(stdout, " [attr: %s]", k);
+		}
+		fprintf(stdout, "\n");
+	}
+	strbuf_release(&sb);
+
+	bundle_uri_string_list_clear(&list);
+
+	return err < 0 ? 1 : 0;
+usage:
+	usage_with_options(usage, options);
+}
+
+int cmd__bundle_uri(int argc, const char **argv)
+{
+	const char *usage[] = {
+		"test-tool bundle-uri <subcommand> [<options>]",
+		NULL
+	};
+	struct option options[] = {
+		OPT_END(),
+	};
+
+	argc = parse_options(argc, argv, NULL, options, usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION |
+			     PARSE_OPT_KEEP_ARGV0);
+	if (argc == 1)
+		goto usage;
+
+	if (!strcmp(argv[1], "parse"))
+		return cmd__bundle_uri_parse(argc - 1, argv + 1);
+	error("there is no test-tool bundle-uri tool '%s'", argv[1]);
+
+usage:
+	usage_with_options(usage, options);
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 3ce5585e53..b6e1ee7b25 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -17,6 +17,7 @@ static struct test_cmd cmds[] = {
 	{ "advise", cmd__advise_if_enabled },
 	{ "bitmap", cmd__bitmap },
 	{ "bloom", cmd__bloom },
+	{ "bundle-uri", cmd__bundle_uri },
 	{ "chmtime", cmd__chmtime },
 	{ "config", cmd__config },
 	{ "crontab", cmd__crontab },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 9f0f522850..ef839ac726 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -7,6 +7,7 @@
 int cmd__advise_if_enabled(int argc, const char **argv);
 int cmd__bitmap(int argc, const char **argv);
 int cmd__bloom(int argc, const char **argv);
+int cmd__bundle_uri(int argc, const char **argv);
 int cmd__chmtime(int argc, const char **argv);
 int cmd__config(int argc, const char **argv);
 int cmd__crontab(int argc, const char **argv);
diff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh
new file mode 100755
index 0000000000..ef1b9fedea
--- /dev/null
+++ b/t/t5750-bundle-uri-parse.sh
@@ -0,0 +1,98 @@
+#!/bin/sh
+
+test_description="Test bundle-uri bundle_uri_parse_line()"
+
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+test_expect_success 'bundle_uri_parse_line() just URIs' '
+	cat >in <<-\EOF &&
+	http://example.com/bundle.bdl
+	https://example.com/bundle.bdl
+	file:///usr/share/git/bundle.bdl
+	EOF
+
+	# For the simple case
+	cp in expect &&
+
+	test-tool bundle-uri parse <in >actual 2>err &&
+	test_must_be_empty err &&
+	test_cmp expect actual
+'
+
+test_expect_success 'bundle_uri_parse_line() with attributes' '
+	cat >in <<-\EOF &&
+	http://example.com/bundle1.bdl attr
+	http://example.com/bundle2.bdl ibute
+	EOF
+
+
+	cat >expect <<-\EOF &&
+	http://example.com/bundle1.bdl [attr: attr]
+	http://example.com/bundle2.bdl [attr: ibute]
+	EOF
+
+	test-tool bundle-uri parse <in >actual 2>err &&
+	test_must_be_empty err &&
+	test_cmp expect actual
+'
+
+test_expect_success 'bundle_uri_parse_line() with attributes and key-value attributes' '
+	cat >in <<-\EOF &&
+	http://example.com/bundle1.bdl x a=b y c=d z e=f a=b
+	EOF
+
+
+	cat >expect <<-\EOF &&
+	http://example.com/bundle1.bdl [attr: x] [kv: a => b] [attr: y] [kv: c => d] [attr: z] [kv: e => f] [kv: a => b]
+	EOF
+
+	test-tool bundle-uri parse <in >actual 2>err &&
+	test_must_be_empty err &&
+	test_cmp expect actual
+'
+
+test_expect_success 'bundle_uri_parse_line() parsing edge cases: extra SP' '
+	cat >in <<-\EOF &&
+	http://example.com/bundle1.bdl one-space
+	http://example.com/bundle1.bdl  two-space
+	http://example.com/bundle1.bdl   three-space
+	EOF
+
+	# We are anal just the one SP
+	cat >expect <<-\EOF &&
+	http://example.com/bundle1.bdl [attr: one-space]
+	http://example.com/bundle1.bdl [attr: ] [attr: two-space]
+	http://example.com/bundle1.bdl [attr: ] [attr: ] [attr: three-space]
+	EOF
+
+	test-tool bundle-uri parse <in >actual 2>err &&
+	test_must_be_empty err &&
+	test_cmp expect actual
+'
+
+test_expect_success 'bundle_uri_parse_line() parsing edge cases: multiple = in key-values' '
+	cat >in <<-\EOF &&
+	http://example.com/bundle1.bdl k=v=extra
+	http://example.com/bundle2.bdl a=b k=v=extra c=d
+	EOF
+
+	cat >err.expect <<-\EOF &&
+	error: expected `k` or `k=v` in column 1 of bundle-uri line '"'"'http://example.com/bundle1.bdl k=v=extra'"'"', got '"'"'k=v=extra'"'"'
+	error: bad line: http://example.com/bundle1.bdl k=v=extra
+	error: expected `k` or `k=v` in column 2 of bundle-uri line '"'"'http://example.com/bundle2.bdl a=b k=v=extra c=d'"'"', got '"'"'k=v=extra'"'"'
+	error: bad line: http://example.com/bundle2.bdl a=b k=v=extra c=d
+	EOF
+
+	# We fail, but try to continue parsing regardless
+	cat >expect <<-\EOF &&
+	http://example.com/bundle1.bdl
+	http://example.com/bundle2.bdl [kv: a => b] [kv: c => d]
+	EOF
+
+	test_must_fail test-tool bundle-uri parse <in >actual 2>err.actual &&
+	test_cmp err.expect err.actual &&
+	test_cmp expect actual
+'
+
+test_done

-- 
2.33.0.rc0.646.g585563e77f
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 5 of 26 in “Add bundle-uri: resumably clones, static "dumb" CDN etc.”
  1. 00/13 Add bundle-uri: resumably clones, static "dumb" CDN etc.Ævar Arnfjörð Bjarmason, Aug 5, 2021
  2. 01/13 serve: add command to advertise bundle URIsÆvar Arnfjörð Bjarmason, Aug 5, 2021
  3. Derrick StoleeAug 10, 2021
  4. Ævar Arnfjörð BjarmasonAug 23, 2021
  5. 02/13 bundle-uri client: add "bundle-uri" parsing + testsÆvar Arnfjörð Bjarmason, Aug 5, 2021
  6. 04/13 bundle-uri client: add minimal NOOP clientÆvar Arnfjörð Bjarmason, Aug 5, 2021
  7. 03/13 connect.c: refactor sending of agent & object-formatÆvar Arnfjörð Bjarmason, Aug 5, 2021
  8. 05/13 bundle-uri client: add "git ls-remote-bundle-uri"Ævar Arnfjörð Bjarmason, Aug 5, 2021
  9. 07/13 bundle-uri client: add boolean transfer.bundleURI settingÆvar Arnfjörð Bjarmason, Aug 5, 2021
  10. 09/13 fetch-pack: add a deref_without_lazy_fetch_extended()Ævar Arnfjörð Bjarmason, Aug 5, 2021
  11. 06/13 bundle-uri client: add transfer.injectBundleURI supportÆvar Arnfjörð Bjarmason, Aug 5, 2021
  12. 08/13 bundle.h: make "fd" version of read_bundle_header() publicÆvar Arnfjörð Bjarmason, Aug 5, 2021
  13. 10/13 fetch-pack: move --keep=* option filling to a functionÆvar Arnfjörð Bjarmason, Aug 5, 2021
  14. 11/13 index-pack: add --progress-title optionÆvar Arnfjörð Bjarmason, Aug 5, 2021
  15. 12/13 bundle-uri client: support for bundle-uri with "clone"Ævar Arnfjörð Bjarmason, Aug 5, 2021
  16. 13/13 bundle-uri docs: add design notesÆvar Arnfjörð Bjarmason, Aug 5, 2021
  17. brian m. carlsonAug 24, 2021
  18. Ævar Arnfjörð BjarmasonAug 24, 2021
  19. Jonathan NiederAug 6, 2021
  20. Ævar Arnfjörð BjarmasonAug 6, 2021
  21. Jonathan NiederAug 6, 2021
  22. Ævar Arnfjörð BjarmasonAug 7, 2021
  23. Derrick StoleeAug 10, 2021
  24. Ævar Arnfjörð BjarmasonAug 23, 2021
  25. Derrick StoleeAug 24, 2021
  26. Ævar Arnfjörð BjarmasonAug 24, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.