From: Thomas Rast <trast@student.ethz.ch>
Date: Mon, 22 Feb 2010 09:57:34 GMT
Subject: Re: [PATCH 3/3] git-svn: Fix discarding of extra parents from svn:mergeinfo
Message-ID: <201002221057.35088.trast@student.ethz.ch>
In-Reply-To: <1266825442-32107-4-git-send-email-tuomas.suutari@gmail.com>

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.
[...]
> -				my $revs = command_oneline(
> -					"rev-list", "-1",
> -					"$new_parents[$i]..$new_parents[$j]",
> +				my $mergebase = command_oneline(
> +					"merge-base",
> +					"$new_parents[$i]", "$new_parents[$j]",
>  				       );
> -				if ( !$revs ) {
> +				if ( $mergebase eq $new_parents[$i] ) {
>  					undef($new_parents[$i]);

I think you swapped the test (or I got confused, which is entirely
possible).  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.  If it's not, please tell
us what you are fixing.  Either way, please change the commit message
appropriately.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch