Re: [PATCH GSoC v14 11/13] cat-file: add remote-object-info to batch-command

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jun 27, 2026, 13:14 UTC
Message-ID: <CAOLa=ZSCKbwckV-j+DyUqOkDkfYcW5xSCPza562mq+OJtQc7DA@mail.gmail.com>
In-Reply-To: <20260625-ps-eric-work-rebase-v14-11-09f7ffe21a53@gmail.com>

Pablo Sabater <pabloosabaterr@gmail.com> writes:

[snip]

Show 26 quoted lines

> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
> index 86b9181599..aba20eb770 100644
> --- a/Documentation/git-cat-file.adoc
> +++ b/Documentation/git-cat-file.adoc
> @@ -169,6 +169,13 @@ info <object>::
>  	Print object info for object reference `<object>`. This corresponds to the
>  	output of `--batch-check`.
>
> +remote-object-info <remote> <object>...::
> +	Print object info for object references `<object>` at specified
> +	`<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.
> +	This command may be combined with `--buffer`.
> +
>  flush::
>  	Used with `--buffer` to execute all preceding commands that were issued
>  	since the beginning or since the last flush was issued. When `--buffer`
> @@ -312,7 +319,8 @@ newline. The available atoms are:
>  	The full hex representation of the object name.
>
>  `objecttype`::
> -	The type of the object (the same as `cat-file -t` reports).
> +	The type of the object (the same as `cat-file -t` reports). See
> +	`CAVEATS` below. Not supported by `remote-object-info`.
>

Do we have to keep adding 'Not supported by `remote-object-info`' to each type? Can't we do the inverse and only add 'Supported by `remote-object-info`' to `objectsize`. This avoid having to add this line to every new type.

>  If no format is specified, the default format is `%(objectname)
> -%(objecttype) %(objectsize)`.
> +%(objecttype) %(objectsize)`, except for `remote-object-info` commands which use
> +`%(objectname) %(objectsize)` for now because "%(objecttype)" is not supported yet.

Nit: I would drop the 'for now' here, since we don't know when the changes
for 'objecttype' will land.

[snip]

Show 20 quoted lines

>  enum batch_mode {
>  	BATCH_MODE_CONTENTS,
> @@ -633,6 +649,81 @@ static void batch_one_object(const char *obj_name,
>  	object_context_release(&ctx);
>  }
>
> +static int get_remote_info(struct batch_options *opt,
> +			   int argc,
> +			   const char **argv,
> +			   struct object_info **remote_object_info,
> +			   struct oid_array *object_info_oids)
> +{
> +	int retval = 0;
> +	struct remote *remote = NULL;
> +	struct object_id oid;
> +	struct string_list object_info_options = STRING_LIST_INIT_NODUP;
> +	struct transport *gtransport;
> +
> +	/*
> +	 * Change the format to "%(objectname) %(objectsize)" when

Nit: perhaps prepend a "TODO"

Show 48 quoted lines

> +	 * remote-object-info command is used. Once we start supporting objecttype
> +	 * the default format should change to DEFAULT_FORMAT.
> +	 */
> +	if (!opt->format)
> +		opt->format = "%(objectname) %(objectsize)";
> +
> +	remote = remote_get(argv[0]);
> +	if (!remote)
> +		die(_("must supply valid remote when using remote-object-info"));
> +
> +	oid_array_clear(object_info_oids);
> +	for (size_t i = 1; i < argc; i++) {
> +		if (get_oid_hex(argv[i], &oid)) {
> +			size_t len = strlen(argv[i]);
> +
> +			if (len < the_hash_algo->hexsz && len >= 4) {
> +				size_t j;
> +				for (j = 0; j < len; j++)
> +					if (!isxdigit(argv[i][j]))
> +						break;
> +				if (j == len)
> +					die(_("remote-object-info does not support "
> +					      "short oids, %d characters required"),
> +					    (int)the_hash_algo->hexsz);
> +			}
> +			die(_("not a valid object name '%s'"), argv[i]);
> +		}
> +		oid_array_append(object_info_oids, &oid);
> +	}
> +
> +	if (!object_info_oids->nr)
> +		die(_("remote-object-info requires objects"));
> +
> +	gtransport = transport_get(remote, NULL);
> +
> +	if (!gtransport->smart_options) {
> +		retval = -1;
> +		goto cleanup;
> +	}
> +
> +	CALLOC_ARRAY(*remote_object_info, object_info_oids->nr);
> +	gtransport->smart_options->object_info = 1;
> +	gtransport->smart_options->object_info_oids = object_info_oids;
> +
> +	/* 'objectsize' is the only option currently supported */
> +	if (!strstr(opt->format, "%(objectsize)"))
> +		die(_("%s is currently not supported with remote-object-info"), opt->format);
> +

Aren't we setting the opt->format ourselves in this function? Why do we need to check it?

Show 37 quoted lines

> +	string_list_append(&object_info_options, "size");
> +
> +	if (object_info_options.nr > 0) {
> +		gtransport->smart_options->object_info_options = &object_info_options;
> +		gtransport->smart_options->object_info_data = *remote_object_info;
> +		retval = transport_fetch_refs(gtransport, NULL);
> +	}
> +cleanup:
> +	string_list_clear(&object_info_options, 0);
> +	transport_disconnect(gtransport);
> +	return retval;
> +}
> +
>  struct object_cb_data {
>  	struct batch_options *opt;
>  	struct expand_data *expand;
> @@ -714,6 +805,57 @@ static void parse_cmd_mailmap(struct batch_options *opt UNUSED,
>  		load_mailmap();
>  }
>
> +static void parse_cmd_remote_object_info(struct batch_options *opt,
> +					 const char *line, struct strbuf *output,
> +					 struct expand_data *data)
> +{
> +	int count;
> +	const char **argv;
> +	char *line_to_split;
> +	struct object_info *remote_object_info = NULL;
> +	struct oid_array object_info_oids = OID_ARRAY_INIT;
> +
> +	if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
> +		die(_("remote-object-info command too long"));
> +
> +	line_to_split = xstrdup(line);
> +	count = split_cmdline(line_to_split, &argv);
> +	if (count < 0)
> +		die(_("split remote-object-info command"));

We should  be using `split_cmdline_strerror()` here

Show 34 quoted lines

> +	if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
> +		die(_("remote-object-info supports at most %d objects"),
> +		    MAX_ALLOWED_OBJ_LIMIT);
> +
> +	if (get_remote_info(opt, count, argv, &remote_object_info,
> +			    &object_info_oids))
> +		goto cleanup;
> +
> +	data->skip_object_info = 1;
> +	for (size_t i = 0; i < object_info_oids.nr; i++) {
> +		data->oid = object_info_oids.oid[i];
> +		if (remote_object_info[i].sizep) {
> +			/*
> +			 * When reaching here, it means remote-object-info can retrieve
> +			 * information from server without downloading them.
> +			 */
> +			data->size = *remote_object_info[i].sizep;
> +			opt->batch_mode = BATCH_MODE_INFO;
> +			batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
> +		} else {
> +			report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
> +		}
> +	}
> +	data->skip_object_info = 0;
> +
> +cleanup:
> +	for (size_t i = 0; i < object_info_oids.nr; i++)
> +		free_object_info_contents(&remote_object_info[i]);
> +	free(line_to_split);
> +	free(argv);
> +	free(remote_object_info);
> +	oid_array_clear(&object_info_oids);
> +}
> +

[snip]

Show 12 quoted lines

> diff --git a/t/meson.build b/t/meson.build
> index 3219264fe7..54d21111a3 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -170,6 +170,7 @@ integration_tests = [
>    't1014-read-tree-confusing.sh',
>    't1015-read-index-unmerged.sh',
>    't1016-compatObjectFormat.sh',
> +  't1017-cat-file-remote-object-info.sh',
>    't1020-subdirectory.sh',
>    't1022-read-tree-partial-clone.sh',
>    't1050-large.sh',

[snip]