Re: [PATCH] wt-status: expand, not dwim, a "detached from" ref

From: Junio C Hamano <gitster@pobox.com>
Date: May 13, 2020, 05:33 UTC
Message-ID: <xmqqftc4wg96.fsf@gitster.c.googlers.com>
In-Reply-To: <20200513004058.34456-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 9 quoted lines

> When a user checks out the upstream branch of HEAD and then runs "git
> branch", like this:
>
>   git checkout @{u}
>   git branch
>
> an error message is printed:
>
>   fatal: HEAD does not point to a branch

I had to spend unreasonable amount of time reproducing this.

    $ git branch --show-current
    master
    $ git checkout -b -t naster master
    $ git branch --show-current
    naster
    $ git checkout @{u}
    $ git branch --show-current
    master
    $ git branch
    * master
      naster

What was missing was that the current branch must be forked from a remote-tracking branch; with a fork from a local branch, the last step, i.e. "git branch", works just fine.

> (This error message when running "git branch" persists even after
> checking out other things - it only stops after checking out a branch.)

Correct. And it is also worth pointing out that this is not "git branch"; the users would see it primarily as a problem with "git status", which would die the same way.

Show 7 quoted lines

> This is because "git branch" attempts to DWIM "@{u}" when determining
> the "HEAD detached" message, but that doesn't work because HEAD no
> longer points to a branch.
>
> Therefore, when calculating the status of a worktree, only expand such a
> string, not DWIM it. Such strings would not match a ref, and "git
> branch" would report "HEAD detached at <hash>" instead.

OK. "Detached at <hash>" is somewhat unsatisfactory (you most likely have detached from refs/remotes/origin/<something>), but it is much better than "fatal" X-<.

Show 7 quoted lines

> One alternative is to plumb down a flag from dwim_ref() to
> interpret_branch_mark() that indicates that (1) don't read HEAD when
> processing, and (2) when encountering a ref of the form "<optional
> ref>@{u}", don't die when the optional ref doesn't exist, is not a
> branch, or does not have an upstream. (We cannot change the
> functionality outright because other parts of Git rely on such a message
> being printed.)

Thanks for a good analysis. That likely is a good direction for a longer term. @{upstream} is "tell me the upstream of the branch that is currently checked out", right? So it is reasonable to expect that it has no good answer in a detached HEAD state. And when on a branch, that branch may not have an upstream, and we should prepare for such a case, too.

Show 13 quoted lines

> diff --git a/wt-status.c b/wt-status.c
> index 98dfa6f73f..f84ebc3e2c 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1562,7 +1562,7 @@ static void wt_status_get_detached_from(struct repository *r,
>  		return;
>  	}
>  
> -	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
> +	if (expand_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
>  	    /* sha1 is a commit? match without further lookup */
>  	    (oideq(&cb.noid, &oid) ||
>  	     /* perhaps sha1 is a tag, try to dereference to a commit */

Hmph, I just did this:

    $ git branch -b -t next origin/next
    $ git checkout next@{upstream}
    $ git status

It used to say "HEAD detached at origin/next" without this change, but now it says "HEAD detached at 046d49d455", which smells like a regression.