Re: [PATCH 3/3] branch: forbid refs/heads/HEAD

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 14, 2017, 02:11 UTC
Message-ID: <xmqqefq6v5qj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171013131501.3qarwhanktfvgjqd@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 12 quoted lines

> On Fri, Oct 13, 2017 at 02:11:32PM +0900, Junio C Hamano wrote:
>
>> strbuf_check_branch_ref() is the central place where many codepaths
>> see if a proposed name is suitable for the name of a branch.  It was
>> designed to allow us to get stricter than the check_refname_format()
>> check used for refnames in general, and we already use it to reject
>> a branch whose name begins with a '-'.
>> 
>> Use it to also reject "HEAD" as a branch name.
>
> Heh, I just pointed somebody to this a day or two ago as #leftoverbit. I
> guess it's taken now. :)

Didn't notice /remember it; sorry about that. I can retract it if you want, but perhaps they cannot unsee it ;-)

Show 21 quoted lines

>> diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
>> index e88349c8a0..3ecb2eab0c 100755
>> --- a/t/t1430-bad-ref-name.sh
>> +++ b/t/t1430-bad-ref-name.sh
>> @@ -331,4 +331,12 @@ test_expect_success 'update-ref --stdin -z fails delete with bad ref name' '
>>  	grep "fatal: invalid ref format: ~a" err
>>  '
>>  
>> +test_expect_success 'branch rejects HEAD as a branch name' '
>> +	test_must_fail git branch HEAD HEAD^
>> +'
>> +
>> +test_expect_success 'checkout -b rejects HEAD as a branch name' '
>> +	test_must_fail git checkout -B HEAD HEAD^
>> +'
>
> Should we test that:
>
>   git update-ref refs/heads/HEAD HEAD^
>
> continues to work?

Perhaps. Also we may want to make sure that "git branch -D HEAD" still works as a way to recover from historical mistakes.