directory rename detection: partially renamed directory testcase/discussion
Add a long note about why we are not considering "partial directory renames" for the current directory rename detection implementation. Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
		
				
					committed by
					
						
						Junio C Hamano
					
				
			
			
				
	
			
			
			
						parent
						
							21b53733a0
						
					
				
				
					commit
					de632e4ed3
				
			@ -735,4 +735,119 @@ test_expect_success '3b-check: Avoid implicit rename if involved as source on cu
 | 
				
			|||||||
#   of a rename on either side of a merge.
 | 
					#   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.
 | 
				
			||||||
 | 
					#
 | 
				
			||||||
 | 
					# Note that the wording of the rule ("We don't do directory rename
 | 
				
			||||||
 | 
					# detection if the directory still exists on both sides of the merge.")
 | 
				
			||||||
 | 
					# also excludes "renaming" of a directory into a subdirectory of itself
 | 
				
			||||||
 | 
					# (e.g. /some/dir/* -> /some/dir/subdir/*).  It may be possible to carve
 | 
				
			||||||
 | 
					# out an exception for "renaming"-beneath-itself cases without opening
 | 
				
			||||||
 | 
					# weird edge/corner cases for other partial directory renames, but for now
 | 
				
			||||||
 | 
					# we are keeping the rule simple.
 | 
				
			||||||
 | 
					#
 | 
				
			||||||
 | 
					# This section contains a test for a 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 &&
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
							git ls-files -s >out &&
 | 
				
			||||||
 | 
							test_line_count = 5 out &&
 | 
				
			||||||
 | 
							git ls-files -u >out &&
 | 
				
			||||||
 | 
							test_line_count = 0 out &&
 | 
				
			||||||
 | 
							git ls-files -o >out &&
 | 
				
			||||||
 | 
							test_line_count = 1 out &&
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
							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
 | 
					test_done
 | 
				
			||||||
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user