Subject: Re: [PATCH v5 4/6] serve: advertise object-info feature

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 01 Aug 2022 20:44:57 +0200

On Mon, Aug 01 2022, Calvin Wan wrote:

>> OK.  Now we will no longer advertise a bare "object-info", but
>> "object-info=size" (and possibly in the future things other than
>> "size").  How would this change affect older clients who knows what
>> to do with "object-info" but not "object-info=<values>" yet?
>
> This was a tricky tradeoff that I definitely think I should have
> discussed more in the commit message. The issue with how object
> info is currently implemented is that it is very inflexible for adding
> new parameters.
>
> This is how object-info currently parses a client request:
>
> while (packet_reader_read(request) == PACKET_READ_NORMAL) {
>     if (!strcmp("size", request->line)) {
>         info.size = 1;
>         continue;
>     }
>
>     if (parse_oid(request->line, &oid_str_list))
>         continue;
>
>     packet_writer_error(&writer,
>     "object-info: unexpected line: '%s'",
>     request->line);
> }
>
> Object-info supports "size" right now but, let's say I want to add
> "type" as a parameter. OK I add another if statement like:
>
> if (!strcmp("type", request->line)) {
>     info.type = 1;
>     continue;
> }
>
> And we update the docs to say "type" is now supported by
> object-info. If a user now attempts to request "size" and "type"
> from a server that has not been updated to support "type",
> then the user gets an error message "object-info: unexpected
> line: 'type'", which is another situation that is a bad experience
> for older clients. The client has no way of knowing that their
> failure is caused by a server version issue.
>
> Essentially I think at some point we have to bite the bullet and say
> we need to rework some part of the object-info advertisement (or
> if anyone has a better idea of achieving the same goal) so that we
> can make future incremental changes to object-info. If the supported
> parameters are posted in the advertisement, then the client doesn't
> have to first make a request to find out that their requested
> parameter isn't support by the server. While you noted that we can't
> make the assumption now that nobody is using the current
> object-info feature, I think the benefit of the change outweighs
> the cost of affecting the possibly small amount of users of this
> feature. (A quick search on stack overflow for "object-info" tagged
> under [git] returned no questions about it so that's what I used as
> a cursory estimate for how popular this feature is).
>
> Curious to hear what your thoughts on this are Junio, since as
> much as I'd like to create a seamless upgrade experience for
> older clients, I'm out of ideas as to how I would do so.

I haven't looked deeply into this case, but in general for such protcol
incompatibilities we could just create an object-info2, and have that
use some extensible calling convention we wish we'd have used from day
1.

Then have new clients understand both (and prefer the new verb), and
older clients use object-info without breakage.

Or we could call the new thing "cat-file", and have it accept any
arbitrary options it does, and then limit it to some sensible subset for
now :)