Re: [RFC PATCH 0/2] Introduce new merge-tree-ort command

From: Elijah Newren <newren@gmail.com>
Date: Jan 12, 2022, 20:06 UTC
Message-ID: <CABPp-BHQdkhAEmTrtc+XMgj5A5ASBVRw0_bXH10NSrMsyRK+oA@mail.gmail.com>
In-Reply-To: <xmqqk0f4x20a.fsf@gitster.g>

On Wed, Jan 12, 2022 at 10:06 AM Junio C Hamano <gitster@pobox.com> wrote:

>
> Elijah Newren <newren@gmail.com> writes:
>

...

Show 6 quoted lines

> I however suspect that Ævar didn't mean by "legacy merge plumbing
> built-in" the strategy backends.  IOW, I had an impression that what
> is on the chopping block is merge-tree and not merge-recursive.
>
> But since you brought up deprecation of recursive, let's spend a few
> minutes on the topic.

Not sure it matters, but for reference, Ævar explicitly brought up merge-recursive.c. The fuller quote was:

> >> I.e. is it really costing us
> >> to just leave these "legacy merge" plumbing built-ins and
> >> merge-recursive.c etc. in place?

Because he brought it up, I decided to address it. It was unclear to me whether he meant builtin/merge-recursive.c or the toplevel merge-recursive.c, so I just addressed both.

> I suspect that we may be able to just invoke ort when recursive is
> invoked, and such a wrapping may even be easier than wrapping "git
> blame" to replace "git annotate" (where a command line option or two
> needs to change behaviour).

Yes, that was the plan I had.

> I doubt there is -X<strategy-option>
> that affects recursive that ORT does not understand,

Technically there are currently two. As documented in merge-strategies.txt, ORT takes the same -X options as recursive, but currently ignores both -Xdiff-algorithm (instead passing HISTOGRAM_DIFF down to ll_merge()), and -Xno-renames (instead passing DIFF_DETECT_RENAME down to diffcore_rename_extended()). I guess there are three if you count -Xpatience separately from -Xdiff-algorithm=patience.

> so it may be quite simple to deprecate "merge -s recursive".

Yes...but why deprecate? I thought the plan was to (eventually) make requests for either `recursive` or `ort` be handled by running the `ort` backend. Making that kind of switch is much smaller than the one we already made to switch the default backend from `recursive` to `ort`, so I'm not sure I see what we gain by doing such a switch in stages. Maybe I'm missing something?