Subject: Re: [topgit] tg update error

From: Junio C Hamano <[email-masked]>
Date: Thu, 12 Feb 2009 13:01:42 -0800

Jeff King <[email-masked]> writes:

> Junio, I think we should probably revert b229d18 (and loosen
> symbolic-ref's check to just "refs/"). Even if you want to argue that
> topgit should be changed to handle this differently, we are still
> breaking existing topgit installations, and who knows what other scripts
> which might have relied on doing something like this.

I'm Ok with the revert (and I agree it is absolutely the right thing to do
at least for the short term).

But I still do agree with the reasoning for the change stated in its
commit log message:

    commit b229d18a809c169314b7f0d048dc5a7632e8f916
    Author: Jeff King <[email-masked]>
    Date:   Thu Jan 29 03:30:16 2009 -0500

        validate_headref: tighten ref-matching to just branches

        When we are trying to determine whether a directory contains
        a git repository, one of the tests we do is to check whether
        HEAD is either a symlink or a symref into the "refs/"
        hierarchy, or a detached HEAD.

        We can tighten this a little more, though: a non-detached
        HEAD should always point to a branch (since checking out
        anything else should result in detachment), so it is safe to
        check for "refs/heads/".

        Signed-off-by: Jeff King <[email-masked]>
        Signed-off-by: Junio C Hamano <[email-masked]>

It would be nice to hear TopGit people defend why setting HEAD to outside
refs/heads/ is justified, why doing so should not break other things, and
why it was needed.

The last one is particularly important to avoid this kind of issue in the
future.  Perhaps they _knew_ some things refuse to work on refs outside
refs/heads/ and wanted to take advantage of that fact to protect their own
refs from vanilla git tools, but if that really is the case, the rules
they want have to be spelled out.

"git checkout" would refuse to switch to "refs/top-bases/frotz", because
it currently considers HEAD pointing outside refs/heads/ is insane, for
example.  But the revert of the above commit *means* that it is not
insane, and somebody may add an option to switch to any refs inside refs/
hierarchy.  If the reason TopGit points HEAD outside refs/heads hierarchy
were because they assume "git checkout" would never do so, such a change
would break them again (I am not seriously suggesting to add such an
option to "git checkout", but I am just using it to illustrate the point.
We would not know what other assumption, warranted or unwarranted, it is
making).