Kaartic Sivaraam <kaartic.sivar...@gmail.com> writes:
>> The only difference is improved tests where we use show-ref to make
>> sure refs/heads/HEAD does not exist when it shouldn't, exercise
>> update-ref to create and delete refs/heads/HEAD, and also make sure
>> it can be deleted with "git branch -d".
>
> In which case you might also like to ensure that it's possible to
> "rename" the branch with a name "HEAD" to recover from historical
> mistakes.
Perhaps. I didn't think it was all that needed---as long as you can
delete, you can recreate at the same commit with a more desirable
name, and it is not like users have tons of repositories with
misnamed branches that they need to fix. The code may already
handle it, or there may need even more code to support the rename; I
didn't check.
>> int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
>> {
>> strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
>> - if (name[0] == '-')
>> + if (*name == '-' || !strcmp(name, "HEAD"))
>> return -1;
>
> I guess this makes the check for "HEAD" in builtin/branch::cmd_branch()
> (line 796) redundant. May be it could be removed?
Perhaps. But I think that is better done as a follow-up "now the
lower level consistently handles, let's remove the extra check that
has become unnecessary" separate patch.
> So, may be the following test could also be added (untested yet),
>
> test_expect_success 'branch -m can rename refs/heads/HEAD' '
> git update-ref refs/heads/HEAD HEAD^ &&
> git branch -m HEAD head &&
> test_must_fail git show-ref refs/heads/HEAD
> '
Yeah, that would be a good material for that separate follow-up
patch.
Thanks.
