Local clone checks out wrong branch based on remote HEAD

Local clone checks out wrong branch based on remote HEAD

[Tom Preston-Werner]

I'm having some unexpected behavior when cloning a remote repo that
has several branches at the same commit. On the remote side, the HEAD
is 'trunk':

git@remote ~/repositories/akincisor/site.git $ cat HEAD
ref: refs/heads/trunk

After cloning this with a standard `git clone`, the refs are:

[11:48][tom@solid:~/dev/sandbox/site(release)]$ git branch -r -v
  origin/HEAD    a52528a Fixed some routing problems
  origin/release a52528a Fixed some routing problems
  origin/trunk   a52528a Fixed some routing problems

And the checked out branch is 'release' instead of 'trunk' as I would expect:

[11:48][tom@solid:~/dev/sandbox/site(release)]$ git branch
* release

I'm guessing that the first branch that matches the remote HEAD
revision is being checked out instead of the actual remote branch. I
would expect the correct branch to be chosen regardless of where the
branches are pointing.

Tom

--
Tom Preston-Werner
github.com/mojombo

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

[Daniel Barkalow]

On Tue, 17 Mar 2009, Tom Preston-Werner wrote:

> I'm having some unexpected behavior when cloning a remote repo that
> has several branches at the same commit. On the remote side, the HEAD
> is 'trunk':
> 
> git@remote ~/repositories/akincisor/site.git $ cat HEAD
> ref: refs/heads/trunk
> 
> After cloning this with a standard `git clone`, the refs are:
> 
> [11:48][tom@solid:~/dev/sandbox/site(release)]$ git branch -r -v
>   origin/HEAD    a52528a Fixed some routing problems
>   origin/release a52528a Fixed some routing problems
>   origin/trunk   a52528a Fixed some routing problems
> 
> And the checked out branch is 'release' instead of 'trunk' as I would expect:
> 
> [11:48][tom@solid:~/dev/sandbox/site(release)]$ git branch
> * release
> 
> I'm guessing that the first branch that matches the remote HEAD
> revision is being checked out instead of the actual remote branch. I
> would expect the correct branch to be chosen regardless of where the
> branches are pointing.

Unfortunately, the current protocol version just sends:

a52528a HEAD
a52528a refs/heads/release
a52528a refs/heads/trunk

It doesn't transmit the fact that HEAD is a pointer to anything, or what 
it's a pointer to. One thing you can do is just change your local repo to 
point origin/HEAD where you want, and check out what you want; the 
defaults are just to get you started. Another thing is that it will guess 
"master" if there is one. I think there's also been discussion of a 
protocol extension to transmit the information, although I don't know 
where that ended up. (The protocol-agnostic transport infrastructure can 
represent the information, but doesn't receive it for the normal protocol)

	-Daniel
*This .sig left intentionally blank*

Local clone checks out wrong branch based on remote HEAD

[Nanako Shiraishi]

Quoting Tom Preston-Werner <tom@github.com>:

> I'm guessing that the first branch that matches the remote HEAD
> revision is being checked out instead of the actual remote branch. I
> would expect the correct branch to be chosen regardless of where the
> branches are pointing.

Isn't this a known issue that can be found out easily from the archive?

  http://article.gmane.org/gmane.comp.version-control.git/27259
  http://thread.gmane.org/gmane.comp.version-control.git/101956/focus=101958
  http://thread.gmane.org/gmane.comp.version-control.git/102039

If I remember correctly, the two patch series from Junio wasn't accepted warmly and they were dropped.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

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

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> ... I think there's also been discussion of a 
> protocol extension to transmit the information, although I don't know 
> where that ended up.

You can find them in the list of threads posted nearby.

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.

We could resurrect it if somebody is interested.

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

[Jeff King]

On Tue, Mar 17, 2009 at 12:19:35PM -0700, Tom Preston-Werner wrote:

> After cloning this with a standard `git clone`, the refs are:
> 
> [11:48][tom@solid:~/dev/sandbox/site(release)]$ git branch -r -v
>   origin/HEAD    a52528a Fixed some routing problems
>   origin/release a52528a Fixed some routing problems
>   origin/trunk   a52528a Fixed some routing problems
> 
> And the checked out branch is 'release' instead of 'trunk' as I would expect:

As others have explained, this is because the information is lacking at
the client and we are forced to make a guess. There is a heuristic in
the guess to prefer "master" if it is an option. I suppose we could make
a similar exception for "trunk", which might make sense to people
working with SVN repositories.

OTOH, I am not sure I want to open the can of worms that is writing an
exhaustive list of heuristics that will work for everybody. Fixing the
protocol itself would probably be easier. :)

Here is what such a heuristic would look like, though (on top of next
and totally untested):

---
diff --git a/remote.c b/remote.c
index 76b1bbd..99d2281 100644
--- a/remote.c
+++ b/remote.c
@@ -1529,11 +1529,18 @@ struct ref *guess_remote_head(const struct ref *head,
 	if (head->symref)
 		return copy_ref(find_ref_by_name(refs, head->symref));
 
-	/* If refs/heads/master could be right, it is. */
+	/* We heuristically prefer certain names */
 	if (!all) {
-		r = find_ref_by_name(refs, "refs/heads/master");
-		if (r && !hashcmp(r->old_sha1, head->old_sha1))
-			return copy_ref(r);
+		const char *rules[] = {
+			"refs/heads/master",
+			"refs/heads/trunk",
+		};
+		int i;
+		for (i = 0; i < ARRAY_SIZE(rules); i++) {
+			r = find_ref_by_name(refs, rules[i]);
+			if (r && !hashcmp(r->old_sha1, head->old_sha1))
+				return copy_ref(r);
+		}
 	}
 
 	/* Look for another ref that points there */

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

[Jeff King]

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

> 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

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

[Michael J Gruber]

Jeff King venit, vidit, dixit 18.03.2009 01:54:
> On Tue, Mar 17, 2009 at 12:19:35PM -0700, Tom Preston-Werner wrote:
> 
>> After cloning this with a standard `git clone`, the refs are:
>>
>> [11:48][tom@solid:~/dev/sandbox/site(release)]$ git branch -r -v
>>   origin/HEAD    a52528a Fixed some routing problems
>>   origin/release a52528a Fixed some routing problems
>>   origin/trunk   a52528a Fixed some routing problems
>>
>> And the checked out branch is 'release' instead of 'trunk' as I would expect:
> 
> As others have explained, this is because the information is lacking at
> the client and we are forced to make a guess. There is a heuristic in
> the guess to prefer "master" if it is an option. I suppose we could make
> a similar exception for "trunk", which might make sense to people
> working with SVN repositories.
> 
> OTOH, I am not sure I want to open the can of worms that is writing an
> exhaustive list of heuristics that will work for everybody. Fixing the
> protocol itself would probably be easier. :)

One might even argue that in case of ambiguities, checking out a
detached head would be most appropriate. Really, why impose creation of
certain local branches on a user at all, unless asked for? Detached
heads are natural in git! But I don't really expect positive consensus
on that one...

Michael

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

[Jay Soffian]

On Wed, Mar 18, 2009 at 6:05 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> One might even argue that in case of ambiguities, checking out a
> detached head would be most appropriate. Really, why impose creation of
> certain local branches on a user at all, unless asked for? Detached
> heads are natural in git! But I don't really expect positive consensus
> on that one...

Shirley, you must be joking. :-)

I think there are two reasonable paths forward:

1) Address Jeff's concerns above so that the symref can be sent.
2) In lieu of (1), have clone at least warn that multiple branches
match and that it just picked one.

j.

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

[Jeff King]

On Wed, Mar 18, 2009 at 11:05:29AM +0100, Michael J Gruber wrote:

> One might even argue that in case of ambiguities, checking out a
> detached head would be most appropriate. Really, why impose creation of
> certain local branches on a user at all, unless asked for? Detached
> heads are natural in git! But I don't really expect positive consensus
> on that one...

I'm not sure that detached HEADs are all that natural. It means that:

  git clone repo-with-ambiguous-HEAD foo
  cd foo
  hack hack hack
  git commit -a -m msg

is putting your commits "nowhere" (i.e., not on any ref). They are not
accessible for pushing, and when you checkout another branch, they will
be lost (except to the reflog).

So it clearly requires that the user be aware of what is going on, and
that they understand the subtleties of detached HEADs (something that
has caused new user confusion before, I think).

-Peff

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

[Jeff King]

On Wed, Mar 18, 2009 at 05:11:25PM -0400, Jay Soffian wrote:

> I think there are two reasonable paths forward:
> 
> 1) Address Jeff's concerns above so that the symref can be sent.
> 2) In lieu of (1), have clone at least warn that multiple branches
> match and that it just picked one.

I think we should do (2) regardless. Even with an updated client,
remote servers may have an older git which does not support (1) for some
time.

So I guess it's time to refactor guess_remote_head _again_. :)

-Peff