simplify-merges: never remove all TREESAME parents

When simplifying an odd merge, such as one that used "-s ours", we may
find ourselves TREESAME to apparently redundant parents. Prevent
simplify_merges() from removing every TREESAME parent; if this would
happen reinstate the first TREESAME parent - the one that the default
log would have followed.

This avoids producing a totally disjoint history from the default log
when the default log is a better explanation of the end result, and aids
visualisation of odd merges.

Signed-off-by: Kevin Bracey <kevin@bracey.fi>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Kevin Bracey 2013-05-16 18:32:36 +03:00 committed by Junio C Hamano
parent d5d2fc8b1a
commit 9c129eab99
3 changed files with 73 additions and 3 deletions

View File

@ -471,7 +471,8 @@ history according to the following rules:
+
* Replace each parent `P` of `C'` with its simplification `P'`. In
the process, drop parents that are ancestors of other parents, and
remove duplicates.
remove duplicates, but take care to never drop all parents that
we are TREESAME to.
+
* If after this parent rewriting, `C'` is a root or merge commit (has
zero or >1 parents), a boundary commit, or !TREESAME, it remains.

View File

@ -2136,6 +2136,73 @@ static int mark_redundant_parents(struct rev_info *revs, struct commit *commit)
return marked;
}
/*
* Awkward naming - this means one parent we are TREESAME to.
* cf mark_treesame_root_parents: root parents that are TREESAME (to an
* empty tree). Better name suggestions?
*/
static int leave_one_treesame_to_parent(struct rev_info *revs, struct commit *commit)
{
struct treesame_state *ts = lookup_decoration(&revs->treesame, &commit->object);
struct commit *unmarked = NULL, *marked = NULL;
struct commit_list *p;
unsigned n;
for (p = commit->parents, n = 0; p; p = p->next, n++) {
if (ts->treesame[n]) {
if (p->item->object.flags & TMP_MARK) {
if (!marked)
marked = p->item;
} else {
if (!unmarked) {
unmarked = p->item;
break;
}
}
}
}
/*
* If we are TREESAME to a marked-for-deletion parent, but not to any
* unmarked parents, unmark the first TREESAME parent. This is the
* parent that the default simplify_history==1 scan would have followed,
* and it doesn't make sense to omit that path when asking for a
* simplified full history. Retaining it improves the chances of
* understanding odd missed merges that took an old version of a file.
*
* Example:
*
* I--------*X A modified the file, but mainline merge X used
* \ / "-s ours", so took the version from I. X is
* `-*A--' TREESAME to I and !TREESAME to A.
*
* Default log from X would produce "I". Without this check,
* --full-history --simplify-merges would produce "I-A-X", showing
* the merge commit X and that it changed A, but not making clear that
* it had just taken the I version. With this check, the topology above
* is retained.
*
* Note that it is possible that the simplification chooses a different
* TREESAME parent from the default, in which case this test doesn't
* activate, and we _do_ drop the default parent. Example:
*
* I------X A modified the file, but it was reverted in B,
* \ / meaning mainline merge X is TREESAME to both
* *A-*B parents.
*
* Default log would produce "I" by following the first parent;
* --full-history --simplify-merges will produce "I-A-B". But this is a
* reasonable result - it presents a logical full history leading from
* I to X, and X is not an important merge.
*/
if (!unmarked && marked) {
marked->object.flags &= ~TMP_MARK;
return 1;
}
return 0;
}
static int remove_marked_parents(struct rev_info *revs, struct commit *commit)
{
struct commit_list **pp, *p;
@ -2238,6 +2305,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c
*/
if (1 < cnt) {
int marked = mark_redundant_parents(revs, commit);
if (marked)
marked -= leave_one_treesame_to_parent(revs, commit);
if (marked)
cnt = remove_marked_parents(revs, commit);
}

View File

@ -116,7 +116,7 @@ check_result 'M L H B A' -- file
check_result '(LH)M (B)L (B)H (A)B A' --parents -- file
check_result 'M L J I H G F D B A' --full-history -- file
check_result '(LH)M (K)L (GJ)K (I)J (G)I (G)H (FB)G (D)F (BA)D (A)B A' --full-history --parents -- file
check_outcome failure '(LH)M (G)H (J)L (I)J (G)I (FB)G (B)F (A)B A' --simplify-merges -- file # drops parent B from G
check_result '(LH)M (G)H (J)L (I)J (G)I (FB)G (B)F (A)B A' --simplify-merges -- file
check_result 'M L K G F D B A' --first-parent
check_result 'M L G F B A' --first-parent -- file
@ -127,7 +127,7 @@ check_result 'M L H' F..M -- file
check_result '(LH)M (B)L (B)H' --parents F..M -- file
check_result 'M L J I H G' F..M --full-history -- file
check_result '(LH)M (K)L (GJ)K (I)J (G)I (G)H (FB)G' F..M --full-history --parents -- file
check_outcome failure '(LH)M (G)H (J)L (I)J (G)I (FB)G' F..M --simplify-merges -- file # drops parent B from G
check_result '(LH)M (G)H (J)L (I)J (G)I (FB)G' F..M --simplify-merges -- file
check_result 'M L K J I H G' F..M --ancestry-path
check_result 'M L J I H G' F..M --ancestry-path -- file
check_result '(LH)M (K)L (GJ)K (I)J (G)I (G)H (FE)G' F..M --ancestry-path --parents -- file