Subject: Re: [PATCH v8 39/44] refs.c: move the check for valid refname to lock_ref_sha1_basic

From: Jonathan Nieder <[email-masked]>
Date: Thu, 22 May 2014 10:44:27 -0700

Ronnie Sahlberg wrote:
> On Wed, May 21, 2014 at 6:42 PM, Jonathan Nieder <[email-masked]> wrote:

>>         $ git rev-parse HEAD >.git/refs/heads/foo..bar
>>         $ git branch -m foo..bar something-saner
>>         fatal: Invalid branch name: 'foo..bar'
>>
>> "git branch -m" has an explicit codepath ("recovery = 1;") to handle
>> this case, but it looks like it was not well tested and regressed in
>> v1.7.8-rc0~19^2~7 (resolve_ref(): verify that the input refname has
>> the right format, 2011-09-15).
>>
>> Is what the recovery codepath of branch -m does misguided?
[...]
> I don't think we should allow external caller any other way to
> access/modify refs than through
> the transaction api. This may make it awkward to handle cases such as
> foo/../bar was created  and how would you now delete it ?
>
> The exception to this I think would be 'git fsck'. We could allow fsck
> to have low level access to the backing store and allow it to access
> the files directly,
> or allow fsck to set magic flags that disable various checks.

Interesting.  Do you mean that 'git fsck' should notice invalid refnames
and have an option to repair them on its own?

That would be a big change from what git fsck currently does --- it's
currently read-only (except for "fsck --lost-found", which is read-only
except in the .git/lost-found/ directory).  If there's an option to
check refnames only and not have to wait for the full check of all
objects then I think it makes sense.

In the meantime, 'git branch -m' and 'git update-ref -d' (which were
the historical ways to do this kind of repair) are already broken, so
at least this patch doesn't seem to make anything worse. :/  (I
haven't checked all callers, though.)

Jonathan