Re: [PATCH 0/4] jk/version-string and google code

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 10, 2012, 15:34 UTC
Message-ID: <7vfw7ulq0a.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120810075342.GA30072@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

> Ugh, the jk/version-string topic breaks fetching from Google Code. With
> my patch, the client unconditionally sends an "agent=foo" capability,
> but the server does not like seeing the unknown capability and ends the
> connection (I'm guessing with some kind of internal exception, since it
> spews "Internal server error" over the protocol channel).

The fix looks sane and the right thing to do.

I've been using 'next' and started seeing this breakage when pushing to code.google.com only a few days ago. My reflog tells that ff5effd (include agent identifier in capability string, 2012-08-03) was merged to my everyday-work branch on this Monday, which is more or less consistent with what I am observing.

Show 16 quoted lines

> This is the right thing to do according to protocol-capabilities.txt,
> which says:
>
>   Client will then send a space separated list of capabilities it wants
>   to be in effect. The client MUST NOT ask for capabilities the server
>   did not say it supports.
>
>   Server MUST diagnose and abort if capabilities it does not understand
>   was sent.  Server MUST NOT ignore capabilities that client requested
>   and server advertised.  As a consequence of these rules, server MUST
>   NOT advertise capabilities it does not understand.
>
> However, that is not how git-core behaves. Its server side will ignore
> an unknown capability coming from the client (so not only is it more
> lenient about what the client does, but it does not follow the "MUST"
> directives in the second paragraph).

Yeah, we probably should fix in the implementation to honor the "MUST". When various implementations of clients can start asking something the server does not support and still yet expect the request to cause the server do unusual things, it will lead to chaos, and honoring that "MUST" is a good way to catch and help diagnosing such a breakage early.

But that is a separate topic and a longer term direction item.

Thanks.