Re: git-send-pack SIGSEGV..

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 16, 2007, 21:05 UTC
Message-ID: <7vfy4r3915.fsf@assigned-by-dhcp.pobox.com>
In-Reply-To: <Pine.LNX.4.64.0706152342490.4740@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 5 quoted lines

> That is, since "push = refs/heads/master" was already the same as "push = 
> refs/heads/master:refs/heads/master", the pattern fell into that code. I 
> was at least confidant enough of this to translate --tags into 
> "refs/tags/*" instead of "refs/tags/*:refs/tags/*" when I got to that 
> point.

This change sounds sensible.

> On the fetch side, the code isn't using my parser yet, anyway. But my 
> parser should be able to distinguish the one-sided refspec case, so that 
> the fetch logic can do whatever is determined to be right with the 
> combination of features.

Yes, but at the same time I know you are planning to rewrite git-fetch using this code, so...

Show 14 quoted lines

>> I think "fetch = refs/heads/*" syntax, if it just fetches
>> without storing, does not make much sense.  In a separate-remote
>> repository, "[remote "foo"] fetch = refs/heads/*" would probably
>> be useful if we treated it as "refs/heads/*:refs/remotes/foo/*".
>> In a bare repository used for mirrors, it would be useful if it
>> stood for "refs/heads/*:refs/heads/*".
>
> I'm kind of uncomfortable with this level of complexity for a fully 
> specified lhs pattern. Maybe "fetch = refs/heads/*" should be prohibited, 
> while something DWIM-y like "fetch = heads/*" or even "fetch = head *" 
> could come up with clever and useful patterns. In any case, the parser 
> should report "refs/heads/*" as pattern,refs/heads/,NULL and let the fetch 
> code decide what to do with it, rather than having the special case in the 
> parser.

Yeah, I am not very happy about DWIMmery either, especially a magic that changes behaviour depending on the bareness [*1*]. As long as the parser can distinguish these five cases, I'd be happy.

                        [push]			[fetch]
 (1) $any/*		same name		TBD
 (2) $any/*:		error			TBD
 (3) $any1/*:$any2/*	map any1 => any2	map any1 => any2
 (4) :$any/*		error			error
 (5) $any1/*:$any2	error			error
     $any1:$any2/*

(1-fetch) should probably be an error to avoid newbie confusion.

(2-push) is clearly an error.

(2-fetch) does not make much sense (octopus of random branches
    you do not even control what they are), but I do not have
    too strong an objection against it, so marked it TBD for now.

(3) is what we have always done.

(4-fetch) is clearly an error.

(4-push) could be "remove all refs", but I'd rather make that an error.

(5) is clearly an error.  If you explicitly have both sides,
    they should both be wildcard, or non-wildcard.

[Footnote]

*1* Sometimes I see an incorrect suggestion given on #git to do "mv .git ../project.git" to "make an incorrectly made repository bare". This is incorrect---at least you must update core.bare, and also should make sure .git/object/info/alternates points at a right directory if you used a relative path to specify one. Luckily, we see much smaller number of such incorrect "help" on the mailing list.