Re: [PATCH 6/6] Add a REFNAME_ALLOW_UNNORMALIZED flag to check_ref_format()

[Michael Haggerty <mhagger@alum.mit.edu>]

On 09/10/2011 01:30 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> Let the callers of check_ref_format() (and normalize_refname()) decide
>> whether to accept unnormalized refnames via a new
>> REFNAME_ALLOW_UNNORMALIZED flag.  Change callers to set this flag,
>> which preserves their current behavior.  (There are likely places
>> where this flag can be removed.)
> 
> [...]
> To put it another way, my knee jerk reaction is that we shouldn't need
> such a "flag". Shouldn't it be sufficient for normalize_refname() and
> nothing else to allow unnormalized input, and everybody else should barf
> when they see an un-normalized input?

That is a good idea.

I will make the current normalize_refname() function static and hide the
REFNAME_ALLOW_UNNORMALIZED option from the outside world.  Then I will
write a new public normalize_refname() function that calls the static
version with REFNAME_ALLOW_UNNORMALIZED set, and change
check_ref_format() to call normalize_refname() with
REFNAME_ALLOW_UNNORMALIZED unset.

What should I do with all of the current callers of check_ref_format(),
given that I don't want to be personally responsible for analyzing and
rewriting them all?  The hard-nosed approach would be to say that they
are calling check_ref_format() without normalizing the refnames, so they
are already broken (albeit perhaps sometimes accidentally functional),
and it is OK that the new behavior of check_ref_format() causes them to
fail explicitly.

A more forgiving approach would be to implement another transition
function like check_ref_format_deprecated_unsafe() that accepts
unnormalized refnames, change the callers to use this function during
the transition, and remove it only after all callers have been fixed.

Suggestions?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/