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

On Tue, Aug 3, 2021 at 9:56 AM Jeff King [off-list ref] wrote:

On Sun, Aug 01, 2021 at 12:07:39AM +0000, Elijah Newren via GitGitGadget wrote:

quoted

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.

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.