Merge branch 'jc/remove-treesame-parent-in-simplify-merges'

The --simplify-merges logic did not cull irrelevant parents from a
merge that is otherwise not interesting with respect to the paths
we are following.

This touches a fairly core part of the revision traversal
infrastructure; even though I think this change is correct, please
report immediately if you find any unintended side effect.

* jc/remove-treesame-parent-in-simplify-merges:
simplify-merges: drop merge from irrelevant side branch

diff --git a/revision.c b/revision.c
index ef6020541282770b9edc4c921cadb7ab1506da56..78397d6486203a6b7242c8ea866780979876e685 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -1970,6 +1970,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;
@@ -2022,10 +2038,18 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c
                if (revs->first_parent_only)
                        break;
        }
-       if (!revs->first_parent_only)
-               cnt = remove_duplicate_parents(commit);
-       else
+
+       if (revs->first_parent_only) {
                cnt = 1;
+       } else {
+               /*
+                * A merge with a tree-same parent is useless
+                */
+               if (commit->parents && commit->parents->next)
+                       remove_treesame_parents(commit);
+
+               cnt = remove_duplicate_parents(commit);
+       }
 
        /*
         * It is possible that we are a merge and one side branch

diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index 839ad97b791c6aa757d0b82eea7fc16369ad4586..8e2ff13423ba01095404ec49232760bc66c68978 100755 (executable)
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -56,7 +56,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'
@@ -79,10 +95,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