graph API: fix bug in graph_is_interesting()

Previously, graph_is_interesting() did not behave quite the same way as
the code in get_revision().  As a result, it would sometimes think
commits were uninteresting, even though get_revision() would return
them.  This resulted in incorrect lines in the graph output.

This change creates a get_commit_action() function, which
graph_is_interesting() and simplify_commit() both now use to determine
if a commit will be shown.  It is identical to the old simplify_commit()
behavior, except that it never calls rewrite_parents().

This problem was reported by Santi Béjar.  The following command
would exhibit the problem before, but now works correctly:

git log --graph --simplify-by-decoration --oneline v1.6.3.3

Previously git graph did not display the output for this command
correctly between f29ac4f and 66996ec, among other places.

Signed-off-by: Adam Simpkins <simpkins@facebook.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/graph.c b/graph.c
index e466770208d73d8e1f5bc7f6b12728b6f2b7c6bb..9087f658495200eb671b87711d15c05ad84089ef 100644 (file)
--- a/graph.c
+++ b/graph.c
@@ -286,9 +286,10 @@ static int graph_is_interesting(struct git_graph *graph, struct commit *commit)
        }
 
        /*
        }
 
        /*

-        * Uninteresting and pruned commits won't be printed
+        * Otherwise, use get_commit_action() to see if this commit is
+        * interesting

         */
         */

-       return (commit->object.flags & (UNINTERESTING | TREESAME)) ? 0 : 1;
+       return get_commit_action(graph->revs, commit) == commit_show;

 }
 
 static struct commit_list *next_interesting_parent(struct git_graph *graph,
 }
 
 static struct commit_list *next_interesting_parent(struct git_graph *graph,

diff --git a/revision.c b/revision.c
index 9f5dac5f1d8a8a654a2ab77e8e26fb134c7e36fa..efa3b7c795291fd2c337cf589977368d83d25ad6 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -1664,7 +1664,7 @@ static inline int want_ancestry(struct rev_info *revs)
        return (revs->rewrite_parents || revs->children.name);
 }
 
        return (revs->rewrite_parents || revs->children.name);
 }
 

-enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
+enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit)

 {
        if (commit->object.flags & SHOWN)
                return commit_ignore;
 {
        if (commit->object.flags & SHOWN)
                return commit_ignore;


@@ -1692,12 +1692,23 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
                        if (!commit->parents || !commit->parents->next)
                                return commit_ignore;
                }
                        if (!commit->parents || !commit->parents->next)
                                return commit_ignore;
                }

-               if (want_ancestry(revs) && rewrite_parents(revs, commit) < 0)
-                       return commit_error;

        }
        return commit_show;
 }
 
        }
        return commit_show;
 }
 

+enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
+{
+       enum commit_action action = get_commit_action(revs, commit);
+
+       if (action == commit_show &&
+           !revs->show_all &&
+           revs->prune && revs->dense && want_ancestry(revs)) {
+               if (rewrite_parents(revs, commit) < 0)
+                       return commit_error;
+       }
+       return action;
+}
+

 static struct commit *get_revision_1(struct rev_info *revs)
 {
        if (!revs->commits)
 static struct commit *get_revision_1(struct rev_info *revs)
 {
        if (!revs->commits)

diff --git a/revision.h b/revision.h
index fb74492714b9276d02992dbe1101efff2cbdb5e6..f0b8ea7bfa205dc776a6f8e20a2032142db35f9f 100644 (file)
--- a/revision.h
+++ b/revision.h
@@ -165,6 +165,7 @@ enum commit_action {
        commit_error
 };
 
        commit_error
 };
 

+extern enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit);

 extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit);
 
 #endif
 extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit);
 
 #endif

diff --git a/t/t6015-rev-list-show-all-parents.sh b/t/t6015-rev-list-show-all-parents.sh
new file mode 100755 (executable)
index 0000000..8b146fb
--- /dev/null
+++ b/t/t6015-rev-list-show-all-parents.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+test_description='--show-all --parents does not rewrite TREESAME commits'
+
+. ./test-lib.sh
+
+test_expect_success 'set up --show-all --parents test' '
+       test_commit one foo.txt &&
+       commit1=`git rev-list -1 HEAD` &&
+       test_commit two bar.txt &&
+       commit2=`git rev-list -1 HEAD` &&
+       test_commit three foo.txt &&
+       commit3=`git rev-list -1 HEAD`
+       '
+
+test_expect_success '--parents rewrites TREESAME parents correctly' '
+       echo $commit3 $commit1 > expected &&
+       echo $commit1 >> expected &&
+       git rev-list --parents HEAD -- foo.txt > actual &&
+       test_cmp expected actual
+       '
+
+test_expect_success '--parents --show-all does not rewrites TREESAME parents' '
+       echo $commit3 $commit2 > expected &&
+       echo $commit2 $commit1 >> expected &&
+       echo $commit1 >> expected &&
+       git rev-list --parents --show-all HEAD -- foo.txt > actual &&
+       test_cmp expected actual
+       '
+
+test_done