Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort

From: Elijah Newren <newren@gmail.com>
Date: Aug 3, 2021, 16:57 UTC
Message-ID: <CABPp-BGgcpMbFrEjaDxpfb8=Wr_5O76qCDq1OSbpemrnYDcsqw@mail.gmail.com>
In-Reply-To: <YQlnNXgk3qcGb5Bk@coredump.intra.peff.net>

On Tue, Aug 3, 2021 at 9:56 AM Jeff King <peff@peff.net> wrote:

Show 29 quoted lines

>
> On Sun, Aug 01, 2021 at 12:07:39AM +0000, Elijah Newren via GitGitGadget wrote:
>
> > This is an RFC series designed to spur feedback about switching the default
> > merge backend (reviewing the patches is of secondary importance at this
> > point). Some questions:
> >
> >  * Are there things others want before this series is considered for
> >    inclusion?
> >  * What kind of timeline do others think is reasonable?
> >  * Would it be beneficial to let this series sit in 'next' for an extended
> >    duration to gain more feedback?
>
> It looks like others gave some more specific review on the patches, but
> on the meta-topic of "do we switch, and when", my response is: yes, and
> soon. :)
>
> Having watched the development of merge-ort, plus all of the weird
> corner cases in merge-recursive we've seen over the years (many of which
> you found and added tests for while working on merge-ort!), my gut
> feeling is that the switch is _much_ more likely to fix problems people
> might see in the wild rather than cause them.
>
> It would make sense to me to do the switch in 'next' early in the
> post-v2.33 cycle. It can cook there for a bit, but I think we have found
> that it's much more likely to see actual use once it hits 'master'. So I
> don't see a particular reason to have it sit in 'next' for a long time.
> We should get as much exposure in 'master' during the v2.34 cycle as
> possible.

Show 6 quoted lines

> The nice thing is that the two strategies can co-exist. So if it does
> turn out to have any regressions, it's an easy revert to switch back,
> and even post-release users can switch at runtime. We have pull.twohead,
> but I don't think we have an equivalent that would impact a bare "git
> merge" or "git rebase -m". Maybe it would be worth adding those as an
> escape hatch?

Actually, pull.twohead is not pull specific; it already affects merge, rebase (-m is the default for rebase, btw), cherry-pick, and revert. pull.twohead has affected a bare "git merge" since 1c7b76be7d ("Build in merge", 2008-07-07). I thought it was weird that "merge strategy" for the merge command was specified via a config option under "pull", and included my misgivings about it in the commit message of 14c4586c2d ("merge,rebase,revert: select ort or recursive by config or environment", 2020-11-02) when I made sequencer.c pay attention to that config option as well:

"""
    Also, allow folks to pick the new algorithm via config setting.  It
    turns out builtin/merge.c already had a way to allow users to specify a
    different default merge algorithm: pull.twohead.  Rather odd
    configuration name (especially to be in the 'pull' namespace rather than
    'merge') but it's there.  Add that same configuration to rebase,
    cherry-pick, and revert.
"""

But no one had an alternate suggestion or opinion on attempting to migrate the configuration to a different name, so it has just stuck. Anyway, if folks want to try out 'ort' with the 2.32 or 2.33 releases, they can set pull.twohead=ort. Once we switch the default, they can set pull.twohead=recursive to get the old default.