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

From: Thomas Rast <trast@student.ethz.ch>
Date: Feb 22, 2010, 09:57 UTC
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.

[...]

Show 10 quoted lines

> -				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