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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html