Re: [PATCH 00/19] Cleanup merge API

From: Elijah Newren <newren@gmail.com>
Date: Jul 26, 2019, 15:15 UTC
Message-ID: <CABPp-BHUf0eR2Dj77+qNa0rS2qAHxQL2MpRgg26Zjrfheim6Gg@mail.gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.1907252055500.21907@tvgsbejvaqbjf.bet>

Hi Dscho,

On Thu, Jul 25, 2019 at 12:15 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 45 quoted lines

>
> Hi Elijah,
>
> On Thu, 25 Jul 2019, Elijah Newren wrote:
>
> >   * All current callers (3 of them?) of merge_recursive() always pass
> >     it a specially created reversed-list for the merge_bases.  Some
> >     history spelunking provides no details on any of these about why;
> >     it appears that the 2nd and 3rd callers reversed the list because
> >     the first did, and I'm guessing the first did in an attempt to
> >     exactly match the git-merge-recursive.py scripts' behavior.
>
> That is part of the reason.
>
> After I worked on converting that Python script with Alex Riesen, I
> tested it on all merge commits I could find at that point in linux.git,
> to compare the outcomes between scripted and non-scripted versions. IIRC
> I even reported those findings back to the mailing list.
>
> *clickety-click*
>
> Ah, I did the extensive test on git.git first [*1*]:
> https://public-inbox.org/git/Pine.LNX.4.63.0608092232000.13885@wbgn013.biozentrum.uni-wuerzburg.de/
>
> As you can see, it seemed that the reverse order of merge bases (trying
> to merge the oldest two merge bases first, then merging with the
> 3rd-oldest, etc) avoided more merge conflicts than the original order.
>
> >     But if the API needs them in a reverse order from what people
> >     would normally expect to pass them in, shouldn't it reverse them
> >     itself instead of making all callers do it?
>
> I would not, as the order the merge bases are passed in defines the
> order in which they are handled.
>
> >     Also, the order shouldn't matter when there are no conflicts, and
> >     when there are conflicts it'd only change which side of the
> >     conflict markers the made-up virtual merge base would list things
> >     in.
>
> That's what I thought, right up until I re-created those merge commits.
>
> I really forgot most of the details, but I seem to remember that there
> was a puzzling one where the reverse order caused no merge conflicts,
> and the original order caused a double merge conflict.

Thanks for providing this history; very helpful. In v2 (which I will be posting soon), I included a comment in merge-recursive.h just above the merge_recursive() function with a summary of these details:

NOTE: empirically, about a decade ago it was determined that with more
      than two merge bases, optimal behavior was found when the
      merge_bases were passed in the order of oldest commit to newest
      commit.  Also, merge_bases will be consumed (emptied) so make a
      copy if you need it.