Web lists-archives.com

Re: [PATCH 07/30] directory rename detection: partially renamed directory testcase/discussion




On Fri, Nov 10, 2017 at 11:05 AM, Elijah Newren <newren@xxxxxxxxx> wrote:
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> ---
>  t/t6043-merge-rename-directories.sh | 100 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 100 insertions(+)
>
> diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
> index 021513ec00..ec054b210a 100755
> --- a/t/t6043-merge-rename-directories.sh
> +++ b/t/t6043-merge-rename-directories.sh
> @@ -650,4 +650,104 @@ 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 last paragraph ("I think") sounds like part of a commit message,
rather than
a note inside a testing script. Not sure if I recommend moving this
text into the
commit message.

> +# 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 A: z/{b,c,d,e}
> +#   Commit B: y/{b,c,d}, z/e
> +#   Commit C: 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.

Makes sense.

> +###########################################################################
> +# 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.

Looking forward for that test case.