revision.c: Make --full-history consider more merges
History simplification previously always treated merges as TREESAME if they were TREESAME to any parent. While this was consistent with the default behaviour, this could be extremely unhelpful when searching detailed history, and could not be overridden. For example, if a merge had ignored a change, as if by "-s ours", then: git log -m -p --full-history -Schange file would successfully locate "change"'s addition but would not locate the merge that resolved against it. Futher, simplify_merges could drop the actual parent that a commit was TREESAME to, leaving it as a normal commit marked TREESAME that isn't actually TREESAME to its remaining parent. Now redefine a commit's TREESAME flag to be true only if a commit is TREESAME to _all_ of its parents. This doesn't affect either the default simplify_history behaviour (because partially TREESAME merges are turned into normal commits), or full-history with parent rewriting (because all merges are output). But it does affect other modes. The clearest difference is that --full-history will show more merges - sufficient to ensure that -m -p --full-history log searches can really explain every change to the file, including those changes' ultimate fate in merges. Also modify simplify_merges to recalculate TREESAME after removing a parent. This is achieved by storing per-parent TREESAME flags on the initial scan, so the combined flag can be easily recomputed. This fixes some t6111 failures, but creates a couple of new ones - we are now showing some merges that don't need to be shown. Signed-off-by: Kevin Bracey <kevin@bracey.fi> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Kevin Bracey committed
May 16, 2013 at 18:32 UTC
d0af663e42abfcd5be6f7c3db21a29e521aa4ca2
5 files changed
+229
-47
Documentation/rev-list-options.txt
+3
-3
@@ -409,10 +409,10 @@ parent lines.
409
the example, we get
410
+
411
-----------------------------------------------------------------------
412
- I A B N D O
412
+ I A B N D O P
413
-----------------------------------------------------------------------
414
+
415
-`P` and `M` were excluded because they are TREESAME to a parent. `E`,
415
+`M` was excluded because it is TREESAME to both parents. `E`,
416
`C` and `B` were all walked, but only `B` was !TREESAME, so the others
417
do not appear.
418
+
@@ -440,7 +440,7 @@ themselves. This results in
440
Compare to '\--full-history' without rewriting above. Note that `E`
441
was pruned away because it is TREESAME, but the parent list of P was
442
rewritten to contain `E`'s parent `I`. The same happened for `C` and
443
-`N`. Note also that `P` was included despite being TREESAME.
443
+`N`.
444
445
In addition to the above settings, you can change whether TREESAME
446
affects inclusion:
revision.c
+211
-30
@@ -429,10 +429,100 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
429
return retval >= 0 && (tree_difference == REV_TREE_SAME);
430
}
431
432
+struct treesame_state {
433
+ unsigned int nparents;
434
+ unsigned char treesame[FLEX_ARRAY];
435
+};
436
+
437
+static struct treesame_state *initialise_treesame(struct rev_info *revs, struct commit *commit)
438
+{
439
+ unsigned n = commit_list_count(commit->parents);
440
+ struct treesame_state *st = xcalloc(1, sizeof(*st) + n);
441
+ st->nparents = n;
442
+ add_decoration(&revs->treesame, &commit->object, st);
443
+ return st;
444
+}
445
+
446
+/*
447
+ * Must be called immediately after removing the nth_parent from a commit's
448
+ * parent list, if we are maintaining the per-parent treesame[] decoration.
449
+ * This does not recalculate the master TREESAME flag - update_treesame()
450
+ * should be called to update it after a sequence of treesame[] modifications
451
+ * that may have affected it.
452
+ */
453
+static int compact_treesame(struct rev_info *revs, struct commit *commit, unsigned nth_parent)
454
+{
455
+ struct treesame_state *st;
456
+ int old_same;
457
+
458
+ if (!commit->parents) {
459
+ /*
460
+ * Have just removed the only parent from a non-merge.
461
+ * Different handling, as we lack decoration.
462
+ */
463
+ if (nth_parent != 0)
464
+ die("compact_treesame %u", nth_parent);
465
+ old_same = !!(commit->object.flags & TREESAME);
466
+ if (rev_same_tree_as_empty(revs, commit))
467
+ commit->object.flags |= TREESAME;
468
+ else
469
+ commit->object.flags &= ~TREESAME;
470
+ return old_same;
471
+ }
472
+
473
+ st = lookup_decoration(&revs->treesame, &commit->object);
474
+ if (!st || nth_parent >= st->nparents)
475
+ die("compact_treesame %u", nth_parent);
476
+
477
+ old_same = st->treesame[nth_parent];
478
+ memmove(st->treesame + nth_parent,
479
+ st->treesame + nth_parent + 1,
480
+ st->nparents - nth_parent - 1);
481
+
482
+ /*
483
+ * If we've just become a non-merge commit, update TREESAME
484
+ * immediately, and remove the no-longer-needed decoration.
485
+ * If still a merge, defer update until update_treesame().
486
+ */
487
+ if (--st->nparents == 1) {
488
+ if (commit->parents->next)
489
+ die("compact_treesame parents mismatch");
490
+ if (st->treesame[0] && revs->dense)
491
+ commit->object.flags |= TREESAME;
492
+ else
493
+ commit->object.flags &= ~TREESAME;
494
+ free(add_decoration(&revs->treesame, &commit->object, NULL));
495
+ }
496
+
497
+ return old_same;
498
+}
499
+
500
+static unsigned update_treesame(struct rev_info *revs, struct commit *commit)
501
+{
502
+ if (commit->parents && commit->parents->next) {
503
+ unsigned n;
504
+ struct treesame_state *st;
505
+
506
+ st = lookup_decoration(&revs->treesame, &commit->object);
507
+ if (!st)
508
+ die("update_treesame %s", sha1_to_hex(commit->object.sha1));
509
+ commit->object.flags |= TREESAME;
510
+ for (n = 0; n < st->nparents; n++) {
511
+ if (!st->treesame[n]) {
512
+ commit->object.flags &= ~TREESAME;
513
+ break;
514
+ }
515
+ }
516
+ }
517
+
518
+ return commit->object.flags & TREESAME;
519
+}
520
+
521
static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
522
{
523
struct commit_list **pp, *parent;
435
- int tree_changed = 0, tree_same = 0, nth_parent = 0;
524
+ struct treesame_state *ts = NULL;
525
+ int tree_changed = 0, nth_parent;
526
527
/*
528
* If we don't do pruning, everything is interesting
@@ -456,25 +546,43 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
546
if (!revs->dense && !commit->parents->next)
547
return;
548
459
- pp = &commit->parents;
460
- while ((parent = *pp) != NULL) {
549
+ for (pp = &commit->parents, nth_parent = 0;
550
+ (parent = *pp) != NULL;
551
+ pp = &parent->next, nth_parent++) {
552
struct commit *p = parent->item;
553
463
- /*
464
- * Do not compare with later parents when we care only about
465
- * the first parent chain, in order to avoid derailing the
466
- * traversal to follow a side branch that brought everything
467
- * in the path we are limited to by the pathspec.
468
- */
469
- if (revs->first_parent_only && nth_parent++)
470
- break;
554
+ if (nth_parent == 1) {
555
+ /*
556
+ * This our second loop iteration - so we now know
557
+ * we're dealing with a merge.
558
+ *
559
+ * Do not compare with later parents when we care only about
560
+ * the first parent chain, in order to avoid derailing the
561
+ * traversal to follow a side branch that brought everything
562
+ * in the path we are limited to by the pathspec.
563
+ */
564
+ if (revs->first_parent_only)
565
+ break;
566
+ /*
567
+ * If this will remain a potentially-simplifiable
568
+ * merge, remember per-parent treesame if needed.
569
+ * Initialise the array with the comparison from our
570
+ * first iteration.
571
+ */
572
+ if (revs->treesame.name &&
573
+ !revs->simplify_history &&
574
+ !(commit->object.flags & UNINTERESTING)) {
575
+ ts = initialise_treesame(revs, commit);
576
+ if (!tree_changed)
577
+ ts->treesame[0] = 1;
578
+ }
579
+ }
580
if (parse_commit(p) < 0)
581
die("cannot simplify commit %s (because of %s)",
582
sha1_to_hex(commit->object.sha1),
583
sha1_to_hex(p->object.sha1));
584
switch (rev_compare_tree(revs, p, commit)) {
585
case REV_TREE_SAME:
477
- tree_same = 1;
586
if (!revs->simplify_history || (p->object.flags & UNINTERESTING)) {
587
/* Even if a merge with an uninteresting
588
* side branch brought the entire change
@@ -482,7 +590,8 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
590
* to lose the other branches of this
591
* merge, so we just keep going.
592
*/
485
- pp = &parent->next;
593
+ if (ts)
594
+ ts->treesame[nth_parent] = 1;
595
continue;
596
}
597
parent->next = NULL;
@@ -511,12 +620,11 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
620
case REV_TREE_OLD:
621
case REV_TREE_DIFFERENT:
622
tree_changed = 1;
514
- pp = &parent->next;
623
continue;
624
}
625
die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
626
}
519
- if (tree_changed && !tree_same)
627
+ if (tree_changed)
628
return;
629
commit->object.flags |= TREESAME;
630
}
@@ -1947,28 +2055,32 @@ static void add_child(struct rev_info *revs, struct commit *parent, struct commi
2055
l->next = add_decoration(&revs->children, &parent->object, l);
2056
}
2057
1950
-static int remove_duplicate_parents(struct commit *commit)
2058
+static int remove_duplicate_parents(struct rev_info *revs, struct commit *commit)
2059
{
2060
+ struct treesame_state *ts = lookup_decoration(&revs->treesame, &commit->object);
2061
struct commit_list **pp, *p;
2062
int surviving_parents;
2063
2064
/* Examine existing parents while marking ones we have seen... */
2065
pp = &commit->parents;
2066
+ surviving_parents = 0;
2067
while ((p = *pp) != NULL) {
2068
struct commit *parent = p->item;
2069
if (parent->object.flags & TMP_MARK) {
2070
*pp = p->next;
2071
+ if (ts)
2072
+ compact_treesame(revs, commit, surviving_parents);
2073
continue;
2074
}
2075
parent->object.flags |= TMP_MARK;
2076
+ surviving_parents++;
2077
pp = &p->next;
2078
}
1966
- /* count them while clearing the temporary mark */
1967
- surviving_parents = 0;
2079
+ /* clear the temporary mark */
2080
for (p = commit->parents; p; p = p->next) {
2081
p->item->object.flags &= ~TMP_MARK;
1970
- surviving_parents++;
2082
}
2083
+ /* no update_treesame() - removing duplicates can't affect TREESAME */
2084
return surviving_parents;
2085
}
2086
@@ -1988,6 +2100,70 @@ static struct merge_simplify_state *locate_simplify_state(struct rev_info *revs,
2100
return st;
2101
}
2102
2103
+static int mark_redundant_parents(struct rev_info *revs, struct commit *commit)
2104
+{
2105
+ struct commit_list *h = reduce_heads(commit->parents);
2106
+ int i = 0, marked = 0;
2107
+ struct commit_list *po, *pn;
2108
+
2109
+ /* Want these for sanity-checking only */
2110
+ int orig_cnt = commit_list_count(commit->parents);
2111
+ int cnt = commit_list_count(h);
2112
+
2113
+ /*
2114
+ * Not ready to remove items yet, just mark them for now, based
2115
+ * on the output of reduce_heads(). reduce_heads outputs the reduced
2116
+ * set in its original order, so this isn't too hard.
2117
+ */
2118
+ po = commit->parents;
2119
+ pn = h;
2120
+ while (po) {
2121
+ if (pn && po->item == pn->item) {
2122
+ pn = pn->next;
2123
+ i++;
2124
+ } else {
2125
+ po->item->object.flags |= TMP_MARK;
2126
+ marked++;
2127
+ }
2128
+ po=po->next;
2129
+ }
2130
+
2131
+ if (i != cnt || cnt+marked != orig_cnt)
2132
+ die("mark_redundant_parents %d %d %d %d", orig_cnt, cnt, i, marked);
2133
+
2134
+ free_commit_list(h);
2135
+
2136
+ return marked;
2137
+}
2138
+
2139
+static int remove_marked_parents(struct rev_info *revs, struct commit *commit)
2140
+{
2141
+ struct commit_list **pp, *p;
2142
+ int nth_parent, removed = 0;
2143
+
2144
+ pp = &commit->parents;
2145
+ nth_parent = 0;
2146
+ while ((p = *pp) != NULL) {
2147
+ struct commit *parent = p->item;
2148
+ if (parent->object.flags & TMP_MARK) {
2149
+ parent->object.flags &= ~TMP_MARK;
2150
+ *pp = p->next;
2151
+ free(p);
2152
+ removed++;
2153
+ compact_treesame(revs, commit, nth_parent);
2154
+ continue;
2155
+ }
2156
+ pp = &p->next;
2157
+ nth_parent++;
2158
+ }
2159
+
2160
+ /* Removing parents can only increase TREESAMEness */
2161
+ if (removed && !(commit->object.flags & TREESAME))
2162
+ update_treesame(revs, commit);
2163
+
2164
+ return nth_parent;
2165
+}
2166
+
2167
static struct commit_list **simplify_one(struct rev_info *revs, struct commit *commit, struct commit_list **tail)
2168
{
2169
struct commit_list *p;
@@ -2032,7 +2208,9 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c
2208
}
2209
2210
/*
2035
- * Rewrite our list of parents.
2211
+ * Rewrite our list of parents. Note that this cannot
2212
+ * affect our TREESAME flags in any way - a commit is
2213
+ * always TREESAME to its simplification.
2214
*/
2215
for (p = commit->parents; p; p = p->next) {
2216
pst = locate_simplify_state(revs, p->item);
@@ -2044,31 +2222,30 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c
2222
if (revs->first_parent_only)
2223
cnt = 1;
2224
else
2047
- cnt = remove_duplicate_parents(commit);
2225
+ cnt = remove_duplicate_parents(revs, commit);
2226
2227
/*
2228
* It is possible that we are a merge and one side branch
2229
* does not have any commit that touches the given paths;
2052
- * in such a case, the immediate parents will be rewritten
2053
- * to different commits.
2230
+ * in such a case, the immediate parent from that branch
2231
+ * will be rewritten to be the merge base.
2232
*
2233
* o----X X: the commit we are looking at;
2234
* / / o: a commit that touches the paths;
2235
* ---o----'
2236
*
2059
- * Further reduce the parents by removing redundant parents.
2237
+ * Detect and simplify this case.
2238
*/
2239
if (1 < cnt) {
2062
- struct commit_list *h = reduce_heads(commit->parents);
2063
- cnt = commit_list_count(h);
2064
- free_commit_list(commit->parents);
2065
- commit->parents = h;
2240
+ int marked = mark_redundant_parents(revs, commit);
2241
+ if (marked)
2242
+ cnt = remove_marked_parents(revs, commit);
2243
}
2244
2245
/*
2246
* A commit simplifies to itself if it is a root, if it is
2247
* UNINTERESTING, if it touches the given paths, or if it is a
2071
- * merge and its parents simplifies to more than one commits
2248
+ * merge and its parents simplify to more than one commit
2249
* (the first two cases are already handled at the beginning of
2250
* this function).
2251
*
@@ -2176,6 +2353,10 @@ int prepare_revision_walk(struct rev_info *revs)
2353
if (!revs->leak_pending)
2354
free(list);
2355
2356
+ /* Signal whether we need per-parent treesame decoration */
2357
+ if (revs->simplify_merges)
2358
+ revs->treesame.name = "treesame";
2359
+
2360
if (revs->no_walk != REVISION_WALK_NO_WALK_UNSORTED)
2361
commit_list_sort_by_date(&revs->commits);
2362
if (revs->no_walk)
@@ -2235,7 +2416,7 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
2416
}
2417
pp = &parent->next;
2418
}
2238
- remove_duplicate_parents(commit);
2419
+ remove_duplicate_parents(revs, commit);
2420
return 0;
2421
}
2422
revision.h
+1
@@ -168,6 +168,7 @@ struct rev_info {
168
struct reflog_walk_info *reflog_info;
169
struct decoration children;
170
struct decoration merge_simplification;
171
+ struct decoration treesame;
172
173
/* notes-specific options: which refs to show */
174
struct display_notes_opt notes_opt;
t/t6019-rev-list-ancestry-path.sh
+1
-1
@@ -100,7 +100,7 @@ test_expect_success 'rev-list G..M -- G.t' '
100
test_cmp expect actual
101
'
102
103
-test_expect_failure 'rev-list --ancestry-path G..M -- G.t' '
103
+test_expect_success 'rev-list --ancestry-path G..M -- G.t' '
104
for c in H J L; do echo $c; done >expect &&
105
git rev-list --ancestry-path --format=%s G..M -- G.t |
106
sed -e "/^commit /d" |
t/t6111-rev-list-treesame.sh
+13
-13
@@ -114,9 +114,9 @@ check_result '(LH)M (K)L (GJ)K (I)J (G)I (G)H (FE)G (D)F (B)E (BC)D (A)C (A)B A'
114
check_result 'M H L K J I G E F D C B A' --topo-order
115
check_result 'M L H B A' -- file
116
check_result '(LH)M (B)L (B)H (A)B A' --parents -- file
117
-check_outcome failure 'M L J I H G F D B A' --full-history -- file # drops G
117
+check_result 'M L J I H G F D B A' --full-history -- file
118
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
119
-check_outcome failure '(LH)M (G)H (J)L (I)J (G)I (FB)G (B)F (A)B A' --simplify-merges -- file # drops G
119
+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
120
check_result 'M L K G F D B A' --first-parent
121
check_result 'M L G F B A' --first-parent -- file
122
@@ -125,11 +125,11 @@ check_result 'M L K J I H G E' F..M
125
check_result 'M H L K J I G E' F..M --topo-order
126
check_result 'M L H' F..M -- file
127
check_result '(LH)M (B)L (B)H' --parents F..M -- file
128
-check_outcome failure 'M L J I H G' F..M --full-history -- file # drops G
128
+check_result 'M L J I H G' F..M --full-history -- file
129
check_result '(LH)M (K)L (GJ)K (I)J (G)I (G)H (FB)G' F..M --full-history --parents -- file
130
-check_outcome failure '(LH)M (G)H (J)L (I)J (G)I (FB)G' F..M --simplify-merges -- file # drops G
130
+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
131
check_result 'M L K J I H G' F..M --ancestry-path
132
-check_outcome failure 'M L J I H G' F..M --ancestry-path -- file # drops G
132
+check_result 'M L J I H G' F..M --ancestry-path -- file
133
check_result '(LH)M (K)L (GJ)K (I)J (G)I (G)H (FE)G' F..M --ancestry-path --parents -- file
134
check_result '(LH)M (G)H (J)L (I)J (G)I (FE)G' F..M --ancestry-path --simplify-merges -- file
135
check_result 'M L K G' F..M --first-parent
@@ -138,7 +138,7 @@ check_result 'M L G' F..M --first-parent -- file
138
# Note that G is pruned when E is the bottom, even if it's the same commit list
139
# If we want history since E, then we're quite happy to ignore G that took E.
140
check_result 'M L K J I H G' E..M --ancestry-path
141
-check_result 'M L J I H' E..M --ancestry-path -- file
141
+check_outcome failure 'M L J I H' E..M --ancestry-path -- file # includes G
142
check_outcome failure '(LH)M (K)L (EJ)K (I)J (E)I (E)H' E..M --ancestry-path --parents -- file # includes G
143
check_outcome failure '(LH)M (E)H (J)L (I)J (E)I' E..M --ancestry-path --simplify-merges -- file # includes G
144
@@ -161,32 +161,32 @@ check_result 'M H L J I' G..M --ancestry-path --simplify-merges -- file
161
# But --full-history shouldn't drop D on its own - without simplification,
162
# we can't decide if the merge from INTERESTING commit C was sensible.
163
check_result 'F D C' B..F
164
-check_result 'F' B..F -- file
164
+check_outcome failure 'F' B..F -- file # includes D
165
check_outcome failure '(B)F' B..F --parents -- file # includes D
166
-check_outcome failure 'F D' B..F --full-history -- file # drops D prematurely
166
+check_result 'F D' B..F --full-history -- file
167
check_result '(D)F (BA)D' B..F --full-history --parents -- file
168
check_result '(B)F' B..F --simplify-merges -- file
169
check_result 'F D' B..F --ancestry-path
170
-check_result 'F' B..F --ancestry-path -- file
170
+check_outcome failure 'F' B..F --ancestry-path -- file # includes D
171
check_outcome failure 'F' B..F --ancestry-path --parents -- file # includes D
172
check_outcome failure 'F' B..F --ancestry-path --simplify-merges -- file # includes D
173
check_result 'F D' B..F --first-parent
174
check_result 'F' B..F --first-parent -- file
175
176
# E...F should be equivalent to E F ^B, and be able to drop D as above.
177
-check_result 'F' E F ^B -- file
178
-check_result 'F' E...F -- file
177
+check_outcome failure 'F' E F ^B -- file # includes D
178
+check_outcome failure 'F' E...F -- file # includes D
179
180
# Any sort of full history of C..F should show D, as it's the connection to C,
181
# and it differs from it.
182
check_result 'F D B' C..F
183
check_result 'F B' C..F -- file
184
check_result '(B)F (A)B' C..F --parents -- file
185
-check_outcome failure 'F D B' C..F --full-history -- file # drops D
185
+check_result 'F D B' C..F --full-history -- file
186
check_result '(D)F (BC)D (A)B' C..F --full-history --parents -- file
187
check_result '(D)F (BC)D (A)B' C..F --simplify-merges -- file
188
check_result 'F D' C..F --ancestry-path
189
-check_outcome failure 'F D' C..F --ancestry-path -- file # drops D
189
+check_result 'F D' C..F --ancestry-path -- file
190
check_result 'F D' C..F --ancestry-path --parents -- file
191
check_result 'F D' C..F --ancestry-path --simplify-merges -- file
192
check_result 'F D B' C..F --first-parent