Re: [PATCH] mv: let 'git mv file no-such-dir/' error out

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Dec 4, 2013, 08:44 UTC
Message-ID: <vpqa9ghhunu.fsf@anie.imag.fr>
In-Reply-To: <CACsJy8CfvdT2LgRHax3P6k94C3S9ScB4fuNRm7bZWGXRW=NqKA@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 19 quoted lines

> On Tue, Dec 3, 2013 at 3:32 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>> Git used to trim the trailing slash, and make the command equivalent to
>> 'git mv file no-such-dir', which created the file no-such-dir (while the
>> trailing slash explicitly stated that it could only be a directory).
>>
>> This patch skips the trailing slash removal for the destination path. The
>> path with its trailing slash is passed to rename(2), which errors out
>> with the appropriate message:
>>
>>   $ git mv file no-such-dir/
>>   fatal: renaming 'file' failed: Not a directory
>
> There's something we probably should check. In d78b0f3 ([PATCH]
> git-mv: add more path normalization - 2006-08-16), it mentions about
>
> git mv something/ somewhere/
>
> there's no test in that commit so I don't know the actual input and
> expected outcome.

To me, the expected outcome is "behave like Unix's mv" (which works with or without the trailing slash if somewhere exists).

> If "somewhere" is a directory, it errors out with this patch and works
> without it.

I can't reproduce. I've added this to my patch (indeed, the area wasn't well tested), and the tests pass.

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index e5c8084..3bfdfed 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -70,12 +70,31 @@ test_expect_success \
 rm -f idontexist untracked1 untracked2 \
      path0/idontexist path0/untracked1 path0/untracked2 \
      .git/index.lock
+rmdir path1
 
 test_expect_success \
-    'moving to target with trailing slash' \
+    'moving to absent target with trailing slash' \
     'test_must_fail git mv path0/COPYING no-such-dir/ &&
      test_must_fail git mv path0/COPYING no-such-dir// &&
-     git mv path0/ no-such-dir/'
+     git mv path0/ no-such-dir/ &&
+     test_path_is_dir no-such-dir'
+
+test_expect_success \
+    'clean up' \
+    'git reset --hard'
+
+test_expect_success \
+    'moving to existing untracked target with trailing slash' \
+    'mkdir path1 &&
+     git mv path0/ path1/ &&
+     test_path_is_dir path1/path0/'
+
+test_expect_success \
+    'moving to existing tracked target with trailing slash' \
+    'mkdir path2 &&
+     >path2/file && git add path2/file &&
+     git mv path1/path0/ path2/ &&
+     test_path_is_dir path2/path0/'
 
 test_expect_success \
     'clean up' \

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/