Re: [PATCH v5 04/12] merge-tree: implement real merges

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 21, 2022, 09:25 UTC
Message-ID: <nycvar.QRO.7.76.6.2202211019490.26495@tvgsbejvaqbjf.bet>
In-Reply-To: <9b65e743-729f-6449-b7ef-c8c9fb130221@web.de>

Hi,

On Sun, 20 Feb 2022, René Scharfe wrote:

Show 15 quoted lines

> Am 20.02.22 um 07:54 schrieb Elijah Newren via GitGitGadget:
>
> > +	/*
> > +	 * Get the merge bases, in reverse order; see comment above
> > +	 * merge_incore_recursive in merge-ort.h
> > +	 */
> > +	common = get_merge_bases(parent1, parent2);
> > +	if (!common)
> > +		die(_("refusing to merge unrelated histories"));
> > +	for (j = common; j; j = j->next)
> > +		commit_list_insert(j->item, &merge_bases);
>
> This loop creates a reversed copy of "common".  You could use
> reverse_commit_list() instead to do it in-place and avoid the
> allocations.  Only the copy, "merge_bases", is used below.

Curious. When I read this first, I immediately assumed this was copy-pasted from `merge-recursive.c`, but it wasn't (https://github.com/git/git/blob/v2.35.1/merge-recursive.c#L3591-L3592):

		merge_bases = get_merge_bases(h1, h2);
		merge_bases = reverse_commit_list(merge_bases);

I tried to figure out where the manual reversal might have been copy-pasted from, but came up empty-handed. The comment in https://github.com/git/git/blob/v2.35.1/merge-ort.h#L40-L48 did not shed any light on it (but took me down memory lane, all the way to 2006!).

Ciao, Dscho