Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch

From: Philip Oakley <philipoakley@iee.org>
Date: May 28, 2017, 11:21 UTC
Message-ID: <48CB2547397B471C99F54CFB3F113C3F@PhilipOakley>
In-Reply-To: <xmqqh905c0tm.fsf@gitster.mtv.corp.google.com>

From: "Junio C Hamano" <gitster@pobox.com>

Show 12 quoted lines

> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> However given the discussion about an unborn HEAD, the option here
>> would be to also pass the NULL sha for the symref and then add the
>> annotation 'HEAD' after an extra \0, in the same way that an active
>> symref could be annotated with the '\0HEAD'. This would kill two birds
>> with one stone!
>
> Are you aware of the symref capability that is already advertised in
> the initial upload-pack response?  Right now, we do so only when
> HEAD actually points at something, and the earlier suggestion by
> Peff is to do so unconditionally, even when HEAD is dangling.

The suggestion is the otherway around. I would argue (as a viewpoint) that what we advertise are object IDs and their associated refs, sorted by ref name. (I'm thinking of the git/Documentation/technical/pack-protocol.txt here). My suggestion was that when we get to the sorted ref that HEAD points to (including the unborn oid) that we annotate that ref.

I didn't quite follow Peff's suggestion as to where the list change went and if that was a protocol change.

There are two current fault scenarios. a. The currently reported case where HEAD has an unborn symref b. The multiple ref HEAD case, where the HEAD oid matches multiple advertised refs, and the correct symref is not the first listed (which is the case I had looked at a few years ago, a prompted me to respond).

With the above discussions, we would have both a NULL oid for the unborn (sym)ref sent (if needed), and the (sym)ref for HEAD would have the extra annotation. That would at least not break the protocol rule that "If HEAD is not a valid ref, HEAD MUST NOT appear in the advertisement list at all" (it is now an annotation to another valid ref [or the unborn symref]).

Show 9 quoted lines

>
> Existing clients that are symref aware do not do anything (good or
> bad) when a HEAD that is dangling [*1*] is advertised, so such a
> change will not hurt (but it would not help by itself either).
> Ancient clients that are not even aware of the symref are not
> affected.
>
> Then new clients _could_ start paying attention to the advertised
> HEAD that is dangling.

My main step was that for case b, so that we don't need to guess_remote_head(). The annotation would have already told us.

>
> The bundle transport is a different beast.  I do not think it
> advertises where HEAD is pointing at, whether it is dangling or
> not.

My understanding was that the Bundle was a tiny wrapper and that it has that same protocol header, which was then decoded using the same code. Hence the hope that it could fix both the bundle and remote clone problems. I'd just avoided stepping into remote clone arena because of other implementations.

Show 10 quoted lines

>
>
> [Footnote]
>
> *1* A HEAD symref that is advertised in the upload-pack response is
>    dangling when its pointee does not appear in the set of refs
>    that are advertised.  Félix's case would have shown HEAD
>    pointing at refs/heads/master in the symref capability extension,
>    but the list of refs and their values would not have included
>    that ref (there was only refs/heads/MASTER "for joke reasons").

I hope I haven't confused the different parts of the negotiation and transfer, which can be confusing.

Philip