threads / discuss / 18360

Local clone checks out wrong branch based on remote HEAD

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

## tl;dr

10 messages between Mar 17, 2009 and Mar 19, 2009.

replies: 9people: 7as markdown or json

Tom Preston-Werner· Mar 17, 2009, 19:19 UTC · lore

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

Daniel Barkalow· Mar 17, 2009, 19:39 UTC · re: Tom Preston-Werner · lore

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

On Tue, 17 Mar 2009, Tom Preston-Werner wrote:
Show 23 quoted lines
> 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*
Junio C Hamano· Mar 17, 2009, 23:40 UTC · re: Daniel Barkalow · lore

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

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.
Jeff King· Mar 18, 2009, 01:51 UTC · re: Junio C Hamano · lore

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

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
Nanako Shiraishi· Mar 17, 2009, 21:31 UTC · re: Tom Preston-Werner · lore
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/
Jeff King· Mar 18, 2009, 00:54 UTC · re: Tom Preston-Werner · lore

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

On Tue, Mar 17, 2009 at 12:19:35PM -0700, Tom Preston-Werner wrote:
Show 8 quoted lines
> 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 */
Michael J Gruber· Mar 18, 2009, 10:05 UTC · re: Jeff King · lore

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

Jeff King venit, vidit, dixit 18.03.2009 01:54:
Show 20 quoted lines
> 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
Jay Soffian· Mar 18, 2009, 21:11 UTC · re: Michael J Gruber · lore

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

On Wed, Mar 18, 2009 at 6:05 AM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

Show 5 quoted lines
> 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.
Jeff King· Mar 19, 2009, 04:04 UTC · re: Jay Soffian · lore

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

On Wed, Mar 18, 2009 at 05:11:25PM -0400, Jay Soffian wrote:
Show 5 quoted lines
> 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
Jeff King· Mar 19, 2009, 04:02 UTC · re: Michael J Gruber · lore

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

On Wed, Mar 18, 2009 at 11:05:29AM +0100, Michael J Gruber wrote:
Show 5 quoted lines
> 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

← back to recent threads