Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

[Junio C Hamano <gitster@pobox.com>]

Josh Steadmon <steadmon@google.com> writes:

>> What I was alludding to was a lot simpler, though.  An advert string
>> "version=0:version=1" from a client that prefers version 0 won't be
>> !strcmp("version=0", advert) but seeing many strcmp() in the patch
>> made me wonder.
>
> Ah I see. The strcmp()s against "version=0" are special cases for where
> it looks like the client does not understand any sort of version
> negotiation. If we see multiple versions listed in the advert, then the
> rest of the selection logic should do the right thing.

OK, let me try to see if I understand correctly by rephrasing.

If the client does not say anything about which version it prefers
(because it only knows about version 0 without even realizing that
there is a version negotiation exchange), we substitute the lack of
proposed versions with string "version=0", and the strcmp()s I saw
and was puzzled by are all about special casing such a client.  But
we would end up behaving the same way (at least when judged only by
externally visible effects) to a client that is aware of version
negotiation and tells us it prefers version 0 (and nothing else)
with the selection logic anyway.

Did I get it right?  If so, yeah, I think it makes sense to avoid
two codepaths that ends up doing the same thing by removing the
special case.

> However, I think that it might work to remove the special cases. In the
> event that the client is so old that it doesn't understand any form of
> version negotiation, it should just ignore the version fields /
> environment vars. If you think it's cleaner to remove the special cases,
> let me know.