Re: bug? in checkout with ambiguous refnames

From: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Date: Jan 9, 2011, 02:43 UTC
Message-ID: <alpine.DEB.1.10.1101081902150.12031@debian>
In-Reply-To: <20110108214011.GA4753@sigill.intra.peff.net>

On Sat, 8 Jan 2011, Jeff King wrote:

Show 26 quoted lines

> On Sat, Jan 08, 2011 at 03:40:33PM -0500, Martin von Zweigbergk wrote:
> 
> > > Yeah, we generally resolve ambiguities in favor of the tag (and that
> > > warning comes from deep within get_sha1_basic). So the real bug here is
> > > that it still said "Switched to branch", which is totally wrong.
> > > 
> > > That being said, it probably would make more sense for "git checkout" to
> > > prefer branches to tags.
> > 
> > What was the rationale for generally favoring tags?
> 
> I don't recall hearing any specific argument, but it has always been
> that way from early on. I think it is from a vague sense of "tags are
> more important than branch tips because they are about marking specific
> points, not lines of development". But maybe other old-timers can say
> more.
> 
> I don't necessarily buy that argument; my only reasoning is that we
> should probably keep historic behavior.
> 
> > Why does that reasoning not apply to 'git checkout' too?
> 
> Because checkout has always been fundamentally about branches. It did
> end up growing sane behavior for "git checkout tag" (i.e., a detached
> HEAD), but branches are still the fundamental unit for most of its
> arguments.

I had a look at the history of the lines Junio mentioned in another message on this thread (lookup_commit_reference_gently() and setup_branch_path()). I don't understand the code, but just looking at where the lines came from, they seem to have been there ever since 782c2d6 (Build in checkout, 2008-02-07). If that is correct (but please check for yourselves), it seems like the broken checkout of ambiguous references causes problems rarely enough that no one has bothered to report it for almost two years. If it has been broken for that long, it seems to me like we could resolve it whichever way makes most sense, without much concern to how it used to work, no?

Show 8 quoted lines

> > Btw, what exactly does "generally" mean, i.e. which other commands
> > don't favor tags? I know rebase is one example of a command that does
> > not favor tags.
> 
> It means "we favor tags in resolve_ref, which is the underlying
> machinery for most commands, so unless a command special-cases it, that
> will be the behavior, and I am too lazy to exhaustively search for such
> special cases".

Sensible definition :-). I was just hoping for an example where it more obviously makes sense to favor branches. Maybe rebase is actually one of the better examples.

Show 10 quoted lines

> > The reason I'm asking is because I just happened to see in the rebase
> > code the other day that it will rebase a detached head if the <branch>
> > parameter is not "completely unqualified". For example 'git rebase
> > master heads/topic' or 'git rebase master refs/heads/topic' will not
> > update refs/heads/topic. I was trying to fix that by using 'git
> > rev-parse --symbolic-full-name' to parse <branch>. That seemed to work
> > fine until I saw this thread :-).
> 
> Heh. I think that would be an argument in favor of changing rev-parse's
> behavior.

I was hoping to do something like

head_name=$(git rev-parse --symbolic-full-name -q --verify "$1") case "$head_name" in refs/heads/*) ;; *) head_name="detached HEAD" ;; esac

plus setting another variable or two in each case. So even if 'git rev-parse --symbolic-full-name' would return refs/tags/$name, it wouldn't actually help here, since rebase currently favors branches. In order to keep that behavior, I will need to add an extra check before the above code.

It still feels like rev-parse should return refs/tags/$name, though.