Re: Local clone checks out wrong branch based on remote HEAD

From: Jeff King <peff@peff.net>
Date: Mar 18, 2009, 01:51 UTC
Message-ID: <20090318015158.GA32119@coredump.intra.peff.net>
In-Reply-To: <7viqm7pvkl.fsf@gitster.siamese.dyndns.org>

On Tue, Mar 17, 2009 at 04:40:26PM -0700, Junio C Hamano wrote:

Show 12 quoted lines

> The first round's protocol extension was not quite backward compatible
> but in a benign way, in that it did not break anything but induced a
> harmless warning from older ls-remote.  The second round did not have such
> flaw but it got a "Yuck". 
> 
>     From: Jeff King <peff@peff.net>
>     Date: Mon, 1 Dec 2008 12:44:15 -0500
>     Message-ID: <20081201174414.GA22185@coredump.intra.peff.net>
>     Subject: Re: [PATCH 5/6 (v2)] upload-pack: send the HEAD information
> 
> I somehow feel that the "Yuck" was addressed not to the patches but to the
> problem the patch needs to address.

Actually, you addressed my original "yuck" as it was a misunderstanding on my part of how the protocol worked. I did lay out a few further complaints in:

  http://thread.gmane.org/gmane.comp.version-control.git/102039/focus=102070

To summarize, they were:

  1. sending the server capabilities repeatedly

  2. extensibility of this technique

  3. handling empty clone

I think (1) is something we can just live with. It's a few dozen extra bytes per symref line. But just look at all the crap a normal HTTP request sends. ;)

For (2), I think it would work to simply define each NUL-separated field after the first as an "extra info" slot, and put a header in that slot. So send something like:

  [0-9a-f]{40} HEAD\0<server capabilities>\0symref refs/heads/master\n

And as we add new "here is something extra about this ref" fields, they get assigned new headers. Sadly it is too late to do such a thing for the server capabilities slot, so slot 1 must remain there. But at least we can keep it open for the future.

For (3), we would have to investigate how badly a 0{40} sha-1 break current clients (which understand empty clone, but maybe not this new "branch to be born" syntax). Or maybe it is OK to say "this is the new way to do empty clone, and everything less than v1.6.3 will not be able to handle your empty clone" (which is true for everything less than v1.6.2 or so, anyway).

-Peff