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

[Patrick Steinhardt <ps@pks.im>]

On Tue, Dec 03, 2024 at 02:23:01PM -0500, Peijian Ju wrote:
> On Mon, Nov 25, 2024 at 4:51 AM Patrick Steinhardt <ps@pks.im> wrote:
> > On Mon, Nov 25, 2024 at 12:36:16AM -0500, Eric Ju wrote:
> > > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > > index 5db55fabc4..ad17be69b0 100644
> > > --- a/builtin/cat-file.c
> > > +++ b/builtin/cat-file.c
> > > @@ -576,6 +582,59 @@ 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)
> > > +{
> > > +     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
> > > +     */
> > > +     if (!opt->format)
> > > +             opt->format = "%(objectname) %(objectsize)";
> >
> > Seems like it isn't stale. Hum.
> >
> 
> No, this isn’t stale. As I mentioned in my response to Junio in
> https://lore.kernel.org/git/CAN2LT1Cmsw3RB1kbRBvoeLs8WaQeZWqrG96EQfMkMe_jdKaO4g@mail.gmail.com/,
> adding type support is planned for the next patch series. Based on the
> documentation at https://git-scm.com/docs/protocol-v2#_object_info, it
> seems type isn’t yet supported on the server side either. My plan is
> to implement the logic for both server and client in the next series.
> 
> Unless the reviewers feel strongly that this must be included now, I’d
> prefer to stick to the original plan.

The problem is that you cannot introduce a different format first and
then change it in a subsequent patch series because that would be a
backwards-incompatible change. So if the follow-up patch series would
implement that you cannot revert back to the default format, and
consequently the behaviour would now be inconsistent with the non-remote
case without a good reason.

Patrick