Re: [PATCH v2 6/6] cat-file: add remote-object-info to batch-command
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Sep 24, 2024, 12:13 UTC
- Message-ID
- <CAP8UFD0DWWi19HesaBEHtWXrcqj0184xKRAehwKCUJ-bhR5saQ@mail.gmail.com>
- In-Reply-To
- <20240720034337.57125-7-eric.peijian@gmail.com>
On Sat, Jul 20, 2024 at 5:44 AM Eric Ju <eric.peijian@gmail.com> wrote:
Show 5 quoted lines
> +remote-object-info <remote> <object>...:: > + Print object info for object references `<object>` at specified <remote> without > + downloading objects from remote. If the object-info capability is not > + supported by the server, the objects will be downloaded instead. > + Error when no object references is provided.
Maybe s/is provided/are provided/
> + This command may be combined with `--buffer`.
Show 15 quoted lines
> `deltabase`:: > If the object is stored as a delta on-disk, this expands to the > full hex representation of the delta base object name. > Otherwise, expands to the null OID (all zeroes). See `CAVEATS` > - below. > + below. Not supported by `remote-object-info`. > > `rest`:: > If this atom is used in the output string, input lines are split > @@ -314,7 +323,9 @@ newline. The available atoms are: > line) are output in place of the `%(rest)` atom. > > If no format is specified, the default format is `%(objectname) > -%(objecttype) %(objectsize)`. > +%(objecttype) %(objectsize)`, except remote-object-info command who uses
s/except remote-object-info command who uses/except for `remote-object-info` commands which use/
Show 10 quoted lines
> +`%(objectname) %(objectsize)` for now because "%(objecttype)" is not supported yet. > +When "%(objecttype)" is supported, default format should be unified. > > If `--batch` is specified, or if `--batch-command` is used with the `contents` > command, the object information is followed by the object contents (consisting > @@ -396,6 +407,10 @@ scripting purposes. > CAVEATS > ------- > > +Note that since objecttype, objectsize:disk and deltabase are currently not supported by the
s/objecttype, objectsize:disk and deltabase/%(objecttype), %(objectsize:disk) and %(deltabase)/
> +remote-object-info, git will error and exit when they are in the format string.
s//remote-object-info, git /`remote-object-info` command, we/
> + > +
Maybe a single blank line is enough.
> Note that the sizes of objects on disk are reported accurately, but care > should be taken in drawing conclusions about which refs or objects are > responsible for disk usage. The size of a packed non-delta object may be
[...]
Show 14 quoted lines
> + gtransport = transport_get(remote, NULL);
> + if (gtransport->smart_options) {
> + CALLOC_ARRAY(remote_object_info, object_info_oids.nr);
> + gtransport->smart_options->object_info = 1;
> + gtransport->smart_options->object_info_oids = &object_info_oids;
> + /*
> + * 'size' is the only option currently supported.
> + * Other options that are passed in the format will exit with error.
> + */
> + if (strstr(opt->format, "%(objectsize)")) {
> + string_list_append(&object_info_options, "size");
> + } else {
> + die(_("%s is currently not supported with remote-object-info"), opt->format);
> + }Something like the following might be a bit shorter and simpler:
/* 'objectsize' is the only option currently supported */
if (!strstr(opt->format, "%(objectsize)"))
die(_("%s is currently not supported with
remote-object-info"), opt->format);string_list_append(&object_info_options, "size");
Show 8 quoted lines
> + 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);
> + }
> + } else {
> + retval = -1;
> + }[...]
Show 11 quoted lines
> + opt->use_remote_info = 1;
> + 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) {
> + data->size = *remote_object_info[i].sizep;
> + } else {
> + /*
> + * When reaching here, it means remote-object-info can't retrives/retrive/retrieve/
> + * infomation from server withoug downloading them, and the objects
s/infomation from server withoug/information from server without/
> + * have been fetched to client already. > + * Print the infomation using the logic for local objects.
s/infomation/information/
Show 33 quoted lines
> + */
> + data->skip_object_info = 0;
> + }
> +
> + opt->batch_mode = BATCH_MODE_INFO;
> + batch_object_write(argv[i+1], output, opt, data, NULL, 0);
> +
> + }
> + opt->use_remote_info = 0;
> + 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);
> +}
> +
> static void dispatch_calls(struct batch_options *opt,
> struct strbuf *output,
> struct expand_data *data,
> @@ -696,9 +803,10 @@ static const struct parse_cmd {
> parse_cmd_fn_t fn;
> unsigned takes_args;
> } commands[] = {
> - { "contents", parse_cmd_contents, 1},
> - { "info", parse_cmd_info, 1},
> - { "flush", NULL, 0},
> + { "contents", parse_cmd_contents, 1 },
> + { "info", parse_cmd_info, 1 },
> + { "remote-object-info", parse_cmd_remote_object_info, 1 },
> + { "flush", NULL, 0 },I am not sure it's a good thing to add a space before "}".
> };