Re: [BUG] branch renamed to 'HEAD'

From: Jeff King <peff@peff.net>
Date: Feb 27, 2017, 08:01 UTC
Message-ID: <20170227080158.de2xarctzscfdsp2@sigill.intra.peff.net>
In-Reply-To: <20170227074915.xljfe5jox756rlyv@sigill.intra.peff.net>

On Mon, Feb 27, 2017 at 02:49:15AM -0500, Jeff King wrote:

Show 7 quoted lines

> > >         $ git branch -m -f orig @
> [...]
> 
> Regardless of the original intent, I think it is wrong to convert "@" to
> a branch named "HEAD". I think the bug is in strbuf_check_branch_ref(),
> which blindly sticks "refs/heads/" in front of any value we get from
> interpret_branch_name(), which clearly does not make sense for HEAD.

I do think the bug is in strbuf_check_branch_ref(), but it's hard for it to do a better job. It needs to feed arbitrary expressions into interpret_branch_name() to resolve things like "@{upstream}", "@{-1}", "foo@{upstream}", etc.

The problem is that it expects a branch name to come out of interpret_branch_name(), which _mostly_ happens. The exception is HEAD, which is a "special" name. But the returned value doesn't indicate whether it is special or not.

My first thought was that we might be handling "@" in the wrong place. But it needs to happen here to make things like "@@{upstream}" work (which turns "@" into HEAD, and then finds its upstream).

So I think the options are:

  1. Before calling interpret_branch_name(), strbuf_check_branch_ref()
     checks for "@". I don't like this because it's making assumptions
     about how the result will be parsed and interpreted.

  2. interpret_branch_name() returns a flag that says "this isn't
     _really_ a branch name".

  3. After interpret_branch_name() returns, check whether the result is
     "HEAD".

Doing (2) is the "right" thing in the sense that the "is it a branch" logic remains with the matching parsing code. But we have to surface that value (maybe across recursion via reinterpret?). Since we're unlikely to ever grow a return value that matches this case except "HEAD", it might be simplest to just do (3).

-Peff