On Tue, Jul 9, 2024 at 3:16 AM Toon claes <toon@xxxxxxxxx> wrote:
>
> Eric Ju <eric.peijian@xxxxxxxxx> writes:
>
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 72a78cdc8c..34958a1747 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > ...
> > +static int get_remote_info(struct batch_options *opt, int argc, const char **argv)
> > +{
> > + int retval = 0;
> > + struct remote *remote = NULL;
> > + struct object_id oid;
> > + struct string_list object_info_options = STRING_LIST_INIT_NODUP;
> > + static struct transport *gtransport;
> > +
> > + /*
> > + * Change the format to "%(objectname) %(objectsize)" when
> > + * remote-object-info command is used. Once we start supporting objecttype
> > + * the default format should change to DEFAULT_FORMAT
> > + */
>
> I believe this comment has become outdated, or got moved around
> incorrectly.
>
Thank you Toon. Sorry, I didn't get it. This comment is not outdated.
It is before this code
if (!opt->format) {
opt->format = "%(objectname) %(objectsize)";
}
And this is related to my 2nd open question in the cover letter
2. Right now, only the size is supported. If the batch command format
contains objectsize:disk or deltabase, it will die. The question
is about objecttype. In the current implementation, it will die too.
But dying on objecttype breaks the default format. We have changed the
default format to %(objectname) %(objectsize) when
remote-object-info is used.
Any suggestions on this approach?
> > diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh
> > new file mode 100755
> > index 0000000000..7a7bdfeb91
> > --- /dev/null
> > +++ b/t/t1017-cat-file-remote-object-info.sh
> > ...
> > +stop_git_daemon
> > +
> > +# Test --batch-command remote-object-info with 'http://' transport
> > +
> > +. "$TEST_DIRECTORY"/lib-httpd.sh
> > +start_httpd
>
> start_httpd skips the remainder of the tests if it fails to start the
> httpd server. That's why I see various other tests which have this at
> the end:
>
> # DO NOT add non-httpd-specific tests here, because the last part of this
> # test script is only executed when httpd is available and enabled.
>
> So I would suggest to add this comment as well, and move the file://
> tests above start_httpd.
>
Thank you. Fixing it in V2
> --
> Toon
