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

From: Elijah Newren <newren@gmail.com>
Date: Feb 22, 2022, 02:28 UTC
Message-ID: <CABPp-BESAh6wLComJoYsf7Q7NF2EMPptKRhfAoy=1-ZRZovnaQ@mail.gmail.com>
In-Reply-To: <9b65e743-729f-6449-b7ef-c8c9fb130221@web.de>

On Sun, Feb 20, 2022 at 1:03 AM René Scharfe <l.s.r@web.de> wrote:

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

[...]

Show 13 quoted lines

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

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?