Re: check-ref-format: include refs/ in the argument or to strip it?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 22, 2014 at 10:46 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Fri, Aug 22, 2014 at 11:45:15AM -0700, Jonathan Nieder wrote:
>
>> Junio C Hamano wrote:
>>
>> >                implication of which is that the 'at least one slash'
>> > rule was to expect things are 'refs/<anything>' so there will be at
>> > least one.  Even back then, that <anything> alone had at least one
>> > slash (e.g. heads/master), but the intention was *never* that we
>> > would forbid <anything> that does not have a slash by feeding
>> > <anything> part alone to check-ref-format, i.e. things like
>> > "refs/stash" were designed to be allowed.
>>
>> Now I'm more confused.  Until 5f7b202a (2008-01-01), there was a
>> comment
>>
>>               if (level < 2)
>>                       return -2; /* at least of form "heads/blah" */
>>
>> and that behavior has been preserved since the beginning.
>>
>> Why do most old callers pass a string that doesn't start with refs/
>> (e.g., see the callers in 03feddd6, 2005-10-13)?  Has the intent been
>> to relax the requirement since then?
>
> Yeah, this weird "do not allow refs/foo" behavior has continually
> confused me. Coincidentally I just noticed a case today where
> "pack-refs" treats "refs/foo" specially for no good reason:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/255729
>
> After much head scratching over the years, I am of the opinion that
> nobody every really _meant_ to prevent "refs/foo", and that code
> comments like the one you quote above were an attempt to document
> existing buggy behavior that was really trying to differentiate "HEAD"
> from "refs/*". That's just my opinion, though. :) I'd be happy if all of
> the special-treatment of "refs/foo" went away and check_refname_format
> always got the full ref.
>

There are also a lot of places where we assume that a refs will start
with "refs/heads/" and not just "refs/"
for_each_branch_ref(), log_ref_setup() (so no reflogs) is_branch() to
name a few.

This makes the api a bit confusing and hard to predict. Which
functions allow refs/foo and which will ignore it?
Are there any compelling reasons why refs/foo should be allowed?
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]