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

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

[Matthieu Moy]

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

Original-patch-by: Duy Nguyen <pclouds@gmail.com>
Tests, tweaks and commit message by: Matthieu Moy <Matthieu.Moy@imag.fr>

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/mv.c  | 23 ++++++++++++++++-------
 t/t7001-mv.sh | 10 ++++++++++
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 2e0e61b..08fbc03 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -16,9 +16,12 @@ static const char * const builtin_mv_usage[] = {
 	NULL
 };
 
+#define DUP_BASENAME 1
+#define KEEP_TRAILING_SLASH 2
+
 static const char **internal_copy_pathspec(const char *prefix,
 					   const char **pathspec,
-					   int count, int base_name)
+					   int count, unsigned flags)
 {
 	int i;
 	const char **result = xmalloc((count + 1) * sizeof(const char *));
@@ -27,11 +30,12 @@ static const char **internal_copy_pathspec(const char *prefix,
 	for (i = 0; i < count; i++) {
 		int length = strlen(result[i]);
 		int to_copy = length;
-		while (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
+		while (!(flags & KEEP_TRAILING_SLASH) &&
+		       to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
 			to_copy--;
-		if (to_copy != length || base_name) {
+		if (to_copy != length || flags & DUP_BASENAME) {
 			char *it = xmemdupz(result[i], to_copy);
-			if (base_name) {
+			if (flags & DUP_BASENAME) {
 				result[i] = xstrdup(basename(it));
 				free(it);
 			} else
@@ -87,16 +91,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
 	source = internal_copy_pathspec(prefix, argv, argc, 0);
 	modes = xcalloc(argc, sizeof(enum update_mode));
-	dest_path = internal_copy_pathspec(prefix, argv + argc, 1, 0);
+	/*
+	 * Keep trailing slash, needed to let
+	 * "git mv file no-such-dir/" error out.
+	 */
+	dest_path = internal_copy_pathspec(prefix, argv + argc, 1,
+					   KEEP_TRAILING_SLASH);
 	submodule_gitfile = xcalloc(argc, sizeof(char *));
 
 	if (dest_path[0][0] == '\0')
 		/* special case: "." was normalized to "" */
-		destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);
+		destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
 	else if (!lstat(dest_path[0], &st) &&
 			S_ISDIR(st.st_mode)) {
 		dest_path[0] = add_slash(dest_path[0]);
-		destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);
+		destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
 	} else {
 		if (argc != 1)
 			die("destination '%s' is not a directory", dest_path[0]);
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index b90e985..e5c8084 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -72,6 +72,16 @@ rm -f idontexist untracked1 untracked2 \
      .git/index.lock
 
 test_expect_success \
+    'moving to 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/'
+
+test_expect_success \
+    'clean up' \
+    'git reset --hard'
+
+test_expect_success \
     'adding another file' \
     'cp "$TEST_DIRECTORY"/../README path0/README &&
      git add path0/README &&
-- 
1.8.5.rc3.4.g8bd3721

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

[Duy Nguyen]

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. If "somewhere" is a directory, it errors out with
this patch and works without it. If "somewhere" does not exist, it
seems to work like Linux "mv" with or without the patch.

> Original-patch-by: Duy Nguyen <pclouds@gmail.com>
> Tests, tweaks and commit message by: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  builtin/mv.c  | 23 ++++++++++++++++-------
>  t/t7001-mv.sh | 10 ++++++++++
>  2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/mv.c b/builtin/mv.c
> index 2e0e61b..08fbc03 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -16,9 +16,12 @@ static const char * const builtin_mv_usage[] = {
>         NULL
>  };
>
> +#define DUP_BASENAME 1
> +#define KEEP_TRAILING_SLASH 2
> +
>  static const char **internal_copy_pathspec(const char *prefix,
>                                            const char **pathspec,
> -                                          int count, int base_name)
> +                                          int count, unsigned flags)
>  {
>         int i;
>         const char **result = xmalloc((count + 1) * sizeof(const char *));
> @@ -27,11 +30,12 @@ static const char **internal_copy_pathspec(const char *prefix,
>         for (i = 0; i < count; i++) {
>                 int length = strlen(result[i]);
>                 int to_copy = length;
> -               while (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
> +               while (!(flags & KEEP_TRAILING_SLASH) &&
> +                      to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
>                         to_copy--;
> -               if (to_copy != length || base_name) {
> +               if (to_copy != length || flags & DUP_BASENAME) {
>                         char *it = xmemdupz(result[i], to_copy);
> -                       if (base_name) {
> +                       if (flags & DUP_BASENAME) {
>                                 result[i] = xstrdup(basename(it));
>                                 free(it);
>                         } else
> @@ -87,16 +91,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>
>         source = internal_copy_pathspec(prefix, argv, argc, 0);
>         modes = xcalloc(argc, sizeof(enum update_mode));
> -       dest_path = internal_copy_pathspec(prefix, argv + argc, 1, 0);
> +       /*
> +        * Keep trailing slash, needed to let
> +        * "git mv file no-such-dir/" error out.
> +        */
> +       dest_path = internal_copy_pathspec(prefix, argv + argc, 1,
> +                                          KEEP_TRAILING_SLASH);
>         submodule_gitfile = xcalloc(argc, sizeof(char *));
>
>         if (dest_path[0][0] == '\0')
>                 /* special case: "." was normalized to "" */
> -               destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);
> +               destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
>         else if (!lstat(dest_path[0], &st) &&
>                         S_ISDIR(st.st_mode)) {
>                 dest_path[0] = add_slash(dest_path[0]);
> -               destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);
> +               destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
>         } else {
>                 if (argc != 1)
>                         die("destination '%s' is not a directory", dest_path[0]);
> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> index b90e985..e5c8084 100755
> --- a/t/t7001-mv.sh
> +++ b/t/t7001-mv.sh
> @@ -72,6 +72,16 @@ rm -f idontexist untracked1 untracked2 \
>       .git/index.lock
>
>  test_expect_success \
> +    'moving to 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/'
> +
> +test_expect_success \
> +    'clean up' \
> +    'git reset --hard'
> +
> +test_expect_success \
>      'adding another file' \
>      'cp "$TEST_DIRECTORY"/../README path0/README &&
>       git add path0/README &&
> --
> 1.8.5.rc3.4.g8bd3721
>



-- 
Duy

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

[Matthieu Moy]

Duy Nguyen <pclouds@gmail.com> writes:

> 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/

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

[Duy Nguyen]

On Wed, Dec 4, 2013 at 3:44 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Tue, Dec 3, 2013 at 3:32 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>> 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.

Now I can't either. Probably some mis-setups or some local bugs in my
tree. Good that we have more tests.
-- 
Duy

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

[Matthieu Moy]

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

Original-patch-by: Duy Nguyen <pclouds@gmail.com>
Tests, tweaks and commit message by: Matthieu Moy <Matthieu.Moy@imag.fr>

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
So, this patch adds more tests, as suggested by Duy. They all pass
without modifying the code.

 builtin/mv.c  | 23 ++++++++++++++++-------
 t/t7001-mv.sh | 29 +++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 2e0e61b..08fbc03 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -16,9 +16,12 @@ static const char * const builtin_mv_usage[] = {
 	NULL
 };
 
+#define DUP_BASENAME 1
+#define KEEP_TRAILING_SLASH 2
+
 static const char **internal_copy_pathspec(const char *prefix,
 					   const char **pathspec,
-					   int count, int base_name)
+					   int count, unsigned flags)
 {
 	int i;
 	const char **result = xmalloc((count + 1) * sizeof(const char *));
@@ -27,11 +30,12 @@ static const char **internal_copy_pathspec(const char *prefix,
 	for (i = 0; i < count; i++) {
 		int length = strlen(result[i]);
 		int to_copy = length;
-		while (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
+		while (!(flags & KEEP_TRAILING_SLASH) &&
+		       to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
 			to_copy--;
-		if (to_copy != length || base_name) {
+		if (to_copy != length || flags & DUP_BASENAME) {
 			char *it = xmemdupz(result[i], to_copy);
-			if (base_name) {
+			if (flags & DUP_BASENAME) {
 				result[i] = xstrdup(basename(it));
 				free(it);
 			} else
@@ -87,16 +91,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
 	source = internal_copy_pathspec(prefix, argv, argc, 0);
 	modes = xcalloc(argc, sizeof(enum update_mode));
-	dest_path = internal_copy_pathspec(prefix, argv + argc, 1, 0);
+	/*
+	 * Keep trailing slash, needed to let
+	 * "git mv file no-such-dir/" error out.
+	 */
+	dest_path = internal_copy_pathspec(prefix, argv + argc, 1,
+					   KEEP_TRAILING_SLASH);
 	submodule_gitfile = xcalloc(argc, sizeof(char *));
 
 	if (dest_path[0][0] == '\0')
 		/* special case: "." was normalized to "" */
-		destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);
+		destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
 	else if (!lstat(dest_path[0], &st) &&
 			S_ISDIR(st.st_mode)) {
 		dest_path[0] = add_slash(dest_path[0]);
-		destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);
+		destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
 	} else {
 		if (argc != 1)
 			die("destination '%s' is not a directory", dest_path[0]);
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index b90e985..2f82478 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -70,6 +70,35 @@ test_expect_success \
 rm -f idontexist untracked1 untracked2 \
      path0/idontexist path0/untracked1 path0/untracked2 \
      .git/index.lock
+rmdir path1
+
+test_expect_success \
+    '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/ &&
+     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' \
+    'git reset --hard'
 
 test_expect_success \
     'adding another file' \
-- 
1.8.5.rc3.4.g8bd3721

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

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> On Wed, Dec 4, 2013 at 3:44 PM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Duy Nguyen <pclouds@gmail.com> writes:
>>
>>> On Tue, Dec 3, 2013 at 3:32 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>>> 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.
>
> Now I can't either. Probably some mis-setups or some local bugs in my
> tree. Good that we have more tests.

OK, I was also scratching my head after seeing your response.

It seems that t7001 needs some face-lifting, by the way.  Perhaps
after this patch solidifies.

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

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> It seems that t7001 needs some face-lifting, by the way.  Perhaps
> after this patch solidifies.

Yes. I followed the style of surrounding code, but it could be
reformatted to follow the current standard. I have no time for it now.

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

[PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too

[Johannes Sixt]

The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
relies on that rename("src", "dst/") fails if directory dst does not
exist (note the trailing slash). This does not work as expected on Windows:
This rename() call is successful. Insert an explicit check for this case.

This changes the error message from

   $ git mv file no-such-dir/
   fatal: renaming 'file' failed: Not a directory

to

   $ git mv file no-such-dir/
   fatal: destination directory does not exist, source=file, destination=no-such-dir/

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 builtin/mv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/mv.c b/builtin/mv.c
index 08fbc03..21c46d1 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 			}
 		} else if (string_list_has_string(&src_for_dst, dst))
 			bad = _("multiple sources for the same target");
+		else if (is_dir_sep(dst[strlen(dst) - 1]))
+			bad = _("destination directory does not exist");
 		else
 			string_list_insert(&src_for_dst, dst);
 
-- 
1.8.5.2.1473.g6546919

Re: [PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
> relies on that rename("src", "dst/") fails if directory dst does not
> exist (note the trailing slash). This does not work as expected on Windows:
> This rename() call is successful. Insert an explicit check for this case.

Could you care to explain "Successful how" a bit here?  Do we see
no-such-dir mkdir'ed and then no-such-dir/file created?  Do we see
file moved to a new file whose name is no-such-dir/?  I am guessing
that it would be the latter, but if that is the case we would need
at least an air-quote around "successful".

> This changes the error message from
>
>    $ git mv file no-such-dir/
>    fatal: renaming 'file' failed: Not a directory
>
> to
>
>    $ git mv file no-such-dir/
>    fatal: destination directory does not exist, source=file, destination=no-such-dir/
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  builtin/mv.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/builtin/mv.c b/builtin/mv.c
> index 08fbc03..21c46d1 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  			}
>  		} else if (string_list_has_string(&src_for_dst, dst))
>  			bad = _("multiple sources for the same target");
> +		else if (is_dir_sep(dst[strlen(dst) - 1]))
> +			bad = _("destination directory does not exist");
>  		else
>  			string_list_insert(&src_for_dst, dst);

Re: [PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too

[Johannes Sixt]

Am 09.01.2014 23:42, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
>> relies on that rename("src", "dst/") fails if directory dst does not
>> exist (note the trailing slash). This does not work as expected on Windows:
>> This rename() call is successful. Insert an explicit check for this case.
> 
> Could you care to explain "Successful how" a bit here?  Do we see
> no-such-dir mkdir'ed and then no-such-dir/file created?  Do we see
> file moved to a new file whose name is no-such-dir/?  I am guessing
> that it would be the latter, but if that is the case we would need
> at least an air-quote around "successful".

The file is renamed to no-such-dir without the slash at the end. The
updated commit message would be:

mv: let 'git mv file no-such-dir/' error out on Windows, too

The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
relies on that rename("src", "dst/") fails if directory dst does not
exist (note the trailing slash). This does not work as expected on Windows:
The rename() call does not fail, but renames src to dst (without the
trailing slash). Insert an explicit check for this case to force an error.

This changes the error message from

   $ git mv file no-such-dir/
   fatal: renaming 'file' failed: Not a directory

to

   $ git mv file no-such-dir/
   fatal: destination directory does not exist, source=file, destination=no-such-dir/

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

> 
>> This changes the error message from
>>
>>    $ git mv file no-such-dir/
>>    fatal: renaming 'file' failed: Not a directory
>>
>> to
>>
>>    $ git mv file no-such-dir/
>>    fatal: destination directory does not exist, source=file, destination=no-such-dir/
>>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> ---
>>  builtin/mv.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/builtin/mv.c b/builtin/mv.c
>> index 08fbc03..21c46d1 100644
>> --- a/builtin/mv.c
>> +++ b/builtin/mv.c
>> @@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>>  			}
>>  		} else if (string_list_has_string(&src_for_dst, dst))
>>  			bad = _("multiple sources for the same target");
>> +		else if (is_dir_sep(dst[strlen(dst) - 1]))
>> +			bad = _("destination directory does not exist");
>>  		else
>>  			string_list_insert(&src_for_dst, dst);
> 

Re: [PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> The file is renamed to no-such-dir without the slash at the end. The
> updated commit message would be:
>
> mv: let 'git mv file no-such-dir/' error out on Windows, too
>
> The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
> relies on that rename("src", "dst/") fails if directory dst does not
> exist (note the trailing slash). This does not work as expected on Windows:
> The rename() call does not fail, but renames src to dst (without the
> trailing slash). Insert an explicit check for this case to force an error.
>
> This changes the error message from
>
>    $ git mv file no-such-dir/
>    fatal: renaming 'file' failed: Not a directory
>
> to
>
>    $ git mv file no-such-dir/
>    fatal: destination directory does not exist, source=file, destination=no-such-dir/
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Thanks.