log --cherry and merges (was [RFC/PATCH 0/2] merge-base: add --merge-child option)

[John Keeping <john@keeping.me.uk>]

On Mon, May 13, 2013 at 04:45:43PM +0200, Michael J Gruber wrote:
> Kevin Bracey venit, vidit, dixit 13.05.2013 16:26:
> > On 13/05/2013 01:22, Junio C Hamano wrote:
> >> Kevin Bracey <kevin@bracey.fi> writes:
> >>
> >>>     git log --ancestry-path --left-right E...F --not $(git merge-base
> >>> --all E F)
> >>>
> >>> which looks like we're having to repeat ourselves because it's not
> >>> paying attention...
> >> You are half wrong; "--left-right" is about "do we show the </>/=
> >> marker in the output?", so it is true that it does not make sense
> >> without "...", but the reverse is not true: A...B does not and
> >> should not imply --left-right.
> >>
> > The repetition I meant is that by the definition of ancestry-path, the 
> > above would seem to be equivalent to
> > 
> >    git log --ancestry-path --left-right E F --not $(git merge-base --all E F) $(git merge-base --all E F)
> > 
> > Anyway, revised separated-out version of the patch follows.
> > 
> > Kevin
> 
> It is certainly true that "git log --cherry" needs much less information
> than what the merge base machinery provides. I've been experimenting
> with that in order to get the speedup which is necessary for replacing
> the "git cherry" code with calls into the revision walker using "--cherry".

I think the revision machinery is the same speed as the "git cherry"
code, it's just that "git cherry" ignores merges and the cherry code in
revision.c doesn't.

Since the patch ID of a merge is just being calculated to its first
parent, I don't think it's meaningful to consider merges for "log
--cherry" but I can't quite convince myself that there's no corner case
where it is.

The following patch makes the revision cherry machinery ignore merges
unconditionally.  With it applied, there's not noticeable difference in
speed between "git cherry" and "git log --cherry".

-- >8 --
diff --git a/revision.c b/revision.c
index a67b615..19d0683 100644
--- a/revision.c
+++ b/revision.c
@@ -640,6 +640,11 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 
 		if (flags & BOUNDARY)
 			continue;
+
+		/* Patch ID is meaningless for merges. */
+		if (commit->parents && commit->parents->next)
+			continue;
+
 		/*
 		 * If we have fewer left, left_first is set and we omit
 		 * commits on the right branch in this loop.  If we have
@@ -661,6 +666,11 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 
 		if (flags & BOUNDARY)
 			continue;
+
+		/* Patch ID is meaningless for merges. */
+		if (commit->parents && commit->parents->next)
+			continue;
+
 		/*
 		 * If we have fewer left, left_first is set and we omit
 		 * commits on the left branch in this loop.
-- 
1.8.3.rc3.372.g721bad8