On Wed, May 21, 2014 at 6:42 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Ronnie Sahlberg wrote:
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2044,6 +2044,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
>> int missing = 0;
>> int attempts_remaining = 3;
>>
>> + if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
>> + return NULL;
>
> Are we sure that no callers are locking a poorly named ref
> as preparation for repairing it?
>
> To see what works already in that vein, I tried:
>
> $ 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? One
> school of thought would be that people with malformed refs are
> responsible for recovering using low-level tools like "mv" and "vi"
> instead of normal git commands since normal git commands should never
> have created such a bad situation. Another school of thought would
> assert that
>
> * git commands can have bugs
> * the format checked by check_refname_format() keeps getting stricter
> over time
>
> which means it's nice when people can recover with 'update-ref -d'
> or 'branch -m'. It's not obvious to me what the right thing to do
> here is (maybe a special flag to be attached to a ref update during
> recovery?).
>
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.
> Hope that helps,
> Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html