Hi Elijah,

On Mon, 21 Feb 2022, Elijah Newren wrote:

On Sun, Feb 20, 2022 at 1:03 AM René Scharfe [off-list ref] wrote:

quoted

Am 20.02.22 um 07:54 schrieb Elijah Newren via GitGitGadget:

[...]

quoted

quoted

+     /*
+      * 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.

Oh, good catch.  I probably should have been aware of this since
someone requested I move the reverse_commit_list() function from
merge-recursive.c to commit.c as part of my merge-ort work, but looks
like I forgot about it and copied this command snippet from
builtin/merge.c instead.  I have no excuse.

Ooops! I missed that the `reverse_commit_list()` function was moved to
`commit.c` by _you_, and had not been there all along (my fault, of
course, see 8918b0c9c26 (merge-recur: try to merge older merge bases
first, 2006-08-09)).

However, I wonder if that means we could also apply this
simplification to the code snippets in builtin/merge.c and sequencer.c
that you can find with
    git grep commit_list_insert.*reversed
?  Maybe #leftoverbits for that part?

Yes, that's a good idea. I summarized this left-over-bit in
https://github.com/gitgitgadget/git/issues/1156

Ciao,
Dscho