All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>, Elijah Newren <newren@gmail.com>
Subject: [PATCH] merge-ort: fix bug with renormalization and rename/delete conflicts
Date: Tue, 28 Dec 2021 00:20:46 +0000	[thread overview]
Message-ID: <pull.1174.git.git.1640650846612.gitgitgadget@gmail.com> (raw)

From: Elijah Newren <newren@gmail.com>

Ever since commit a492d5331c ("merge-ort: ensure we consult df_conflict
and path_conflicts", 2021-06-30), when renormalization is active AND a
file is involved in a rename/delete conflict BUT the file is unmodified
(either before or after renormalization), merge-ort was running into an
assertion failure.  Prior to that commit (or if assertions were compiled
out), merge-ort would mis-merge instead, ignoring the rename/delete
conflict and just deleting the file.

Remove the assertions, fix the code appropriately, leave some good
comments in the code, and add a testcase for this situation.

Reported-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
    merge-ort: fix bug with renormalization and rename/delete conflicts
    
    Original report:
    https://lore.kernel.org/git/CAN0XMOK8iHZnbtYw7CPAQGJcmuVSDxQoFNFEwiaa41V89F1rzA@mail.gmail.com/
    
    Built in v2.34.1, but rebases onto and/or merges cleanly with newer
    versions.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1174%2Fnewren%2Fmerge-ort-rename-delete-renormalization-bug-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1174/newren/merge-ort-rename-delete-renormalization-bug-v1
Pull-Request: https://github.com/git/git/pull/1174

 merge-ort.c                | 19 ++++++++++++++++---
 t/t6418-merge-text-auto.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/merge-ort.c b/merge-ort.c
index 0342f104836..c3197970219 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -3841,9 +3841,22 @@ static void process_entry(struct merge_options *opt,
 		if (opt->renormalize &&
 		    blob_unchanged(opt, &ci->stages[0], &ci->stages[side],
 				   path)) {
-			ci->merged.is_null = 1;
-			ci->merged.clean = 1;
-			assert(!ci->df_conflict && !ci->path_conflict);
+			if (!ci->path_conflict) {
+				/*
+				 * Blob unchanged after renormalization, so
+				 * there's no modify/delete conflict after all;
+				 * we can just remove the file.
+				 */
+				ci->merged.is_null = 1;
+				ci->merged.clean = 1;
+				 /*
+				  * file goes away => even if there was a
+				  * directory/file conflict there isn't one now.
+				  */
+				ci->df_conflict = 0;
+			} else {
+				/* rename/delete, so conflict remains */
+			}
 		} else if (ci->path_conflict &&
 			   oideq(&ci->stages[0].oid, &ci->stages[side].oid)) {
 			/*
diff --git a/t/t6418-merge-text-auto.sh b/t/t6418-merge-text-auto.sh
index 1e0296dd172..41288a60ceb 100755
--- a/t/t6418-merge-text-auto.sh
+++ b/t/t6418-merge-text-auto.sh
@@ -204,4 +204,30 @@ test_expect_success 'Test delete/normalize conflict' '
 	test_path_is_missing file
 '
 
+test_expect_success 'rename/delete vs. renormalization' '
+	git init subrepo &&
+	(
+		cd subrepo &&
+		echo foo >oldfile &&
+		git add oldfile &&
+		git commit -m original &&
+
+		git branch rename &&
+		git branch nuke &&
+
+		git checkout rename &&
+		git mv oldfile newfile &&
+		git commit -m renamed &&
+
+		git checkout nuke &&
+		git rm oldfile &&
+		git commit -m deleted &&
+
+		git checkout rename^0 &&
+		test_must_fail git -c merge.renormalize=true merge nuke >out &&
+
+		grep "rename/delete" out
+	)
+'
+
 test_done

base-commit: e9d7761bb94f20acc98824275e317fa82436c25d
-- 
gitgitgadget

             reply	other threads:[~2021-12-28  0:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-28  0:20 Elijah Newren via GitGitGadget [this message]
2021-12-28 13:55 ` [PATCH] merge-ort: fix bug with renormalization and rename/delete conflicts Derrick Stolee
2021-12-30 22:56   ` Junio C Hamano
2021-12-30 23:35     ` Elijah Newren
2021-12-30 22:08 ` [PATCH v2] " 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=pull.1174.git.git.1640650846612.gitgitgadget@gmail.com \
    --to=gitgitgadget@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.