Re: [PATCH v20 43/48] refs.c: move the check for valid refname to lock_ref_sha1_basic

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

 



On Wed, Aug 20, 2014 at 1:11 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> On 08/20/2014 09:45 PM, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
>>
>>> I think we can get away with not including broken refnames when
>>> iterating.  After all, the main goal of tolerating them is to let them
>>> be deleted, right?
>>
>> Or read from a ref whose name has retroactively made invalid, in
>> order to create a similar but validly named ref to serve as its
>> replacement?  So at least we need to give the users some way of
>> reading from them, in addition to deleting them, no?
>
> The die() error message would presumably include the name of the invalid
> reference.
>
> If we change the rules for reference names and a bunch of names become
> invalid, then it would admittedly be cumbersome to run git N times to
> find the N invalid names.  But if we change the rules, then *at that
> time* we could always build in iteration over broken reference names.
>
> It's not that I have something against building it iteration over broken
> reference names now, as long as it is clearly segregated from "normal"
> iteration to prevent illegal names from getting loose in the code.

We already have iterators that show also bad refs.
For example, git branch uses
DO_FOR_EACH_INCLUDE_BROKEN
which is a flag to include also broken refs that can not be resolved
which allows them to be listed :



$ echo "this is not a valid sha1" >.git/refs/heads/broken
$ git branch
  broken
  foo
* master
$ git branch -D broken
error: branch 'broken' not found.

(allowing -D to remove it is in a different patch further down my series)


Since we already display broken/unresolvable refs, I think the most
consistent path is to also allow showing the refs broken/illegal-names
too in the list. (when DO_FOR_EACH_INCLUDE_BROKEN is specified)
Of course, an end user could fix this by deleting the file but since
it is easy to add the special casing to 'git branch -D' to handle this
case I think this would be more userfriendly since then the user can
use git branch -D regardless of the reason why the ref is broken.


Ronnie
--
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]