Re: [PATCH v2] gitk: fix direction of symmetric difference in optimized mode

Paul Mackerras wrote:

Thomas Rast writes:

quoted

The confusing part of this, but also how I stumbled across the real
bug, was that I was playing with --show-all and that flipped the
direction *again*.  Turns out the option is not recognized by gitk and
lets it go back to unoptimized mode, where the bug does not exist.

What does --show-all do?  Maybe I need to add support for it to gitk.

Support for _displaying_ it was added to gitk in 1407ade, even before
the option was added to the revision walker.  Since it's a debugging
option, I doubt it's worth handling this in the optimized code path.

But anyway, it does not do what I hoped :-(

It shows commits that were walked, but found uninteresting, with a ^
in front.  See the long explanation in 3131b71; you can try

  gitk --show-all origin/next..origin/pu

for a nice example in git.git.

I was _actually_ looking for an option to make --cherry-pick
--left-right history connected again, as I was trying to make sense of
an SVN history basically by saying

  gitk --left-right --cherry-pick svn/2.2...svn/trunk

(Incidentally this SVN is publicly available at
https://secure.a-eskwadraat.nl/svn/domjudge, but I doubt it's worth
the cloning.)

The problem with this is that it disconnects history, so I was looking
for an option to either get back the commits omitted by --cherry-pick
(but of course flagged in some way that shows they're duplicated) or
fix the parent pointers so that history becomes connected again.  Some
of my guesses were --sparse, --full-history and --show-all, but none
achieve this.

[I *think* --sparse comes closest, but it's about TREESAME-type
uninteresting commits, not about --cherry-pick.  --full-history is
only about the merges that are TREESAME, so that's out.  --show-all
apparently is something entirely different.]

Sadly, it's really the underlying git-rev-list that is "broken" in the
sense that it does not fix the parent lists.  And git log --graph
handles it much worse than gitk.  I've added the authors of the
relevant features to Cc; maybe you can help?

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

The current --cherry-pick declares commits SHOWN that are found to be
duplicates.  Unfortunately this disconnects the history at every such
duplicate, making it quite hard to follow in graphical viewers.

Add an extra stage of parent rewriting after scanning for duplicates,
which simplifies the history to omit all duplicate commits.  This
cannot easily be shifted to the existing parent rewriting because
cherry_pick_list() always comes last in the entire filtering process
(presumably because it is the most expensive).

Signed-off-by: Thomas Rast <redacted>
---

I wrote:

The problem with [gitk --left-right --cherry-pick A...B] is that it
disconnects history

[...]

Sadly, it's really the underlying git-rev-list that is "broken" in the
sense that it does not fix the parent lists.  And git log --graph
handles it much worse than gitk.

Maybe this is an approach.  It unfortunately breaks down if merges can
disappear because of patch-ids too.  Can they?

(In the case where a merge is flagged SHOWN, it might have its parent
list reduced to one at some point, and then later filterings would
simplify it away whereas earlier ones didn't.)

Also, I'm not entirely sure we want to do this without any guards
except rewrite_parents.

On the plus side, the issues with git log --graph vanish because
history is again connected :-)


 revision.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/revision.c b/revision.c
index 9f5dac5..9e24514 100644
--- a/revision.c
+++ b/revision.c

@@ -517,6 +517,8 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,
 	return 0;
 }
 
+static int remove_duplicate_parents(struct commit *commit);
+
 static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 {
 	struct commit_list *p;

@@ -599,6 +601,25 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 		commit->util = NULL;
 	}
 
+	if (revs->rewrite_parents) {
+		/* Prune away commits we've just found to be duplicates */
+		for (p = list; p; p = p->next) {
+			struct commit *commit = p->item;
+			struct commit_list *pp;
+
+			for (pp = commit->parents; pp; pp = pp->next) {
+				struct commit *parent = pp->item;
+				while (parent->object.flags & SHOWN
+				       && parent->parents
+				       && !parent->parents->next)
+					parent = parent->parents->item;
+				pp->item = parent;
+			}
+
+			remove_duplicate_parents(commit);
+		}
+	}
+
 	free_patch_ids(&ids);
 }
 

-- 
1.6.4.199.g24c3