On Tue, Sep 24, 2024 at 8:13 AM Christian Couder
<christian.couder@xxxxxxxxx> wrote:
>
> On Sat, Jul 20, 2024 at 5:44 AM Eric Ju <eric.peijian@xxxxxxxxx> wrote:
>
> > +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/
Thank you. Fixed in V3.
>
>
> > + This command may be combined with `--buffer`.
>
> > `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/
Thank you. Fixed in V3.
>
>
> > +`%(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)/
>
Thank you. Fixed in V3.
>
> > +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/
>
Thank you. Fixed in V3.
>
> > +
> > +
>
> Maybe a single blank line is enough.
Thank you. Fixed in V3.
>
>
> > 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
>
> [...]
>
> > + 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");
Thank you. Revised in V3.
>
>
> > + 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;
> > + }
>
> [...]
>
> > + 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 retrive
>
> s/retrive/retrieve/
>
Thank you. Fixed in V3.
>
> > + * infomation from server withoug downloading them, and the objects
>
> s/infomation from server withoug/information from server without/
>
Thank you. Fixed in V3.
> > + * have been fetched to client already.
> > + * Print the infomation using the logic for local objects.
>
> s/infomation/information/
Thank you. Fixed in V3.
>
>
> > + */
> > + 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 "}".
>
Thank you. Fixed in V3.
>
> > };
