Re: [PATCH] log: --remerge-diff needs to keep around commit parents

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 11, 2024, 18:32 UTC
Message-ID: <b7488a36-0ba6-0847-47cb-6a5f3edec689@gmx.de>
In-Reply-To: <xmqq7c9ak0e9.fsf@gitster.g>

Hi Junio,

On Mon, 11 Nov 2024, Junio C Hamano wrote:

Show 23 quoted lines

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > 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
> > cb115748ec0d (Some more memory leak avoidance, 2006-06-17) is to release
> > the commit's list of parents immediately after showing it. This can break
> > the merge base computation.
>
> > Note that it matters more clearly 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.
>
> Ouch.
>
> I am curious about "more clearly" in the above, though.

I consider these examples less clear, but they are still affected:

	git show --remerge-diff v2.45.2^0
	vs
	git show --remerge-diff v2.44.2^0 v2.45.2^0

	git show --remerge-diff v2.45.1~1
	vs
	git log --topo-order --first-parent --remerge-diff v2.44.2 v2.45.1

Concretely, these diffs should be empty, but are not:

	git diff --no-index \
		<(git show --remerge-diff v2.45.2^0 | sed 1d) \
		<(git show --remerge-diff v2.44.2^0 v2.45.2^0 | sed 1,/^commit/d)

and

	git diff --no-index \
		<(git show --remerge-diff v2.45.1~1 | grep -v ^commit) \
		<(git log --topo-order --first-parent -6 --remerge-diff v2.44.2 v2.45.1 |
		  sed '1,/^commit 1c00f92eb5ee4a48ab615eefa41f2dd6024d43bc/d;/^commit/,$d')

No `--reverse` required, not even clock skew.

Ciao, Johannes