Re: [PATCH v2 6/6] cat-file: add remote-object-info to batch-command

On Tue, Sep 24, 2024 at 8:13 AM Christian Couder
[off-list ref] wrote:

On Sat, Jul 20, 2024 at 5:44 AM Eric Ju [off-list ref] wrote:

quoted

+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.

quoted

+       This command may be combined with `--buffer`.

quoted

 `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.

quoted

+`%(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.

quoted

+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.

quoted

+
+

Maybe a single blank line is enough.

Thank you. Fixed in V3.

quoted

 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

[...]

quoted

+       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.

quoted

+               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;
+       }

[...]

quoted

+       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.

quoted

+                        * infomation from server withoug downloading them, and the objects

s/infomation from server withoug/information from server without/

Thank you. Fixed in V3.

quoted

+                        * have been fetched to client already.
+                        * Print the infomation using the logic for local objects.

s/infomation/information/

Thank you. Fixed in V3.

quoted

+                        */
+                       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.

quoted

 };