git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / 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 v3 11/11] t6425: be more flexible with rename/delete conflict messages
Date: Mon, 10 Aug 2020 22:29:19 +0000	[thread overview]
Message-ID: <0c8dcbf01cc7bb78f62291b645d56d344b24e5fd.1597098559.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.827.v3.git.git.1597098559.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

t6425 was very picky about the exact output message produced by a
rename/delete conflict, in a way that just scratches the surface of the
mess that was built into merge-recursive.  The idea was that it would
try to find the possible combinations of different conflict types, and
when more than one was present for one path, it would try to provide a
combined message that covered all the cases.

There's a lot to unravel here...

First, there's a basic conflict type known as modify/delete, which is a
content conflict.  It occurs when one side deletes a file, but the other
modifies it.

There is also a path conflict known as a rename/delete.  This occurs
when one side deletes a path, and the other renames it.  This is not a
content conflict, it is a path conflict.  It will often occur in
combination with a content conflict, though, namely a modify/delete.  As
such, these two were often combined.

Another type of conflict that can exist is a directory/file conflict.
For example, one side adds a new file at some path, and the other side
of history adds a directory at the same path.  The path that was "added"
could have been put there by a rename, though.  Thus, we have the
possibility of a single path being affected by a modify/delete, a
rename/delete, and a directory/file conflict.

In part, this was a natural by-product of merge-recursive's design.
Since it was doing a four way merge with the contents of the working
tree being the fourth factor it had to consider, it had working tree
handling spread all over the code.  It also had directory/file conflict
handling spread everywhere through all the other types of conflicts.
And our testsuite has a huge number of directory/file conflict tests
because trying to get them right required modifying so many different
codepaths.  A natural outgrowth of this kind of structure is conflict
messages that combine all the different types that the current codepath
is considering.

However, if we want to make the different conflict types orthogonal and
avoid repeating ourselves and getting very brittle code, then we need to
split the messages from these different conflict types apart.  Besides,
trying to determine all possible permutations is a _royal_ mess.  The
code to handle the rename/delete/directory/file conflict output is
already somewhat hard to parse, and is somewhat brittle.  But if we
really wanted to go that route, then we'd have to have special handling
for the following types of combinations:
  * rename/add/delete:
      on side of history that didn't rename the given file, remove the file
      instead and place an unrelated file in the way of the rename
  * rename/rename(2to1)/mode conflict/delete/delete:
      two different files, one executable and the other not, are renamed
      to the same location, each side deletes the source file that the
      other side renames
  * rename/rename(1to2)/add/add:
      file renamed differently on each side of history, with each side
      placing an unrelated file in the way of the other
  * rename/rename(1to2)/content conflict/file location/(D/F)/(D/F)/:
      both sides modify a file in conflicting way, both rename that file
      but to different paths, one side renames the directory which the
      other side had renamed that file into causing it to possibly need a
      transitive rename, and each side puts a directory in the way of the
      other's path.

Let's back away from this path of insanity, and allow the different
types of conflicts to be handled by separate pieces of non-repeated code
by allowing the conflict messages to be split into their separate types.
(If multiple conflict types affect a single path, the conflict messages
can be printed sequentially.)  Start this path with a simple change:
modify this test to be more flexible and accept the output either merge
backend (recursive or the new ort) will produce.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6425-merge-rename-delete.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t6425-merge-rename-delete.sh b/t/t6425-merge-rename-delete.sh
index 5d33577d2f..f79d021590 100755
--- a/t/t6425-merge-rename-delete.sh
+++ b/t/t6425-merge-rename-delete.sh
@@ -17,7 +17,8 @@ test_expect_success 'rename/delete' '
 	git commit -m "delete" &&
 
 	test_must_fail git merge --strategy=recursive rename >output &&
-	test_i18ngrep "CONFLICT (rename/delete): A deleted in HEAD and renamed to B in rename. Version rename of B left in tree." output
+	test_i18ngrep "CONFLICT (rename/delete): A.* renamed .*to B.* in rename" output &&
+	test_i18ngrep "CONFLICT (rename/delete): A.*deleted in HEAD." output
 '
 
 test_done
-- 
gitgitgadget

      parent reply	other threads:[~2020-08-10 22:29 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-05 16:29 [PATCH] Collect merge-related tests to t64xx Elijah Newren via GitGitGadget
2020-08-08 17:01 ` [PATCH v2 00/11] Start preparing merge-related tests to work with multiple merge backends Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 01/11] Collect merge-related tests to t64xx Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 02/11] t6418: tighten delete/normalize conflict testcase Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 03/11] t6422: fix bad check against missing file Elijah Newren via GitGitGadget
2020-08-09  2:30     ` Eric Sunshine
2020-08-08 17:01   ` [PATCH v2 04/11] t6416, t6422: fix incorrect untracked file count Elijah Newren via GitGitGadget
2020-08-09  3:12     ` Eric Sunshine
2020-08-09  5:34       ` Elijah Newren
2020-08-08 17:01   ` [PATCH v2 05/11] t6423: fix test setup for a couple tests Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 06/11] t6422: fix multiple errors with the mod6 test expectations Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 07/11] t6416, t6423: clarify some comments and fix some typos Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 08/11] t6423: add an explanation about why one of the tests does not pass Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 09/11] t6422, t6426: be more flexible for add/add conflicts involving renames Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 10/11] t642[23]: be more flexible for add/add conflicts involving pair renames Elijah Newren via GitGitGadget
2020-08-08 17:01   ` [PATCH v2 11/11] t6425: be more flexible with rename/delete conflict messages Elijah Newren via GitGitGadget
2020-08-10 22:29   ` [PATCH v3 00/11] Start preparing merge-related tests to work with multiple merge backends Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 01/11] Collect merge-related tests to t64xx Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 02/11] t6418: tighten delete/normalize conflict testcase Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 03/11] t6422: fix bad check against missing file Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 04/11] t6416, t6422: fix incorrect untracked file count Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 05/11] t6423: fix test setup for a couple tests Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 06/11] t6422: fix multiple errors with the mod6 test expectations Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 07/11] t6416, t6423: clarify some comments and fix some typos Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 08/11] t6423: add an explanation about why one of the tests does not pass Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 09/11] t6422, t6426: be more flexible for add/add conflicts involving renames Elijah Newren via GitGitGadget
2020-08-10 22:29     ` [PATCH v3 10/11] t642[23]: be more flexible for add/add conflicts involving pair renames Elijah Newren via GitGitGadget
2020-08-10 22:29     ` Elijah Newren via GitGitGadget [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=0c8dcbf01cc7bb78f62291b645d56d344b24e5fd.1597098559.git.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).