Re: [RFC PATCH 2/2] merge-ort: add t/t4310-merge-tree-ort.sh

[Elijah Newren <newren@gmail.com>]

On Wed, Jan 5, 2022 at 8:33 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> This adds a few tests for the new merge-tree-ort command. They have
> been copy-pasted from t4300-merge-tree.sh, and then the expected
> output has been adjusted.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  t/t4310-merge-tree-ort.sh | 162 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 162 insertions(+)
>  create mode 100755 t/t4310-merge-tree-ort.sh
>
> diff --git a/t/t4310-merge-tree-ort.sh b/t/t4310-merge-tree-ort.sh
> new file mode 100755
> index 0000000000..9a54508e82
> --- /dev/null
> +++ b/t/t4310-merge-tree-ort.sh
> @@ -0,0 +1,162 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2010 Will Palmer
> +# Copyright (c) 2021 Christian Couder
> +#
> +
> +test_description='git merge-tree-ort'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +       test_commit "initial" "initial-file" "initial"
> +'
> +
> +test_expect_success 'file add A, !B' '
> +       git reset --hard initial &&
> +       test_commit "add-a-not-b" "ONE" "AAA" &&
> +       git merge-tree-ort initial initial add-a-not-b >actual &&
> +       cat >expected <<EXPECTED &&
> +result tree: ee38e20a5c0e1698539ac99d55616079a04fce26

So the tests only work on sha1?  My tests in
https://lore.kernel.org/git/pull.1114.git.git.1640927044.gitgitgadget@gmail.com/
are sha256 compatible.

> +clean: 1
> +diff with branch1:
> +:000000 100644 0000000 43d5a8e A       ONE
> +
> +diff --git a/ONE b/ONE
> +new file mode 100644
> +index 0000000..43d5a8e
> +--- /dev/null
> ++++ b/ONE
> +@@ -0,0 +1 @@
> ++AAA

Oh, this isn't just a --raw diff, but both a raw and full diff?  I
missed that reading over the previous patch.  This seems potentially
*extremely* expensive for big repos; dramatically more so than the
merge portion of the operation.  (Any files modified on just one side
can be trivially merged without looking at the contents.  In fact,
directories only modified on one side can usually be trivially merged
without looking at the contents.  But merges are going to modify lots
of files relative to either of the two sides and especially relative
to the merge base, and doing a full diff is going to have to crack
open every one of those files -- multiple times since you do it
against the base as well -- to show this output).  I don't think this
is what you want.

> +diff with branch2:
> +diff with base:
> +:000000 100644 0000000 43d5a8e A       ONE
> +
> +diff --git a/ONE b/ONE
> +new file mode 100644
> +index 0000000..43d5a8e
> +--- /dev/null
> ++++ b/ONE
> +@@ -0,0 +1 @@
> ++AAA
> +EXPECTED
> +
> +       test_cmp expected actual
> +'
> +
> +test_expect_success 'file add !A, B' '
> +       git reset --hard initial &&
> +       test_commit "add-not-a-b" "ONE" "AAA" &&
> +       git merge-tree-ort initial add-not-a-b initial >actual &&
> +       cat >expected <<EXPECTED &&
> +result tree: ee38e20a5c0e1698539ac99d55616079a04fce26
> +clean: 1
> +diff with branch1:
> +diff with branch2:
> +:000000 100644 0000000 43d5a8e A       ONE
> +
> +diff --git a/ONE b/ONE
> +new file mode 100644
> +index 0000000..43d5a8e
> +--- /dev/null
> ++++ b/ONE
> +@@ -0,0 +1 @@
> ++AAA
> +diff with base:
> +:000000 100644 0000000 43d5a8e A       ONE
> +
> +diff --git a/ONE b/ONE
> +new file mode 100644
> +index 0000000..43d5a8e
> +--- /dev/null
> ++++ b/ONE
> +@@ -0,0 +1 @@
> ++AAA
> +EXPECTED
> +
> +       test_cmp expected actual
> +'
> +
> +test_expect_success 'file add A, B (same)' '
> +       git reset --hard initial &&
> +       test_commit "add-a-b-same-A" "ONE" "AAA" &&
> +       git reset --hard initial &&
> +       test_commit "add-a-b-same-B" "ONE" "AAA" &&
> +       git merge-tree-ort initial add-a-b-same-A add-a-b-same-B >actual &&
> +       cat >expected <<EXPECTED &&
> +result tree: ee38e20a5c0e1698539ac99d55616079a04fce26
> +clean: 1
> +diff with branch1:
> +diff with branch2:
> +diff with base:
> +:000000 100644 0000000 43d5a8e A       ONE
> +
> +diff --git a/ONE b/ONE
> +new file mode 100644
> +index 0000000..43d5a8e
> +--- /dev/null
> ++++ b/ONE
> +@@ -0,0 +1 @@
> ++AAA
> +EXPECTED
> +
> +       test_cmp expected actual
> +'
> +
> +test_expect_success 'file add A, B (different)' '
> +       git reset --hard initial &&
> +       test_commit "add-a-b-diff-A" "ONE" "AAA" &&
> +       git reset --hard initial &&
> +       test_commit "add-a-b-diff-B" "ONE" "BBB" &&
> +       git merge-tree-ort initial add-a-b-diff-A add-a-b-diff-B >actual &&
> +       cat >expected <<EXPECTED &&
> +result tree: 3aa938e8cc8be73c81cbaf629ea55a16e7c39319
> +clean: 0
> +diff with branch1:
> +:100644 100644 43d5a8e 1e462bc M       ONE
> +
> +diff --git a/ONE b/ONE
> +index 43d5a8e..1e462bc 100644
> +--- a/ONE
> ++++ b/ONE
> +@@ -1 +1,5 @@
> ++<<<<<<< add-a-b-diff-A
> + AAA
> ++=======
> ++BBB
> ++>>>>>>> add-a-b-diff-B
> +diff with branch2:
> +:100644 100644 ba62923 1e462bc M       ONE
> +
> +diff --git a/ONE b/ONE
> +index ba62923..1e462bc 100644
> +--- a/ONE
> ++++ b/ONE
> +@@ -1 +1,5 @@
> ++<<<<<<< add-a-b-diff-A
> ++AAA
> ++=======
> + BBB
> ++>>>>>>> add-a-b-diff-B
> +diff with base:
> +:000000 100644 0000000 1e462bc A       ONE
> +
> +diff --git a/ONE b/ONE
> +new file mode 100644
> +index 0000000..1e462bc
> +--- /dev/null
> ++++ b/ONE
> +@@ -0,0 +1,5 @@
> ++<<<<<<< add-a-b-diff-A
> ++AAA
> ++=======
> ++BBB
> ++>>>>>>> add-a-b-diff-B
> +EXPECTED
> +
> +       test_cmp expected actual
> +'
> +
> +test_done
> --

I've focused a bit on the things that I didn't care as much for, but
the usage of the merge-ort API was solid and there are pieces here
that look quite simliar to what I'd expect...and in fact, to what I
also implemented.  Perhaps there are multiple things I also overlooked
in my implementation of this idea; would be great to get your comments
on that, over at [1].

And, as a heads up, as noted at [2], I'm also working on the
server-side cherry-pick/rebase.


[1] https://lore.kernel.org/git/pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/CABPp-BHpK8hPsiuHoYsf5D_rjcGLSW-_faL3ODoh56pG_2Luwg@mail.gmail.com/