Skip to content

Commit f94bfa1

Browse files
dschogitster
authored andcommitted
log: --remerge-diff needs to keep around commit parents
To show a remerge diff, the merge needs to be recreated. For that to work, the merge base(s) need to be found, which means that the commits' parents have to be traversed until common ancestors are found (if any). However, one optimization that hails all the way back to cb11574 (Some more memory leak avoidance, 2006-06-17) is to release the commit's list of parents immediately after showing it _and to set that parent list to `NULL`_. This can break the merge base computation. This problem is most obvious when traversing the commits in reverse: In that instance, if a parent of a merge commit has been shown as part of the `git log` command, by the time the merge commit's diff needs to be computed, that parent commit's list of parent commits will have been set to `NULL` and as a result no merge base will be found (even if one should be found). Traversing commits in reverse is far from the only circumstance in which this problem occurs, though. There are many avenues to traversing at least one commit in the revision walk that will later be part of a merge base computation, for example when not even walking any revisions in `git show <merge1> <merge2>` where `<merge1>` is part of the commit graph between the parents of `<merge2>`. Another way to force a scenario where a commit is traversed before it has to be traversed again as part of a merge base computation is to start with two revisions (where the first one is reachable from the second but not in a first-parent ancestry) and show the commit log with `--topo-order` and `--first-parent`. Let's fix this by special-casing the `remerge_diff` mode, similar to what we did with reflogs in f35650d (log: do not free parents when walking reflog, 2017-07-07). Signed-off-by: Johannes Schindelin <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent dbecc61 commit f94bfa1

File tree

2 files changed

+13
-2
lines changed

2 files changed

+13
-2
lines changed

builtin/log.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,10 +511,14 @@ static int cmd_log_walk_no_free(struct rev_info *rev)
511511
* but we didn't actually show the commit.
512512
*/
513513
rev->max_count++;
514-
if (!rev->reflog_info) {
514+
if (!rev->reflog_info && !rev->remerge_diff) {
515515
/*
516516
* We may show a given commit multiple times when
517-
* walking the reflogs.
517+
* walking the reflogs. Therefore we still need it.
518+
*
519+
* Likewise, we potentially still need the parents
520+
* of * already shown commits to determine merge
521+
* bases when showing remerge diffs.
518522
*/
519523
free_commit_buffer(the_repository->parsed_objects,
520524
commit);

t/t4069-remerge-diff.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,4 +317,11 @@ test_expect_success 'remerge-diff turns off history simplification' '
317317
test_cmp expect actual
318318
'
319319

320+
test_expect_success 'remerge-diff with --reverse' '
321+
git log -1 --remerge-diff --oneline ab_resolution^ >expect &&
322+
git log -1 --remerge-diff --oneline ab_resolution >>expect &&
323+
git log -2 --remerge-diff --oneline ab_resolution --reverse >actual &&
324+
test_cmp expect actual
325+
'
326+
320327
test_done

0 commit comments

Comments
 (0)