Re: git-rev-list bug?

Junio C Hamano [off-list ref] writes:

To my surprise, it turns out that this regression was not very
recent.  Bisecting points at this commite:
...
I haven't had time to dig into this deeper yet...

I am wondering why try_to_simplify_commit() skips parents marked
with UNINTERESTING.  I think this is causing a problem Catalin
found with rev-list.

If you have (time flows from top to bottom):

	commit#1	(it does not matter what this does)
        commit#2        change file B only
        commit#3	change file A only
        commit#4	change file B only

"git-rev-list commit#1..commit#4 -- A" shows commit#3 (correct)
and commit#2 (incorrect).

It pushes commit#1 (UNINTERESTING) and commit#4 (~UNINTERESTING)
and starts traversing.  try-to-simplify(commit#4) says "no tree
change between #3 and #4" and it returns without marking
commit#4 with TREECHANGE flag.  But when looking at commit#2 and
trying to simplify it, it says "Ah, its parent is uninteresting,
so I would not do compare_tree()".  Iteration over parents of
commit#2 leaves the while() loop and at the end of function the
commit is marked with TREECHANGE and is shown.

The attached patch seems to fix it (without losing the logic to
omit tree comparison with UNINTERESTING parent, which I do not
quite understand).



---

diff --git a/revision.c b/revision.c
index 713f27e..23c9b9d 100644
--- a/revision.c
+++ b/revision.c

@@ -282,6 +282,7 @@ static int same_tree_as_empty(struct tre
 static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 {
 	struct commit_list **pp, *parent;
+	int changed = 0;
 
 	if (!commit->tree)
 		return;

@@ -315,12 +316,14 @@ static void try_to_simplify_commit(struc
 			}
 		/* fallthrough */
 		case TREE_DIFFERENT:
+			changed = 1;
 			pp = &parent->next;
 			continue;
 		}
 		die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
 	}
-	commit->object.flags |= TREECHANGE;
+	if (changed)
+		commit->object.flags |= TREECHANGE;
 }
 
 static void add_parents_to_list(struct rev_info *revs, struct commit *commit, struct commit_list **list)