Re: [PATCH v3 1/1] protocol: advertise multiple supported versions
- From
Josh Steadmon <steadmon@google.com>
- Date
- Nov 16, 2018, 19:59 UTC
- Message-ID
- <20181116195932.GA9703@google.com>
- In-Reply-To
- <xmqq4lchn3jy.fsf@gitster-ct.c.googlers.com>
On 2018.11.16 11:45, Junio C Hamano wrote:
Show 27 quoted lines
> 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.Yes, that is correct. The next version will remove the special cases here.
Show 5 quoted lines
> > 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.