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

On Mon, May 13, 2013 at 04:45:43PM +0200, Michael J Gruber wrote:

Kevin Bracey venit, vidit, dixit 13.05.2013 16:26:

quoted

On 13/05/2013 01:22, Junio C Hamano wrote:

quoted

Kevin Bracey [off-list ref] writes:

quoted

    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