From: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date: Sun, 23 Jun 2013 13:35:16 GMT
Subject: Re: [PATCH 1/3] remote: Add warnings about mixin --mirror and other remotes
Message-ID: <1371994516.24315.8.camel@localhost>
In-Reply-To: <7v8v23mhjy.fsf@alter.siamese.dyndns.org>

On vr, 2013-06-21 at 11:42 -0700, Junio C Hamano wrote:
> > +                (!fnmatch(refspec, remote->fetch[i].dst, 0) ||
> > +                 !fnmatch(remote->fetch[i].dst, refspec, 0))) {
> 
> Does .dst always exist and is never a NULL?  My quick scan of
> remote.c::parse_refspec_internal() tells me otherwise.
> 
> Also what are you matching with what?  refs/* against
> refs/remotes/origin/*?
> 
> What if remote->fetch[i] is not a wildcarded refspec, e.g.
> 
>         [remote "origin"]
>                 fetch = +refs/heads/master:refs/heads/origin
>                 fetch = +refs/heads/next:refs/remotes/origin/next
> 
> You would want to check for equality in such a case against the RHS
> of the refspeec you have.

Thanks for all the feedback, I've incorporated it all in a reroll that's
in progress, except for the above.

I've added a prefix check, so refs/foo and refs/foo/bar will be
considered clashes, but not yet an equality check. Equality for
wildcards is allowed and tested for, so do we really want to 'outlaw'
equality of non-wildcard refspecs?

-- 
Dennis Kaarsemaker
www.kaarsemaker.net