On Fri, Oct 17, 2008 at 04:08:42PM -0700, Junio C Hamano wrote:
> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
