Re: [PATCH v2 04/13] merge-tree: implement real merges

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Wed, 2 Feb 2022, Elijah Newren wrote:

> On Wed, Feb 2, 2022 at 1:30 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> > > +Performs a merge, but does not make any new commits and does not read
> > > +from or write to either the working tree or index.
> > > +
> > > +The second form is deprecated and supported only for backward
> > > +compatibility.  It will likely be removed in the future, and will not
> > > +be discussed further in this manual.
> >
> > This, especially the deletion of the original description on what
> > trivial merge does, may be premature, especially if it is still
> > "supported for backward compatibility".
>
> I actually extended it, but Dscho suggested removing it entirely --
> https://lore.kernel.org/git/nycvar.QRO.7.76.6.2201251804250.2121@tvgsbejvaqbjf.bet/.
> I can restore it; does that paragraph look good to you (you can see
> the full thing even if it's split by Dscho's commentary).

I do not necessarily feel strong about keeping the paragraph, but I would
appreciate a better rationale for that than "because it was there
previously".

At this point there cannot be any doubt that the "trivial mode" is worth
deprecating and getting rid off because it has been in little or no use.

I mean, elsewhere we are overzealously removing stuff "because _core Git_
does not use it", and here we keep something for... reasons?

> > > +test_expect_success 'Content merge and a few conflicts' '
> > > +     git checkout side1^0 &&
> > > +     test_must_fail git merge side2 &&
> > > +     expected_tree=$(cat .git/AUTO_MERGE) &&
> > > +
> > > +     # We will redo the merge, while we are still in a conflicted state!
> > > +     test_when_finished "git reset --hard" &&
> > > +
> > > +     test_expect_code 1 git merge-tree --write-tree side1 side2 >RESULT &&
> > > +     actual_tree=$(head -n 1 RESULT) &&
> > > +
> > > +     # Due to differences of e.g. "HEAD" vs "side1", the results will not
> > > +     # exactly match.  Dig into individual files.
> > > +
> > > +     # Numbers should have three-way merged cleanly
> > > +     test_write_lines 0 1 2 3 4 5 6 >expect &&
> > > +     git show ${actual_tree}:numbers >actual &&
> > > +     test_cmp expect actual &&
> > > +
> > > +     # whatever and whatever~<branch> should have same HASHES
> > > +     git rev-parse ${expected_tree}:whatever ${expected_tree}:whatever~HEAD >expect &&
> > > +     git rev-parse ${actual_tree}:whatever ${actual_tree}:whatever~side1 >actual &&
> > > +     test_cmp expect actual &&
> > > +
> > > +     # greeting should have a merge conflict
> > > +     git show ${expected_tree}:greeting >tmp &&
> > > +     cat tmp | sed -e s/HEAD/side1/ >expect &&
> > > +     git show ${actual_tree}:greeting >actual &&
> > > +     test_cmp expect actual
> > > +'
> >
> > It is somewhat sad that we need to reivent merge test cases over and
> > over, instead of easily reuse an existing one by replacing
> >
> >         git checkout one &&
> >         git merge two
> >
> > with
> >
> >         git checkout one &&
> >         T=$(git merge-tree HEAD two) &&
> >         C=$(git commit-tree $T -p HEAD -p two) &&
> >         git reset --hard $C
> >
> > ;-)
>
> Sorry...I'm afraid I'm not following.

I cannot speak for Junio (nor do I want to). Having said that, I interpret
the quoted section as a lament on the somewhat repetitive complexity that
is required by our tests merely to set up a scenario where we can run the
actual command we want to test.

If my interpretation is correct, then it is probably fair to suggest using
`test_merge` in a setup-type of test case (because that not only runs `git
merge` but also tags the result), and then use `git reset --hard <tag>` in
subsequent test cases instead of re-running the `checkout && merge` dance.

Ciao,
Dscho