git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Elijah Newren" <newren@gmail.com>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Elijah Newren" <newren@gmail.com>
Subject: [PATCH v2 7/9] t6423, t6436: note improved ort handling with dirty files
Date: Mon, 26 Oct 2020 17:01:42 +0000	[thread overview]
Message-ID: <c5350fd0aef44e5d7658ae1082d745a6a148cc73.1603731704.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.769.v2.git.1603731704.gitgitgadget@gmail.com>

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


  parent reply	other threads:[~2020-10-26 17:02 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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:48   ` Junio C Hamano
2020-10-23 17:25     ` Elijah Newren
2020-10-23 18:27       ` Elijah Newren
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
2020-10-26 14:56         ` Elijah Newren
2020-10-26 17:43           ` Junio C Hamano
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
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 ` [PATCH 4/9] t6404, t6423: expect improved rename/delete handling in ort backend Elijah Newren via GitGitGadget
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 ` [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
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 ` [PATCH 8/9] t6423: note improved ort handling with untracked files 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-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
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   ` [PATCH v2 3/9] t6416: correct expectation for rename/rename(1to2) + directory/file 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
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   ` [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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c5350fd0aef44e5d7658ae1082d745a6a148cc73.1603731704.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).