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

Ronnie Sahlberg wrote:

On Wed, May 21, 2014 at 6:42 PM, Jonathan Nieder [off-list ref] wrote:

quoted

        $ 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