[PATCH RFC 3/3] cat-file: add --remote-max-batch option

[Pablo Sabater <pabloosabaterr@gmail.com>]

Now that remote-object-info works by requesting in batches, a user might
not want to use the default of 50k OIDs per batch.

Add a new --remote-max-batch option that changes the size of the
batches.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 Documentation/git-cat-file.adoc        | 16 ++++++--
 builtin/cat-file.c                     | 46 ++++++++++++++++++++---
 t/t1017-cat-file-remote-object-info.sh | 68 ++++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
index c4ea2524cf..a21a4d7fd4 100644
--- a/Documentation/git-cat-file.adoc
+++ b/Documentation/git-cat-file.adoc
@@ -14,7 +14,7 @@ SYNOPSIS
 	     [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]
 'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]
 	     [--buffer] [--follow-symlinks] [--unordered]
-	     [--textconv | --filters] [-Z]
+	     [--textconv | --filters] [-Z] [--remote-max-batch=<n>]
 
 DESCRIPTION
 -----------
@@ -143,8 +143,8 @@ are not of the requested type.
 --batch-command::
 --batch-command=<format>::
 	Enter a command mode that reads commands and arguments from stdin. May
-	only be combined with `--buffer`, `--textconv`, `--use-mailmap` or
-	`--filters`.
+	only be combined with `--buffer`, `--textconv`, `--use-mailmap`,
+	`--filters` or `--remote-max-batch`.
 +
 --
 	* When used with `--textconv` or `--filters`, the input lines must
@@ -174,6 +174,9 @@ remote-object-info <remote> <object>...::
 	`<remote>` without downloading objects from the remote.
 	Raise an error when the `object-info` capability is not supported by the remote.
 	Raise an error when no object references are provided.
+	There is no limit on the number of objects; they are requested from
+	the remote in batches over a single connection, see
+	`--remote-max-batch`.
 	This command may be combined with `--buffer`.
 
 flush::
@@ -215,6 +218,13 @@ flush::
 	only once, even if it is stored multiple times in the
 	repository.
 
+--remote-max-batch=<n>::
+	Maximum number of objects that `remote-object-info` asks for in a
+	single `object-info` request. If a `remote-object-info` command is
+	given more objects, the rest are requested in further batches over
+	the same connection. `<n>` must be a positive integer. Defaults
+	to 50000. Requires `--batch-command`.
+
 --follow-symlinks::
 	With `--batch` or `--batch-check`, follow symlinks inside the
 	repository when requesting objects with extended SHA-1
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 4f78194bbd..ca2122c67b 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -59,6 +59,8 @@ struct batch_options {
 	char input_delim;
 	char output_delim;
 	const char *format;
+	size_t remote_max_batch;
+	unsigned custom_object_info_batch;
 };
 
 static const char *force_path;
@@ -799,9 +801,9 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 	char *line_to_split;
 	struct fetch_object_info_results results = FETCH_OBJECT_INFO_RESULTS_INIT;
 	struct oid_array object_info_oids = OID_ARRAY_INIT;
-	size_t batch_size = DEFAULT_REMOTE_BATCH_SIZE;
 	struct transport *transport;
 	struct remote *remote;
+	size_t object_nr;
 
 	line_to_split = xstrdup(line);
 	count = split_cmdline(line_to_split, &argv);
@@ -811,6 +813,7 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 
 	if (count < 2)
 		die(_("remote-object-info requires objects"));
+	object_nr = (size_t)count - 1;
 
 	if (data->info.sizep)
 		results.wants_size = 1;
@@ -826,12 +829,16 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 	if (!transport->smart_options)
 		die(_("smart options required"));
 
-	for (size_t j = 0; j < count - 1; j += batch_size) {
+	for (size_t j = 0; j < object_nr; j += opt->remote_max_batch) {
 		const char **objects = argv + 1 + j;
-		size_t remaining = count - 1 - j;
-		size_t objects_nr = batch_size < remaining ? batch_size : remaining;
+		size_t objects_in_batch_nr;
 
-		get_remote_info(transport, objects, objects_nr, &results, &object_info_oids);
+		if (opt->remote_max_batch < object_nr - j)
+			objects_in_batch_nr = opt->remote_max_batch;
+		else
+			objects_in_batch_nr = object_nr - j;
+
+		get_remote_info(transport, objects, objects_in_batch_nr, &results, &object_info_oids);
 
 		string_list_clear(&data->remote_allowed_atoms, 0);
 		string_list_append(&data->remote_allowed_atoms, "objectname");
@@ -1145,6 +1152,24 @@ static int git_cat_file_config(const char *var, const char *value,
 	return git_default_config(var, value, ctx, cb);
 }
 
+static int set_remote_batch_size(const struct option *opt,
+				 const char *arg,
+				 int unset)
+{
+	struct batch_options *bo = opt->value;
+	size_t n;
+
+	BUG_ON_OPT_NEG(unset);
+
+	if (!git_parse_size_t(arg, &n) || !n)
+		return error(_("%s expects a positive number"), opt->long_name);
+
+	bo->remote_max_batch = n;
+	bo->custom_object_info_batch = 1;
+
+	return 0;
+}
+
 static int batch_option_callback(const struct option *opt,
 				 const char *arg,
 				 int unset)
@@ -1184,6 +1209,7 @@ int cmd_cat_file(int argc,
 	const char *exp_type = NULL, *obj_name = NULL;
 	struct batch_options batch = {
 		.objects_filter = LIST_OBJECTS_FILTER_INIT,
+		.remote_max_batch = DEFAULT_REMOTE_BATCH_SIZE,
 	};
 	int unknown_type = 0;
 	int input_nul_terminated = 0;
@@ -1197,7 +1223,7 @@ int cmd_cat_file(int argc,
 		   "             [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]"),
 		N_("git cat-file (--batch | --batch-check | --batch-command) [--batch-all-objects]\n"
 		   "             [--buffer] [--follow-symlinks] [--unordered]\n"
-		   "             [--textconv | --filters] [-Z]"),
+		   "             [--textconv | --filters] [-Z] [--remote-max-batch=<n>]"),
 		NULL
 	};
 	const struct option options[] = {
@@ -1240,6 +1266,9 @@ int cmd_cat_file(int argc,
 			 N_("follow in-tree symlinks")),
 		OPT_BOOL(0, "unordered", &batch.unordered,
 			 N_("do not order objects before emitting them")),
+		OPT_CALLBACK_F(0, "remote-max-batch", &batch, N_("n"),
+			       N_("set a custom object-info batch size"),
+			       PARSE_OPT_NONEG, set_remote_batch_size),
 		/* Textconv options, stand-ole*/
 		OPT_GROUP(N_("Emit object (blob or tree) with conversion or filter (stand-alone, or with batch)")),
 		OPT_CMDMODE(0, "textconv", &opt,
@@ -1307,6 +1336,11 @@ int cmd_cat_file(int argc,
 		usage_msg_optf(_("'%s' requires a batch mode"), builtin_catfile_usage,
 			       options, "-Z");
 
+	if (batch.custom_object_info_batch &&
+	    batch.batch_mode != BATCH_MODE_QUEUE_AND_DISPATCH)
+		usage_msg_optf(_("'%s' requires '%s'"), builtin_catfile_usage,
+			       options, "--remote-max-batch", "--batch-command");
+
 	batch.input_delim = batch.output_delim = '\n';
 	if (input_nul_terminated)
 		batch.input_delim = '\0';
diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh
index e2919aa061..75624a9498 100755
--- a/t/t1017-cat-file-remote-object-info.sh
+++ b/t/t1017-cat-file-remote-object-info.sh
@@ -61,6 +61,18 @@ set_transport_variables () {
 # Since "%(objecttype)" is currently not supported by the command remote-object-info ,
 # the filters are set to "%(objectname) %(objectsize)" in some test cases.
 
+test_expect_success '--remote-max-batch rejects zero and non-numbers' '
+	test_must_fail git cat-file --batch-command --remote-max-batch=0 </dev/null 2>err &&
+	test_grep "remote-max-batch expects a positive number" err &&
+	test_must_fail git cat-file --batch-command --remote-max-batch=foo </dev/null 2>err &&
+	test_grep "remote-max-batch expects a positive number" err
+'
+
+test_expect_success '--remote-max-batch requires --batch-command' '
+	test_must_fail git cat-file --batch-check --remote-max-batch=2 </dev/null 2>err &&
+	test_grep "requires .--batch-command." err
+'
+
 # Test --batch-command remote-object-info with 'git://' transport with
 # transfer.advertiseobjectinfo set to true, i.e. server has object-info capability
 . "$TEST_DIRECTORY"/lib-git-daemon.sh
@@ -134,6 +146,25 @@ test_expect_success 'batch-command remote-object-info git:// multiple sha1 per l
 	)
 '
 
+test_expect_success 'remote-object-info git:// across batch boundaries' '
+	(
+		set_transport_variables "$daemon_parent" &&
+		cd "$daemon_parent/daemon_client_empty" &&
+
+		echo "$hello_oid $hello_size" >expect &&
+		echo "$tree_oid $tree_size" >>expect &&
+		echo "$unstored_oid missing" >>expect &&
+		echo "$commit_oid $commit_size" >>expect &&
+		echo "$tag_oid $tag_size" >>expect &&
+
+		git cat-file --batch-command="%(objectname) %(objectsize)" \
+			--remote-max-batch=2 >actual <<-EOF &&
+		remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid $tree_oid $unstored_oid $commit_oid $tag_oid
+		EOF
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'batch-command remote-object-info git:// default filter' '
 	(
 		set_transport_variables "$daemon_parent" &&
@@ -461,6 +492,25 @@ test_expect_success 'batch-command remote-object-info file:// default filter' '
 	)
 '
 
+test_expect_success 'remote-object-info file:// with objects a multiple of the batch size' '
+	(
+		set_transport_variables "server" &&
+		server_path="$(pwd)/server" &&
+		cd file_client_empty &&
+
+		echo "$hello_oid $hello_size" >expect &&
+		echo "$tree_oid $tree_size" >>expect &&
+		echo "$commit_oid $commit_size" >>expect &&
+		echo "$tag_oid $tag_size" >>expect &&
+
+		git cat-file --batch-command="%(objectname) %(objectsize)" \
+			--remote-max-batch=2 >actual <<-EOF &&
+		remote-object-info "file://${server_path}" $hello_oid $tree_oid $commit_oid $tag_oid
+		EOF
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'batch-command -Z remote-object-info file:// default filter' '
 	(
 		set_transport_variables "server" &&
@@ -741,6 +791,24 @@ test_expect_success 'remote-object-info fails on not providing OID' '
 	)
 '
 
+test_expect_success 'remote-object-info http:// across batch boundaries' '
+	(
+		set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+		cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" &&
+
+		echo "$hello_oid $hello_size" >expect &&
+		echo "$tree_oid $tree_size" >>expect &&
+		echo "$unstored_oid missing" >>expect &&
+		echo "$commit_oid $commit_size" >>expect &&
+		echo "$tag_oid $tag_size" >>expect &&
+
+		git cat-file --batch-command="%(objectname) %(objectsize)" \
+			--remote-max-batch=2 >actual <<-EOF &&
+		remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid $tree_oid $unstored_oid $commit_oid $tag_oid
+		EOF
+		test_cmp expect actual
+	)
+'
 
 # Test --batch-command remote-object-info with 'http://' transport and
 # transfer.advertiseobjectinfo set to false, i.e. server does not have object-info capability

-- 
2.56.0