Re: Detached checkout will clobber branch head when using symlink HEAD

From: Jeff King <peff@peff.net>
Date: Oct 19, 2008, 13:00 UTC
Message-ID: <20081019130050.GA1822@coredump.intra.peff.net>
In-Reply-To: <7vfxmuhlad.fsf@gitster.siamese.dyndns.org>

On Fri, Oct 17, 2008 at 04:08:42PM -0700, Junio C Hamano wrote:

Show 20 quoted lines

> I do not think that is a correct approach.
> 
> The offending callchain is:
> 
>   update_ref(..., "HEAD", REF_NODEREF, ...);
>   -> lock_any_ref_for_update("HEAD", ..., REF_NODEREF);
>    -> lock_ref_sha1_basic("HEAD", ..., REF_NODEREF, ...);
>       . calls resolve_ref() to read HEAD to arrive at
>         refs/heads/master
>       . however, it notices REF_NODEREF and adjusts the ref to be updated
>         back to "HEAD";
>     -> hold_lock_file_for_update(..., "HEAD", 1);
>      -> lock_file(..., "HEAD");
>         . resolves symlink "HEAD" to "refs/heads/master", and
>           locks it!  This creates "refs/heads/master.lock", that is
>           then renamed to "refs/heads/master" when unlocked.
> 
> In other words, the breakage is in lock_file() and not in resolve_ref().
> The latter gives the same output to the caller whether the HEAD is
> symbolic link or textual symref -- at least it should.

To be fair, my patch fixed the problem in lock_ref_sha1_basic, not resolve_ref. It merely asked resolve_ref to annotate the resolution to show whether a symlink was found, since it already had that information.

But if I am reading your patch right, you are actually enabling detached HEAD in this instance, which is much better. Unfortunately, I had quite a few conflicts in applying your patches on top of master (with or without the patch from "checkout --track -b broken"), and when I thought I had fixed up the result, the test actually still failed.

So I will take your word that it actually works, and that I screwed up resolving the conflicts.

-Peff

PS If you are rebasing or resolving anyway, as I suspect you will have to, there is a typo in the test: s/detch/detach/