Re: git-rev-list bug?
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:21
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · git-rev-list bug? · Catalin Marinas <hidden>
- 2016-06-15 · Re: git-rev-list bug? · Junio C Hamano <hidden>
- 2016-06-15 · Re: git-rev-list bug? · Junio C Hamano <hidden>
- 2016-06-15 · Re: git-rev-list bug? · Junio C Hamano <hidden>
- 2016-06-15 · Re: git-rev-list bug? · Junio C Hamano <hidden>
Junio C Hamano [off-list ref] writes:
It however has a side effect -- uninteresting commits were never parsed here, but now they get parsed. I am not sure if there are correctness implications...
Actually there is. If a merge with an uninteresting side branch was the only thing that brought changes to paths we are interested in, we do not want TREE_SAME logic to remove other parents (i.e. the branches we are interested in) from the merge commit. So we would need a combination of both, something like this? ---
diff --git a/revision.c b/revision.c
index 713f27e..c8d93ff 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 tree_changed = 0; if (!commit->tree) return;
@@ -296,14 +297,19 @@ static void try_to_simplify_commit(struc while ((parent = *pp) != NULL) { struct commit *p = parent->item; - if (p->object.flags & UNINTERESTING) { - pp = &parent->next; - continue; - } - parse_commit(p); switch (compare_tree(p->tree, commit->tree)) { case TREE_SAME: + if (p->object.flags & UNINTERESTING) { + /* Even if a merge with an uninteresting + * side branch brought the entire change + * we are interested in, we do not want + * to lose the other branches of this + * merge, so we just keep going. + */ + pp = &parent->next; + continue; + } parent->next = NULL; commit->parents = parent; return;
@@ -315,12 +321,14 @@ static void try_to_simplify_commit(struc } /* fallthrough */ case TREE_DIFFERENT: + tree_changed = 1; pp = &parent->next; continue; } die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1)); } - commit->object.flags |= TREECHANGE; + if (tree_changed) + commit->object.flags |= TREECHANGE; } static void add_parents_to_list(struct rev_info *revs, struct commit *commit, struct commit_list **list)