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

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

[Félix Saparelli]

Hi,

I created a git repository that, for joke reasons, has a single branch
called MASTER (in uppercase). Upon cloning this repo, git attempts to
checkout the master branch (in lowercase), which does not exist.
Checking out the MASTER branch manually afterwards works.

$ git clone git@github.com:passcod/UPPERCASE-NPM.git
Cloning into 'UPPERCASE-NPM'...
remote: Counting objects: 14, done.
remote: Compressing objects: 100% (11/11), done.
remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0
Receiving objects: 100% (14/14), done.
Resolving deltas: 100% (3/3), done.
warning: remote HEAD refers to nonexistent ref, unable to checkout.

$ cd UPPERCASE-NPM
$ ls -a
. .. .git
$ git branch
$ git checkout MASTER
Branch MASTER set up to track remote branch MASTER from origin.
Switched to a new branch 'MASTER'
$ ls -a
. .. .git NPM package.json README
$ git branch
* MASTER

Some platform information:

$ git version
git version 2.12.2

$ uname -a
Linux felix-probook 4.10.13-1-ARCH #1 SMP PREEMPT Thu Apr 27 12:15:09
CEST 2017 x86_64 GNU/Linux

Git was installed from the default Arch Linux package.

Thanks,
Félix

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

[Junio C Hamano]

Félix Saparelli <felix@passcod.name> writes:

> I created a git repository that, for joke reasons, has a single branch
> called MASTER (in uppercase). Upon cloning this repo, git attempts to
> checkout the master branch (in lowercase), which does not exist.

See what Git told you carefully and you can guess, I think.

> $ git clone git@github.com:passcod/UPPERCASE-NPM.git
> Cloning into 'UPPERCASE-NPM'...
> remote: Counting objects: 14, done.
> remote: Compressing objects: 100% (11/11), done.
> remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0
> Receiving objects: 100% (14/14), done.
> Resolving deltas: 100% (3/3), done.
> warning: remote HEAD refers to nonexistent ref, unable to checkout.

So you have MASTER but not master, but your HEAD still is pointing
at non-existing master.  As HEAD is the way the remote tells the
clone what the default branch to be checked out is, the command
reports "we cannot do a checkout of the default branch."

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

[Samuel Lijin]

For some reason the repo on GH does not have a HEAD pointer:

$ git ls-remote https://github.com/passcod/UPPERCASE-NPM.git
efc7dbfd6ca155d5d19ce67eb98603896062f35a        refs/heads/MASTER
e60ea8e6ec45ec45ff44ac8939cb4105b16477da        refs/pull/1/head
f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c        refs/pull/2/head
0d9b3a1268ff39350e04a7183af0add912b686e6        refs/tags/V1.0.0
efc7dbfd6ca155d5d19ce67eb98603896062f35a        refs/tags/V1.0.1

I'm not sure how you managed to do that, since GH rejects attempts to
delete the current branch, but I believe if you set the default branch
to MASTER it will work correctly.

On Mon, May 22, 2017 at 5:42 PM, Félix Saparelli <felix@passcod.name> wrote:
> Hi,
>
> I created a git repository that, for joke reasons, has a single branch
> called MASTER (in uppercase). Upon cloning this repo, git attempts to
> checkout the master branch (in lowercase), which does not exist.
> Checking out the MASTER branch manually afterwards works.
>
> $ git clone git@github.com:passcod/UPPERCASE-NPM.git
> Cloning into 'UPPERCASE-NPM'...
> remote: Counting objects: 14, done.
> remote: Compressing objects: 100% (11/11), done.
> remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0
> Receiving objects: 100% (14/14), done.
> Resolving deltas: 100% (3/3), done.
> warning: remote HEAD refers to nonexistent ref, unable to checkout.
>
> $ cd UPPERCASE-NPM
> $ ls -a
> . .. .git
> $ git branch
> $ git checkout MASTER
> Branch MASTER set up to track remote branch MASTER from origin.
> Switched to a new branch 'MASTER'
> $ ls -a
> . .. .git NPM package.json README
> $ git branch
> * MASTER
>
> Some platform information:
>
> $ git version
> git version 2.12.2
>
> $ uname -a
> Linux felix-probook 4.10.13-1-ARCH #1 SMP PREEMPT Thu Apr 27 12:15:09
> CEST 2017 x86_64 GNU/Linux
>
> Git was installed from the default Arch Linux package.
>
> Thanks,
> Félix
>

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

[Jeff King]

On Tue, May 23, 2017 at 04:01:06AM -0400, Samuel Lijin wrote:

> For some reason the repo on GH does not have a HEAD pointer:
> 
> $ git ls-remote https://github.com/passcod/UPPERCASE-NPM.git
> efc7dbfd6ca155d5d19ce67eb98603896062f35a        refs/heads/MASTER
> e60ea8e6ec45ec45ff44ac8939cb4105b16477da        refs/pull/1/head
> f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c        refs/pull/2/head
> 0d9b3a1268ff39350e04a7183af0add912b686e6        refs/tags/V1.0.0
> efc7dbfd6ca155d5d19ce67eb98603896062f35a        refs/tags/V1.0.1
> 
> I'm not sure how you managed to do that, since GH rejects attempts to
> delete the current branch, but I believe if you set the default branch
> to MASTER it will work correctly.

The HEAD branch on the server side is still pointing at
refs/heads/master. But since that doesn't exist, upload-pack doesn't
advertise HEAD at all (it has no sha1).

You can recreate this situation with:

  cd /some/repo
  git init --bare dst.git
  git push dst.git HEAD:refs/heads/MASTER
  git ls-remote dst.git

No current-branch deletion required.

Félix should be able to re-point the server-side HEAD to MASTER via
GitHub's web interface.

GitHub's post-receive hook usually does that automatically if you push a
branch to a repository with an unborn HEAD. But I think there may be
corner cases where it gets confused.

-Peff

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

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> Félix Saparelli <felix@passcod.name> writes:
>
>> I created a git repository that, for joke reasons, has a single branch
>> called MASTER (in uppercase). Upon cloning this repo, git attempts to
>> checkout the master branch (in lowercase), which does not exist.
>
> See what Git told you carefully and you can guess, I think.

guessing is rarely a good user experience ;-) however...
>
>> $ git clone git@github.com:passcod/UPPERCASE-NPM.git
>> Cloning into 'UPPERCASE-NPM'...
>> remote: Counting objects: 14, done.
>> remote: Compressing objects: 100% (11/11), done.
>> remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0
>> Receiving objects: 100% (14/14), done.
>> Resolving deltas: 100% (3/3), done.
>> warning: remote HEAD refers to nonexistent ref, unable to checkout.

Perhaps here the warning message could include the value of the ref provided 
by the remote's HEAD which would then more clearly indicate to the user what 
was expected.

I haven't had chance to look at how easy that maybe in the code - perhaps a 
bit of low hanging fruit for someone?

>
> So you have MASTER but not master, but your HEAD still is pointing
> at non-existing master.  As HEAD is the way the remote tells the
> clone what the default branch to be checked out is, the command
> reports "we cannot do a checkout of the default branch."
>
--
Philip 

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

[Jeff King]

On Wed, May 24, 2017 at 12:24:52AM +0100, Philip Oakley wrote:

> > > $ git clone git@github.com:passcod/UPPERCASE-NPM.git
> > > Cloning into 'UPPERCASE-NPM'...
> > > remote: Counting objects: 14, done.
> > > remote: Compressing objects: 100% (11/11), done.
> > > remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0
> > > Receiving objects: 100% (14/14), done.
> > > Resolving deltas: 100% (3/3), done.
> > > warning: remote HEAD refers to nonexistent ref, unable to checkout.
> 
> Perhaps here the warning message could include the value of the ref provided
> by the remote's HEAD which would then more clearly indicate to the user what
> was expected.
> 
> I haven't had chance to look at how easy that maybe in the code - perhaps a
> bit of low hanging fruit for someone?

Unfortunately, it can't, because the ref doesn't exist:

  $ git ls-remote git://github.com/passcod/UPPERCASE-NPM.git
  efc7dbfd6ca155d5d19ce67eb98603896062f35a	refs/heads/MASTER
  e60ea8e6ec45ec45ff44ac8939cb4105b16477da	refs/pull/1/head
  f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c	refs/pull/2/head
  0d9b3a1268ff39350e04a7183af0add912b686e6	refs/tags/V1.0.0
  efc7dbfd6ca155d5d19ce67eb98603896062f35a	refs/tags/V1.0.1

There is no HEAD line at all, so we have no information about it on the
client side.  Likewise, if you run with GIT_TRACE_PACKET=1, you'll see
that the capabilities line does not include a symref marker either.

So if we wanted to improve this, I think the first step would be for the
server to start sending symref lines for HEAD, even when it does not
resolve to anything.

That would also make a case like this:

  git -C dst.git symbolic-ref refs/heads/does-not-exist
  git clone dst.git local

use "does-not-exist" as the default branch name in our local clone
(rather than just falling back to "master", which presumably the other
side never plans to use).

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Unfortunately, it can't, because the ref doesn't exist:
>
>   $ git ls-remote git://github.com/passcod/UPPERCASE-NPM.git
>   efc7dbfd6ca155d5d19ce67eb98603896062f35a	refs/heads/MASTER
>   e60ea8e6ec45ec45ff44ac8939cb4105b16477da	refs/pull/1/head
>   f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c	refs/pull/2/head
>   0d9b3a1268ff39350e04a7183af0add912b686e6	refs/tags/V1.0.0
>   efc7dbfd6ca155d5d19ce67eb98603896062f35a	refs/tags/V1.0.1
>
> There is no HEAD line at all, so we have no information about it on the
> client side.  Likewise, if you run with GIT_TRACE_PACKET=1, you'll see
> that the capabilities line does not include a symref marker either.
>
> So if we wanted to improve this, I think the first step would be for the
> server to start sending symref lines for HEAD, even when it does not
> resolve to anything.

Yup, noticed the same and I agree with your conclusion.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> Unfortunately, it can't, because the ref doesn't exist:
>>
>>   $ git ls-remote git://github.com/passcod/UPPERCASE-NPM.git
>>   efc7dbfd6ca155d5d19ce67eb98603896062f35a	refs/heads/MASTER
>>   e60ea8e6ec45ec45ff44ac8939cb4105b16477da	refs/pull/1/head
>>   f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c	refs/pull/2/head
>>   0d9b3a1268ff39350e04a7183af0add912b686e6	refs/tags/V1.0.0
>>   efc7dbfd6ca155d5d19ce67eb98603896062f35a	refs/tags/V1.0.1
>>
>> There is no HEAD line at all, so we have no information about it on the
>> client side.  Likewise, if you run with GIT_TRACE_PACKET=1, you'll see
>> that the capabilities line does not include a symref marker either.
>>
>> So if we wanted to improve this, I think the first step would be for the
>> server to start sending symref lines for HEAD, even when it does not
>> resolve to anything.
>
> Yup, noticed the same and I agree with your conclusion.

We probably should make head_ref_namespaced() to take the
resolve_flags that is passed down thru refs_read_ref_full() down to
refs_resolve_ref_unsafe().  For the purpose of the first call to it
in upload-pack.c to call back find_symref(), we do not need and want
to say RESOLVE_REF_READING (which requires a symref to be pointing
at an existing ref).  I suspect all the other calls (there are 2
other in unload-pack.c and yet another in http-backend.c) to the
function should keep passing RESOLVE_REF_READING, as they do want to
omit a symbolic ref that points at an unborn branch.

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

[Jeff King]

On Thu, May 25, 2017 at 12:13:03PM +0900, Junio C Hamano wrote:

> >> So if we wanted to improve this, I think the first step would be for the
> >> server to start sending symref lines for HEAD, even when it does not
> >> resolve to anything.
> >
> > Yup, noticed the same and I agree with your conclusion.
> 
> We probably should make head_ref_namespaced() to take the
> resolve_flags that is passed down thru refs_read_ref_full() down to
> refs_resolve_ref_unsafe().  For the purpose of the first call to it
> in upload-pack.c to call back find_symref(), we do not need and want
> to say RESOLVE_REF_READING (which requires a symref to be pointing
> at an existing ref).  I suspect all the other calls (there are 2
> other in unload-pack.c and yet another in http-backend.c) to the
> function should keep passing RESOLVE_REF_READING, as they do want to
> omit a symbolic ref that points at an unborn branch.

That would make head_ref() not function-pointer compatible with all the
other for_each_ref functions. I don't know how much that matters. The
revision.c parser does use function pointers, but it doesn't handle HEAD
specially.

So I kind of wonder if that code should simply be calling
resolve_ref_unsafe() itself in the first place. We know the only value
it's going to get is HEAD.

OTOH, it's possible that we would eventually want to report all symrefs,
including ones we find while traversing the refs. And in that sense, all
of the for_each_ref functions would want to learn about this. But for
ref advertisement I think that would need a protocol change anyway, so
I'm not sure it's worth worrying about.

The just-HEAD case could look like:

diff --git a/upload-pack.c b/upload-pack.c
index 97da13e6a..04a913ad1 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -959,28 +959,25 @@ static int send_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
-static int find_symref(const char *refname, const struct object_id *oid,
-		       int flag, void *cb_data)
+static void find_symref(const char *refname, struct string_list *out)
 {
 	const char *symref_target;
 	struct string_list_item *item;
 	struct object_id unused;
+	int flag;
 
-	if ((flag & REF_ISSYMREF) == 0)
-		return 0;
 	symref_target = resolve_ref_unsafe(refname, 0, unused.hash, &flag);
 	if (!symref_target || (flag & REF_ISSYMREF) == 0)
-		die("'%s' is a symref but it is not?", refname);
-	item = string_list_append(cb_data, refname);
+		return;
+	item = string_list_append(out, refname);
 	item->util = xstrdup(symref_target);
-	return 0;
 }
 
 static void upload_pack(void)
 {
 	struct string_list symref = STRING_LIST_INIT_DUP;
 
-	head_ref_namespaced(find_symref, &symref);
+	find_symref("HEAD", &symref);
 
 	if (advertise_refs || !stateless_rpc) {
 		reset_timeout();

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

[Jeff King]

On Thu, May 25, 2017 at 11:59:24AM -0400, Jeff King wrote:

> The just-HEAD case could look like:

This patch does work, in the sense that upload-pack advertises the
unborn HEAD symref. But the client doesn't actually do anything with it.
The capability parsing happens in get_remote_heads(), which passes the
data out by adding an annotation to the "struct ref" list. But of course
we have no HEAD ref to annotate.

So either get_remote_heads() would have to start returning a bogus HEAD
ref (with a null sha1, I guess, which all callers would have to
recognize). Or clone (and probably "remote set-head -a") would have to
start reaching across the transport-module boundary and asking for any
symref values for "HEAD". I'm not excited about more special-casing of
"HEAD", though. In theory we'd want this for other symrefs in the long
run, and it would be nice if clients were ready to handle that (even if
the protocol isn't quite there).

I dunno. I was thinking there might be a quick tweak, but I'm wondering
if this arcane case is worth the restructuring we'd have to do to
support it. It only comes up when you've moved the server repo's HEAD to
an unborn branch _and_ you have other refs (since otherwise we don't
even send capabilities at all!).

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I dunno. I was thinking there might be a quick tweak, but I'm wondering
> if this arcane case is worth the restructuring we'd have to do to
> support it. It only comes up when you've moved the server repo's HEAD to
> an unborn branch _and_ you have other refs (since otherwise we don't
> even send capabilities at all!).

Thanks for digging.  You made me to start doubting it is worth
doing, too.

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

[Philip Oakley]

been trying to keep up...

From: "Jeff King" <peff@peff.net>
> On Thu, May 25, 2017 at 11:59:24AM -0400, Jeff King wrote:
>
>> The just-HEAD case could look like:
>
> This patch does work, in the sense that upload-pack advertises the
> unborn HEAD symref. But the client doesn't actually do anything with it.
> The capability parsing happens in get_remote_heads(), which passes the
> data out by adding an annotation to the "struct ref" list. But of course
> we have no HEAD ref to annotate.
>
> So either get_remote_heads() would have to start returning a bogus HEAD
> ref (with a null sha1, I guess, which all callers would have to
> recognize). Or clone (and probably "remote set-head -a") would have to
> start reaching across the transport-module boundary and asking for any
> symref values for "HEAD". I'm not excited about more special-casing of
> "HEAD", though. In theory we'd want this for other symrefs in the long
> run, and it would be nice if clients were ready to handle that (even if
> the protocol isn't quite there).
>
> I dunno. I was thinking there might be a quick tweak, but I'm wondering
> if this arcane case is worth the restructuring we'd have to do to
> support it. It only comes up when you've moved the server repo's HEAD to
> an unborn branch _and_ you have other refs (since otherwise we don't
> even send capabilities at all!).
>
> -Peff

My original comment regarding Felix's report was based on when I was looking 
at the bundle code's disambiguation of refs which matched HEAD's sha1. Hence 
I had a mis-remembered impression that the HEAD - symref matching was 
avaibable.

At that time, Junio had suggested that, at least in the bundle file, the 
HEAD symref could be advertised immediately after a nul on the ref line.

At least for regular git, the sha1and symref value would included in the 
read line, and the current string processing [1] would not notice the extra 
symref data. This extra data could then be read (if present) from the end of 
the line, and the HEAD symref set.

What I then noticed (but didn't report to the list) was the option of adding 
that extra info to the PKLINE protocol.

<from my notes>
In technical\pack-protocol.txt #L136;158-160
Reference Discovery:

If HEAD is a valid ref, HEAD MUST appear as the first advertised
ref.  If HEAD is not a valid ref, HEAD MUST NOT appear in the
advertisement list at all, but other refs may still appear.
-

So, (for both upload pack, and bundle refs) the place to hide the HEAD is 
after the later ref that HEAD points to.
 e.g.(updating the example at #L147):
   00441d3fcd5ced445d1abc402225c0b8a1299641f497 
refs/heads/integration\0HEAD[LF]

The potential issue is if there is a passed ref that is HEAD, but that HEAD 
itself isn't passed (especially for bundle)
<\from my notes>
--

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!

These are still protocol changes but should squeeze into the existing 
processing using the \0 trick.

In the absence of the information, and the multi-use of the warning 
function, the current message is possible the best we can get.

Philip

[1] the question would be whether other git versions also use the same 
string processing so could be 'fooled' in the same way? I'd be interested to 
know if that was a possibility.

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

[Philip Oakley]

adding and updating an example..
From: "Philip Oakley" <philipoakley@iee.org>
> been trying to keep up...
>
> From: "Jeff King" <peff@peff.net>
>> On Thu, May 25, 2017 at 11:59:24AM -0400, Jeff King wrote:
>>
>>> The just-HEAD case could look like:
>>
>> This patch does work, in the sense that upload-pack advertises the
>> unborn HEAD symref. But the client doesn't actually do anything with it.
>> The capability parsing happens in get_remote_heads(), which passes the
>> data out by adding an annotation to the "struct ref" list. But of course
>> we have no HEAD ref to annotate.
>>
>> So either get_remote_heads() would have to start returning a bogus HEAD
>> ref (with a null sha1, I guess, which all callers would have to
>> recognize). Or clone (and probably "remote set-head -a") would have to
>> start reaching across the transport-module boundary and asking for any
>> symref values for "HEAD". I'm not excited about more special-casing of
>> "HEAD", though. In theory we'd want this for other symrefs in the long
>> run, and it would be nice if clients were ready to handle that (even if
>> the protocol isn't quite there).
>>
>> I dunno. I was thinking there might be a quick tweak, but I'm wondering
>> if this arcane case is worth the restructuring we'd have to do to
>> support it. It only comes up when you've moved the server repo's HEAD to
>> an unborn branch _and_ you have other refs (since otherwise we don't
>> even send capabilities at all!).
>>
>> -Peff
>
> My original comment regarding Felix's report was based on when I was 
> looking at the bundle code's disambiguation of refs which matched HEAD's 
> sha1. Hence I had a mis-remembered impression that the HEAD - symref 
> matching was avaibable.
>
> At that time, Junio had suggested that, at least in the bundle file, the 
> HEAD symref could be advertised immediately after a nul on the ref line.
>
> At least for regular git, the sha1and symref value would included in the 
> read line, and the current string processing [1] would not notice the 
> extra symref data. This extra data could then be read (if present) from 
> the end of the line, and the HEAD symref set.
>
> What I then noticed (but didn't report to the list) was the option of 
> adding that extra info to the PKLINE protocol.
>
> <from my notes>
> In technical\pack-protocol.txt #L136;158-160
> Reference Discovery:
>
> If HEAD is a valid ref, HEAD MUST appear as the first advertised
> ref.  If HEAD is not a valid ref, HEAD MUST NOT appear in the
> advertisement list at all, but other refs may still appear.
> -
>
> So, (for both upload pack, and bundle refs) the place to hide the HEAD is 
> after the later ref that HEAD points to.
> e.g.(updating the example at #L147):
>   00441d3fcd5ced445d1abc402225c0b8a1299641f497 
> refs/heads/integration\0HEAD[LF]
>
> The potential issue is if there is a passed ref that is HEAD, but that 
> HEAD itself isn't passed (especially for bundle)
> <\from my notes>
> --
>
> 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!
>
> These are still protocol changes but should squeeze into the existing 
> processing using the \0 trick.
>
> In the absence of the information, and the multi-use of the warning 
> function, the current message is possible the best we can get.
>
> Philip
>
> [1] the question would be whether other git versions also use the same 
> string processing so could be 'fooled' in the same way? I'd be interested 
> to know if that was a possibility.
>
Updating the original example with the suggestion of adding the unborn ref 
and a \0HEAD marker (sort order may be an issue if it is the first entry 
which 'clashes' with the capability string... - I've been lenient here)

  $ git ls-remote git://github.com/passcod/UPPERCASE-NPM.git
  efc7dbfd6ca155d5d19ce67eb98603896062f35a refs/heads/MASTER
  0000000000000000000000000000000000000000 refs/heads/master\0HEAD
  e60ea8e6ec45ec45ff44ac8939cb4105b16477da refs/pull/1/head
  f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c refs/pull/2/head
  0d9b3a1268ff39350e04a7183af0add912b686e6 refs/tags/V1.0.0
  efc7dbfd6ca155d5d19ce67eb98603896062f35a refs/tags/V1.0.1

--
Philip

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

[Junio C Hamano]

"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.

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.

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


[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").

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

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> "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]).
>
> 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.
>
>
> [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 

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

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Junio C Hamano" <gitster@pobox.com>
>> "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's not the part of the protocol I explained Peff's suggestion to
you about.  That's ref advertisement proper, and its first line has
a trailing NUL followed by "protocol capability" list.  There is one
"capability" that tells the receiver specifically about HEAD symref
(if and only if HEAD is a symref).  There are two reasons why the
current code does not help even though that necessary protocol bits
are *already* there (i.e. you do not need any protocol extension).
One is that existing servers do not use the symref capability for
HEAD if HEAD is pointing at an unborn branch (i.e. dangling). The
other is that the existing code sitting on the receiving end is not
prepared to handle one, even if the server end sent one.

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

[Jeff King]

On Sun, May 28, 2017 at 12:21:50PM +0100, Philip Oakley wrote:

> > 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.

It's hard to do that in a backwards-compatible way. The reason the
symref capability works like it does (and supports only HEAD) is that
the bits after the NUL are treated as a capability string, and older
clients will actually _replace_ any earlier capability string they saw
with the new one. So:

  1234abcd refs/heads/a\0symref=refs/heads/target1
  5678abcd refs/heads/b\0symref=refs/heads/target2

doesn't work as you'd like.

But even if it did, I don't think that solves the dangling symref
problem by itself. As you note, we'd need to advertise a null sha1 line
and annotate that. I didn't test, but I'd suspect that's another
compatibility problem; clients will probably choke on the null sha1.

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

The current protocol just advertises symref=source:target in the
capability line, where "source" does not have to be the ref that is
currently being advertised at all. So you are free to do:

  1234abcd refs/heads/unrelated\0...symref=HEAD:refs/heads/target

But the server does not bother to do so when the HEAD symref doesn't
point to anything. We could fix that without changing the protocol
syntax. It's possible that some picky client would complain that we
mentioned the HEAD symref even though it was not advertised, but
certainly older git.git clients are fine with it. They'd still need a
client-side update in order to do something useful with it, but at least
its presence would not make older clients behave any worse.

-Peff