From: Karthik Nayak Date: Sat, 27 Jun 2026 13:14:57 GMT Subject: Re: [PATCH GSoC v14 11/13] cat-file: add remote-object-info to batch-command Message-ID: In-Reply-To: <20260625-ps-eric-work-rebase-v14-11-09f7ffe21a53@gmail.com> Pablo Sabater writes: [snip] > 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 :: > Print object info for object reference ``. This corresponds to the > output of `--batch-check`. > > +remote-object-info ...:: > + Print object info for object references `` at specified > + `` 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] > 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" > + * 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? > + 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 > + 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] > 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]