Re: [PATCH 3/3] git-svn: Fix discarding of extra parents from svn:mergeinfo
On Mon 2010-02-22 11:57:34 Thomas Rast wrote:
Show 5 quoted lines
> 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.
Show 15 quoted lines
> 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?
> 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
Tuomas Suutari
