* [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-23 16:48 ` Junio C Hamano
2020-10-24 10:49 ` Đoàn Trần Công Danh
2020-10-23 16:01 ` [PATCH 2/9] merge tests: expect improved directory/file conflict handling in ort Elijah Newren via GitGitGadget
` (8 subsequent siblings)
9 siblings, 2 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
There are a number of tests that the "recursive" backend does not handle
correctly but which the redesign in "ort" will. Add a new helper in
lib-merge.sh for selecting a different test expectation based on the
setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to
document which ones we expect to fail under recursive but pass under
ort.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/lib-merge.sh | 15 +++++++++++++++
t/t6416-recursive-corner-cases.sh | 11 ++++++-----
t/t6422-merge-rename-corner-cases.sh | 7 ++++---
t/t6423-merge-rename-directories.sh | 13 +++++++------
t/t6426-merge-skip-unneeded-updates.sh | 3 ++-
t/t6430-merge-recursive.sh | 3 ++-
6 files changed, 36 insertions(+), 16 deletions(-)
create mode 100644 t/lib-merge.sh
diff --git a/t/lib-merge.sh b/t/lib-merge.sh
new file mode 100644
index 0000000000..fac2bc5919
--- /dev/null
+++ b/t/lib-merge.sh
@@ -0,0 +1,15 @@
+# Helper functions used by merge tests.
+
+test_expect_merge_algorithm () {
+ status_for_recursive=$1
+ shift
+ status_for_ort=$1
+ shift
+
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_expect_${status_for_ort} "$@"
+ else
+ test_expect_${status_for_recursive} "$@"
+ fi
+}
diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
index fd98989b14..8b3a4fc843 100755
--- a/t/t6416-recursive-corner-cases.sh
+++ b/t/t6416-recursive-corner-cases.sh
@@ -3,6 +3,7 @@
test_description='recursive merge corner cases involving criss-cross merges'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
#
# L1 L2
@@ -1069,7 +1070,7 @@ test_expect_success 'setup symlink modify/modify' '
)
'
-test_expect_failure 'check symlink modify/modify' '
+test_expect_merge_algorithm failure success 'check symlink modify/modify' '
(
cd symlink-modify-modify &&
@@ -1135,7 +1136,7 @@ test_expect_success 'setup symlink add/add' '
)
'
-test_expect_failure 'check symlink add/add' '
+test_expect_merge_algorithm failure success 'check symlink add/add' '
(
cd symlink-add-add &&
@@ -1223,7 +1224,7 @@ test_expect_success 'setup submodule modify/modify' '
)
'
-test_expect_failure 'check submodule modify/modify' '
+test_expect_merge_algorithm failure success 'check submodule modify/modify' '
(
cd submodule-modify-modify &&
@@ -1311,7 +1312,7 @@ test_expect_success 'setup submodule add/add' '
)
'
-test_expect_failure 'check submodule add/add' '
+test_expect_merge_algorithm failure success 'check submodule add/add' '
(
cd submodule-add-add &&
@@ -1386,7 +1387,7 @@ test_expect_success 'setup conflicting entry types (submodule vs symlink)' '
)
'
-test_expect_failure 'check conflicting entry types (submodule vs symlink)' '
+test_expect_merge_algorithm failure success 'check conflicting entry types (submodule vs symlink)' '
(
cd submodule-symlink-add-add &&
diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh
index 3375eaf4e7..58729593ba 100755
--- a/t/t6422-merge-rename-corner-cases.sh
+++ b/t/t6422-merge-rename-corner-cases.sh
@@ -4,6 +4,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses"
# t6036 has corner cases that involve both criss-cross merges and renames
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
test_setup_rename_delete_untracked () {
test_create_repo rename-delete-untracked &&
@@ -878,7 +879,7 @@ test_setup_rad () {
)
}
-test_expect_failure 'rad-check: rename/add/delete conflict' '
+test_expect_merge_algorithm failure success 'rad-check: rename/add/delete conflict' '
test_setup_rad &&
(
cd rad &&
@@ -951,7 +952,7 @@ test_setup_rrdd () {
)
}
-test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
+test_expect_merge_algorithm failure success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
test_setup_rrdd &&
(
cd rrdd &&
@@ -1040,7 +1041,7 @@ test_setup_mod6 () {
)
}
-test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
+test_expect_merge_algorithm failure success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
test_setup_mod6 &&
(
cd mod6 &&
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 06b46af765..807a424a52 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -26,6 +26,7 @@ test_description="recursive merge with directory renames"
# files that might be renamed into each other's paths.)
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
###########################################################################
@@ -1339,7 +1340,7 @@ test_setup_6b1 () {
)
}
-test_expect_failure '6b1: Same renames done on both sides, plus another rename' '
+test_expect_merge_algorithm failure success '6b1: Same renames done on both sides, plus another rename' '
test_setup_6b1 &&
(
cd 6b1 &&
@@ -1412,7 +1413,7 @@ test_setup_6b2 () {
)
}
-test_expect_failure '6b2: Same rename done on both sides' '
+test_expect_merge_algorithm failure success '6b2: Same rename done on both sides' '
test_setup_6b2 &&
(
cd 6b2 &&
@@ -3471,7 +3472,7 @@ test_setup_10e () {
)
}
-test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' '
+test_expect_merge_algorithm failure success '10e: Does git complain about untracked file that is not really in the way?' '
test_setup_10e &&
(
cd 10e &&
@@ -4104,7 +4105,7 @@ test_setup_12b1 () {
)
}
-test_expect_failure '12b1: Moving two directory hierarchies into each other' '
+test_expect_merge_algorithm failure success '12b1: Moving two directory hierarchies into each other' '
test_setup_12b1 &&
(
cd 12b1 &&
@@ -4272,7 +4273,7 @@ test_setup_12c1 () {
)
}
-test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' '
+test_expect_merge_algorithm failure success '12c1: Moving one directory hierarchy into another w/ content merge' '
test_setup_12c1 &&
(
cd 12c1 &&
@@ -4632,7 +4633,7 @@ test_setup_12f () {
)
}
-test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' '
+test_expect_merge_algorithm failure success '12f: Trivial directory resolve, caching, all kinds of fun' '
test_setup_12f &&
(
cd 12f &&
diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh
index 699813671c..d7eeee4310 100755
--- a/t/t6426-merge-skip-unneeded-updates.sh
+++ b/t/t6426-merge-skip-unneeded-updates.sh
@@ -23,6 +23,7 @@ test_description="merge cases"
# files that might be renamed into each other's paths.)
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
###########################################################################
@@ -666,7 +667,7 @@ test_setup_4a () {
# correct requires doing the merge in-memory first, then realizing that no
# updates to the file are necessary, and thus that we can just leave the path
# alone.
-test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' '
+test_expect_merge_algorithm failure success '4a: Change on A, change on B subset of A, dirty mods present' '
test_setup_4a &&
(
cd 4a &&
diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh
index a328260d42..9c08e63af2 100755
--- a/t/t6430-merge-recursive.sh
+++ b/t/t6430-merge-recursive.sh
@@ -3,6 +3,7 @@
test_description='merge-recursive backend test'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
test_expect_success 'setup 1' '
@@ -641,7 +642,7 @@ test_expect_success 'merge-recursive copy vs. rename' '
test_cmp expected actual
'
-test_expect_failure 'merge-recursive rename vs. rename/symlink' '
+test_expect_merge_algorithm failure success 'merge-recursive rename vs. rename/symlink' '
git checkout -f rename &&
git merge rename-ln &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-23 16:01 ` [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive Elijah Newren via GitGitGadget
@ 2020-10-23 16:48 ` Junio C Hamano
2020-10-23 17:25 ` Elijah Newren
2020-10-24 10:49 ` Đoàn Trần Công Danh
1 sibling, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2020-10-23 16:48 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Elijah Newren <newren@gmail.com>
>
> There are a number of tests that the "recursive" backend does not handle
> correctly but which the redesign in "ort" will. Add a new helper in
> lib-merge.sh for selecting a different test expectation based on the
> setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to
> document which ones we expect to fail under recursive but pass under
> ort.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> t/lib-merge.sh | 15 +++++++++++++++
> t/t6416-recursive-corner-cases.sh | 11 ++++++-----
> t/t6422-merge-rename-corner-cases.sh | 7 ++++---
> t/t6423-merge-rename-directories.sh | 13 +++++++------
> t/t6426-merge-skip-unneeded-updates.sh | 3 ++-
> t/t6430-merge-recursive.sh | 3 ++-
> 6 files changed, 36 insertions(+), 16 deletions(-)
> create mode 100644 t/lib-merge.sh
>
> diff --git a/t/lib-merge.sh b/t/lib-merge.sh
> new file mode 100644
> index 0000000000..fac2bc5919
> --- /dev/null
> +++ b/t/lib-merge.sh
> @@ -0,0 +1,15 @@
> +# Helper functions used by merge tests.
> +
> +test_expect_merge_algorithm () {
> + status_for_recursive=$1
> + shift
> + status_for_ort=$1
> + shift
Smaller than minor, but I'd find
status_for_recursive=$1 status_for_ort=$2
shift 2
easier to see that which one is for which by matching the order in
which the calling sites, e.g.
test_expect_merge_algorithm success failure \
here comes the commands being tested
lists them.
> + if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> + then
> + test_expect_${status_for_ort} "$@"
> + else
> + test_expect_${status_for_recursive} "$@"
> + fi
I expect this to be purely transitory, so it is fine. If not,
something along the lines of ...
eval test_expect='$'status_for_"$GIT_TEST_MERGE_ALGORITHM"
$test_expect "$@"
... might be what I would suggest, though ;-).
And the users are just too pleasant to see, with full of "failure
sucess", which is the second best outcome we want to see ;-)
> +test_expect_merge_algorithm failure success 'check symlink mo...
> +test_expect_merge_algorithm failure success 'check symlink ad...
> +test_expect_merge_algorithm failure success 'check submodule ...
> +test_expect_merge_algorithm failure success 'check submodule ...
> +test_expect_merge_algorithm failure success 'check conflictin...
> +test_expect_merge_algorithm failure success 'rad-check: renam...
> +test_expect_merge_algorithm failure success 'rrdd-check: rena...
> +test_expect_merge_algorithm failure success 'mod6-check: chai...
> +test_expect_merge_algorithm failure success '6b1: Same rename...
> +test_expect_merge_algorithm failure success '6b2: Same rename...
> +test_expect_merge_algorithm failure success '10e: Does git co...
> +test_expect_merge_algorithm failure success '12b1: Moving two...
> +test_expect_merge_algorithm failure success '12c1: Moving one...
> +test_expect_merge_algorithm failure success '12f: Trivial dir...
> +test_expect_merge_algorithm failure success '4a: Change on A,...
> +test_expect_merge_algorithm failure success 'merge-recursive ...
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-23 16:48 ` Junio C Hamano
@ 2020-10-23 17:25 ` Elijah Newren
2020-10-23 18:27 ` Elijah Newren
0 siblings, 1 reply; 31+ messages in thread
From: Elijah Newren @ 2020-10-23 17:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, Git Mailing List
On Fri, Oct 23, 2020 at 9:48 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > There are a number of tests that the "recursive" backend does not handle
> > correctly but which the redesign in "ort" will. Add a new helper in
> > lib-merge.sh for selecting a different test expectation based on the
> > setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to
> > document which ones we expect to fail under recursive but pass under
> > ort.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> > t/lib-merge.sh | 15 +++++++++++++++
> > t/t6416-recursive-corner-cases.sh | 11 ++++++-----
> > t/t6422-merge-rename-corner-cases.sh | 7 ++++---
> > t/t6423-merge-rename-directories.sh | 13 +++++++------
> > t/t6426-merge-skip-unneeded-updates.sh | 3 ++-
> > t/t6430-merge-recursive.sh | 3 ++-
> > 6 files changed, 36 insertions(+), 16 deletions(-)
> > create mode 100644 t/lib-merge.sh
> >
> > diff --git a/t/lib-merge.sh b/t/lib-merge.sh
> > new file mode 100644
> > index 0000000000..fac2bc5919
> > --- /dev/null
> > +++ b/t/lib-merge.sh
> > @@ -0,0 +1,15 @@
> > +# Helper functions used by merge tests.
> > +
> > +test_expect_merge_algorithm () {
> > + status_for_recursive=$1
> > + shift
> > + status_for_ort=$1
> > + shift
>
> Smaller than minor, but I'd find
>
> status_for_recursive=$1 status_for_ort=$2
> shift 2
>
> easier to see that which one is for which by matching the order in
> which the calling sites, e.g.
>
> test_expect_merge_algorithm success failure \
> here comes the commands being tested
>
> lists them.
I can fix that up.
>
> > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> > + then
> > + test_expect_${status_for_ort} "$@"
> > + else
> > + test_expect_${status_for_recursive} "$@"
> > + fi
>
> I expect this to be purely transitory, so it is fine. If not,
> something along the lines of ...
>
> eval test_expect='$'status_for_"$GIT_TEST_MERGE_ALGORITHM"
> $test_expect "$@"
>
> ... might be what I would suggest, though ;-).
I also expect it to be transitory, but how long that transition is
depends more on how long it takes for others to become comfortable
with removing merge-recursive.c once merge-ort.c is in place. I
suspect at a minimum it's a cycle or two after ort becomes the default
(which at a minimum would come a cycle or two after ort is finished
and available). If for some reason folks don't eventually become
comfortable with removing merge-recursive, then we might need to
revisit.
> And the users are just too pleasant to see, with full of "failure
> sucess", which is the second best outcome we want to see ;-)
>
> > +test_expect_merge_algorithm failure success 'check symlink mo...
> > +test_expect_merge_algorithm failure success 'check symlink ad...
> > +test_expect_merge_algorithm failure success 'check submodule ...
> > +test_expect_merge_algorithm failure success 'check submodule ...
> > +test_expect_merge_algorithm failure success 'check conflictin...
> > +test_expect_merge_algorithm failure success 'rad-check: renam...
> > +test_expect_merge_algorithm failure success 'rrdd-check: rena...
> > +test_expect_merge_algorithm failure success 'mod6-check: chai...
> > +test_expect_merge_algorithm failure success '6b1: Same rename...
> > +test_expect_merge_algorithm failure success '6b2: Same rename...
> > +test_expect_merge_algorithm failure success '10e: Does git co...
> > +test_expect_merge_algorithm failure success '12b1: Moving two...
> > +test_expect_merge_algorithm failure success '12c1: Moving one...
> > +test_expect_merge_algorithm failure success '12f: Trivial dir...
> > +test_expect_merge_algorithm failure success '4a: Change on A,...
> > +test_expect_merge_algorithm failure success 'merge-recursive ...
:-)
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-23 17:25 ` Elijah Newren
@ 2020-10-23 18:27 ` Elijah Newren
0 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren @ 2020-10-23 18:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: Elijah Newren via GitGitGadget, Git Mailing List, Jonathan Nieder
One more comment...
On Fri, Oct 23, 2020 at 10:25 AM Elijah Newren <newren@gmail.com> wrote:
>
> On Fri, Oct 23, 2020 at 9:48 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> > And the users are just too pleasant to see, with full of "failure
> > sucess", which is the second best outcome we want to see ;-)
> >
> > > +test_expect_merge_algorithm failure success 'check symlink mo...
> > > +test_expect_merge_algorithm failure success 'check symlink ad...
> > > +test_expect_merge_algorithm failure success 'check submodule ...
> > > +test_expect_merge_algorithm failure success 'check submodule ...
> > > +test_expect_merge_algorithm failure success 'check conflictin...
> > > +test_expect_merge_algorithm failure success 'rad-check: renam...
> > > +test_expect_merge_algorithm failure success 'rrdd-check: rena...
> > > +test_expect_merge_algorithm failure success 'mod6-check: chai...
> > > +test_expect_merge_algorithm failure success '6b1: Same rename...
> > > +test_expect_merge_algorithm failure success '6b2: Same rename...
> > > +test_expect_merge_algorithm failure success '10e: Does git co...
> > > +test_expect_merge_algorithm failure success '12b1: Moving two...
> > > +test_expect_merge_algorithm failure success '12c1: Moving one...
> > > +test_expect_merge_algorithm failure success '12f: Trivial dir...
> > > +test_expect_merge_algorithm failure success '4a: Change on A,...
> > > +test_expect_merge_algorithm failure success 'merge-recursive ...
>
> :-)
Actually, there are another 12 submodule-related tests that pass under
ort but not under recursive, spread across t3512, t3513, t5572, t6437,
and t6438. I didn't (yet) apply the same change there, so they all
show up as "TODO passed" if you check out the 'ort' branch of my repo
and run the tests with GIT_TEST_MERGE_ALGORITHM=ort. I delayed
marking them as expecting success under ort because I suspect that
nearby tests should also pass but are just coded too stringently.
(For example, perhaps they expected a directory/submodule conflict to
result in all files within the conflicting directory to be renamed out
of the way instead of expecting the submodule to be moved aside --
moving the submodule aside results in massively less rename handling
pressure and is an easier way to make sure that the files under the
conflicting directory aren't written into and over entries within the
submodule.)
I was hoping to get a submodule expert to look over those tests and
provide some opinions...
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-23 16:01 ` [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive Elijah Newren via GitGitGadget
2020-10-23 16:48 ` Junio C Hamano
@ 2020-10-24 10:49 ` Đoàn Trần Công Danh
2020-10-24 16:53 ` Elijah Newren
1 sibling, 1 reply; 31+ messages in thread
From: Đoàn Trần Công Danh @ 2020-10-24 10:49 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren
On 2020-10-23 16:01:16+0000, Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote:
> +test_expect_merge_algorithm () {
> + status_for_recursive=$1
> + shift
> + status_for_ort=$1
> + shift
> +
> + if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> + then
> + test_expect_${status_for_ort} "$@"
> + else
> + test_expect_${status_for_recursive} "$@"
> -test_expect_failure 'check symlink modify/modify' '
> +test_expect_merge_algorithm failure success 'check symlink modify/modify' '
I find this series of "failure success" hard to decode without
understanding what it would be, then I need to keep rememberring which
status is corresponding with with algorithm.
Perhaps this patch is a bit easier to read. This is largely based on
your patch. (I haven't read other patches, yet).
What do you think?
-------------8<------------
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
<congdanhqx@gmail.com>
Date: Sat, 24 Oct 2020 17:41:02 +0700
Subject: [PATCH] t/: new helper for testing merge that allow failure for some
algorithm
There are a number of tests that the "recursive" backend does not handle
correctly but which the redesign in "ort" will.
Add a new helper in lib-merge.sh for selecting a different test expectation
based on the setting of GIT_TEST_MERGE_ALGORITHM, and use it in various
testcases to document that we expect them to be success by default
but failure with certain algorithm.
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
t/lib-merge.sh | 19 +++++++++++++++++++
t/t6416-recursive-corner-cases.sh | 11 ++++++-----
t/t6422-merge-rename-corner-cases.sh | 7 ++++---
t/t6423-merge-rename-directories.sh | 13 +++++++------
t/t6426-merge-skip-unneeded-updates.sh | 3 ++-
t/t6430-merge-recursive.sh | 3 ++-
6 files changed, 40 insertions(+), 16 deletions(-)
create mode 100644 t/lib-merge.sh
diff --git a/t/lib-merge.sh b/t/lib-merge.sh
new file mode 100644
index 0000000000..efd8b9615c
--- /dev/null
+++ b/t/lib-merge.sh
@@ -0,0 +1,19 @@
+# Helper functions used by merge tests.
+
+test_expect_merge_success() {
+ exception="$1"
+ : "${GIT_TEST_MERGE_ALGORITHM:=recursive}"
+ case ",$exception," in
+ *,$GIT_TEST_MERGE_ALGORITHM=failure,*)
+ shift
+ test_expect_failure "$@" ;;
+ *,$GIT_TEST_MERGE_ALGORITHM=*)
+ BUG "exception must be failure only" ;;
+ *=failure,)
+ shift
+ test_expect_success "$@" ;;
+ *)
+ # No exception
+ test_expect_success "$@" ;;
+ esac
+}
diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
index fd98989b14..d13c1afb4a 100755
--- a/t/t6416-recursive-corner-cases.sh
+++ b/t/t6416-recursive-corner-cases.sh
@@ -3,6 +3,7 @@
test_description='recursive merge corner cases involving criss-cross merges'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
#
# L1 L2
@@ -1069,7 +1070,7 @@ test_expect_success 'setup symlink modify/modify' '
)
'
-test_expect_failure 'check symlink modify/modify' '
+test_expect_merge_success recursive=failure 'check symlink modify/modify' '
(
cd symlink-modify-modify &&
@@ -1135,7 +1136,7 @@ test_expect_success 'setup symlink add/add' '
)
'
-test_expect_failure 'check symlink add/add' '
+test_expect_merge_success recursive=failure 'check symlink add/add' '
(
cd symlink-add-add &&
@@ -1223,7 +1224,7 @@ test_expect_success 'setup submodule modify/modify' '
)
'
-test_expect_failure 'check submodule modify/modify' '
+test_expect_merge_success recursive=failure 'check submodule modify/modify' '
(
cd submodule-modify-modify &&
@@ -1311,7 +1312,7 @@ test_expect_success 'setup submodule add/add' '
)
'
-test_expect_failure 'check submodule add/add' '
+test_expect_merge_success recursive=failure 'check submodule add/add' '
(
cd submodule-add-add &&
@@ -1386,7 +1387,7 @@ test_expect_success 'setup conflicting entry types (submodule vs symlink)' '
)
'
-test_expect_failure 'check conflicting entry types (submodule vs symlink)' '
+test_expect_merge_success recursive=failure 'check conflicting entry types (submodule vs symlink)' '
(
cd submodule-symlink-add-add &&
diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh
index 3375eaf4e7..0c8eb4df8a 100755
--- a/t/t6422-merge-rename-corner-cases.sh
+++ b/t/t6422-merge-rename-corner-cases.sh
@@ -4,6 +4,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses"
# t6036 has corner cases that involve both criss-cross merges and renames
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
test_setup_rename_delete_untracked () {
test_create_repo rename-delete-untracked &&
@@ -878,7 +879,7 @@ test_setup_rad () {
)
}
-test_expect_failure 'rad-check: rename/add/delete conflict' '
+test_expect_merge_success recursive=failure 'rad-check: rename/add/delete conflict' '
test_setup_rad &&
(
cd rad &&
@@ -951,7 +952,7 @@ test_setup_rrdd () {
)
}
-test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
+test_expect_merge_success recursive=failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
test_setup_rrdd &&
(
cd rrdd &&
@@ -1040,7 +1041,7 @@ test_setup_mod6 () {
)
}
-test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
+test_expect_merge_success recursive=failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
test_setup_mod6 &&
(
cd mod6 &&
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 06b46af765..249fbb6853 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -26,6 +26,7 @@ test_description="recursive merge with directory renames"
# files that might be renamed into each other's paths.)
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
###########################################################################
@@ -1339,7 +1340,7 @@ test_setup_6b1 () {
)
}
-test_expect_failure '6b1: Same renames done on both sides, plus another rename' '
+test_expect_merge_success recursive=failure '6b1: Same renames done on both sides, plus another rename' '
test_setup_6b1 &&
(
cd 6b1 &&
@@ -1412,7 +1413,7 @@ test_setup_6b2 () {
)
}
-test_expect_failure '6b2: Same rename done on both sides' '
+test_expect_merge_success recursive=failure '6b2: Same rename done on both sides' '
test_setup_6b2 &&
(
cd 6b2 &&
@@ -3471,7 +3472,7 @@ test_setup_10e () {
)
}
-test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' '
+test_expect_merge_success recursive=failure '10e: Does git complain about untracked file that is not really in the way?' '
test_setup_10e &&
(
cd 10e &&
@@ -4104,7 +4105,7 @@ test_setup_12b1 () {
)
}
-test_expect_failure '12b1: Moving two directory hierarchies into each other' '
+test_expect_merge_success recursive=failure '12b1: Moving two directory hierarchies into each other' '
test_setup_12b1 &&
(
cd 12b1 &&
@@ -4272,7 +4273,7 @@ test_setup_12c1 () {
)
}
-test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' '
+test_expect_merge_success recursive=failure '12c1: Moving one directory hierarchy into another w/ content merge' '
test_setup_12c1 &&
(
cd 12c1 &&
@@ -4632,7 +4633,7 @@ test_setup_12f () {
)
}
-test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' '
+test_expect_merge_success recursive=failure '12f: Trivial directory resolve, caching, all kinds of fun' '
test_setup_12f &&
(
cd 12f &&
diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh
index 699813671c..8510d4da8b 100755
--- a/t/t6426-merge-skip-unneeded-updates.sh
+++ b/t/t6426-merge-skip-unneeded-updates.sh
@@ -23,6 +23,7 @@ test_description="merge cases"
# files that might be renamed into each other's paths.)
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
###########################################################################
@@ -666,7 +667,7 @@ test_setup_4a () {
# correct requires doing the merge in-memory first, then realizing that no
# updates to the file are necessary, and thus that we can just leave the path
# alone.
-test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' '
+test_expect_merge_success recursive=failure '4a: Change on A, change on B subset of A, dirty mods present' '
test_setup_4a &&
(
cd 4a &&
diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh
index a328260d42..4795a7abd0 100755
--- a/t/t6430-merge-recursive.sh
+++ b/t/t6430-merge-recursive.sh
@@ -3,6 +3,7 @@
test_description='merge-recursive backend test'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
test_expect_success 'setup 1' '
@@ -641,7 +642,7 @@ test_expect_success 'merge-recursive copy vs. rename' '
test_cmp expected actual
'
-test_expect_failure 'merge-recursive rename vs. rename/symlink' '
+test_expect_merge_success recursive=failure 'merge-recursive rename vs. rename/symlink' '
git checkout -f rename &&
git merge rename-ln &&
--
2.29.0.rc1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-24 10:49 ` Đoàn Trần Công Danh
@ 2020-10-24 16:53 ` Elijah Newren
2020-10-25 13:49 ` Đoàn Trần Công Danh
0 siblings, 1 reply; 31+ messages in thread
From: Elijah Newren @ 2020-10-24 16:53 UTC (permalink / raw)
To: Đoàn Trần Công Danh
Cc: Elijah Newren via GitGitGadget, Git Mailing List
On Sat, Oct 24, 2020 at 3:49 AM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:
>
> On 2020-10-23 16:01:16+0000, Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote:
> > +test_expect_merge_algorithm () {
> > + status_for_recursive=$1
> > + shift
> > + status_for_ort=$1
> > + shift
> > +
> > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> > + then
> > + test_expect_${status_for_ort} "$@"
> > + else
> > + test_expect_${status_for_recursive} "$@"
> > -test_expect_failure 'check symlink modify/modify' '
> > +test_expect_merge_algorithm failure success 'check symlink modify/modify' '
>
> I find this series of "failure success" hard to decode without
> understanding what it would be, then I need to keep rememberring which
> status is corresponding with with algorithm.
>
> Perhaps this patch is a bit easier to read. This is largely based on
> your patch. (I haven't read other patches, yet).
>
> What do you think?
It is easier to read and I think something along these lines would
make a lot of sense if this weren't a transient change (the idea is to
eventually drop the recursive backend in favor of ort, and then these
can all switch to just using test_expect_success). Maybe it still
makes sense to make further changes here anyway, but if we do go this
route, there are 1-2 things we can/should change:
First, while a lot of my contributions aren't that important, and the
new test_expect_* function certainly falls in that category, one of
the driving goals behind a new merge algorithm was fixing up edge and
corner cases that were just too problematic in the recursive backend.
Thus, the patch where I get to flip the test expectation is one that I
care about more than most out of the (I'm guessing on this number)
100+ patches that will be part of this new merge algorithm. Having
you take over ownership of that patch thus isn't right; we should
instead keep my original patch and apply your suggested changes on top
(or have a patch from you introducing a new function first, and then
have a patch from me using it to flip test expectations on top).
Second, I think that lines like
test_expect_merge_success recursive=failure ...
read like a contradiction and are also confusing. I think it'd be
better if it read something like
test_expect_merge recursive=failure ort=success ...
or something along those lines.
> -------------8<------------
> From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
> <congdanhqx@gmail.com>
> Date: Sat, 24 Oct 2020 17:41:02 +0700
> Subject: [PATCH] t/: new helper for testing merge that allow failure for some
> algorithm
>
> There are a number of tests that the "recursive" backend does not handle
> correctly but which the redesign in "ort" will.
>
> Add a new helper in lib-merge.sh for selecting a different test expectation
> based on the setting of GIT_TEST_MERGE_ALGORITHM, and use it in various
> testcases to document that we expect them to be success by default
> but failure with certain algorithm.
>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
> t/lib-merge.sh | 19 +++++++++++++++++++
> t/t6416-recursive-corner-cases.sh | 11 ++++++-----
> t/t6422-merge-rename-corner-cases.sh | 7 ++++---
> t/t6423-merge-rename-directories.sh | 13 +++++++------
> t/t6426-merge-skip-unneeded-updates.sh | 3 ++-
> t/t6430-merge-recursive.sh | 3 ++-
> 6 files changed, 40 insertions(+), 16 deletions(-)
> create mode 100644 t/lib-merge.sh
>
> diff --git a/t/lib-merge.sh b/t/lib-merge.sh
> new file mode 100644
> index 0000000000..efd8b9615c
> --- /dev/null
> +++ b/t/lib-merge.sh
> @@ -0,0 +1,19 @@
> +# Helper functions used by merge tests.
> +
> +test_expect_merge_success() {
> + exception="$1"
> + : "${GIT_TEST_MERGE_ALGORITHM:=recursive}"
> + case ",$exception," in
> + *,$GIT_TEST_MERGE_ALGORITHM=failure,*)
> + shift
> + test_expect_failure "$@" ;;
> + *,$GIT_TEST_MERGE_ALGORITHM=*)
> + BUG "exception must be failure only" ;;
> + *=failure,)
> + shift
> + test_expect_success "$@" ;;
> + *)
> + # No exception
> + test_expect_success "$@" ;;
> + esac
> +}
> diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
> index fd98989b14..d13c1afb4a 100755
> --- a/t/t6416-recursive-corner-cases.sh
> +++ b/t/t6416-recursive-corner-cases.sh
> @@ -3,6 +3,7 @@
> test_description='recursive merge corner cases involving criss-cross merges'
>
> . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-merge.sh
>
> #
> # L1 L2
> @@ -1069,7 +1070,7 @@ test_expect_success 'setup symlink modify/modify' '
> )
> '
>
> -test_expect_failure 'check symlink modify/modify' '
> +test_expect_merge_success recursive=failure 'check symlink modify/modify' '
> (
> cd symlink-modify-modify &&
>
> @@ -1135,7 +1136,7 @@ test_expect_success 'setup symlink add/add' '
> )
> '
>
> -test_expect_failure 'check symlink add/add' '
> +test_expect_merge_success recursive=failure 'check symlink add/add' '
> (
> cd symlink-add-add &&
>
> @@ -1223,7 +1224,7 @@ test_expect_success 'setup submodule modify/modify' '
> )
> '
>
> -test_expect_failure 'check submodule modify/modify' '
> +test_expect_merge_success recursive=failure 'check submodule modify/modify' '
> (
> cd submodule-modify-modify &&
>
> @@ -1311,7 +1312,7 @@ test_expect_success 'setup submodule add/add' '
> )
> '
>
> -test_expect_failure 'check submodule add/add' '
> +test_expect_merge_success recursive=failure 'check submodule add/add' '
> (
> cd submodule-add-add &&
>
> @@ -1386,7 +1387,7 @@ test_expect_success 'setup conflicting entry types (submodule vs symlink)' '
> )
> '
>
> -test_expect_failure 'check conflicting entry types (submodule vs symlink)' '
> +test_expect_merge_success recursive=failure 'check conflicting entry types (submodule vs symlink)' '
> (
> cd submodule-symlink-add-add &&
>
> diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh
> index 3375eaf4e7..0c8eb4df8a 100755
> --- a/t/t6422-merge-rename-corner-cases.sh
> +++ b/t/t6422-merge-rename-corner-cases.sh
> @@ -4,6 +4,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses"
> # t6036 has corner cases that involve both criss-cross merges and renames
>
> . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-merge.sh
>
> test_setup_rename_delete_untracked () {
> test_create_repo rename-delete-untracked &&
> @@ -878,7 +879,7 @@ test_setup_rad () {
> )
> }
>
> -test_expect_failure 'rad-check: rename/add/delete conflict' '
> +test_expect_merge_success recursive=failure 'rad-check: rename/add/delete conflict' '
> test_setup_rad &&
> (
> cd rad &&
> @@ -951,7 +952,7 @@ test_setup_rrdd () {
> )
> }
>
> -test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
> +test_expect_merge_success recursive=failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
> test_setup_rrdd &&
> (
> cd rrdd &&
> @@ -1040,7 +1041,7 @@ test_setup_mod6 () {
> )
> }
>
> -test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
> +test_expect_merge_success recursive=failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
> test_setup_mod6 &&
> (
> cd mod6 &&
> diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
> index 06b46af765..249fbb6853 100755
> --- a/t/t6423-merge-rename-directories.sh
> +++ b/t/t6423-merge-rename-directories.sh
> @@ -26,6 +26,7 @@ test_description="recursive merge with directory renames"
> # files that might be renamed into each other's paths.)
>
> . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-merge.sh
>
>
> ###########################################################################
> @@ -1339,7 +1340,7 @@ test_setup_6b1 () {
> )
> }
>
> -test_expect_failure '6b1: Same renames done on both sides, plus another rename' '
> +test_expect_merge_success recursive=failure '6b1: Same renames done on both sides, plus another rename' '
> test_setup_6b1 &&
> (
> cd 6b1 &&
> @@ -1412,7 +1413,7 @@ test_setup_6b2 () {
> )
> }
>
> -test_expect_failure '6b2: Same rename done on both sides' '
> +test_expect_merge_success recursive=failure '6b2: Same rename done on both sides' '
> test_setup_6b2 &&
> (
> cd 6b2 &&
> @@ -3471,7 +3472,7 @@ test_setup_10e () {
> )
> }
>
> -test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' '
> +test_expect_merge_success recursive=failure '10e: Does git complain about untracked file that is not really in the way?' '
> test_setup_10e &&
> (
> cd 10e &&
> @@ -4104,7 +4105,7 @@ test_setup_12b1 () {
> )
> }
>
> -test_expect_failure '12b1: Moving two directory hierarchies into each other' '
> +test_expect_merge_success recursive=failure '12b1: Moving two directory hierarchies into each other' '
> test_setup_12b1 &&
> (
> cd 12b1 &&
> @@ -4272,7 +4273,7 @@ test_setup_12c1 () {
> )
> }
>
> -test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' '
> +test_expect_merge_success recursive=failure '12c1: Moving one directory hierarchy into another w/ content merge' '
> test_setup_12c1 &&
> (
> cd 12c1 &&
> @@ -4632,7 +4633,7 @@ test_setup_12f () {
> )
> }
>
> -test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' '
> +test_expect_merge_success recursive=failure '12f: Trivial directory resolve, caching, all kinds of fun' '
> test_setup_12f &&
> (
> cd 12f &&
> diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh
> index 699813671c..8510d4da8b 100755
> --- a/t/t6426-merge-skip-unneeded-updates.sh
> +++ b/t/t6426-merge-skip-unneeded-updates.sh
> @@ -23,6 +23,7 @@ test_description="merge cases"
> # files that might be renamed into each other's paths.)
>
> . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-merge.sh
>
>
> ###########################################################################
> @@ -666,7 +667,7 @@ test_setup_4a () {
> # correct requires doing the merge in-memory first, then realizing that no
> # updates to the file are necessary, and thus that we can just leave the path
> # alone.
> -test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' '
> +test_expect_merge_success recursive=failure '4a: Change on A, change on B subset of A, dirty mods present' '
> test_setup_4a &&
> (
> cd 4a &&
> diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh
> index a328260d42..4795a7abd0 100755
> --- a/t/t6430-merge-recursive.sh
> +++ b/t/t6430-merge-recursive.sh
> @@ -3,6 +3,7 @@
> test_description='merge-recursive backend test'
>
> . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-merge.sh
>
> test_expect_success 'setup 1' '
>
> @@ -641,7 +642,7 @@ test_expect_success 'merge-recursive copy vs. rename' '
> test_cmp expected actual
> '
>
> -test_expect_failure 'merge-recursive rename vs. rename/symlink' '
> +test_expect_merge_success recursive=failure 'merge-recursive rename vs. rename/symlink' '
>
> git checkout -f rename &&
> git merge rename-ln &&
> --
> 2.29.0.rc1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-24 16:53 ` Elijah Newren
@ 2020-10-25 13:49 ` Đoàn Trần Công Danh
2020-10-26 14:56 ` Elijah Newren
0 siblings, 1 reply; 31+ messages in thread
From: Đoàn Trần Công Danh @ 2020-10-25 13:49 UTC (permalink / raw)
To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, Git Mailing List
On 2020-10-24 09:53:18-0700, Elijah Newren <newren@gmail.com> wrote:
> On Sat, Oct 24, 2020 at 3:49 AM Đoàn Trần Công Danh
> <congdanhqx@gmail.com> wrote:
> >
> > On 2020-10-23 16:01:16+0000, Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote:
> > > +test_expect_merge_algorithm () {
> > > + status_for_recursive=$1
> > > + shift
> > > + status_for_ort=$1
> > > + shift
> > > +
> > > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> > > + then
> > > + test_expect_${status_for_ort} "$@"
> > > + else
> > > + test_expect_${status_for_recursive} "$@"
> > > -test_expect_failure 'check symlink modify/modify' '
> > > +test_expect_merge_algorithm failure success 'check symlink modify/modify' '
> >
> > I find this series of "failure success" hard to decode without
> > understanding what it would be, then I need to keep rememberring which
> > status is corresponding with with algorithm.
> >
> > Perhaps this patch is a bit easier to read. This is largely based on
> > your patch. (I haven't read other patches, yet).
> >
> > What do you think?
>
> It is easier to read and I think something along these lines would
> make a lot of sense if this weren't a transient change (the idea is to
> eventually drop the recursive backend in favor of ort, and then these
> can all switch to just using test_expect_success). Maybe it still
> makes sense to make further changes here anyway, but if we do go this
> route, there are 1-2 things we can/should change:
>
> First, while a lot of my contributions aren't that important, and the
Mine aren't that important, either
> new test_expect_* function certainly falls in that category, one of
> the driving goals behind a new merge algorithm was fixing up edge and
> corner cases that were just too problematic in the recursive backend.
> Thus, the patch where I get to flip the test expectation is one that I
> care about more than most out of the (I'm guessing on this number)
Make sense.
> 100+ patches that will be part of this new merge algorithm. Having
> you take over ownership of that patch thus isn't right; we should
> instead keep my original patch and apply your suggested changes on top
> (or have a patch from you introducing a new function first, and then
> have a patch from me using it to flip test expectations on top).
You can take back the ownership, the patch was based on yours, anyway.
I wrote like that since I need to rewrite part of the message to match
with my changes ;)
No need to generate extra noise of additional patch.
> Second, I think that lines like
> test_expect_merge_success recursive=failure ...
> read like a contradiction and are also confusing. I think it'd be
> better if it read something like
> test_expect_merge recursive=failure ort=success ...
> or something along those lines.
When I wrote the patch, I was expecting something like
test_expect_merge_success recursive=failure,other=failure ...
in order to merge all algorithm into single parameters.
How about something like:
test_expect_merge_success exception=recursive,other ...
Not that we have "other" algorithm to begin with.
Thanks,
-- Danh
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-25 13:49 ` Đoàn Trần Công Danh
@ 2020-10-26 14:56 ` Elijah Newren
2020-10-26 17:43 ` Junio C Hamano
0 siblings, 1 reply; 31+ messages in thread
From: Elijah Newren @ 2020-10-26 14:56 UTC (permalink / raw)
To: Đoàn Trần Công Danh
Cc: Elijah Newren via GitGitGadget, Git Mailing List
On Sun, Oct 25, 2020 at 6:49 AM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:
>
> On 2020-10-24 09:53:18-0700, Elijah Newren <newren@gmail.com> wrote:
> > On Sat, Oct 24, 2020 at 3:49 AM Đoàn Trần Công Danh
> > <congdanhqx@gmail.com> wrote:
> > >
> > > On 2020-10-23 16:01:16+0000, Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote:
> > > > +test_expect_merge_algorithm () {
> > > > + status_for_recursive=$1
> > > > + shift
> > > > + status_for_ort=$1
> > > > + shift
> > > > +
> > > > + if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> > > > + then
> > > > + test_expect_${status_for_ort} "$@"
> > > > + else
> > > > + test_expect_${status_for_recursive} "$@"
> > > > -test_expect_failure 'check symlink modify/modify' '
> > > > +test_expect_merge_algorithm failure success 'check symlink modify/modify' '
> > >
> > > I find this series of "failure success" hard to decode without
> > > understanding what it would be, then I need to keep rememberring which
> > > status is corresponding with with algorithm.
> > >
> > > Perhaps this patch is a bit easier to read. This is largely based on
> > > your patch. (I haven't read other patches, yet).
> > >
> > > What do you think?
> >
> > It is easier to read and I think something along these lines would
> > make a lot of sense if this weren't a transient change (the idea is to
> > eventually drop the recursive backend in favor of ort, and then these
> > can all switch to just using test_expect_success). Maybe it still
> > makes sense to make further changes here anyway, but if we do go this
> > route, there are 1-2 things we can/should change:
> >
> > First, while a lot of my contributions aren't that important, and the
>
> Mine aren't that important, either
>
> > new test_expect_* function certainly falls in that category, one of
> > the driving goals behind a new merge algorithm was fixing up edge and
> > corner cases that were just too problematic in the recursive backend.
> > Thus, the patch where I get to flip the test expectation is one that I
> > care about more than most out of the (I'm guessing on this number)
>
> Make sense.
>
> > 100+ patches that will be part of this new merge algorithm. Having
> > you take over ownership of that patch thus isn't right; we should
> > instead keep my original patch and apply your suggested changes on top
> > (or have a patch from you introducing a new function first, and then
> > have a patch from me using it to flip test expectations on top).
>
> You can take back the ownership, the patch was based on yours, anyway.
>
> I wrote like that since I need to rewrite part of the message to match
> with my changes ;)
>
> No need to generate extra noise of additional patch.
Just to be clear, others have made suggestions like yours in the past
where they've taken over ownership of a patch in a series and I've
been totally fine with it. Your suggestion to do the same would have
been fine here, but I'm just kinda attached to being able to flip the
test expectation for these tests; I've been working towards it for a
_long_ time.
I think an extra patch, attributed to you, actually makes the most sense here.
> > Second, I think that lines like
> > test_expect_merge_success recursive=failure ...
> > read like a contradiction and are also confusing. I think it'd be
> > better if it read something like
> > test_expect_merge recursive=failure ort=success ...
> > or something along those lines.
>
> When I wrote the patch, I was expecting something like
>
> test_expect_merge_success recursive=failure,other=failure ...
>
> in order to merge all algorithm into single parameters.
>
> How about something like:
>
> test_expect_merge_success exception=recursive,other ...
>
> Not that we have "other" algorithm to begin with.
Sure, sounds great. I wouldn't spend any time trying to make it work
with a 3rd backend, though. The goal is to have two merge backends
only long enough for people to become comfortable with the new backend
and discover any unknown issues with it that we can fix, then we'll
rip it out the old "recursive" backend and we'll translate any
requests for "recursive" to mean "ort". We'll also rip the
test_expect_merge_success() function out since it'll be unneeded (so
efforts towards future proofing of that function will be wasted).
Then years will go by before another merge backend comes along, if one
ever does.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-26 14:56 ` Elijah Newren
@ 2020-10-26 17:43 ` Junio C Hamano
0 siblings, 0 replies; 31+ messages in thread
From: Junio C Hamano @ 2020-10-26 17:43 UTC (permalink / raw)
To: Elijah Newren
Cc: Đoàn Trần Công Danh,
Elijah Newren via GitGitGadget, Git Mailing List
Elijah Newren <newren@gmail.com> writes:
>> When I wrote the patch, I was expecting something like
>>
>> test_expect_merge_success recursive=failure,other=failure ...
>>
>> in order to merge all algorithm into single parameters.
>>
>> How about something like:
>>
>> test_expect_merge_success exception=recursive,other ...
>>
>> Not that we have "other" algorithm to begin with.
>
> Sure, sounds great. I wouldn't spend any time trying to make it work
> with a 3rd backend, though.
Yup. I'd appreciate if the lines become a bit shorter while at it,
though.
test_merge_both [<exception>] '<title>' '
<body>
'
that expects success unless otherwise told, and <exception> like
"failure=recursive" can be used to tell us to expect differently,
would work well?
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/9] merge tests: expect improved directory/file conflict handling in ort
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
2020-10-23 16:01 ` [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-23 17:40 ` Elijah Newren
2020-10-23 16:01 ` [PATCH 3/9] t6416: correct expectation for rename/rename(1to2) + directory/file Elijah Newren via GitGitGadget
` (7 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
merge-recursive.c is built on the idea of running unpack_trees() and
then "doing minor touch-ups" to get the result. Unfortunately,
unpack_trees() was run in an update-as-it-goes mode, leading
merge-recursive.c to follow suit and end up with an immediate evaluation
and fix-it-up-as-you-go design. Some things like directory/file
conflicts are not well representable in the index data structure, and
required special extra code to handle. But then when it was discovered
that rename/delete conflicts could also be involved in directory/file
conflicts, the special directory/file conflict handling code had to be
copied to the rename/delete codepath. ...and then it had to be copied
for modify/delete, and for rename/rename(1to2) conflicts, ...and yet it
still missed some. Further, when it was discovered that there were also
file/submodule conflicts and submodule/directory conflicts, we needed to
copy the special submodule handling code to all the special cases
throughout the codebase.
And then it was discovered that our handling of directory/file conflicts
was suboptimal because it would create untracked files to store the
contents of the conflicting file, which would not be cleaned up if
someone were to run a 'git merge --abort' or 'git rebase --abort'. It
was also difficult or scary to try to add or remove the index entries
corresponding to these files given the directory/file conflict in the
index. But changing merge-recursive.c to handle these correctly was a
royal pain because there were so many sites in the code with similar but
not identical code for handling directory/file/submodule conflicts that
would all need to be updated.
I have worked hard to push all directory/file/submodule conflict
handling in merge-ort through a single codepath, and avoid creating
untracked files for storing tracked content (it does record things at
alternate paths, but makes sure they have higher-order stages in the
index).
Since updating merge-recursive is too much work and we don't want to
destabilize it, instead update the testsuite to have different
expectations for relevant directory/file/submodule conflict tests.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6400-merge-df.sh | 14 +-
t/t6402-merge-rename.sh | 108 ++++++++++++----
t/t6416-recursive-corner-cases.sh | 185 +++++++++++++++++++--------
t/t6422-merge-rename-corner-cases.sh | 30 +++--
t/t6423-merge-rename-directories.sh | 53 +++++---
t/t7610-mergetool.sh | 32 ++++-
6 files changed, 314 insertions(+), 108 deletions(-)
diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh
index f1b84617af..9da0838216 100755
--- a/t/t6400-merge-df.sh
+++ b/t/t6400-merge-df.sh
@@ -81,7 +81,12 @@ test_expect_success 'modify/delete + directory/file conflict' '
test 5 -eq $(git ls-files -s | wc -l) &&
test 4 -eq $(git ls-files -u | wc -l) &&
- test 1 -eq $(git ls-files -o | wc -l) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 0 -eq $(git ls-files -o | wc -l)
+ else
+ test 1 -eq $(git ls-files -o | wc -l)
+ fi &&
test_path_is_file letters/file &&
test_path_is_file letters.txt &&
@@ -97,7 +102,12 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
test 5 -eq $(git ls-files -s | wc -l) &&
test 4 -eq $(git ls-files -u | wc -l) &&
- test 1 -eq $(git ls-files -o | wc -l) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 0 -eq $(git ls-files -o | wc -l)
+ else
+ test 1 -eq $(git ls-files -o | wc -l)
+ fi &&
test_path_is_file letters/file &&
test_path_is_file letters.txt &&
diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index bbbba3dcbf..47d4434d64 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -397,7 +397,12 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t
test_must_fail git merge --strategy=recursive dir-in-way &&
test 5 -eq "$(git ls-files -u | wc -l)" &&
- test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
+ else
+ test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+ fi &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
test_must_fail git diff --quiet &&
@@ -415,7 +420,12 @@ test_expect_success 'Same as previous, but merged other way' '
test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
test 5 -eq "$(git ls-files -u | wc -l)" &&
- test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
+ else
+ test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+ fi &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
test_must_fail git diff --quiet &&
@@ -471,7 +481,12 @@ test_expect_success 'both rename source and destination involved in D/F conflict
git checkout -q rename-dest^0 &&
test_must_fail git merge --strategy=recursive source-conflict &&
- test 1 -eq "$(git ls-files -u | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 2 -eq "$(git ls-files -u | wc -l)"
+ else
+ test 1 -eq "$(git ls-files -u | wc -l)"
+ fi &&
test_must_fail git diff --quiet &&
@@ -505,34 +520,63 @@ test_expect_success 'setup pair rename to parent of other (D/F conflicts)' '
git commit -m "Rename one/file -> two"
'
-test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
- git checkout -q rename-one^0 &&
- mkdir one &&
- test_must_fail git merge --strategy=recursive rename-two &&
+if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+then
+ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
+ git checkout -q rename-one^0 &&
+ mkdir one &&
+ test_must_fail git merge --strategy=recursive rename-two &&
- test 2 -eq "$(git ls-files -u | wc -l)" &&
- test 1 -eq "$(git ls-files -u one | wc -l)" &&
- test 1 -eq "$(git ls-files -u two | wc -l)" &&
+ test 4 -eq "$(git ls-files -u | wc -l)" &&
+ test 2 -eq "$(git ls-files -u one | wc -l)" &&
+ test 2 -eq "$(git ls-files -u two | wc -l)" &&
- test_must_fail git diff --quiet &&
+ test_must_fail git diff --quiet &&
- test 4 -eq $(find . | grep -v .git | wc -l) &&
+ test 3 -eq $(find . | grep -v .git | wc -l) &&
- test_path_is_dir one &&
- test_path_is_file one~rename-two &&
- test_path_is_file two &&
- test "other" = $(cat one~rename-two) &&
- test "stuff" = $(cat two)
-'
+ test_path_is_file one &&
+ test_path_is_file two &&
+ test "other" = $(cat one) &&
+ test "stuff" = $(cat two)
+ '
+else
+ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
+ git checkout -q rename-one^0 &&
+ mkdir one &&
+ test_must_fail git merge --strategy=recursive rename-two &&
+
+ test 2 -eq "$(git ls-files -u | wc -l)" &&
+ test 1 -eq "$(git ls-files -u one | wc -l)" &&
+ test 1 -eq "$(git ls-files -u two | wc -l)" &&
+
+ test_must_fail git diff --quiet &&
+
+ test 4 -eq $(find . | grep -v .git | wc -l) &&
+
+ test_path_is_dir one &&
+ test_path_is_file one~rename-two &&
+ test_path_is_file two &&
+ test "other" = $(cat one~rename-two) &&
+ test "stuff" = $(cat two)
+ '
+fi
test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean start' '
git reset --hard &&
git clean -fdqx &&
test_must_fail git merge --strategy=recursive rename-two &&
- test 2 -eq "$(git ls-files -u | wc -l)" &&
- test 1 -eq "$(git ls-files -u one | wc -l)" &&
- test 1 -eq "$(git ls-files -u two | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 4 -eq "$(git ls-files -u | wc -l)" &&
+ test 2 -eq "$(git ls-files -u one | wc -l)" &&
+ test 2 -eq "$(git ls-files -u two | wc -l)"
+ else
+ test 2 -eq "$(git ls-files -u | wc -l)" &&
+ test 1 -eq "$(git ls-files -u one | wc -l)" &&
+ test 1 -eq "$(git ls-files -u two | wc -l)"
+ fi &&
test_must_fail git diff --quiet &&
@@ -572,12 +616,22 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
git checkout -q first-rename^0 &&
test_must_fail git merge --strategy=recursive second-rename &&
- test 5 -eq "$(git ls-files -s | wc -l)" &&
- test 3 -eq "$(git ls-files -u | wc -l)" &&
- test 1 -eq "$(git ls-files -u one | wc -l)" &&
- test 1 -eq "$(git ls-files -u two | wc -l)" &&
- test 1 -eq "$(git ls-files -u original | wc -l)" &&
- test 2 -eq "$(git ls-files -o | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 5 -eq "$(git ls-files -s | wc -l)" &&
+ test 3 -eq "$(git ls-files -u | wc -l)" &&
+ test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" &&
+ test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
+ test 1 -eq "$(git ls-files -u original | wc -l)" &&
+ test 0 -eq "$(git ls-files -o | wc -l)"
+ else
+ test 5 -eq "$(git ls-files -s | wc -l)" &&
+ test 3 -eq "$(git ls-files -u | wc -l)" &&
+ test 1 -eq "$(git ls-files -u one | wc -l)" &&
+ test 1 -eq "$(git ls-files -u two | wc -l)" &&
+ test 1 -eq "$(git ls-files -u original | wc -l)" &&
+ test 2 -eq "$(git ls-files -o | wc -l)"
+ fi &&
test_path_is_file one/file &&
test_path_is_file two/file &&
diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
index 8b3a4fc843..0317e83970 100755
--- a/t/t6416-recursive-corner-cases.sh
+++ b/t/t6416-recursive-corner-cases.sh
@@ -538,9 +538,15 @@ test_expect_success 'setup differently handled merges of directory/file conflict
git checkout B^0 &&
test_must_fail git merge C^0 &&
- git clean -fd &&
- git rm -rf a/ &&
- git rm a &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rm -rf a/ &&
+ git rm a~HEAD
+ else
+ git clean -fd &&
+ git rm -rf a/ &&
+ git rm a
+ fi &&
git cat-file -p B:a >a2 &&
git add a2 &&
git commit -m D2 &&
@@ -559,7 +565,12 @@ test_expect_success 'setup differently handled merges of directory/file conflict
git checkout C^0 &&
test_must_fail git merge B^0 &&
- git clean -fd &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rm a~B^0
+ else
+ git clean -fd
+ fi &&
git rm -rf a/ &&
test_write_lines 1 2 3 4 5 6 7 8 >a &&
git add a &&
@@ -568,9 +579,15 @@ test_expect_success 'setup differently handled merges of directory/file conflict
git checkout C^0 &&
test_must_fail git merge B^0 &&
- git clean -fd &&
- git rm -rf a/ &&
- git rm a &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rm -rf a/ &&
+ git rm a~B^0
+ else
+ git clean -fd &&
+ git rm -rf a/ &&
+ git rm a
+ fi &&
test_write_lines 1 2 3 4 5 6 7 8 >a2 &&
git add a2 &&
git commit -m E4 &&
@@ -588,18 +605,34 @@ test_expect_success 'merge of D1 & E1 fails but has appropriate contents' '
test_must_fail git merge -s recursive E1^0 &&
- git ls-files -s >out &&
- test_line_count = 2 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 1 out &&
-
- git rev-parse >expect \
- A:ignore-me B:a &&
- git rev-parse >actual \
- :0:ignore-me :2:a &&
- test_cmp expect actual
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ A:ignore-me B:a D1:a &&
+ git rev-parse >actual \
+ :0:ignore-me :1:a :2:a &&
+ test_cmp expect actual
+ else
+ git ls-files -s >out &&
+ test_line_count = 2 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ A:ignore-me B:a &&
+ git rev-parse >actual \
+ :0:ignore-me :2:a &&
+ test_cmp expect actual
+ fi
)
'
@@ -613,18 +646,34 @@ test_expect_success 'merge of E1 & D1 fails but has appropriate contents' '
test_must_fail git merge -s recursive D1^0 &&
- git ls-files -s >out &&
- test_line_count = 2 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 1 out &&
-
- git rev-parse >expect \
- A:ignore-me B:a &&
- git rev-parse >actual \
- :0:ignore-me :3:a &&
- test_cmp expect actual
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ A:ignore-me B:a D1:a &&
+ git rev-parse >actual \
+ :0:ignore-me :1:a :3:a &&
+ test_cmp expect actual
+ else
+ git ls-files -s >out &&
+ test_line_count = 2 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ A:ignore-me B:a &&
+ git rev-parse >actual \
+ :0:ignore-me :3:a &&
+ test_cmp expect actual
+ fi
)
'
@@ -638,17 +687,32 @@ test_expect_success 'merge of D1 & E2 fails but has appropriate contents' '
test_must_fail git merge -s recursive E2^0 &&
- git ls-files -s >out &&
- test_line_count = 4 out &&
- git ls-files -u >out &&
- test_line_count = 3 out &&
- git ls-files -o >out &&
- test_line_count = 2 out &&
-
- git rev-parse >expect \
- B:a E2:a/file C:a/file A:ignore-me &&
- git rev-parse >actual \
- :2:a :3:a/file :1:a/file :0:ignore-me &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 5 out &&
+ git ls-files -u >out &&
+ test_line_count = 4 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ B:a D1:a E2:a/file C:a/file A:ignore-me &&
+ git rev-parse >actual \
+ :1:a~HEAD :2:a~HEAD :3:a/file :1:a/file :0:ignore-me
+ else
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 2 out &&
+
+ git rev-parse >expect \
+ B:a E2:a/file C:a/file A:ignore-me &&
+ git rev-parse >actual \
+ :2:a :3:a/file :1:a/file :0:ignore-me
+ fi &&
test_cmp expect actual &&
test_path_is_file a~HEAD
@@ -665,17 +729,32 @@ test_expect_success 'merge of E2 & D1 fails but has appropriate contents' '
test_must_fail git merge -s recursive D1^0 &&
- git ls-files -s >out &&
- test_line_count = 4 out &&
- git ls-files -u >out &&
- test_line_count = 3 out &&
- git ls-files -o >out &&
- test_line_count = 2 out &&
-
- git rev-parse >expect \
- B:a E2:a/file C:a/file A:ignore-me &&
- git rev-parse >actual \
- :3:a :2:a/file :1:a/file :0:ignore-me &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 5 out &&
+ git ls-files -u >out &&
+ test_line_count = 4 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ B:a D1:a E2:a/file C:a/file A:ignore-me &&
+ git rev-parse >actual \
+ :1:a~D1^0 :3:a~D1^0 :2:a/file :1:a/file :0:ignore-me
+ else
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 2 out &&
+
+ git rev-parse >expect \
+ B:a E2:a/file C:a/file A:ignore-me &&
+ git rev-parse >actual \
+ :3:a :2:a/file :1:a/file :0:ignore-me
+ fi &&
test_cmp expect actual &&
test_path_is_file a~D1^0
diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh
index 58729593ba..78bfaf17f0 100755
--- a/t/t6422-merge-rename-corner-cases.sh
+++ b/t/t6422-merge-rename-corner-cases.sh
@@ -313,15 +313,18 @@ test_expect_success 'rename/directory conflict + clean content merge' '
git ls-files -u >out &&
test_line_count = 1 out &&
git ls-files -o >out &&
- test_line_count = 2 out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_line_count = 1 out
+ else
+ test_line_count = 2 out
+ fi &&
echo 0 >expect &&
git cat-file -p base:file >>expect &&
echo 7 >>expect &&
test_cmp expect newfile~HEAD &&
- test $(git rev-parse :2:newfile) = $(git hash-object expect) &&
-
test_path_is_file newfile/realfile &&
test_path_is_file newfile~HEAD
)
@@ -344,7 +347,12 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
git ls-files -u >out &&
test_line_count = 3 out &&
git ls-files -o >out &&
- test_line_count = 2 out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_line_count = 1 out
+ else
+ test_line_count = 2 out
+ fi &&
git cat-file -p left-conflict:newfile >left &&
git cat-file -p base:file >base &&
@@ -356,10 +364,16 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
left base right &&
test_cmp left newfile~HEAD &&
- git rev-parse >expect \
- base:file left-conflict:newfile right:file &&
- git rev-parse >actual \
- :1:newfile :2:newfile :3:newfile &&
+ git rev-parse >expect \
+ base:file left-conflict:newfile right:file &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rev-parse >actual \
+ :1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD
+ else
+ git rev-parse >actual \
+ :1:newfile :2:newfile :3:newfile
+ fi &&
test_cmp expect actual &&
test_path_is_file newfile/realfile &&
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 807a424a52..5ea77564d7 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -1177,10 +1177,18 @@ test_expect_success '5d: Directory/file/file conflict due to directory rename' '
git ls-files -u >out &&
test_line_count = 1 out &&
git ls-files -o >out &&
- test_line_count = 2 out &&
-
- git rev-parse >actual \
- :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d :0:y/d/e &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d~HEAD :0:y/d/e
+ else
+ test_line_count = 2 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d :0:y/d/e
+ fi &&
git rev-parse >expect \
O:z/b O:z/c B:z/d B:z/f A:y/d B:y/d/e &&
test_cmp expect actual &&
@@ -2017,17 +2025,32 @@ test_expect_success '7e: transitive rename in rename/delete AND dirs in the way'
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out &&
test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out &&
- git ls-files -s >out &&
- test_line_count = 5 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 2 out &&
-
- git rev-parse >actual \
- :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d &&
- git rev-parse >expect \
- A:x/d/f A:y/d/g O:z/b O:z/c O:x/d &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:x/d/f :0:y/d/g :0:y/b :0:y/c :1:y/d~B^0 :3:y/d~B^0 &&
+ git rev-parse >expect \
+ A:x/d/f A:y/d/g O:z/b O:z/c O:x/d O:x/d
+ else
+ git ls-files -s >out &&
+ test_line_count = 5 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 2 out &&
+
+ git rev-parse >actual \
+ :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d &&
+ git rev-parse >expect \
+ A:x/d/f A:y/d/g O:z/b O:z/c O:x/d
+ fi &&
test_cmp expect actual &&
git hash-object y/d~B^0 >actual &&
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index ad288ddc69..70afdd06fa 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -532,7 +532,14 @@ test_expect_success 'file vs modified submodule' '
yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
yes "" | git mergetool both &&
yes "d" | git mergetool file11 file12 &&
- yes "l" | git mergetool submod &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ yes "c" | git mergetool submod~HEAD &&
+ git rm submod &&
+ git mv submod~HEAD submod
+ else
+ yes "l" | git mergetool submod
+ fi &&
git submodule update -N &&
echo "not a submodule" >expect &&
test_cmp expect submod &&
@@ -549,7 +556,15 @@ test_expect_success 'file vs modified submodule' '
yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
yes "" | git mergetool both &&
yes "d" | git mergetool file11 file12 &&
- yes "r" | git mergetool submod &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ mv submod submod.orig &&
+ git rm --cached submod &&
+ yes "c" | git mergetool submod~test19 &&
+ git mv submod~test19 submod
+ else
+ yes "r" | git mergetool submod
+ fi &&
test -d submod.orig &&
git submodule update -N &&
echo "not a submodule" >expect &&
@@ -567,6 +582,10 @@ test_expect_success 'file vs modified submodule' '
yes "" | git mergetool both &&
yes "d" | git mergetool file11 file12 &&
yes "l" | git mergetool submod &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ yes "d" | git mergetool submod~test19
+ fi &&
echo "master submodule" >expect &&
test_cmp expect submod/bar &&
git submodule update -N &&
@@ -664,7 +683,14 @@ test_expect_success 'directory vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
test ! -e submod.orig &&
- yes "r" | git mergetool submod &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ yes "r" | git mergetool submod~master &&
+ git mv submod submod.orig &&
+ git mv submod~master submod
+ else
+ yes "r" | git mergetool submod
+ fi &&
test -d submod.orig &&
echo "not a submodule" >expect &&
test_cmp expect submod.orig/file16 &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 2/9] merge tests: expect improved directory/file conflict handling in ort
2020-10-23 16:01 ` [PATCH 2/9] merge tests: expect improved directory/file conflict handling in ort Elijah Newren via GitGitGadget
@ 2020-10-23 17:40 ` Elijah Newren
0 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren @ 2020-10-23 17:40 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget; +Cc: Git Mailing List
On Fri, Oct 23, 2020 at 9:01 AM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Elijah Newren <newren@gmail.com>
>
> merge-recursive.c is built on the idea of running unpack_trees() and
> then "doing minor touch-ups" to get the result. Unfortunately,
> unpack_trees() was run in an update-as-it-goes mode, leading
> merge-recursive.c to follow suit and end up with an immediate evaluation
> and fix-it-up-as-you-go design. Some things like directory/file
> conflicts are not well representable in the index data structure, and
> required special extra code to handle. But then when it was discovered
> that rename/delete conflicts could also be involved in directory/file
> conflicts, the special directory/file conflict handling code had to be
> copied to the rename/delete codepath. ...and then it had to be copied
> for modify/delete, and for rename/rename(1to2) conflicts, ...and yet it
> still missed some. Further, when it was discovered that there were also
> file/submodule conflicts and submodule/directory conflicts, we needed to
> copy the special submodule handling code to all the special cases
> throughout the codebase.
>
> And then it was discovered that our handling of directory/file conflicts
> was suboptimal because it would create untracked files to store the
> contents of the conflicting file, which would not be cleaned up if
> someone were to run a 'git merge --abort' or 'git rebase --abort'. It
> was also difficult or scary to try to add or remove the index entries
> corresponding to these files given the directory/file conflict in the
> index. But changing merge-recursive.c to handle these correctly was a
> royal pain because there were so many sites in the code with similar but
> not identical code for handling directory/file/submodule conflicts that
> would all need to be updated.
>
> I have worked hard to push all directory/file/submodule conflict
> handling in merge-ort through a single codepath, and avoid creating
> untracked files for storing tracked content (it does record things at
> alternate paths, but makes sure they have higher-order stages in the
> index).
>
> Since updating merge-recursive is too much work and we don't want to
> destabilize it, instead update the testsuite to have different
> expectations for relevant directory/file/submodule conflict tests.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
Perhaps I should also mention that these changes to directory/file
conflict handling were discussed previously in the thread at
https://lore.kernel.org/git/xmqqbmabcuhf.fsf@gitster-ct.c.googlers.com/.
I just never got around to a complete implementation within
merge-recursive.c, but did implement it in merge-ort.c
I still haven't gotten around to fixing up git-mv and git-rm as
suggested by Junio in that thread; but at least I've finally gotten
the merge machinery side written...
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/9] t6416: correct expectation for rename/rename(1to2) + directory/file
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
2020-10-23 16:01 ` [PATCH 1/9] t/: new helper for tests that pass with ort but fail with recursive Elijah Newren via GitGitGadget
2020-10-23 16:01 ` [PATCH 2/9] merge tests: expect improved directory/file conflict handling in ort Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-23 16:01 ` [PATCH 4/9] t6404, t6423: expect improved rename/delete handling in ort backend Elijah Newren via GitGitGadget
` (6 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
When files are renamed and modified, we need to do three-way content
merges to get the appropriate content in the right location. When we
have a rename/rename(1to2) conflict (both sides rename the same file,
but differently), that merged content should be placed in each of the
two resulting files. merge-recursive handled that fine when that was
all that was involved, but when one or more of the two resulting files
were ALSO involved in a directory/file conflict, it failed to propagate
the merged content to that file. Unfortunately, the one test in t6416
that touched on this combination of cases had been coded to not expect
the merged contents to be present.
Fix the test to check for the right behavior, and record how the
different merge backends will be expected to handle it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6416-recursive-corner-cases.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
index 0317e83970..887c2195a9 100755
--- a/t/t6416-recursive-corner-cases.sh
+++ b/t/t6416-recursive-corner-cases.sh
@@ -786,7 +786,7 @@ test_expect_success 'merge of D1 & E3 succeeds' '
)
'
-test_expect_success 'merge of D1 & E4 notifies user a and a2 are related' '
+test_expect_merge_algorithm failure success 'merge of D1 & E4 puts merge of a and a2 in both a and a2' '
test_when_finished "git -C directory-file reset --hard" &&
test_when_finished "git -C directory-file clean -fdqx" &&
(
@@ -804,7 +804,7 @@ test_expect_success 'merge of D1 & E4 notifies user a and a2 are related' '
test_line_count = 1 out &&
git rev-parse >expect \
- A:ignore-me B:a D1:a E4:a2 &&
+ A:ignore-me B:a E4:a2 E4:a2 &&
git rev-parse >actual \
:0:ignore-me :1:a~Temporary\ merge\ branch\ 2 :2:a :3:a2 &&
test_cmp expect actual
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 4/9] t6404, t6423: expect improved rename/delete handling in ort backend
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (2 preceding siblings ...)
2020-10-23 16:01 ` [PATCH 3/9] t6416: correct expectation for rename/rename(1to2) + directory/file Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-23 16:01 ` [PATCH 5/9] t6423: expect improved conflict markers labels in the " Elijah Newren via GitGitGadget
` (5 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
When a file is renamed and has content conflicts, merge-recursive does
not have some stages for the old filename and some stages for the new
filename in the index; instead it copies all the stages corresponding to
the old filename over to the corresponding locations for the new
filename, so that there are three higher order stages all corresponding
to the new filename. Doing things this way makes it easier for the user
to access the different versions and to resolve the conflict (no need to
manually 'git rm' the old version as well as 'git add' the new one).
rename/deletes should be handled similarly -- there should be two stages
for the renamed file rather than just one. We do not want to
destabilize merge-recursive right now, so instead update relevant tests
to have different expectations depending on whether the "recursive" or
"ort" merge strategies are in use.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6404-recursive-merge.sh | 14 +++++-
t/t6423-merge-rename-directories.sh | 70 ++++++++++++++++++++---------
2 files changed, 62 insertions(+), 22 deletions(-)
diff --git a/t/t6404-recursive-merge.sh b/t/t6404-recursive-merge.sh
index 332cfc53fd..b1c3d4dda4 100755
--- a/t/t6404-recursive-merge.sh
+++ b/t/t6404-recursive-merge.sh
@@ -118,12 +118,22 @@ test_expect_success 'mark rename/delete as unmerged' '
test_tick &&
git commit -m rename &&
test_must_fail git merge delete &&
- test 1 = $(git ls-files --unmerged | wc -l) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 2 = $(git ls-files --unmerged | wc -l)
+ else
+ test 1 = $(git ls-files --unmerged | wc -l)
+ fi &&
git rev-parse --verify :2:a2 &&
test_must_fail git rev-parse --verify :3:a2 &&
git checkout -f delete &&
test_must_fail git merge rename &&
- test 1 = $(git ls-files --unmerged | wc -l) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 2 = $(git ls-files --unmerged | wc -l)
+ else
+ test 1 = $(git ls-files --unmerged | wc -l)
+ fi &&
test_must_fail git rev-parse --verify :2:a2 &&
git rev-parse --verify :3:a2
'
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 5ea77564d7..f9a3f24039 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -1271,17 +1271,32 @@ test_expect_success '6a: Tricky rename/delete' '
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out &&
test_i18ngrep "CONFLICT (rename/delete).*z/c.*y/c" out &&
- git ls-files -s >out &&
- test_line_count = 2 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 1 out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
- git rev-parse >actual \
- :0:y/b :3:y/c &&
- git rev-parse >expect \
- O:z/b O:z/c &&
+ git rev-parse >actual \
+ :0:y/b :1:y/c :3:y/c &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:z/c
+ else
+ git ls-files -s >out &&
+ test_line_count = 2 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:y/b :3:y/c &&
+ git rev-parse >expect \
+ O:z/b O:z/c
+ fi &&
test_cmp expect actual
)
'
@@ -1934,17 +1949,32 @@ test_expect_success '7d: transitive rename involved in rename/delete; how is it
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out &&
test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out &&
- git ls-files -s >out &&
- test_line_count = 3 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 1 out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
- git rev-parse >actual \
- :0:y/b :0:y/c :3:y/d &&
- git rev-parse >expect \
- O:z/b O:z/c O:x/d &&
+ git rev-parse >actual \
+ :0:y/b :0:y/c :1:y/d :3:y/d &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:x/d O:x/d
+ else
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :3:y/d &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:x/d
+ fi &&
test_cmp expect actual
)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 5/9] t6423: expect improved conflict markers labels in the ort backend
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (3 preceding siblings ...)
2020-10-23 16:01 ` [PATCH 4/9] t6404, t6423: expect improved rename/delete handling in ort backend Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-23 16:01 ` [PATCH 6/9] merge tests: expect slight differences in output for recursive vs. ort Elijah Newren via GitGitGadget
` (4 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
Conflict markers carry an extra annotation of the form
REF-OR-COMMIT:FILENAME
to help distinguish where the content is coming from, with the :FILENAME
piece being left off if it is the same for both sides of history (thus
only renames with content conflicts carry that part of the annotation).
However, there were cases where the :FILENAME annotation was
accidentally left off, due to merge-recursive's
every-codepath-needs-a-copy-of-all-special-case-code format.
Update a few tests to have the correct :FILENAME extension on relevant
paths with the ort backend, while leaving the expectation for
merge-recursive the same to avoid destabilizing it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6423-merge-rename-directories.sh | 38 +++++++++++++++++++++--------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index f9a3f24039..7e32626913 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -302,11 +302,20 @@ test_expect_success '1d: Directory renames cause a rename/rename(2to1) conflict'
git cat-file -p :2:x/wham >expect &&
git cat-file -p :3:x/wham >other &&
>empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- expect empty other &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge-file \
+ -L "HEAD:y/wham" \
+ -L "" \
+ -L "B^0:z/wham" \
+ expect empty other
+ else
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ expect empty other
+ fi &&
test_cmp expect x/wham
)
'
@@ -1823,11 +1832,20 @@ test_expect_success '7b: rename/rename(2to1), but only due to transitive rename'
git cat-file -p :2:y/d >expect &&
git cat-file -p :3:y/d >other &&
>empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- expect empty other &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge-file \
+ -L "HEAD:y/d" \
+ -L "" \
+ -L "B^0:z/d" \
+ expect empty other
+ else
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ expect empty other
+ fi &&
test_cmp expect y/d
)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 6/9] merge tests: expect slight differences in output for recursive vs. ort
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (4 preceding siblings ...)
2020-10-23 16:01 ` [PATCH 5/9] t6423: expect improved conflict markers labels in the " Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-24 16:06 ` Elijah Newren
2020-10-23 16:01 ` [PATCH 7/9] t6423, t6436: note improved ort handling with dirty files Elijah Newren via GitGitGadget
` (3 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
The ort merge strategy has some slight differences in commit
descriptions (shortened hashes), stdout vs stderr, and in conflict
messages. Also, builtin/merge.c reports usage of "ort" as "Merge made
by the 'ort' strategy" -- while it is meant as a drop in replacement for
"recursive" it is not yet treated as though it is recursive. Update the
testcases to expect different output for the different merge backends.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6402-merge-rename.sh | 14 ++++++++++++--
t/t6437-submodule-merge.sh | 25 +++++++++++++++++++++----
t/t7602-merge-octopus-many.sh | 4 ++++
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 47d4434d64..3f64f62224 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -320,7 +320,12 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
test_i18ngrep "Auto-merging dir" output &&
- test_i18ngrep "Adding as dir~HEAD instead" output &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_i18ngrep "moving it to dir~HEAD instead" output
+ else
+ test_i18ngrep "Adding as dir~HEAD instead" output
+ fi &&
test 3 -eq "$(git ls-files -u | wc -l)" &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
@@ -342,7 +347,12 @@ test_expect_success 'Same as previous, but merged other way' '
! grep "error: refusing to lose untracked file at" errors &&
test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
test_i18ngrep "Auto-merging dir" output &&
- test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_i18ngrep "moving it to dir~renamed-file-has-no-conflicts instead" output
+ else
+ test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output
+ fi &&
test 3 -eq "$(git ls-files -u | wc -l)" &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
index 6a1e5f8232..3ead2b726f 100755
--- a/t/t6437-submodule-merge.sh
+++ b/t/t6437-submodule-merge.sh
@@ -127,7 +127,12 @@ test_expect_success 'merging should conflict for non fast-forward' '
git checkout -b test-nonforward b &&
(cd sub &&
git rev-parse sub-d > ../expect) &&
- test_must_fail git merge c 2> actual &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge c >actual
+ else
+ test_must_fail git merge c 2> actual
+ fi &&
grep $(cat expect) actual > /dev/null &&
git reset --hard)
'
@@ -138,9 +143,21 @@ test_expect_success 'merging should fail for ambiguous common parent' '
(cd sub &&
git checkout -b ambiguous sub-b &&
git merge sub-c &&
- git rev-parse sub-d > ../expect1 &&
- git rev-parse ambiguous > ../expect2) &&
- test_must_fail git merge c 2> actual &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rev-parse --short sub-d >../expect1 &&
+ git rev-parse --short ambiguous >../expect2
+ else
+ git rev-parse sub-d > ../expect1 &&
+ git rev-parse ambiguous > ../expect2
+ fi
+ ) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge c >actual
+ else
+ test_must_fail git merge c 2> actual
+ fi &&
grep $(cat expect1) actual > /dev/null &&
grep $(cat expect2) actual > /dev/null &&
git reset --hard)
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 6abe441ae3..b205b5619e 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -77,6 +77,10 @@ Merge made by the 'recursive' strategy.
EOF
test_expect_success 'merge reduces irrelevant remote heads' '
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ sed -i'' s/recursive/ort/ expected
+ fi &&
GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
test_i18ncmp expected actual
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 6/9] merge tests: expect slight differences in output for recursive vs. ort
2020-10-23 16:01 ` [PATCH 6/9] merge tests: expect slight differences in output for recursive vs. ort Elijah Newren via GitGitGadget
@ 2020-10-24 16:06 ` Elijah Newren
0 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren @ 2020-10-24 16:06 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget; +Cc: Git Mailing List
On Fri, Oct 23, 2020 at 9:01 AM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
> index 6abe441ae3..b205b5619e 100755
> --- a/t/t7602-merge-octopus-many.sh
> +++ b/t/t7602-merge-octopus-many.sh
> @@ -77,6 +77,10 @@ Merge made by the 'recursive' strategy.
> EOF
>
> test_expect_success 'merge reduces irrelevant remote heads' '
> + if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> + then
> + sed -i'' s/recursive/ort/ expected
> + fi &&
> GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
> test_i18ncmp expected actual
> '
I've got a portability fix for this local hunk that I'll include along
with fixes for other feedback. (I could have sworn I had this fixed
before I sent it out, but I guess I sent an older version somehow?)
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 7/9] t6423, t6436: note improved ort handling with dirty files
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (5 preceding siblings ...)
2020-10-23 16:01 ` [PATCH 6/9] merge tests: expect slight differences in output for recursive vs. ort Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-23 16:01 ` [PATCH 8/9] t6423: note improved ort handling with untracked files Elijah Newren via GitGitGadget
` (2 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
The "recursive" backend relies on unpack_trees() to check if unstaged
changes would be overwritten by a merge, but unpack_trees() does not
understand renames -- and once it returns, it has already written many
updates to the working tree and index. As such, "recursive" had to do a
special 4-way merge where it would need to also treat the working copy
as an extra source of differences that we had to carefully avoid
overwriting and resulting in moving files to new locations to avoid
conflicts.
The "ort" backend, by contrast, does the complete merge inmemory, and
only updates the index and working copy as a post-processing step. If
there are dirty files in the way, it can simply abort the merge.
Update t6423 and t6436 to reflect the better merge abilities and
expectations we have for ort, while still leaving the
best-case-as-good-as-recursive-can-do expectations there for the
recursive backend so we retain its stability until we are ready to
deprecate and remove it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6423-merge-rename-directories.sh | 266 ++++++++++++++++------------
t/t6436-merge-overwrite.sh | 18 +-
2 files changed, 165 insertions(+), 119 deletions(-)
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 7e32626913..32e6925ca4 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -3634,28 +3634,35 @@ test_expect_success '11a: Avoid losing dirty contents with simple rename' '
echo stuff >>z/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "Refusing to lose dirty file at z/c" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "Refusing to lose dirty file at z/c" out &&
- test_seq 1 10 >expected &&
- echo stuff >>expected &&
- test_cmp expected z/c &&
+ git ls-files -s >out &&
+ test_line_count = 2 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- git ls-files -s >out &&
- test_line_count = 2 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 4 out &&
+ git rev-parse >actual \
+ :0:z/a :2:z/c &&
+ git rev-parse >expect \
+ O:z/a B:z/b &&
+ test_cmp expect actual &&
- git rev-parse >actual \
- :0:z/a :2:z/c &&
- git rev-parse >expect \
- O:z/a B:z/b &&
- test_cmp expect actual &&
+ git hash-object z/c~HEAD >actual &&
+ git rev-parse B:z/b >expect &&
+ test_cmp expect actual
+ fi &&
+
+ test_seq 1 10 >expected &&
+ echo stuff >>expected &&
+ test_cmp expected z/c
- git hash-object z/c~HEAD >actual &&
- git rev-parse B:z/b >expect &&
- test_cmp expect actual
)
'
@@ -3706,32 +3713,39 @@ test_expect_success '11b: Avoid losing dirty file involved in directory rename'
git checkout A^0 &&
echo stuff >>z/c &&
- git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "Refusing to lose dirty file at z/c" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
+ test_i18ngrep "Refusing to lose dirty file at z/c" out &&
- grep -q stuff z/c &&
- test_seq 1 10 >expected &&
- echo stuff >>expected &&
- test_cmp expected z/c &&
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -m >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- git ls-files -s >out &&
- test_line_count = 3 out &&
- git ls-files -u >out &&
- test_line_count = 0 out &&
- git ls-files -m >out &&
- test_line_count = 0 out &&
- git ls-files -o >out &&
- test_line_count = 4 out &&
+ git rev-parse >actual \
+ :0:x/b :0:y/a :0:y/c &&
+ git rev-parse >expect \
+ O:x/b O:z/a B:x/c &&
+ test_cmp expect actual &&
- git rev-parse >actual \
- :0:x/b :0:y/a :0:y/c &&
- git rev-parse >expect \
- O:x/b O:z/a B:x/c &&
- test_cmp expect actual &&
+ git hash-object y/c >actual &&
+ git rev-parse B:x/c >expect &&
+ test_cmp expect actual
+ fi &&
- git hash-object y/c >actual &&
- git rev-parse B:x/c >expect &&
- test_cmp expect actual
+ grep -q stuff z/c &&
+ test_seq 1 10 >expected &&
+ echo stuff >>expected &&
+ test_cmp expected z/c
)
'
@@ -3783,7 +3797,13 @@ test_expect_success '11c: Avoid losing not-uptodate with rename + D/F conflict'
echo stuff >>y/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "following files would be overwritten by merge" err &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "following files would be overwritten by merge" err
+ fi &&
grep -q stuff y/c &&
test_seq 1 10 >expected &&
@@ -3851,29 +3871,35 @@ test_expect_success '11d: Avoid losing not-uptodate with rename + D/F conflict'
echo stuff >>z/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "Refusing to lose dirty file at z/c" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "Refusing to lose dirty file at z/c" out &&
- grep -q stuff z/c &&
- test_seq 1 10 >expected &&
- echo stuff >>expected &&
- test_cmp expected z/c &&
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 4 out &&
- git ls-files -s >out &&
- test_line_count = 4 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 5 out &&
+ git rev-parse >actual \
+ :0:x/b :0:y/a :0:y/c/d :3:y/c &&
+ git rev-parse >expect \
+ O:x/b O:z/a B:y/c/d B:x/c &&
+ test_cmp expect actual &&
- git rev-parse >actual \
- :0:x/b :0:y/a :0:y/c/d :3:y/c &&
- git rev-parse >expect \
- O:x/b O:z/a B:y/c/d B:x/c &&
- test_cmp expect actual &&
+ git hash-object y/c~HEAD >actual &&
+ git rev-parse B:x/c >expect &&
+ test_cmp expect actual
+ fi &&
- git hash-object y/c~HEAD >actual &&
- git rev-parse B:x/c >expect &&
- test_cmp expect actual
+ grep -q stuff z/c &&
+ test_seq 1 10 >expected &&
+ echo stuff >>expected &&
+ test_cmp expected z/c
)
'
@@ -3931,37 +3957,43 @@ test_expect_success '11e: Avoid deleting not-uptodate with dir rename/rename(1to
echo mods >>y/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose dirty file at y/c" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose dirty file at y/c" out &&
- git ls-files -s >out &&
- test_line_count = 7 out &&
- git ls-files -u >out &&
- test_line_count = 4 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 7 out &&
+ git ls-files -u >out &&
+ test_line_count = 4 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
+
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :2:y/c :3:y/c &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c &&
+ test_cmp expect actual &&
+
+ # See if y/c~merged has expected contents; requires manually
+ # doing the expected file merge
+ git cat-file -p A:y/c >c1 &&
+ git cat-file -p B:z/c >c2 &&
+ >empty &&
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ c1 empty c2 &&
+ test_cmp c1 y/c~merged
+ fi &&
echo different >expected &&
echo mods >>expected &&
- test_cmp expected y/c &&
-
- git rev-parse >actual \
- :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :2:y/c :3:y/c &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c &&
- test_cmp expect actual &&
-
- # See if y/c~merged has expected contents; requires manually
- # doing the expected file merge
- git cat-file -p A:y/c >c1 &&
- git cat-file -p B:z/c >c2 &&
- >empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- c1 empty c2 &&
- test_cmp c1 y/c~merged
+ test_cmp expected y/c
)
'
@@ -4014,38 +4046,44 @@ test_expect_success '11f: Avoid deleting not-uptodate with dir rename/rename(2to
echo important >>y/wham &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose dirty file at y/wham" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose dirty file at y/wham" out &&
- git ls-files -s >out &&
- test_line_count = 4 out &&
- git ls-files -u >out &&
- test_line_count = 2 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- test_seq 1 10 >expected &&
- echo important >>expected &&
- test_cmp expected y/wham &&
+ test_must_fail git rev-parse :1:y/wham &&
- test_must_fail git rev-parse :1:y/wham &&
+ git rev-parse >actual \
+ :0:y/a :0:y/b :2:y/wham :3:y/wham &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/c O:x/d &&
+ test_cmp expect actual &&
- git rev-parse >actual \
- :0:y/a :0:y/b :2:y/wham :3:y/wham &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/c O:x/d &&
- test_cmp expect actual &&
+ # Test that two-way merge in y/wham~merged is as expected
+ git cat-file -p :2:y/wham >expect &&
+ git cat-file -p :3:y/wham >other &&
+ >empty &&
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ expect empty other &&
+ test_cmp expect y/wham~merged
+ fi &&
- # Test that the two-way merge in y/wham~merged is as expected
- git cat-file -p :2:y/wham >expect &&
- git cat-file -p :3:y/wham >other &&
- >empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- expect empty other &&
- test_cmp expect y/wham~merged
+ test_seq 1 10 >expected &&
+ echo important >>expected &&
+ test_cmp expected y/wham
)
'
diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index dd8ab7ede1..dd9376842f 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -97,11 +97,19 @@ test_expect_success 'will not overwrite unstaged changes in renamed file' '
git mv c1.c other.c &&
git commit -m rename &&
cp important other.c &&
- test_must_fail git merge c1a >out &&
- test_i18ngrep "Refusing to lose dirty file at other.c" out &&
- test_path_is_file other.c~HEAD &&
- test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) &&
- test_cmp important other.c
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge c1a >out 2>err &&
+ test_i18ngrep "would be overwritten by merge" err &&
+ test_cmp important other.c &&
+ test_path_is_missing .git/MERGE_HEAD
+ else
+ test_must_fail git merge c1a >out &&
+ test_i18ngrep "Refusing to lose dirty file at other.c" out &&
+ test_path_is_file other.c~HEAD &&
+ test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) &&
+ test_cmp important other.c
+ fi
'
test_expect_success 'will not overwrite untracked subtree' '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 8/9] t6423: note improved ort handling with untracked files
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (6 preceding siblings ...)
2020-10-23 16:01 ` [PATCH 7/9] t6423, t6436: note improved ort handling with dirty files Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-23 16:01 ` [PATCH 9/9] t6423: add more details about direct resolution of directories Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
9 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
Similar to the previous commit, since the "recursive" backend relies on
unpack_trees() to check if unstaged or untracked files would be
overwritten by a merge, and unpack_trees() does not understand renames
-- it has false positives and false negatives. Once it has run, since
it updates as it goes, merge-recursive then has to handle completing the
merge as best it can despite extra changes in the working copy.
However, this is not just an issue for dirty files, but also for
untracked files because directory renames can cause file contents to
need to be written to a location that was not tracked on either side of
history.
Since the "ort" backend does the complete merge inmemory, and only
updates the index and working copy as a post-processing step, if there
are untracked files in the way it can simply abort the merge much like
checkout does.
Update t6423 to reflect the better merge abilities and expectations for
ort, while still leaving the best-case-as-good-as-recursive-can-do
expectations there for the recursive backend so we retain its stability
until we are ready to deprecate and remove it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6423-merge-rename-directories.sh | 195 ++++++++++++++++++----------
1 file changed, 124 insertions(+), 71 deletions(-)
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 32e6925ca4..db694a6316 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -3214,6 +3214,7 @@ test_expect_success '10a: Overwrite untracked with normal rename/delete' '
echo important >z/d &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
+ test_path_is_missing .git/MERGE_HEAD &&
test_i18ngrep "The following untracked working tree files would be overwritten by merge" err &&
git ls-files -s >out &&
@@ -3283,21 +3284,34 @@ test_expect_success '10b: Overwrite untracked with dir rename + delete' '
echo contents >y/e &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out &&
- test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err &&
- git ls-files -s >out &&
- test_line_count = 3 out &&
- git ls-files -u >out &&
- test_line_count = 2 out &&
- git ls-files -o >out &&
- test_line_count = 5 out &&
+ git ls-files -s >out &&
+ test_line_count = 1 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 5 out
+ else
+ test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out &&
+ test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out &&
- git rev-parse >actual \
- :0:y/b :3:y/d :3:y/e &&
- git rev-parse >expect \
- O:z/b O:z/c B:z/e &&
- test_cmp expect actual &&
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 5 out &&
+
+ git rev-parse >actual \
+ :0:y/b :3:y/d :3:y/e &&
+ git rev-parse >expect \
+ O:z/b O:z/c B:z/e &&
+ test_cmp expect actual
+ fi &&
echo very >expect &&
test_cmp expect y/c &&
@@ -3360,25 +3374,38 @@ test_expect_success '10c1: Overwrite untracked with dir rename/rename(1to2)' '
echo important >y/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err &&
- git ls-files -s >out &&
- test_line_count = 6 out &&
- git ls-files -u >out &&
- test_line_count = 3 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out &&
- git rev-parse >actual \
- :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :3:y/c &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
- test_cmp expect actual &&
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- git hash-object y/c~B^0 >actual &&
- git rev-parse O:x/c >expect &&
- test_cmp expect actual &&
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :3:y/c &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
+ test_cmp expect actual &&
+
+ git hash-object y/c~B^0 >actual &&
+ git rev-parse O:x/c >expect &&
+ test_cmp expect actual
+ fi &&
echo important >expect &&
test_cmp expect y/c
@@ -3398,25 +3425,38 @@ test_expect_success '10c2: Overwrite untracked with dir rename/rename(1to2), oth
echo important >y/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive A^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err &&
- git ls-files -s >out &&
- test_line_count = 6 out &&
- git ls-files -u >out &&
- test_line_count = 3 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out &&
- git rev-parse >actual \
- :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
- test_cmp expect actual &&
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- git hash-object y/c~HEAD >actual &&
- git rev-parse O:x/c >expect &&
- test_cmp expect actual &&
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
+ test_cmp expect actual &&
+
+ git hash-object y/c~HEAD >actual &&
+ git rev-parse O:x/c >expect &&
+ test_cmp expect actual
+ fi &&
echo important >expect &&
test_cmp expect y/c
@@ -3474,37 +3514,50 @@ test_expect_success '10d: Delete untracked with dir rename/rename(2to1)' '
echo important >y/wham &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose untracked file at y/wham" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err &&
- git ls-files -s >out &&
- test_line_count = 6 out &&
- git ls-files -u >out &&
- test_line_count = 2 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose untracked file at y/wham" out &&
- git rev-parse >actual \
- :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/d O:x/e O:z/c O:x/f &&
- test_cmp expect actual &&
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
+
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/d O:x/e O:z/c O:x/f &&
+ test_cmp expect actual &&
- test_must_fail git rev-parse :1:y/wham &&
+ test_must_fail git rev-parse :1:y/wham &&
- echo important >expect &&
- test_cmp expect y/wham &&
+ # Test that two-way merge in y/wham~merged is as expected
+ git cat-file -p :2:y/wham >expect &&
+ git cat-file -p :3:y/wham >other &&
+ >empty &&
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ expect empty other &&
+ test_cmp expect y/wham~merged
+ fi &&
- # Test that the two-way merge in y/wham~merged is as expected
- git cat-file -p :2:y/wham >expect &&
- git cat-file -p :3:y/wham >other &&
- >empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- expect empty other &&
- test_cmp expect y/wham~merged
+ echo important >expect &&
+ test_cmp expect y/wham
)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 9/9] t6423: add more details about direct resolution of directories
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (7 preceding siblings ...)
2020-10-23 16:01 ` [PATCH 8/9] t6423: note improved ort handling with untracked files Elijah Newren via GitGitGadget
@ 2020-10-23 16:01 ` Elijah Newren via GitGitGadget
2020-10-23 20:12 ` Elijah Newren
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
9 siblings, 1 reply; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-23 16:01 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6423-merge-rename-directories.sh | 37 ++++++++++++++++-------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index db694a6316..9e51320ee0 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -4725,20 +4725,22 @@ test_expect_success '12e: Rename/merge subdir into the root, variant 2' '
# folder/subdir/newsubdir/e_Merge2
# folder/subdir/tweaked/{h,Makefile_SUB_Merge1,newfile.py}
# folder/unchanged/<LOTS OF FILES>
-#
-# Notes: This testcase happens to exercise lots of the
-# optimization-specific codepaths in merge-ort, and also
-# demonstrated a failing of the directory rename detection algorithm
-# in merge-recursive; newfile.c should not get pushed into
-# folder/subdir/newsubdir/, yet merge-recursive put it there because
-# the rename of dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d}
-# looks like
-# dir/ -> folder/,
-# whereas the rename of dir/subdir/e -> folder/subdir/newsubdir/e
-# looks like
-# dir/subdir/ -> folder/subdir/newsubdir/
-# and if we note that newfile.c is found in dir/subdir/, we might
-# overlook the dir/ -> folder/ rule that has more weight.
+# Things being checked here:
+# 1. dir/subdir/newfile.c does not get pushed into folder/subdir/newsubdir/.
+# dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d} looks like
+# dir/ -> folder/,
+# whereas dir/subdir/e -> folder/subdir/newsubdir/e looks like
+# dir/subdir/ -> folder/subdir/newsubdir/
+# and if we note that newfile.c is found in dir/subdir/, we might overlook
+# the dir/ -> folder/ rule that has more weight. Older git versions did
+# this.
+# 2. The code to do trivial directory resolves. Note that
+# dir/subdir/unchanged/ is unchanged and can be deleted, and files in the
+# new folder/subdir/unchanged/ are not needed as a target to any renames.
+# Thus, in the second collect_merge_info_callback() we can just resolve
+# these two directories trivially without recursing.)
+# 3. Exercising the codepaths for caching renames and deletes from one cherry
+# pick and re-applying them in the subsequent one.
test_setup_12f () {
test_create_repo 12f &&
@@ -4803,7 +4805,7 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac
git checkout A^0 &&
git branch Bmod B &&
- git -c merge.directoryRenames=true rebase A Bmod &&
+ GIT_TRACE2_PERF="$(pwd)/trace.output" git -c merge.directoryRenames=true rebase A Bmod &&
echo Checking the pick of B1... &&
@@ -4884,7 +4886,10 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac
test_seq 2 12 >e_Merge2 &&
git hash-object e_Merge2 >expect &&
git rev-parse Bmod:folder/subdir/newsubdir/e >actual &&
- test_cmp expect actual
+ test_cmp expect actual &&
+
+ grep region_enter.*collect_merge_info trace.output >collect &&
+ test_line_count = 4 collect
)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 9/9] t6423: add more details about direct resolution of directories
2020-10-23 16:01 ` [PATCH 9/9] t6423: add more details about direct resolution of directories Elijah Newren via GitGitGadget
@ 2020-10-23 20:12 ` Elijah Newren
0 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren @ 2020-10-23 20:12 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget; +Cc: Git Mailing List
On Fri, Oct 23, 2020 at 9:01 AM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Elijah Newren <newren@gmail.com>
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> t/t6423-merge-rename-directories.sh | 37 ++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 16 deletions(-)
Whoops, I had meant for this last patch to be kept local for now and
only send it in after adding more of the merge-ort implementation. To
make matters worse, it's slightly out of date with what I have on my
'ort' branch. Oh, well, I guess it doesn't hurt...at least not if I
get the up-to-date version of the changes. I'll include those in v2,
along with the small change Junio highlighted for patch 1, but wait a
few days for more feedback to come in on this series.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable
2020-10-23 16:01 [PATCH 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (8 preceding siblings ...)
2020-10-23 16:01 ` [PATCH 9/9] t6423: add more details about direct resolution of directories Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 1/9] t/: new helper for tests that pass with ort but fail with recursive Elijah Newren via GitGitGadget
` (8 more replies)
9 siblings, 9 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh, Elijah Newren
This patch series builds on top of en/dir-rename-tests. It can be applied
independent of my "new merge-ort" API series I submitted a couple days
ago[1], but this series uses the same environment variable as patch 4 of
that series.
As promised, here is a series that makes the testsuite changes needed to
simultaneously support both merge backends, keyed off a
GIT_TEST_MERGE_ALGORITHM environment variable setting.
NOTE: The tests do not yet pass with GIT_TEST_MERGE_ALGORITHM=ort, because I
haven't submitted the implementation of the merge-ort functions -- yet. I
figured they are useful as a high level overview of the differences in
behavior between the two backends, and thus am providing these before the
implementation.
Changes since v1:
* Minor tweak to test_expect_merge_algorithm() suggested by Junio
* Updated a test for portability
* I had meant to leave the final patch, patch 9, out of the submission.
But, since I submitted it, update it to match my local version of that
test with one additional thing checked in the result.
[1]
https://lore.kernel.org/git/pull.895.git.git.1603286555.gitgitgadget@gmail.com/
Elijah Newren (9):
t/: new helper for tests that pass with ort but fail with recursive
merge tests: expect improved directory/file conflict handling in ort
t6416: correct expectation for rename/rename(1to2) + directory/file
t6404, t6423: expect improved rename/delete handling in ort backend
t6423: expect improved conflict markers labels in the ort backend
merge tests: expect slight differences in output for recursive vs. ort
t6423, t6436: note improved ort handling with dirty files
t6423: note improved ort handling with untracked files
t6423: add more details about direct resolution of directories
t/lib-merge.sh | 13 +
t/t6400-merge-df.sh | 14 +-
t/t6402-merge-rename.sh | 122 ++++-
t/t6404-recursive-merge.sh | 14 +-
t/t6416-recursive-corner-cases.sh | 200 ++++---
t/t6422-merge-rename-corner-cases.sh | 37 +-
t/t6423-merge-rename-directories.sh | 706 +++++++++++++++----------
t/t6426-merge-skip-unneeded-updates.sh | 3 +-
t/t6430-merge-recursive.sh | 3 +-
t/t6436-merge-overwrite.sh | 18 +-
t/t6437-submodule-merge.sh | 25 +-
t/t7602-merge-octopus-many.sh | 6 +
t/t7610-mergetool.sh | 32 +-
13 files changed, 807 insertions(+), 386 deletions(-)
create mode 100644 t/lib-merge.sh
base-commit: c64432aacda9054459ce550eca95929897c301bd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-769%2Fnewren%2Ftests-support-both-merge-backends-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-769/newren/tests-support-both-merge-backends-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/769
Range-diff vs v1:
1: a8d4825a32 ! 1: f5d3295262 t/: new helper for tests that pass with ort but fail with recursive
@@ t/lib-merge.sh (new)
+# Helper functions used by merge tests.
+
+test_expect_merge_algorithm () {
-+ status_for_recursive=$1
-+ shift
-+ status_for_ort=$1
-+ shift
++ status_for_recursive=$1 status_for_ort=$2
++ shift 2
+
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
2: 9394d526f1 = 2: f27f12e8e5 merge tests: expect improved directory/file conflict handling in ort
3: 1eae84b787 = 3: 38919982c4 t6416: correct expectation for rename/rename(1to2) + directory/file
4: 554feb2aa7 = 4: 6e47fa19f8 t6404, t6423: expect improved rename/delete handling in ort backend
5: 5bead7fa0d = 5: e8a7d6ac81 t6423: expect improved conflict markers labels in the ort backend
6: 2aa44c1f13 ! 6: 4f32450b08 merge tests: expect slight differences in output for recursive vs. ort
@@ t/t7602-merge-octopus-many.sh: Merge made by the 'recursive' strategy.
test_expect_success 'merge reduces irrelevant remote heads' '
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
-+ sed -i'' s/recursive/ort/ expected
++ mv expected expected.tmp &&
++ sed s/recursive/ort/ expected.tmp >expected &&
++ rm expected.tmp
+ fi &&
GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
test_i18ncmp expected actual
7: 9d46cc1ef3 = 7: c5350fd0ae t6423, t6436: note improved ort handling with dirty files
8: 436577106e = 8: 6fd224247d t6423: note improved ort handling with untracked files
9: 2659d1cb98 ! 9: 6e308768ff t6423: add more details about direct resolution of directories
@@ t/t6423-merge-rename-directories.sh: test_expect_merge_algorithm failure success
+ test_cmp expect actual &&
+
+ grep region_enter.*collect_merge_info trace.output >collect &&
-+ test_line_count = 4 collect
++ test_line_count = 4 collect &&
++ grep region_enter.*process_entries$ trace.output >process &&
++ test_line_count = 2 process
)
'
--
gitgitgadget
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v2 1/9] t/: new helper for tests that pass with ort but fail with recursive
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 2/9] merge tests: expect improved directory/file conflict handling in ort Elijah Newren via GitGitGadget
` (7 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
There are a number of tests that the "recursive" backend does not handle
correctly but which the redesign in "ort" will. Add a new helper in
lib-merge.sh for selecting a different test expectation based on the
setting of GIT_TEST_MERGE_ALGORITHM, and use it in various testcases to
document which ones we expect to fail under recursive but pass under
ort.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/lib-merge.sh | 13 +++++++++++++
t/t6416-recursive-corner-cases.sh | 11 ++++++-----
t/t6422-merge-rename-corner-cases.sh | 7 ++++---
t/t6423-merge-rename-directories.sh | 13 +++++++------
t/t6426-merge-skip-unneeded-updates.sh | 3 ++-
t/t6430-merge-recursive.sh | 3 ++-
6 files changed, 34 insertions(+), 16 deletions(-)
create mode 100644 t/lib-merge.sh
diff --git a/t/lib-merge.sh b/t/lib-merge.sh
new file mode 100644
index 0000000000..8734ebfc17
--- /dev/null
+++ b/t/lib-merge.sh
@@ -0,0 +1,13 @@
+# Helper functions used by merge tests.
+
+test_expect_merge_algorithm () {
+ status_for_recursive=$1 status_for_ort=$2
+ shift 2
+
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_expect_${status_for_ort} "$@"
+ else
+ test_expect_${status_for_recursive} "$@"
+ fi
+}
diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
index fd98989b14..8b3a4fc843 100755
--- a/t/t6416-recursive-corner-cases.sh
+++ b/t/t6416-recursive-corner-cases.sh
@@ -3,6 +3,7 @@
test_description='recursive merge corner cases involving criss-cross merges'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
#
# L1 L2
@@ -1069,7 +1070,7 @@ test_expect_success 'setup symlink modify/modify' '
)
'
-test_expect_failure 'check symlink modify/modify' '
+test_expect_merge_algorithm failure success 'check symlink modify/modify' '
(
cd symlink-modify-modify &&
@@ -1135,7 +1136,7 @@ test_expect_success 'setup symlink add/add' '
)
'
-test_expect_failure 'check symlink add/add' '
+test_expect_merge_algorithm failure success 'check symlink add/add' '
(
cd symlink-add-add &&
@@ -1223,7 +1224,7 @@ test_expect_success 'setup submodule modify/modify' '
)
'
-test_expect_failure 'check submodule modify/modify' '
+test_expect_merge_algorithm failure success 'check submodule modify/modify' '
(
cd submodule-modify-modify &&
@@ -1311,7 +1312,7 @@ test_expect_success 'setup submodule add/add' '
)
'
-test_expect_failure 'check submodule add/add' '
+test_expect_merge_algorithm failure success 'check submodule add/add' '
(
cd submodule-add-add &&
@@ -1386,7 +1387,7 @@ test_expect_success 'setup conflicting entry types (submodule vs symlink)' '
)
'
-test_expect_failure 'check conflicting entry types (submodule vs symlink)' '
+test_expect_merge_algorithm failure success 'check conflicting entry types (submodule vs symlink)' '
(
cd submodule-symlink-add-add &&
diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh
index 3375eaf4e7..58729593ba 100755
--- a/t/t6422-merge-rename-corner-cases.sh
+++ b/t/t6422-merge-rename-corner-cases.sh
@@ -4,6 +4,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses"
# t6036 has corner cases that involve both criss-cross merges and renames
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
test_setup_rename_delete_untracked () {
test_create_repo rename-delete-untracked &&
@@ -878,7 +879,7 @@ test_setup_rad () {
)
}
-test_expect_failure 'rad-check: rename/add/delete conflict' '
+test_expect_merge_algorithm failure success 'rad-check: rename/add/delete conflict' '
test_setup_rad &&
(
cd rad &&
@@ -951,7 +952,7 @@ test_setup_rrdd () {
)
}
-test_expect_failure 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
+test_expect_merge_algorithm failure success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
test_setup_rrdd &&
(
cd rrdd &&
@@ -1040,7 +1041,7 @@ test_setup_mod6 () {
)
}
-test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
+test_expect_merge_algorithm failure success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
test_setup_mod6 &&
(
cd mod6 &&
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 06b46af765..807a424a52 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -26,6 +26,7 @@ test_description="recursive merge with directory renames"
# files that might be renamed into each other's paths.)
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
###########################################################################
@@ -1339,7 +1340,7 @@ test_setup_6b1 () {
)
}
-test_expect_failure '6b1: Same renames done on both sides, plus another rename' '
+test_expect_merge_algorithm failure success '6b1: Same renames done on both sides, plus another rename' '
test_setup_6b1 &&
(
cd 6b1 &&
@@ -1412,7 +1413,7 @@ test_setup_6b2 () {
)
}
-test_expect_failure '6b2: Same rename done on both sides' '
+test_expect_merge_algorithm failure success '6b2: Same rename done on both sides' '
test_setup_6b2 &&
(
cd 6b2 &&
@@ -3471,7 +3472,7 @@ test_setup_10e () {
)
}
-test_expect_failure '10e: Does git complain about untracked file that is not really in the way?' '
+test_expect_merge_algorithm failure success '10e: Does git complain about untracked file that is not really in the way?' '
test_setup_10e &&
(
cd 10e &&
@@ -4104,7 +4105,7 @@ test_setup_12b1 () {
)
}
-test_expect_failure '12b1: Moving two directory hierarchies into each other' '
+test_expect_merge_algorithm failure success '12b1: Moving two directory hierarchies into each other' '
test_setup_12b1 &&
(
cd 12b1 &&
@@ -4272,7 +4273,7 @@ test_setup_12c1 () {
)
}
-test_expect_failure '12c1: Moving one directory hierarchy into another w/ content merge' '
+test_expect_merge_algorithm failure success '12c1: Moving one directory hierarchy into another w/ content merge' '
test_setup_12c1 &&
(
cd 12c1 &&
@@ -4632,7 +4633,7 @@ test_setup_12f () {
)
}
-test_expect_failure '12f: Trivial directory resolve, caching, all kinds of fun' '
+test_expect_merge_algorithm failure success '12f: Trivial directory resolve, caching, all kinds of fun' '
test_setup_12f &&
(
cd 12f &&
diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh
index 699813671c..d7eeee4310 100755
--- a/t/t6426-merge-skip-unneeded-updates.sh
+++ b/t/t6426-merge-skip-unneeded-updates.sh
@@ -23,6 +23,7 @@ test_description="merge cases"
# files that might be renamed into each other's paths.)
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
###########################################################################
@@ -666,7 +667,7 @@ test_setup_4a () {
# correct requires doing the merge in-memory first, then realizing that no
# updates to the file are necessary, and thus that we can just leave the path
# alone.
-test_expect_failure '4a: Change on A, change on B subset of A, dirty mods present' '
+test_expect_merge_algorithm failure success '4a: Change on A, change on B subset of A, dirty mods present' '
test_setup_4a &&
(
cd 4a &&
diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh
index a328260d42..9c08e63af2 100755
--- a/t/t6430-merge-recursive.sh
+++ b/t/t6430-merge-recursive.sh
@@ -3,6 +3,7 @@
test_description='merge-recursive backend test'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-merge.sh
test_expect_success 'setup 1' '
@@ -641,7 +642,7 @@ test_expect_success 'merge-recursive copy vs. rename' '
test_cmp expected actual
'
-test_expect_failure 'merge-recursive rename vs. rename/symlink' '
+test_expect_merge_algorithm failure success 'merge-recursive rename vs. rename/symlink' '
git checkout -f rename &&
git merge rename-ln &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 2/9] merge tests: expect improved directory/file conflict handling in ort
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 1/9] t/: new helper for tests that pass with ort but fail with recursive Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 3/9] t6416: correct expectation for rename/rename(1to2) + directory/file Elijah Newren via GitGitGadget
` (6 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
merge-recursive.c is built on the idea of running unpack_trees() and
then "doing minor touch-ups" to get the result. Unfortunately,
unpack_trees() was run in an update-as-it-goes mode, leading
merge-recursive.c to follow suit and end up with an immediate evaluation
and fix-it-up-as-you-go design. Some things like directory/file
conflicts are not well representable in the index data structure, and
required special extra code to handle. But then when it was discovered
that rename/delete conflicts could also be involved in directory/file
conflicts, the special directory/file conflict handling code had to be
copied to the rename/delete codepath. ...and then it had to be copied
for modify/delete, and for rename/rename(1to2) conflicts, ...and yet it
still missed some. Further, when it was discovered that there were also
file/submodule conflicts and submodule/directory conflicts, we needed to
copy the special submodule handling code to all the special cases
throughout the codebase.
And then it was discovered that our handling of directory/file conflicts
was suboptimal because it would create untracked files to store the
contents of the conflicting file, which would not be cleaned up if
someone were to run a 'git merge --abort' or 'git rebase --abort'. It
was also difficult or scary to try to add or remove the index entries
corresponding to these files given the directory/file conflict in the
index. But changing merge-recursive.c to handle these correctly was a
royal pain because there were so many sites in the code with similar but
not identical code for handling directory/file/submodule conflicts that
would all need to be updated.
I have worked hard to push all directory/file/submodule conflict
handling in merge-ort through a single codepath, and avoid creating
untracked files for storing tracked content (it does record things at
alternate paths, but makes sure they have higher-order stages in the
index).
Since updating merge-recursive is too much work and we don't want to
destabilize it, instead update the testsuite to have different
expectations for relevant directory/file/submodule conflict tests.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6400-merge-df.sh | 14 +-
t/t6402-merge-rename.sh | 108 ++++++++++++----
t/t6416-recursive-corner-cases.sh | 185 +++++++++++++++++++--------
t/t6422-merge-rename-corner-cases.sh | 30 +++--
t/t6423-merge-rename-directories.sh | 53 +++++---
t/t7610-mergetool.sh | 32 ++++-
6 files changed, 314 insertions(+), 108 deletions(-)
diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh
index f1b84617af..9da0838216 100755
--- a/t/t6400-merge-df.sh
+++ b/t/t6400-merge-df.sh
@@ -81,7 +81,12 @@ test_expect_success 'modify/delete + directory/file conflict' '
test 5 -eq $(git ls-files -s | wc -l) &&
test 4 -eq $(git ls-files -u | wc -l) &&
- test 1 -eq $(git ls-files -o | wc -l) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 0 -eq $(git ls-files -o | wc -l)
+ else
+ test 1 -eq $(git ls-files -o | wc -l)
+ fi &&
test_path_is_file letters/file &&
test_path_is_file letters.txt &&
@@ -97,7 +102,12 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
test 5 -eq $(git ls-files -s | wc -l) &&
test 4 -eq $(git ls-files -u | wc -l) &&
- test 1 -eq $(git ls-files -o | wc -l) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 0 -eq $(git ls-files -o | wc -l)
+ else
+ test 1 -eq $(git ls-files -o | wc -l)
+ fi &&
test_path_is_file letters/file &&
test_path_is_file letters.txt &&
diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index bbbba3dcbf..47d4434d64 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -397,7 +397,12 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t
test_must_fail git merge --strategy=recursive dir-in-way &&
test 5 -eq "$(git ls-files -u | wc -l)" &&
- test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
+ else
+ test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+ fi &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
test_must_fail git diff --quiet &&
@@ -415,7 +420,12 @@ test_expect_success 'Same as previous, but merged other way' '
test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
test 5 -eq "$(git ls-files -u | wc -l)" &&
- test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
+ else
+ test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+ fi &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
test_must_fail git diff --quiet &&
@@ -471,7 +481,12 @@ test_expect_success 'both rename source and destination involved in D/F conflict
git checkout -q rename-dest^0 &&
test_must_fail git merge --strategy=recursive source-conflict &&
- test 1 -eq "$(git ls-files -u | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 2 -eq "$(git ls-files -u | wc -l)"
+ else
+ test 1 -eq "$(git ls-files -u | wc -l)"
+ fi &&
test_must_fail git diff --quiet &&
@@ -505,34 +520,63 @@ test_expect_success 'setup pair rename to parent of other (D/F conflicts)' '
git commit -m "Rename one/file -> two"
'
-test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
- git checkout -q rename-one^0 &&
- mkdir one &&
- test_must_fail git merge --strategy=recursive rename-two &&
+if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+then
+ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
+ git checkout -q rename-one^0 &&
+ mkdir one &&
+ test_must_fail git merge --strategy=recursive rename-two &&
- test 2 -eq "$(git ls-files -u | wc -l)" &&
- test 1 -eq "$(git ls-files -u one | wc -l)" &&
- test 1 -eq "$(git ls-files -u two | wc -l)" &&
+ test 4 -eq "$(git ls-files -u | wc -l)" &&
+ test 2 -eq "$(git ls-files -u one | wc -l)" &&
+ test 2 -eq "$(git ls-files -u two | wc -l)" &&
- test_must_fail git diff --quiet &&
+ test_must_fail git diff --quiet &&
- test 4 -eq $(find . | grep -v .git | wc -l) &&
+ test 3 -eq $(find . | grep -v .git | wc -l) &&
- test_path_is_dir one &&
- test_path_is_file one~rename-two &&
- test_path_is_file two &&
- test "other" = $(cat one~rename-two) &&
- test "stuff" = $(cat two)
-'
+ test_path_is_file one &&
+ test_path_is_file two &&
+ test "other" = $(cat one) &&
+ test "stuff" = $(cat two)
+ '
+else
+ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked dir' '
+ git checkout -q rename-one^0 &&
+ mkdir one &&
+ test_must_fail git merge --strategy=recursive rename-two &&
+
+ test 2 -eq "$(git ls-files -u | wc -l)" &&
+ test 1 -eq "$(git ls-files -u one | wc -l)" &&
+ test 1 -eq "$(git ls-files -u two | wc -l)" &&
+
+ test_must_fail git diff --quiet &&
+
+ test 4 -eq $(find . | grep -v .git | wc -l) &&
+
+ test_path_is_dir one &&
+ test_path_is_file one~rename-two &&
+ test_path_is_file two &&
+ test "other" = $(cat one~rename-two) &&
+ test "stuff" = $(cat two)
+ '
+fi
test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean start' '
git reset --hard &&
git clean -fdqx &&
test_must_fail git merge --strategy=recursive rename-two &&
- test 2 -eq "$(git ls-files -u | wc -l)" &&
- test 1 -eq "$(git ls-files -u one | wc -l)" &&
- test 1 -eq "$(git ls-files -u two | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 4 -eq "$(git ls-files -u | wc -l)" &&
+ test 2 -eq "$(git ls-files -u one | wc -l)" &&
+ test 2 -eq "$(git ls-files -u two | wc -l)"
+ else
+ test 2 -eq "$(git ls-files -u | wc -l)" &&
+ test 1 -eq "$(git ls-files -u one | wc -l)" &&
+ test 1 -eq "$(git ls-files -u two | wc -l)"
+ fi &&
test_must_fail git diff --quiet &&
@@ -572,12 +616,22 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
git checkout -q first-rename^0 &&
test_must_fail git merge --strategy=recursive second-rename &&
- test 5 -eq "$(git ls-files -s | wc -l)" &&
- test 3 -eq "$(git ls-files -u | wc -l)" &&
- test 1 -eq "$(git ls-files -u one | wc -l)" &&
- test 1 -eq "$(git ls-files -u two | wc -l)" &&
- test 1 -eq "$(git ls-files -u original | wc -l)" &&
- test 2 -eq "$(git ls-files -o | wc -l)" &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 5 -eq "$(git ls-files -s | wc -l)" &&
+ test 3 -eq "$(git ls-files -u | wc -l)" &&
+ test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" &&
+ test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
+ test 1 -eq "$(git ls-files -u original | wc -l)" &&
+ test 0 -eq "$(git ls-files -o | wc -l)"
+ else
+ test 5 -eq "$(git ls-files -s | wc -l)" &&
+ test 3 -eq "$(git ls-files -u | wc -l)" &&
+ test 1 -eq "$(git ls-files -u one | wc -l)" &&
+ test 1 -eq "$(git ls-files -u two | wc -l)" &&
+ test 1 -eq "$(git ls-files -u original | wc -l)" &&
+ test 2 -eq "$(git ls-files -o | wc -l)"
+ fi &&
test_path_is_file one/file &&
test_path_is_file two/file &&
diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
index 8b3a4fc843..0317e83970 100755
--- a/t/t6416-recursive-corner-cases.sh
+++ b/t/t6416-recursive-corner-cases.sh
@@ -538,9 +538,15 @@ test_expect_success 'setup differently handled merges of directory/file conflict
git checkout B^0 &&
test_must_fail git merge C^0 &&
- git clean -fd &&
- git rm -rf a/ &&
- git rm a &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rm -rf a/ &&
+ git rm a~HEAD
+ else
+ git clean -fd &&
+ git rm -rf a/ &&
+ git rm a
+ fi &&
git cat-file -p B:a >a2 &&
git add a2 &&
git commit -m D2 &&
@@ -559,7 +565,12 @@ test_expect_success 'setup differently handled merges of directory/file conflict
git checkout C^0 &&
test_must_fail git merge B^0 &&
- git clean -fd &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rm a~B^0
+ else
+ git clean -fd
+ fi &&
git rm -rf a/ &&
test_write_lines 1 2 3 4 5 6 7 8 >a &&
git add a &&
@@ -568,9 +579,15 @@ test_expect_success 'setup differently handled merges of directory/file conflict
git checkout C^0 &&
test_must_fail git merge B^0 &&
- git clean -fd &&
- git rm -rf a/ &&
- git rm a &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rm -rf a/ &&
+ git rm a~B^0
+ else
+ git clean -fd &&
+ git rm -rf a/ &&
+ git rm a
+ fi &&
test_write_lines 1 2 3 4 5 6 7 8 >a2 &&
git add a2 &&
git commit -m E4 &&
@@ -588,18 +605,34 @@ test_expect_success 'merge of D1 & E1 fails but has appropriate contents' '
test_must_fail git merge -s recursive E1^0 &&
- git ls-files -s >out &&
- test_line_count = 2 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 1 out &&
-
- git rev-parse >expect \
- A:ignore-me B:a &&
- git rev-parse >actual \
- :0:ignore-me :2:a &&
- test_cmp expect actual
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ A:ignore-me B:a D1:a &&
+ git rev-parse >actual \
+ :0:ignore-me :1:a :2:a &&
+ test_cmp expect actual
+ else
+ git ls-files -s >out &&
+ test_line_count = 2 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ A:ignore-me B:a &&
+ git rev-parse >actual \
+ :0:ignore-me :2:a &&
+ test_cmp expect actual
+ fi
)
'
@@ -613,18 +646,34 @@ test_expect_success 'merge of E1 & D1 fails but has appropriate contents' '
test_must_fail git merge -s recursive D1^0 &&
- git ls-files -s >out &&
- test_line_count = 2 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 1 out &&
-
- git rev-parse >expect \
- A:ignore-me B:a &&
- git rev-parse >actual \
- :0:ignore-me :3:a &&
- test_cmp expect actual
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ A:ignore-me B:a D1:a &&
+ git rev-parse >actual \
+ :0:ignore-me :1:a :3:a &&
+ test_cmp expect actual
+ else
+ git ls-files -s >out &&
+ test_line_count = 2 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ A:ignore-me B:a &&
+ git rev-parse >actual \
+ :0:ignore-me :3:a &&
+ test_cmp expect actual
+ fi
)
'
@@ -638,17 +687,32 @@ test_expect_success 'merge of D1 & E2 fails but has appropriate contents' '
test_must_fail git merge -s recursive E2^0 &&
- git ls-files -s >out &&
- test_line_count = 4 out &&
- git ls-files -u >out &&
- test_line_count = 3 out &&
- git ls-files -o >out &&
- test_line_count = 2 out &&
-
- git rev-parse >expect \
- B:a E2:a/file C:a/file A:ignore-me &&
- git rev-parse >actual \
- :2:a :3:a/file :1:a/file :0:ignore-me &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 5 out &&
+ git ls-files -u >out &&
+ test_line_count = 4 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ B:a D1:a E2:a/file C:a/file A:ignore-me &&
+ git rev-parse >actual \
+ :1:a~HEAD :2:a~HEAD :3:a/file :1:a/file :0:ignore-me
+ else
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 2 out &&
+
+ git rev-parse >expect \
+ B:a E2:a/file C:a/file A:ignore-me &&
+ git rev-parse >actual \
+ :2:a :3:a/file :1:a/file :0:ignore-me
+ fi &&
test_cmp expect actual &&
test_path_is_file a~HEAD
@@ -665,17 +729,32 @@ test_expect_success 'merge of E2 & D1 fails but has appropriate contents' '
test_must_fail git merge -s recursive D1^0 &&
- git ls-files -s >out &&
- test_line_count = 4 out &&
- git ls-files -u >out &&
- test_line_count = 3 out &&
- git ls-files -o >out &&
- test_line_count = 2 out &&
-
- git rev-parse >expect \
- B:a E2:a/file C:a/file A:ignore-me &&
- git rev-parse >actual \
- :3:a :2:a/file :1:a/file :0:ignore-me &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 5 out &&
+ git ls-files -u >out &&
+ test_line_count = 4 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ B:a D1:a E2:a/file C:a/file A:ignore-me &&
+ git rev-parse >actual \
+ :1:a~D1^0 :3:a~D1^0 :2:a/file :1:a/file :0:ignore-me
+ else
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 2 out &&
+
+ git rev-parse >expect \
+ B:a E2:a/file C:a/file A:ignore-me &&
+ git rev-parse >actual \
+ :3:a :2:a/file :1:a/file :0:ignore-me
+ fi &&
test_cmp expect actual &&
test_path_is_file a~D1^0
diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh
index 58729593ba..78bfaf17f0 100755
--- a/t/t6422-merge-rename-corner-cases.sh
+++ b/t/t6422-merge-rename-corner-cases.sh
@@ -313,15 +313,18 @@ test_expect_success 'rename/directory conflict + clean content merge' '
git ls-files -u >out &&
test_line_count = 1 out &&
git ls-files -o >out &&
- test_line_count = 2 out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_line_count = 1 out
+ else
+ test_line_count = 2 out
+ fi &&
echo 0 >expect &&
git cat-file -p base:file >>expect &&
echo 7 >>expect &&
test_cmp expect newfile~HEAD &&
- test $(git rev-parse :2:newfile) = $(git hash-object expect) &&
-
test_path_is_file newfile/realfile &&
test_path_is_file newfile~HEAD
)
@@ -344,7 +347,12 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
git ls-files -u >out &&
test_line_count = 3 out &&
git ls-files -o >out &&
- test_line_count = 2 out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_line_count = 1 out
+ else
+ test_line_count = 2 out
+ fi &&
git cat-file -p left-conflict:newfile >left &&
git cat-file -p base:file >base &&
@@ -356,10 +364,16 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
left base right &&
test_cmp left newfile~HEAD &&
- git rev-parse >expect \
- base:file left-conflict:newfile right:file &&
- git rev-parse >actual \
- :1:newfile :2:newfile :3:newfile &&
+ git rev-parse >expect \
+ base:file left-conflict:newfile right:file &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rev-parse >actual \
+ :1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD
+ else
+ git rev-parse >actual \
+ :1:newfile :2:newfile :3:newfile
+ fi &&
test_cmp expect actual &&
test_path_is_file newfile/realfile &&
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 807a424a52..5ea77564d7 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -1177,10 +1177,18 @@ test_expect_success '5d: Directory/file/file conflict due to directory rename' '
git ls-files -u >out &&
test_line_count = 1 out &&
git ls-files -o >out &&
- test_line_count = 2 out &&
-
- git rev-parse >actual \
- :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d :0:y/d/e &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d~HEAD :0:y/d/e
+ else
+ test_line_count = 2 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :0:z/d :0:y/f :2:y/d :0:y/d/e
+ fi &&
git rev-parse >expect \
O:z/b O:z/c B:z/d B:z/f A:y/d B:y/d/e &&
test_cmp expect actual &&
@@ -2017,17 +2025,32 @@ test_expect_success '7e: transitive rename in rename/delete AND dirs in the way'
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out &&
test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out &&
- git ls-files -s >out &&
- test_line_count = 5 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 2 out &&
-
- git rev-parse >actual \
- :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d &&
- git rev-parse >expect \
- A:x/d/f A:y/d/g O:z/b O:z/c O:x/d &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:x/d/f :0:y/d/g :0:y/b :0:y/c :1:y/d~B^0 :3:y/d~B^0 &&
+ git rev-parse >expect \
+ A:x/d/f A:y/d/g O:z/b O:z/c O:x/d O:x/d
+ else
+ git ls-files -s >out &&
+ test_line_count = 5 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 2 out &&
+
+ git rev-parse >actual \
+ :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d &&
+ git rev-parse >expect \
+ A:x/d/f A:y/d/g O:z/b O:z/c O:x/d
+ fi &&
test_cmp expect actual &&
git hash-object y/d~B^0 >actual &&
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index ad288ddc69..70afdd06fa 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -532,7 +532,14 @@ test_expect_success 'file vs modified submodule' '
yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
yes "" | git mergetool both &&
yes "d" | git mergetool file11 file12 &&
- yes "l" | git mergetool submod &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ yes "c" | git mergetool submod~HEAD &&
+ git rm submod &&
+ git mv submod~HEAD submod
+ else
+ yes "l" | git mergetool submod
+ fi &&
git submodule update -N &&
echo "not a submodule" >expect &&
test_cmp expect submod &&
@@ -549,7 +556,15 @@ test_expect_success 'file vs modified submodule' '
yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
yes "" | git mergetool both &&
yes "d" | git mergetool file11 file12 &&
- yes "r" | git mergetool submod &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ mv submod submod.orig &&
+ git rm --cached submod &&
+ yes "c" | git mergetool submod~test19 &&
+ git mv submod~test19 submod
+ else
+ yes "r" | git mergetool submod
+ fi &&
test -d submod.orig &&
git submodule update -N &&
echo "not a submodule" >expect &&
@@ -567,6 +582,10 @@ test_expect_success 'file vs modified submodule' '
yes "" | git mergetool both &&
yes "d" | git mergetool file11 file12 &&
yes "l" | git mergetool submod &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ yes "d" | git mergetool submod~test19
+ fi &&
echo "master submodule" >expect &&
test_cmp expect submod/bar &&
git submodule update -N &&
@@ -664,7 +683,14 @@ test_expect_success 'directory vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
test ! -e submod.orig &&
- yes "r" | git mergetool submod &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ yes "r" | git mergetool submod~master &&
+ git mv submod submod.orig &&
+ git mv submod~master submod
+ else
+ yes "r" | git mergetool submod
+ fi &&
test -d submod.orig &&
echo "not a submodule" >expect &&
test_cmp expect submod.orig/file16 &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 3/9] t6416: correct expectation for rename/rename(1to2) + directory/file
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 1/9] t/: new helper for tests that pass with ort but fail with recursive Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 2/9] merge tests: expect improved directory/file conflict handling in ort Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 4/9] t6404, t6423: expect improved rename/delete handling in ort backend Elijah Newren via GitGitGadget
` (5 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
When files are renamed and modified, we need to do three-way content
merges to get the appropriate content in the right location. When we
have a rename/rename(1to2) conflict (both sides rename the same file,
but differently), that merged content should be placed in each of the
two resulting files. merge-recursive handled that fine when that was
all that was involved, but when one or more of the two resulting files
were ALSO involved in a directory/file conflict, it failed to propagate
the merged content to that file. Unfortunately, the one test in t6416
that touched on this combination of cases had been coded to not expect
the merged contents to be present.
Fix the test to check for the right behavior, and record how the
different merge backends will be expected to handle it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6416-recursive-corner-cases.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
index 0317e83970..887c2195a9 100755
--- a/t/t6416-recursive-corner-cases.sh
+++ b/t/t6416-recursive-corner-cases.sh
@@ -786,7 +786,7 @@ test_expect_success 'merge of D1 & E3 succeeds' '
)
'
-test_expect_success 'merge of D1 & E4 notifies user a and a2 are related' '
+test_expect_merge_algorithm failure success 'merge of D1 & E4 puts merge of a and a2 in both a and a2' '
test_when_finished "git -C directory-file reset --hard" &&
test_when_finished "git -C directory-file clean -fdqx" &&
(
@@ -804,7 +804,7 @@ test_expect_success 'merge of D1 & E4 notifies user a and a2 are related' '
test_line_count = 1 out &&
git rev-parse >expect \
- A:ignore-me B:a D1:a E4:a2 &&
+ A:ignore-me B:a E4:a2 E4:a2 &&
git rev-parse >actual \
:0:ignore-me :1:a~Temporary\ merge\ branch\ 2 :2:a :3:a2 &&
test_cmp expect actual
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 4/9] t6404, t6423: expect improved rename/delete handling in ort backend
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (2 preceding siblings ...)
2020-10-26 17:01 ` [PATCH v2 3/9] t6416: correct expectation for rename/rename(1to2) + directory/file Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 5/9] t6423: expect improved conflict markers labels in the " Elijah Newren via GitGitGadget
` (4 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
When a file is renamed and has content conflicts, merge-recursive does
not have some stages for the old filename and some stages for the new
filename in the index; instead it copies all the stages corresponding to
the old filename over to the corresponding locations for the new
filename, so that there are three higher order stages all corresponding
to the new filename. Doing things this way makes it easier for the user
to access the different versions and to resolve the conflict (no need to
manually 'git rm' the old version as well as 'git add' the new one).
rename/deletes should be handled similarly -- there should be two stages
for the renamed file rather than just one. We do not want to
destabilize merge-recursive right now, so instead update relevant tests
to have different expectations depending on whether the "recursive" or
"ort" merge strategies are in use.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6404-recursive-merge.sh | 14 +++++-
t/t6423-merge-rename-directories.sh | 70 ++++++++++++++++++++---------
2 files changed, 62 insertions(+), 22 deletions(-)
diff --git a/t/t6404-recursive-merge.sh b/t/t6404-recursive-merge.sh
index 332cfc53fd..b1c3d4dda4 100755
--- a/t/t6404-recursive-merge.sh
+++ b/t/t6404-recursive-merge.sh
@@ -118,12 +118,22 @@ test_expect_success 'mark rename/delete as unmerged' '
test_tick &&
git commit -m rename &&
test_must_fail git merge delete &&
- test 1 = $(git ls-files --unmerged | wc -l) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 2 = $(git ls-files --unmerged | wc -l)
+ else
+ test 1 = $(git ls-files --unmerged | wc -l)
+ fi &&
git rev-parse --verify :2:a2 &&
test_must_fail git rev-parse --verify :3:a2 &&
git checkout -f delete &&
test_must_fail git merge rename &&
- test 1 = $(git ls-files --unmerged | wc -l) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test 2 = $(git ls-files --unmerged | wc -l)
+ else
+ test 1 = $(git ls-files --unmerged | wc -l)
+ fi &&
test_must_fail git rev-parse --verify :2:a2 &&
git rev-parse --verify :3:a2
'
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 5ea77564d7..f9a3f24039 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -1271,17 +1271,32 @@ test_expect_success '6a: Tricky rename/delete' '
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out &&
test_i18ngrep "CONFLICT (rename/delete).*z/c.*y/c" out &&
- git ls-files -s >out &&
- test_line_count = 2 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 1 out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
- git rev-parse >actual \
- :0:y/b :3:y/c &&
- git rev-parse >expect \
- O:z/b O:z/c &&
+ git rev-parse >actual \
+ :0:y/b :1:y/c :3:y/c &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:z/c
+ else
+ git ls-files -s >out &&
+ test_line_count = 2 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:y/b :3:y/c &&
+ git rev-parse >expect \
+ O:z/b O:z/c
+ fi &&
test_cmp expect actual
)
'
@@ -1934,17 +1949,32 @@ test_expect_success '7d: transitive rename involved in rename/delete; how is it
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out &&
test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out &&
- git ls-files -s >out &&
- test_line_count = 3 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 1 out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
- git rev-parse >actual \
- :0:y/b :0:y/c :3:y/d &&
- git rev-parse >expect \
- O:z/b O:z/c O:x/d &&
+ git rev-parse >actual \
+ :0:y/b :0:y/c :1:y/d :3:y/d &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:x/d O:x/d
+ else
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :3:y/d &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:x/d
+ fi &&
test_cmp expect actual
)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 5/9] t6423: expect improved conflict markers labels in the ort backend
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (3 preceding siblings ...)
2020-10-26 17:01 ` [PATCH v2 4/9] t6404, t6423: expect improved rename/delete handling in ort backend Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 6/9] merge tests: expect slight differences in output for recursive vs. ort Elijah Newren via GitGitGadget
` (3 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
Conflict markers carry an extra annotation of the form
REF-OR-COMMIT:FILENAME
to help distinguish where the content is coming from, with the :FILENAME
piece being left off if it is the same for both sides of history (thus
only renames with content conflicts carry that part of the annotation).
However, there were cases where the :FILENAME annotation was
accidentally left off, due to merge-recursive's
every-codepath-needs-a-copy-of-all-special-case-code format.
Update a few tests to have the correct :FILENAME extension on relevant
paths with the ort backend, while leaving the expectation for
merge-recursive the same to avoid destabilizing it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6423-merge-rename-directories.sh | 38 +++++++++++++++++++++--------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index f9a3f24039..7e32626913 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -302,11 +302,20 @@ test_expect_success '1d: Directory renames cause a rename/rename(2to1) conflict'
git cat-file -p :2:x/wham >expect &&
git cat-file -p :3:x/wham >other &&
>empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- expect empty other &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge-file \
+ -L "HEAD:y/wham" \
+ -L "" \
+ -L "B^0:z/wham" \
+ expect empty other
+ else
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ expect empty other
+ fi &&
test_cmp expect x/wham
)
'
@@ -1823,11 +1832,20 @@ test_expect_success '7b: rename/rename(2to1), but only due to transitive rename'
git cat-file -p :2:y/d >expect &&
git cat-file -p :3:y/d >other &&
>empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- expect empty other &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge-file \
+ -L "HEAD:y/d" \
+ -L "" \
+ -L "B^0:z/d" \
+ expect empty other
+ else
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ expect empty other
+ fi &&
test_cmp expect y/d
)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 6/9] merge tests: expect slight differences in output for recursive vs. ort
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (4 preceding siblings ...)
2020-10-26 17:01 ` [PATCH v2 5/9] t6423: expect improved conflict markers labels in the " Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 7/9] t6423, t6436: note improved ort handling with dirty files Elijah Newren via GitGitGadget
` (2 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
The ort merge strategy has some slight differences in commit
descriptions (shortened hashes), stdout vs stderr, and in conflict
messages. Also, builtin/merge.c reports usage of "ort" as "Merge made
by the 'ort' strategy" -- while it is meant as a drop in replacement for
"recursive" it is not yet treated as though it is recursive. Update the
testcases to expect different output for the different merge backends.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6402-merge-rename.sh | 14 ++++++++++++--
t/t6437-submodule-merge.sh | 25 +++++++++++++++++++++----
t/t7602-merge-octopus-many.sh | 6 ++++++
3 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 47d4434d64..3f64f62224 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -320,7 +320,12 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
test_i18ngrep "Auto-merging dir" output &&
- test_i18ngrep "Adding as dir~HEAD instead" output &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_i18ngrep "moving it to dir~HEAD instead" output
+ else
+ test_i18ngrep "Adding as dir~HEAD instead" output
+ fi &&
test 3 -eq "$(git ls-files -u | wc -l)" &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
@@ -342,7 +347,12 @@ test_expect_success 'Same as previous, but merged other way' '
! grep "error: refusing to lose untracked file at" errors &&
test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
test_i18ngrep "Auto-merging dir" output &&
- test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_i18ngrep "moving it to dir~renamed-file-has-no-conflicts instead" output
+ else
+ test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output
+ fi &&
test 3 -eq "$(git ls-files -u | wc -l)" &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
index 6a1e5f8232..3ead2b726f 100755
--- a/t/t6437-submodule-merge.sh
+++ b/t/t6437-submodule-merge.sh
@@ -127,7 +127,12 @@ test_expect_success 'merging should conflict for non fast-forward' '
git checkout -b test-nonforward b &&
(cd sub &&
git rev-parse sub-d > ../expect) &&
- test_must_fail git merge c 2> actual &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge c >actual
+ else
+ test_must_fail git merge c 2> actual
+ fi &&
grep $(cat expect) actual > /dev/null &&
git reset --hard)
'
@@ -138,9 +143,21 @@ test_expect_success 'merging should fail for ambiguous common parent' '
(cd sub &&
git checkout -b ambiguous sub-b &&
git merge sub-c &&
- git rev-parse sub-d > ../expect1 &&
- git rev-parse ambiguous > ../expect2) &&
- test_must_fail git merge c 2> actual &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ git rev-parse --short sub-d >../expect1 &&
+ git rev-parse --short ambiguous >../expect2
+ else
+ git rev-parse sub-d > ../expect1 &&
+ git rev-parse ambiguous > ../expect2
+ fi
+ ) &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge c >actual
+ else
+ test_must_fail git merge c 2> actual
+ fi &&
grep $(cat expect1) actual > /dev/null &&
grep $(cat expect2) actual > /dev/null &&
git reset --hard)
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 6abe441ae3..13859ec859 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -77,6 +77,12 @@ Merge made by the 'recursive' strategy.
EOF
test_expect_success 'merge reduces irrelevant remote heads' '
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ mv expected expected.tmp &&
+ sed s/recursive/ort/ expected.tmp >expected &&
+ rm expected.tmp
+ fi &&
GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
test_i18ncmp expected actual
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 7/9] t6423, t6436: note improved ort handling with dirty files
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (5 preceding siblings ...)
2020-10-26 17:01 ` [PATCH v2 6/9] merge tests: expect slight differences in output for recursive vs. ort Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 8/9] t6423: note improved ort handling with untracked files Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 9/9] t6423: add more details about direct resolution of directories Elijah Newren via GitGitGadget
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
The "recursive" backend relies on unpack_trees() to check if unstaged
changes would be overwritten by a merge, but unpack_trees() does not
understand renames -- and once it returns, it has already written many
updates to the working tree and index. As such, "recursive" had to do a
special 4-way merge where it would need to also treat the working copy
as an extra source of differences that we had to carefully avoid
overwriting and resulting in moving files to new locations to avoid
conflicts.
The "ort" backend, by contrast, does the complete merge inmemory, and
only updates the index and working copy as a post-processing step. If
there are dirty files in the way, it can simply abort the merge.
Update t6423 and t6436 to reflect the better merge abilities and
expectations we have for ort, while still leaving the
best-case-as-good-as-recursive-can-do expectations there for the
recursive backend so we retain its stability until we are ready to
deprecate and remove it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6423-merge-rename-directories.sh | 266 ++++++++++++++++------------
t/t6436-merge-overwrite.sh | 18 +-
2 files changed, 165 insertions(+), 119 deletions(-)
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 7e32626913..32e6925ca4 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -3634,28 +3634,35 @@ test_expect_success '11a: Avoid losing dirty contents with simple rename' '
echo stuff >>z/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "Refusing to lose dirty file at z/c" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "Refusing to lose dirty file at z/c" out &&
- test_seq 1 10 >expected &&
- echo stuff >>expected &&
- test_cmp expected z/c &&
+ git ls-files -s >out &&
+ test_line_count = 2 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- git ls-files -s >out &&
- test_line_count = 2 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 4 out &&
+ git rev-parse >actual \
+ :0:z/a :2:z/c &&
+ git rev-parse >expect \
+ O:z/a B:z/b &&
+ test_cmp expect actual &&
- git rev-parse >actual \
- :0:z/a :2:z/c &&
- git rev-parse >expect \
- O:z/a B:z/b &&
- test_cmp expect actual &&
+ git hash-object z/c~HEAD >actual &&
+ git rev-parse B:z/b >expect &&
+ test_cmp expect actual
+ fi &&
+
+ test_seq 1 10 >expected &&
+ echo stuff >>expected &&
+ test_cmp expected z/c
- git hash-object z/c~HEAD >actual &&
- git rev-parse B:z/b >expect &&
- test_cmp expect actual
)
'
@@ -3706,32 +3713,39 @@ test_expect_success '11b: Avoid losing dirty file involved in directory rename'
git checkout A^0 &&
echo stuff >>z/c &&
- git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "Refusing to lose dirty file at z/c" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
+ test_i18ngrep "Refusing to lose dirty file at z/c" out &&
- grep -q stuff z/c &&
- test_seq 1 10 >expected &&
- echo stuff >>expected &&
- test_cmp expected z/c &&
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -m >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- git ls-files -s >out &&
- test_line_count = 3 out &&
- git ls-files -u >out &&
- test_line_count = 0 out &&
- git ls-files -m >out &&
- test_line_count = 0 out &&
- git ls-files -o >out &&
- test_line_count = 4 out &&
+ git rev-parse >actual \
+ :0:x/b :0:y/a :0:y/c &&
+ git rev-parse >expect \
+ O:x/b O:z/a B:x/c &&
+ test_cmp expect actual &&
- git rev-parse >actual \
- :0:x/b :0:y/a :0:y/c &&
- git rev-parse >expect \
- O:x/b O:z/a B:x/c &&
- test_cmp expect actual &&
+ git hash-object y/c >actual &&
+ git rev-parse B:x/c >expect &&
+ test_cmp expect actual
+ fi &&
- git hash-object y/c >actual &&
- git rev-parse B:x/c >expect &&
- test_cmp expect actual
+ grep -q stuff z/c &&
+ test_seq 1 10 >expected &&
+ echo stuff >>expected &&
+ test_cmp expected z/c
)
'
@@ -3783,7 +3797,13 @@ test_expect_success '11c: Avoid losing not-uptodate with rename + D/F conflict'
echo stuff >>y/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "following files would be overwritten by merge" err &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "following files would be overwritten by merge" err
+ fi &&
grep -q stuff y/c &&
test_seq 1 10 >expected &&
@@ -3851,29 +3871,35 @@ test_expect_success '11d: Avoid losing not-uptodate with rename + D/F conflict'
echo stuff >>z/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "Refusing to lose dirty file at z/c" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "Refusing to lose dirty file at z/c" out &&
- grep -q stuff z/c &&
- test_seq 1 10 >expected &&
- echo stuff >>expected &&
- test_cmp expected z/c &&
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 4 out &&
- git ls-files -s >out &&
- test_line_count = 4 out &&
- git ls-files -u >out &&
- test_line_count = 1 out &&
- git ls-files -o >out &&
- test_line_count = 5 out &&
+ git rev-parse >actual \
+ :0:x/b :0:y/a :0:y/c/d :3:y/c &&
+ git rev-parse >expect \
+ O:x/b O:z/a B:y/c/d B:x/c &&
+ test_cmp expect actual &&
- git rev-parse >actual \
- :0:x/b :0:y/a :0:y/c/d :3:y/c &&
- git rev-parse >expect \
- O:x/b O:z/a B:y/c/d B:x/c &&
- test_cmp expect actual &&
+ git hash-object y/c~HEAD >actual &&
+ git rev-parse B:x/c >expect &&
+ test_cmp expect actual
+ fi &&
- git hash-object y/c~HEAD >actual &&
- git rev-parse B:x/c >expect &&
- test_cmp expect actual
+ grep -q stuff z/c &&
+ test_seq 1 10 >expected &&
+ echo stuff >>expected &&
+ test_cmp expected z/c
)
'
@@ -3931,37 +3957,43 @@ test_expect_success '11e: Avoid deleting not-uptodate with dir rename/rename(1to
echo mods >>y/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose dirty file at y/c" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose dirty file at y/c" out &&
- git ls-files -s >out &&
- test_line_count = 7 out &&
- git ls-files -u >out &&
- test_line_count = 4 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 7 out &&
+ git ls-files -u >out &&
+ test_line_count = 4 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
+
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :2:y/c :3:y/c &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c &&
+ test_cmp expect actual &&
+
+ # See if y/c~merged has expected contents; requires manually
+ # doing the expected file merge
+ git cat-file -p A:y/c >c1 &&
+ git cat-file -p B:z/c >c2 &&
+ >empty &&
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ c1 empty c2 &&
+ test_cmp c1 y/c~merged
+ fi &&
echo different >expected &&
echo mods >>expected &&
- test_cmp expected y/c &&
-
- git rev-parse >actual \
- :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :2:y/c :3:y/c &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c &&
- test_cmp expect actual &&
-
- # See if y/c~merged has expected contents; requires manually
- # doing the expected file merge
- git cat-file -p A:y/c >c1 &&
- git cat-file -p B:z/c >c2 &&
- >empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- c1 empty c2 &&
- test_cmp c1 y/c~merged
+ test_cmp expected y/c
)
'
@@ -4014,38 +4046,44 @@ test_expect_success '11f: Avoid deleting not-uptodate with dir rename/rename(2to
echo important >>y/wham &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose dirty file at y/wham" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: Your local changes to the following files would be overwritten by merge" err
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose dirty file at y/wham" out &&
- git ls-files -s >out &&
- test_line_count = 4 out &&
- git ls-files -u >out &&
- test_line_count = 2 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- test_seq 1 10 >expected &&
- echo important >>expected &&
- test_cmp expected y/wham &&
+ test_must_fail git rev-parse :1:y/wham &&
- test_must_fail git rev-parse :1:y/wham &&
+ git rev-parse >actual \
+ :0:y/a :0:y/b :2:y/wham :3:y/wham &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/c O:x/d &&
+ test_cmp expect actual &&
- git rev-parse >actual \
- :0:y/a :0:y/b :2:y/wham :3:y/wham &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/c O:x/d &&
- test_cmp expect actual &&
+ # Test that two-way merge in y/wham~merged is as expected
+ git cat-file -p :2:y/wham >expect &&
+ git cat-file -p :3:y/wham >other &&
+ >empty &&
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ expect empty other &&
+ test_cmp expect y/wham~merged
+ fi &&
- # Test that the two-way merge in y/wham~merged is as expected
- git cat-file -p :2:y/wham >expect &&
- git cat-file -p :3:y/wham >other &&
- >empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- expect empty other &&
- test_cmp expect y/wham~merged
+ test_seq 1 10 >expected &&
+ echo important >>expected &&
+ test_cmp expected y/wham
)
'
diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index dd8ab7ede1..dd9376842f 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -97,11 +97,19 @@ test_expect_success 'will not overwrite unstaged changes in renamed file' '
git mv c1.c other.c &&
git commit -m rename &&
cp important other.c &&
- test_must_fail git merge c1a >out &&
- test_i18ngrep "Refusing to lose dirty file at other.c" out &&
- test_path_is_file other.c~HEAD &&
- test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) &&
- test_cmp important other.c
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_must_fail git merge c1a >out 2>err &&
+ test_i18ngrep "would be overwritten by merge" err &&
+ test_cmp important other.c &&
+ test_path_is_missing .git/MERGE_HEAD
+ else
+ test_must_fail git merge c1a >out &&
+ test_i18ngrep "Refusing to lose dirty file at other.c" out &&
+ test_path_is_file other.c~HEAD &&
+ test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) &&
+ test_cmp important other.c
+ fi
'
test_expect_success 'will not overwrite untracked subtree' '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 8/9] t6423: note improved ort handling with untracked files
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (6 preceding siblings ...)
2020-10-26 17:01 ` [PATCH v2 7/9] t6423, t6436: note improved ort handling with dirty files Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
2020-10-26 17:01 ` [PATCH v2 9/9] t6423: add more details about direct resolution of directories Elijah Newren via GitGitGadget
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
Similar to the previous commit, since the "recursive" backend relies on
unpack_trees() to check if unstaged or untracked files would be
overwritten by a merge, and unpack_trees() does not understand renames
-- it has false positives and false negatives. Once it has run, since
it updates as it goes, merge-recursive then has to handle completing the
merge as best it can despite extra changes in the working copy.
However, this is not just an issue for dirty files, but also for
untracked files because directory renames can cause file contents to
need to be written to a location that was not tracked on either side of
history.
Since the "ort" backend does the complete merge inmemory, and only
updates the index and working copy as a post-processing step, if there
are untracked files in the way it can simply abort the merge much like
checkout does.
Update t6423 to reflect the better merge abilities and expectations for
ort, while still leaving the best-case-as-good-as-recursive-can-do
expectations there for the recursive backend so we retain its stability
until we are ready to deprecate and remove it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6423-merge-rename-directories.sh | 195 ++++++++++++++++++----------
1 file changed, 124 insertions(+), 71 deletions(-)
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 32e6925ca4..db694a6316 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -3214,6 +3214,7 @@ test_expect_success '10a: Overwrite untracked with normal rename/delete' '
echo important >z/d &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
+ test_path_is_missing .git/MERGE_HEAD &&
test_i18ngrep "The following untracked working tree files would be overwritten by merge" err &&
git ls-files -s >out &&
@@ -3283,21 +3284,34 @@ test_expect_success '10b: Overwrite untracked with dir rename + delete' '
echo contents >y/e &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out &&
- test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err &&
- git ls-files -s >out &&
- test_line_count = 3 out &&
- git ls-files -u >out &&
- test_line_count = 2 out &&
- git ls-files -o >out &&
- test_line_count = 5 out &&
+ git ls-files -s >out &&
+ test_line_count = 1 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 5 out
+ else
+ test_i18ngrep "CONFLICT (rename/delete).*Version B\^0 of y/d left in tree at y/d~B\^0" out &&
+ test_i18ngrep "Error: Refusing to lose untracked file at y/e; writing to y/e~B\^0 instead" out &&
- git rev-parse >actual \
- :0:y/b :3:y/d :3:y/e &&
- git rev-parse >expect \
- O:z/b O:z/c B:z/e &&
- test_cmp expect actual &&
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 5 out &&
+
+ git rev-parse >actual \
+ :0:y/b :3:y/d :3:y/e &&
+ git rev-parse >expect \
+ O:z/b O:z/c B:z/e &&
+ test_cmp expect actual
+ fi &&
echo very >expect &&
test_cmp expect y/c &&
@@ -3360,25 +3374,38 @@ test_expect_success '10c1: Overwrite untracked with dir rename/rename(1to2)' '
echo important >y/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err &&
- git ls-files -s >out &&
- test_line_count = 6 out &&
- git ls-files -u >out &&
- test_line_count = 3 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~B\^0 instead" out &&
- git rev-parse >actual \
- :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :3:y/c &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
- test_cmp expect actual &&
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- git hash-object y/c~B^0 >actual &&
- git rev-parse O:x/c >expect &&
- test_cmp expect actual &&
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:x/d :1:x/c :2:w/c :3:y/c &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
+ test_cmp expect actual &&
+
+ git hash-object y/c~B^0 >actual &&
+ git rev-parse O:x/c >expect &&
+ test_cmp expect actual
+ fi &&
echo important >expect &&
test_cmp expect y/c
@@ -3398,25 +3425,38 @@ test_expect_success '10c2: Overwrite untracked with dir rename/rename(1to2), oth
echo important >y/c &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive A^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err &&
- git ls-files -s >out &&
- test_line_count = 6 out &&
- git ls-files -u >out &&
- test_line_count = 3 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out &&
- git rev-parse >actual \
- :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
- test_cmp expect actual &&
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
- git hash-object y/c~HEAD >actual &&
- git rev-parse O:x/c >expect &&
- test_cmp expect actual &&
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
+ test_cmp expect actual &&
+
+ git hash-object y/c~HEAD >actual &&
+ git rev-parse O:x/c >expect &&
+ test_cmp expect actual
+ fi &&
echo important >expect &&
test_cmp expect y/c
@@ -3474,37 +3514,50 @@ test_expect_success '10d: Delete untracked with dir rename/rename(2to1)' '
echo important >y/wham &&
test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err &&
- test_i18ngrep "CONFLICT (rename/rename)" out &&
- test_i18ngrep "Refusing to lose untracked file at y/wham" out &&
+ if test "$GIT_TEST_MERGE_ALGORITHM" = ort
+ then
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_i18ngrep "error: The following untracked working tree files would be overwritten by merge" err &&
- git ls-files -s >out &&
- test_line_count = 6 out &&
- git ls-files -u >out &&
- test_line_count = 2 out &&
- git ls-files -o >out &&
- test_line_count = 3 out &&
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 0 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out
+ else
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose untracked file at y/wham" out &&
- git rev-parse >actual \
- :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham &&
- git rev-parse >expect \
- O:z/a O:z/b O:x/d O:x/e O:z/c O:x/f &&
- test_cmp expect actual &&
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
+
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham &&
+ git rev-parse >expect \
+ O:z/a O:z/b O:x/d O:x/e O:z/c O:x/f &&
+ test_cmp expect actual &&
- test_must_fail git rev-parse :1:y/wham &&
+ test_must_fail git rev-parse :1:y/wham &&
- echo important >expect &&
- test_cmp expect y/wham &&
+ # Test that two-way merge in y/wham~merged is as expected
+ git cat-file -p :2:y/wham >expect &&
+ git cat-file -p :3:y/wham >other &&
+ >empty &&
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "B^0" \
+ expect empty other &&
+ test_cmp expect y/wham~merged
+ fi &&
- # Test that the two-way merge in y/wham~merged is as expected
- git cat-file -p :2:y/wham >expect &&
- git cat-file -p :3:y/wham >other &&
- >empty &&
- test_must_fail git merge-file \
- -L "HEAD" \
- -L "" \
- -L "B^0" \
- expect empty other &&
- test_cmp expect y/wham~merged
+ echo important >expect &&
+ test_cmp expect y/wham
)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 9/9] t6423: add more details about direct resolution of directories
2020-10-26 17:01 ` [PATCH v2 0/9] Support both merge backends in the testsuite, via environment variable Elijah Newren via GitGitGadget
` (7 preceding siblings ...)
2020-10-26 17:01 ` [PATCH v2 8/9] t6423: note improved ort handling with untracked files Elijah Newren via GitGitGadget
@ 2020-10-26 17:01 ` Elijah Newren via GitGitGadget
8 siblings, 0 replies; 31+ messages in thread
From: Elijah Newren via GitGitGadget @ 2020-10-26 17:01 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Đoàn Trần Công Danh,
Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6423-merge-rename-directories.sh | 39 +++++++++++++++++------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index db694a6316..4ab133f489 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -4725,20 +4725,22 @@ test_expect_success '12e: Rename/merge subdir into the root, variant 2' '
# folder/subdir/newsubdir/e_Merge2
# folder/subdir/tweaked/{h,Makefile_SUB_Merge1,newfile.py}
# folder/unchanged/<LOTS OF FILES>
-#
-# Notes: This testcase happens to exercise lots of the
-# optimization-specific codepaths in merge-ort, and also
-# demonstrated a failing of the directory rename detection algorithm
-# in merge-recursive; newfile.c should not get pushed into
-# folder/subdir/newsubdir/, yet merge-recursive put it there because
-# the rename of dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d}
-# looks like
-# dir/ -> folder/,
-# whereas the rename of dir/subdir/e -> folder/subdir/newsubdir/e
-# looks like
-# dir/subdir/ -> folder/subdir/newsubdir/
-# and if we note that newfile.c is found in dir/subdir/, we might
-# overlook the dir/ -> folder/ rule that has more weight.
+# Things being checked here:
+# 1. dir/subdir/newfile.c does not get pushed into folder/subdir/newsubdir/.
+# dir/subdir/{a,b,c,d} -> folder/subdir/{a,b,c,d} looks like
+# dir/ -> folder/,
+# whereas dir/subdir/e -> folder/subdir/newsubdir/e looks like
+# dir/subdir/ -> folder/subdir/newsubdir/
+# and if we note that newfile.c is found in dir/subdir/, we might overlook
+# the dir/ -> folder/ rule that has more weight. Older git versions did
+# this.
+# 2. The code to do trivial directory resolves. Note that
+# dir/subdir/unchanged/ is unchanged and can be deleted, and files in the
+# new folder/subdir/unchanged/ are not needed as a target to any renames.
+# Thus, in the second collect_merge_info_callback() we can just resolve
+# these two directories trivially without recursing.)
+# 3. Exercising the codepaths for caching renames and deletes from one cherry
+# pick and re-applying them in the subsequent one.
test_setup_12f () {
test_create_repo 12f &&
@@ -4803,7 +4805,7 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac
git checkout A^0 &&
git branch Bmod B &&
- git -c merge.directoryRenames=true rebase A Bmod &&
+ GIT_TRACE2_PERF="$(pwd)/trace.output" git -c merge.directoryRenames=true rebase A Bmod &&
echo Checking the pick of B1... &&
@@ -4884,7 +4886,12 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac
test_seq 2 12 >e_Merge2 &&
git hash-object e_Merge2 >expect &&
git rev-parse Bmod:folder/subdir/newsubdir/e >actual &&
- test_cmp expect actual
+ test_cmp expect actual &&
+
+ grep region_enter.*collect_merge_info trace.output >collect &&
+ test_line_count = 4 collect &&
+ grep region_enter.*process_entries$ trace.output >process &&
+ test_line_count = 2 process
)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 31+ messages in thread