Re: [PATCH v3 04/15] merge-tree: implement real merges

From: Elijah Newren <newren@gmail.com>
Date: Feb 2, 2022, 21:56 UTC
Message-ID: <CABPp-BGpD6g5QH3=4X_dCuSX0Bs0utHn5hyuU4_UiwNhU0h8sg@mail.gmail.com>
In-Reply-To: <xmqqy22tx8t1.fsf@gitster.g>

On Wed, Feb 2, 2022 at 1:22 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 59 quoted lines

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > @@ -392,7 +395,46 @@ struct merge_tree_options {
> >  static int real_merge(struct merge_tree_options *o,
> >                     const char *branch1, const char *branch2)
> >  {
> > -     die(_("real merges are not yet implemented"));
> > +     struct commit *parent1, *parent2;
> > +     struct commit_list *common;
> > +     struct commit_list *merge_bases = NULL;
> > +     struct commit_list *j;
> > +     struct merge_options opt;
> > +     struct merge_result result = { 0 };
> > +
> > +     parent1 = get_merge_parent(branch1);
> > +     if (!parent1)
> > +             help_unknown_ref(branch1, "merge-tree",
> > +                              _("not something we can merge"));
> > +
> > +     parent2 = get_merge_parent(branch2);
> > +     if (!parent2)
> > +             help_unknown_ref(branch2, "merge-tree",
> > +                              _("not something we can merge"));
> > +
> > +     init_merge_options(&opt, the_repository);
> > +
> > +     opt.show_rename_progress = 0;
> > +
> > +     opt.branch1 = branch1;
> > +     opt.branch2 = branch2;
> > +
> > +     /*
> > +      * 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"));
>
> It appears to me that "merge-tree" in this mode, with the above
> code, cannot be used as a workhorse to implement server-side
> cherry-pick (or revert), which needs to allow the user to specify an
> arbitrary "common ancestor", instead of computing on its own.
>
> To replay the change made by commit A on top of commit X (i.e.
> "cherry-pick A on X"), we have to be able to say "compute the
> three-way merge between A and X, pretending as if A^ were their
> common ancestor".  The story is the same for revert---we compute
> three-way merge between A^ and X, pretending as if A were their
> common ancestor.
>
> The above interface into this function, sadly, does not seem to
> allow such a request, unless I am missing something.
>
> And if I am correct, it is a shame---after all, the point of the
> merge-trees command is to take three trees and run a three-way
> merge, and not being able to merge three "trees" and require
> "commits" makes this mode much less useful than its potential.

Yes, you are reading right. I think the cherry-pick/rebase replacement actually deserves a separate command from what merges should use; replaying a sequence of commits just has a number of UI differences and abilities that I think pull it in a different direction. I didn't want to wait and submit everything all at once (this series is long enough), and I figured that providing the in-core equivalent to `git merge` was a simpler first step worth submitting first before later providing the in-core equivalents to `git rebase/git cherry-pick`.

(Also, I'm a bit wary of providing a command meant to just do a single three-way merge with a defined merge-base, because that seems to be a path towards returning to a scripted rebase. Having a way to do only a single such special merge is fine but we should avoid encouraging people to go down that route.)