All of lore.kernel.org
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: sbeller@google.com, gitster@pobox.com, Elijah Newren <newren@gmail.com>
Subject: [PATCH v4 07/34] directory rename detection: partially renamed directory testcase/discussion
Date: Tue, 28 Nov 2017 17:42:10 -0800	[thread overview]
Message-ID: <20171129014237.32570-8-newren@gmail.com> (raw)
In-Reply-To: <20171129014237.32570-1-newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6043-merge-rename-directories.sh | 104 ++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index 0ccabed4a2..1dcf010aa6 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -714,4 +714,108 @@ test_expect_success '3b-check: Avoid implicit rename if involved as source on cu
 #   of a rename on either side of a merge.
 ###########################################################################
 
+
+###########################################################################
+# SECTION 4: Partially renamed directory; still exists on both sides of merge
+#
+# What if we were to attempt to do directory rename detection when someone
+# "mostly" moved a directory but still left some files around, or,
+# equivalently, fully renamed a directory in one commmit and then recreated
+# that directory in a later commit adding some new files and then tried to
+# merge?
+#
+# It's hard to divine user intent in these cases, because you can make an
+# argument that, depending on the intermediate history of the side being
+# merged, that some users will want files in that directory to
+# automatically be detected and renamed, while users with a different
+# intermediate history wouldn't want that rename to happen.
+#
+# I think that it is best to simply not have directory rename detection
+# apply to such cases.  My reasoning for this is four-fold: (1) it's
+# easiest for users in general to figure out what happened if we don't
+# apply directory rename detection in any such case, (2) it's an easy rule
+# to explain ["We don't do directory rename detection if the directory
+# still exists on both sides of the merge"], (3) we can get some hairy
+# edge/corner cases that would be really confusing and possibly not even
+# representable in the index if we were to even try, and [related to 3] (4)
+# attempting to resolve this issue of divining user intent by examining
+# intermediate history goes against the spirit of three-way merges and is a
+# path towards crazy corner cases that are far more complex than what we're
+# already dealing with.
+#
+# This section contains a test for this partially-renamed-directory case.
+###########################################################################
+
+# Testcase 4a, Directory split, with original directory still present
+#   (Related to testcase 1f)
+#   Commit O: z/{b,c,d,e}
+#   Commit A: y/{b,c,d}, z/e
+#   Commit B: z/{b,c,d,e,f}
+#   Expected: y/{b,c,d}, z/{e,f}
+#   NOTE: Even though most files from z moved to y, we don't want f to follow.
+
+test_expect_success '4a-setup: Directory split, with original directory still present' '
+	test_create_repo 4a &&
+	(
+		cd 4a &&
+
+		mkdir z &&
+		echo b >z/b &&
+		echo c >z/c &&
+		echo d >z/d &&
+		echo e >z/e &&
+		git add z &&
+		test_tick &&
+		git commit -m "O" &&
+
+		git branch O &&
+		git branch A &&
+		git branch B &&
+
+		git checkout A &&
+		mkdir y &&
+		git mv z/b y/ &&
+		git mv z/c y/ &&
+		git mv z/d y/ &&
+		test_tick &&
+		git commit -m "A" &&
+
+		git checkout B &&
+		echo f >z/f &&
+		git add z/f &&
+		test_tick &&
+		git commit -m "B"
+	)
+'
+
+test_expect_success '4a-check: Directory split, with original directory still present' '
+	(
+		cd 4a &&
+
+		git checkout A^0 &&
+
+		git merge -s recursive B^0 &&
+
+		test 5 -eq $(git ls-files -s | wc -l) &&
+		test 0 -eq $(git ls-files -u | wc -l) &&
+		test 0 -eq $(git ls-files -o | wc -l) &&
+
+		git rev-parse >actual \
+			HEAD:y/b HEAD:y/c HEAD:y/d HEAD:z/e HEAD:z/f &&
+		git rev-parse >expect \
+			O:z/b O:z/c O:z/d O:z/e B:z/f &&
+		test_cmp expect actual
+	)
+'
+
+###########################################################################
+# Rules suggested by section 4:
+#
+#   Directory-rename-detection should be turned off for any directories (as
+#   a source for renames) that exist on both sides of the merge.  (The "as
+#   a source for renames" clarification is due to cases like 1c where
+#   the target directory exists on both sides and we do want the rename
+#   detection.)  But, sadly, see testcase 8b.
+###########################################################################
+
 test_done
-- 
2.15.0.408.g850bc54b15


  parent reply	other threads:[~2017-11-29  1:44 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-29  1:42 [PATCH v4 00/34] Add directory rename detection to git Elijah Newren
2017-11-29  1:42 ` [PATCH v4 01/34] Tighten and correct a few testcases for merging and cherry-picking Elijah Newren
2017-11-29  1:42 ` [PATCH v4 02/34] merge-recursive: fix logic ordering issue Elijah Newren
2017-11-29  1:42 ` [PATCH v4 03/34] merge-recursive: add explanation for src_entry and dst_entry Elijah Newren
2017-11-29  1:42 ` [PATCH v4 04/34] directory rename detection: basic testcases Elijah Newren
2017-11-29  1:42 ` [PATCH v4 05/34] directory rename detection: directory splitting testcases Elijah Newren
2017-11-29  1:42 ` [PATCH v4 06/34] directory rename detection: testcases to avoid taking detection too far Elijah Newren
2017-11-29  1:42 ` Elijah Newren [this message]
2017-11-29  1:42 ` [PATCH v4 08/34] directory rename detection: files/directories in the way of some renames Elijah Newren
2017-11-29  1:42 ` [PATCH v4 09/34] directory rename detection: testcases checking which side did the rename Elijah Newren
2017-11-29  1:42 ` [PATCH v4 10/34] directory rename detection: more involved edge/corner testcases Elijah Newren
2017-11-29  1:42 ` [PATCH v4 11/34] directory rename detection: testcases exploring possibly suboptimal merges Elijah Newren
2017-11-29  1:42 ` [PATCH v4 12/34] directory rename detection: miscellaneous testcases to complete coverage Elijah Newren
2017-11-29  1:42 ` [PATCH v4 13/34] directory rename detection: tests for handling overwriting untracked files Elijah Newren
2017-11-29  1:42 ` [PATCH v4 14/34] directory rename detection: tests for handling overwriting dirty files Elijah Newren
2017-11-29  1:42 ` [PATCH v4 15/34] merge-recursive: move the get_renames() function Elijah Newren
2017-11-29  1:42 ` [PATCH v4 16/34] merge-recursive: introduce new functions to handle rename logic Elijah Newren
2017-11-29  1:42 ` [PATCH v4 17/34] merge-recursive: fix leaks of allocated renames and diff_filepairs Elijah Newren
2017-11-29  1:42 ` [PATCH v4 18/34] merge-recursive: make !o->detect_rename codepath more obvious Elijah Newren
2017-11-29  1:42 ` [PATCH v4 19/34] merge-recursive: split out code for determining diff_filepairs Elijah Newren
2017-11-29  1:42 ` [PATCH v4 20/34] merge-recursive: add a new hashmap for storing directory renames Elijah Newren
2017-11-29  1:42 ` [PATCH v4 21/34] merge-recursive: make a helper function for cleanup for handle_renames Elijah Newren
2017-11-29  1:42 ` [PATCH v4 22/34] merge-recursive: add get_directory_renames() Elijah Newren
2017-11-29  1:42 ` [PATCH v4 23/34] merge-recursive: check for directory level conflicts Elijah Newren
2017-11-29  1:42 ` [PATCH v4 24/34] merge-recursive: add a new hashmap for storing file collisions Elijah Newren
2017-11-29  1:42 ` [PATCH v4 25/34] merge-recursive: add computation of collisions due to dir rename & merging Elijah Newren
2017-11-29  1:42 ` [PATCH v4 26/34] merge-recursive: check for file level conflicts then get new name Elijah Newren
2017-11-29  1:42 ` [PATCH v4 27/34] merge-recursive: when comparing files, don't include trees Elijah Newren
2017-11-29  1:42 ` [PATCH v4 28/34] merge-recursive: apply necessary modifications for directory renames Elijah Newren
2017-11-29  1:42 ` [PATCH v4 29/34] merge-recursive: avoid clobbering untracked files with " Elijah Newren
2017-11-29  1:42 ` [PATCH v4 30/34] merge-recursive: fix overwriting dirty files involved in renames Elijah Newren
2017-11-29  1:42 ` [PATCH v4 31/34] merge-recursive: fix remaining directory rename + dirty overwrite cases Elijah Newren
2017-11-29  1:42 ` [PATCH v4 32/34] directory rename detection: new testcases showcasing a pair of bugs Elijah Newren
2017-11-29  1:42 ` [PATCH v4 33/34] merge-recursive: avoid spurious rename/rename conflict from dir renames Elijah Newren
2017-11-29  1:42 ` [PATCH v4 34/34] merge-recursive: ensure we write updates for directory-renamed file Elijah Newren
2017-12-13  1:06 ` [PATCH v4 00/34] Add directory rename detection to git Junio C Hamano
2017-12-13  2:01   ` Junio C Hamano
2017-12-13 15:38     ` Elijah Newren
2017-12-13 18:15   ` Ramsay Jones
2017-12-13 19:05     ` Junio C Hamano

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=20171129014237.32570-8-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.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.