threads / discuss / 32642

--simplify-merges returns too many references

Subject: --simplify-merges returns too many references

## tl;dr

3 messages between Jan 15, 2013 and Jan 17, 2013.

replies: 2people: 2as markdown or json

Phil Hord· Jan 15, 2013, 23:12 UTC · lore

I thought I understood the intent of the various history simplification switches, but maybe I am still confused.

In git.git, I see three commits which touch stripspace.c:

$ git log --oneline -- builtin/stripspace.c 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory

With --full-history and also with --dense, I see the same three commits:

$ git log --full-history --oneline -- builtin/stripspace.c 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory

$ git log --dense --oneline -- builtin/stripspace.c 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory

But with --simplify-merges, I see _more_ commits.

$ git log --simplify-merges --oneline -- builtin/stripspace.c 634392b Add 'contrib/subtree/' from commit ... 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory 610f043 Import branch 'git-p4' of git://repo.or.cz/fast-export b4d2b04 Merge git-gui 0a8f4f0 Merge git://git.kernel.org/pub/scm/git/gitweb 98e031f Merge git-tools repository under "tools" subdirectory 5569bf9 Do a cross-project merge of Paul Mackerras' gitk visualizer

None of the "new" commits touches this file. The man page suggests that simplify-merges should result in fewer commits than full-history.

"       --simplify-merges
       Additional option to --full-history to remove some needless merges from
       the resulting history, as there are no selected commits contributing to
       this merge."
Am I confused or is git?
Phil
Junio C Hamano· Jan 15, 2013, 23:48 UTC · re: Phil Hord · lore

Re: --simplify-merges returns too many references

Phil Hord <phil.hord@gmail.com> writes:
> But with --simplify-merges, I see _more_ commits.
>
> $ git log  --simplify-merges --oneline -- builtin/stripspace.c
> 634392b Add 'contrib/subtree/' from commit ...
	> 497215d Update documentation for stripspace
	> c2857fb stripspace: fix outdated comment
	> 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory
Show 5 quoted lines
> 610f043 Import branch 'git-p4' of git://repo.or.cz/fast-export
> b4d2b04 Merge git-gui
> 0a8f4f0 Merge git://git.kernel.org/pub/scm/git/gitweb
> 98e031f Merge git-tools repository under "tools" subdirectory
> 5569bf9 Do a cross-project merge of Paul Mackerras' gitk visualizer

After indenting away what should be shown, I notice all these extra are merges without any common ancestors.

Just an observation; I didn't think things through.
Junio C Hamano· Jan 17, 2013, 22:23 UTC · re: Junio C Hamano · lore

[PATCH] simplify-merges: drop merge from irrelevant side branch

The merge simplification rule stated in 6546b59 (revision traversal: show full history with merge simplification, 2008-07-31) still treated merge commits too specially. Namely, in a history with this shape:

	---o---o---M
	          /
         x---x---x

where three 'x' were on a history completely unrelated to the main history 'o' and do not touch any of the paths we are following, we still said that after simplifying all of the parents of M, 'x' (which is the leftmost 'x' that rightmost 'x simplifies down to) and 'o' (which would be the last commit on the main history that touches the paths we are following) are independent from each other, and both need to be kept.

That is incorrect; when the side branch 'x' never touches the paths, it should be removed to allow M to simplify down to the last commit on the main history that touches the paths.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * The old test vector had a history ending at I; the updates to it
   adds an unrelated side branch rooted at J, merge of I and J which
   is K, and then an extra commit L on top.
	A---(some merge mess)---I---K---L
                                   /
                                  J
 revision.c                   | 19 +++++++++++++++++++
 t/t6012-rev-list-simplify.sh | 26 +++++++++++++++++++++-----
 2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/revision.c b/revision.c
index 33cb207..532611b 100644
--- a/revision.c
+++ b/revision.c
@@ -1424,6 +1424,22 @@ static struct merge_simplify_state *locate_simplify_state(struct rev_info *revs,
 	return st;
 }
 
+static void remove_treesame_parents(struct commit *commit)
+{
+	struct commit_list **pp, *p;
+
+	pp = &commit->parents;
+	while ((p = *pp) != NULL) {
+		struct commit *parent = p->item;
+		if (parent->object.flags & TREESAME) {
+			*pp = p->next;
+			free(p);
+			continue;
+		}
+		pp = &p->next;
+	}
+}
+
 static struct commit_list **simplify_one(struct rev_info *revs, struct commit *commit, struct commit_list **tail)
 {
 	struct commit_list *p;
@@ -1469,6 +1485,9 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c
 		pst = locate_simplify_state(revs, p->item);
 		p->item = pst->simplified;
 	}
+
+	remove_treesame_parents(commit);
+
 	cnt = remove_duplicate_parents(commit);
 
 	/*
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index 510bb96..d6d79c4 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -59,7 +59,23 @@ test_expect_success setup '
 
 	echo "Final change" >file &&
 	test_tick && git commit -a -m "Final change" &&
-	note I
+	note I &&
+
+	git symbolic-ref HEAD refs/heads/unrelated &&
+	git rm -f "*" &&
+	echo "Unrelated branch" >side &&
+	git add side &&
+	test_tick && git commit -m "Side root" &&
+	note J &&
+
+	git checkout master &&
+	test_tick && git merge -m "Coolest" unrelated &&
+	note K &&
+
+	echo "Immaterial" >elif &&
+	git add elif &&
+	test_tick && git commit -m "Last" &&
+	note L
 '
 
 FMT='tformat:%P 	%H | %s'
@@ -82,10 +98,10 @@ check_result () {
 	'
 }
 
-check_result 'I H G F E D C B A' --full-history
-check_result 'I H E C B A' --full-history -- file
-check_result 'I H E C B A' --full-history --topo-order -- file
-check_result 'I H E C B A' --full-history --date-order -- file
+check_result 'L K J I H G F E D C B A' --full-history
+check_result 'K I H E C B A' --full-history -- file
+check_result 'K I H E C B A' --full-history --topo-order -- file
+check_result 'K I H E C B A' --full-history --date-order -- file
 check_result 'I E C B A' --simplify-merges -- file
 check_result 'I B A' -- file
 check_result 'I B A' --topo-order -- file
-- 
1.8.1.1.431.g469ab37

← back to recent threads