Re: [PATCH 3/3] git-svn: Fix discarding of extra parents from svn:mergeinfo

[Tuomas Suutari <tuomas.suutari@gmail.com>]

On Mon 2010-02-22 11:57:34 Thomas Rast wrote:
> On Monday 22 February 2010 08:57:22 Tuomas Suutari wrote:
> > Use merge-base rather than rev-list for detecting if a parent is an
> > ancestor of another, because rev-list gives incorrect results
> > sometimes.
> 
[...]
> 
> I think you swapped the test (or I got confused, which is entirely
> possible).

Ah, seems so. I was already writing a long reply explaining why rev-list 
can't be used but only merge-base just to realize that, indeed, rev-list can 
be used after all. The code just used to discard the wrong parent.

> Let I = $new_parents[$i] and J = $new_parents[$j].  The
> old one was
> 
>   test -z "$(git rev-list -1 I..J)"
> 
> which reads "unless there are any commits on J which are not on I",
> i.e., it fails unless J is an ancestor of I.
>
> But the new one is
> 
>   "$(git merge-base I J)" == I.
> 
> so suddenly I must be an ancestor of J.
>
> Is that what you were fixing?

Yes.

> Because I don't think the 'rev-list -1'
> test is any worse than the merge-base test.

You're right. I failed to see that I can get the same results by swapping $i 
with $j in the undef($new_parents[$i]) statement.

My pitfall was that I considered only changing the "if ( !$revs )" part to 
"if ( $revs )" with the rev-list approach. That wouldn't have worked, 
because then the other test case, included in PATCH 2, would have failed.
(When there are two distinct branches merged to one.)

> If it's not, please tell
> us what you are fixing.  Either way, please change the commit message
> appropriately.

Yes, the commit message was horrible. I was hoping that, by writing a test 
case as a "description" about the problem, I would avoid writing so much 
English... ;)

I will send another patch soon. Hopefully with better commit message.

-- 
Tuomas