{"thread":{"id":"35459","subject":"[PATCH] mv: let 'git mv file no-such-dir/' error out","startedAt":"2013-12-03T08:32:04Z","lastAt":"2014-01-10T19:30:54Z","messageCount":11,"participants":["Matthieu Moy","Duy Nguyen","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"231412","messageId":"1386059524-14442-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"35459","inReplyTo":null,"subject":"[PATCH] mv: let 'git mv file no-such-dir/' error out","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-12-03T08:32:04Z","receivedAt":"2013-12-03T08:32:04Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Git used to trim the trailing slash, and make the command equivalent to\n'git mv file no-such-dir', which created the file no-such-dir (while the\ntrailing slash explicitly stated that it could only be a directory).\n\nThis patch skips the trailing slash removal for the destination path. The\npath with its trailing slash is passed to rename(2), which errors out\nwith the appropriate message:\n\n  $ git mv file no-such-dir/\n  fatal: renaming 'file' failed: Not a directory\n\nOriginal-patch-by: Duy Nguyen <pclouds@gmail.com>\nTests, tweaks and commit message by: Matthieu Moy <Matthieu.Moy@imag.fr>\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n builtin/mv.c  | 23 ++++++++++++++++-------\n t/t7001-mv.sh | 10 ++++++++++\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 2e0e61b..08fbc03 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -16,9 +16,12 @@ static const char * const builtin_mv_usage[] = {\n \tNULL\n };\n \n+#define DUP_BASENAME 1\n+#define KEEP_TRAILING_SLASH 2\n+\n static const char **internal_copy_pathspec(const char *prefix,\n \t\t\t\t\t   const char **pathspec,\n-\t\t\t\t\t   int count, int base_name)\n+\t\t\t\t\t   int count, unsigned flags)\n {\n \tint i;\n \tconst char **result = xmalloc((count + 1) * sizeof(const char *));\n@@ -27,11 +30,12 @@ static const char **internal_copy_pathspec(const char *prefix,\n \tfor (i = 0; i < count; i++) {\n \t\tint length = strlen(result[i]);\n \t\tint to_copy = length;\n-\t\twhile (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))\n+\t\twhile (!(flags & KEEP_TRAILING_SLASH) &&\n+\t\t       to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))\n \t\t\tto_copy--;\n-\t\tif (to_copy != length || base_name) {\n+\t\tif (to_copy != length || flags & DUP_BASENAME) {\n \t\t\tchar *it = xmemdupz(result[i], to_copy);\n-\t\t\tif (base_name) {\n+\t\t\tif (flags & DUP_BASENAME) {\n \t\t\t\tresult[i] = xstrdup(basename(it));\n \t\t\t\tfree(it);\n \t\t\t} else\n@@ -87,16 +91,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \n \tsource = internal_copy_pathspec(prefix, argv, argc, 0);\n \tmodes = xcalloc(argc, sizeof(enum update_mode));\n-\tdest_path = internal_copy_pathspec(prefix, argv + argc, 1, 0);\n+\t/*\n+\t * Keep trailing slash, needed to let\n+\t * \"git mv file no-such-dir/\" error out.\n+\t */\n+\tdest_path = internal_copy_pathspec(prefix, argv + argc, 1,\n+\t\t\t\t\t   KEEP_TRAILING_SLASH);\n \tsubmodule_gitfile = xcalloc(argc, sizeof(char *));\n \n \tif (dest_path[0][0] == '\\0')\n \t\t/* special case: \".\" was normalized to \"\" */\n-\t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, 1);\n+\t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);\n \telse if (!lstat(dest_path[0], &st) &&\n \t\t\tS_ISDIR(st.st_mode)) {\n \t\tdest_path[0] = add_slash(dest_path[0]);\n-\t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, 1);\n+\t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);\n \t} else {\n \t\tif (argc != 1)\n \t\t\tdie(\"destination '%s' is not a directory\", dest_path[0]);\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex b90e985..e5c8084 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -72,6 +72,16 @@ rm -f idontexist untracked1 untracked2 \\\n      .git/index.lock\n \n test_expect_success \\\n+    'moving to target with trailing slash' \\\n+    'test_must_fail git mv path0/COPYING no-such-dir/ &&\n+     test_must_fail git mv path0/COPYING no-such-dir// &&\n+     git mv path0/ no-such-dir/'\n+\n+test_expect_success \\\n+    'clean up' \\\n+    'git reset --hard'\n+\n+test_expect_success \\\n     'adding another file' \\\n     'cp \"$TEST_DIRECTORY\"/../README path0/README &&\n      git add path0/README &&\n-- \n1.8.5.rc3.4.g8bd3721\n"},{"id":"231418","messageId":"CACsJy8CfvdT2LgRHax3P6k94C3S9ScB4fuNRm7bZWGXRW=NqKA@mail.gmail.com","threadId":"35459","inReplyTo":"1386059524-14442-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] mv: let 'git mv file no-such-dir/' error out","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-03T10:06:08Z","receivedAt":"2013-12-03T10:06:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 3, 2013 at 3:32 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:\n> Git used to trim the trailing slash, and make the command equivalent to\n> 'git mv file no-such-dir', which created the file no-such-dir (while the\n> trailing slash explicitly stated that it could only be a directory).\n>\n> This patch skips the trailing slash removal for the destination path. The\n> path with its trailing slash is passed to rename(2), which errors out\n> with the appropriate message:\n>\n>   $ git mv file no-such-dir/\n>   fatal: renaming 'file' failed: Not a directory\n\nThere's something we probably should check. In d78b0f3 ([PATCH]\ngit-mv: add more path normalization - 2006-08-16), it mentions about\n\ngit mv something/ somewhere/\n\nthere's no test in that commit so I don't know the actual input and\nexpected outcome. If \"somewhere\" is a directory, it errors out with\nthis patch and works without it. If \"somewhere\" does not exist, it\nseems to work like Linux \"mv\" with or without the patch.\n\n> Original-patch-by: Duy Nguyen <pclouds@gmail.com>\n> Tests, tweaks and commit message by: Matthieu Moy <Matthieu.Moy@imag.fr>\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n>  builtin/mv.c  | 23 ++++++++++++++++-------\n>  t/t7001-mv.sh | 10 ++++++++++\n>  2 files changed, 26 insertions(+), 7 deletions(-)\n>\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 2e0e61b..08fbc03 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -16,9 +16,12 @@ static const char * const builtin_mv_usage[] = {\n>         NULL\n>  };\n>\n> +#define DUP_BASENAME 1\n> +#define KEEP_TRAILING_SLASH 2\n> +\n>  static const char **internal_copy_pathspec(const char *prefix,\n>                                            const char **pathspec,\n> -                                          int count, int base_name)\n> +                                          int count, unsigned flags)\n>  {\n>         int i;\n>         const char **result = xmalloc((count + 1) * sizeof(const char *));\n> @@ -27,11 +30,12 @@ static const char **internal_copy_pathspec(const char *prefix,\n>         for (i = 0; i < count; i++) {\n>                 int length = strlen(result[i]);\n>                 int to_copy = length;\n> -               while (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))\n> +               while (!(flags & KEEP_TRAILING_SLASH) &&\n> +                      to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))\n>                         to_copy--;\n> -               if (to_copy != length || base_name) {\n> +               if (to_copy != length || flags & DUP_BASENAME) {\n>                         char *it = xmemdupz(result[i], to_copy);\n> -                       if (base_name) {\n> +                       if (flags & DUP_BASENAME) {\n>                                 result[i] = xstrdup(basename(it));\n>                                 free(it);\n>                         } else\n> @@ -87,16 +91,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n>\n>         source = internal_copy_pathspec(prefix, argv, argc, 0);\n>         modes = xcalloc(argc, sizeof(enum update_mode));\n> -       dest_path = internal_copy_pathspec(prefix, argv + argc, 1, 0);\n> +       /*\n> +        * Keep trailing slash, needed to let\n> +        * \"git mv file no-such-dir/\" error out.\n> +        */\n> +       dest_path = internal_copy_pathspec(prefix, argv + argc, 1,\n> +                                          KEEP_TRAILING_SLASH);\n>         submodule_gitfile = xcalloc(argc, sizeof(char *));\n>\n>         if (dest_path[0][0] == '\\0')\n>                 /* special case: \".\" was normalized to \"\" */\n> -               destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);\n> +               destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);\n>         else if (!lstat(dest_path[0], &st) &&\n>                         S_ISDIR(st.st_mode)) {\n>                 dest_path[0] = add_slash(dest_path[0]);\n> -               destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);\n> +               destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);\n>         } else {\n>                 if (argc != 1)\n>                         die(\"destination '%s' is not a directory\", dest_path[0]);\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> index b90e985..e5c8084 100755\n> --- a/t/t7001-mv.sh\n> +++ b/t/t7001-mv.sh\n> @@ -72,6 +72,16 @@ rm -f idontexist untracked1 untracked2 \\\n>       .git/index.lock\n>\n>  test_expect_success \\\n> +    'moving to target with trailing slash' \\\n> +    'test_must_fail git mv path0/COPYING no-such-dir/ &&\n> +     test_must_fail git mv path0/COPYING no-such-dir// &&\n> +     git mv path0/ no-such-dir/'\n> +\n> +test_expect_success \\\n> +    'clean up' \\\n> +    'git reset --hard'\n> +\n> +test_expect_success \\\n>      'adding another file' \\\n>      'cp \"$TEST_DIRECTORY\"/../README path0/README &&\n>       git add path0/README &&\n> --\n> 1.8.5.rc3.4.g8bd3721\n>\n\n\n\n-- \nDuy\n"},{"id":"231496","messageId":"vpqa9ghhunu.fsf@anie.imag.fr","threadId":"35459","inReplyTo":"CACsJy8CfvdT2LgRHax3P6k94C3S9ScB4fuNRm7bZWGXRW=NqKA@mail.gmail.com","subject":"Re: [PATCH] mv: let 'git mv file no-such-dir/' error out","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-12-04T08:44:53Z","receivedAt":"2013-12-04T08:44:53Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Tue, Dec 3, 2013 at 3:32 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:\n>> Git used to trim the trailing slash, and make the command equivalent to\n>> 'git mv file no-such-dir', which created the file no-such-dir (while the\n>> trailing slash explicitly stated that it could only be a directory).\n>>\n>> This patch skips the trailing slash removal for the destination path. The\n>> path with its trailing slash is passed to rename(2), which errors out\n>> with the appropriate message:\n>>\n>>   $ git mv file no-such-dir/\n>>   fatal: renaming 'file' failed: Not a directory\n>\n> There's something we probably should check. In d78b0f3 ([PATCH]\n> git-mv: add more path normalization - 2006-08-16), it mentions about\n>\n> git mv something/ somewhere/\n>\n> there's no test in that commit so I don't know the actual input and\n> expected outcome.\n\nTo me, the expected outcome is \"behave like Unix's mv\" (which works with\nor without the trailing slash if somewhere exists).\n\n> If \"somewhere\" is a directory, it errors out with this patch and works\n> without it.\n\nI can't reproduce. I've added this to my patch (indeed, the area wasn't\nwell tested), and the tests pass.\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex e5c8084..3bfdfed 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -70,12 +70,31 @@ test_expect_success \\\n rm -f idontexist untracked1 untracked2 \\\n      path0/idontexist path0/untracked1 path0/untracked2 \\\n      .git/index.lock\n+rmdir path1\n \n test_expect_success \\\n-    'moving to target with trailing slash' \\\n+    'moving to absent target with trailing slash' \\\n     'test_must_fail git mv path0/COPYING no-such-dir/ &&\n      test_must_fail git mv path0/COPYING no-such-dir// &&\n-     git mv path0/ no-such-dir/'\n+     git mv path0/ no-such-dir/ &&\n+     test_path_is_dir no-such-dir'\n+\n+test_expect_success \\\n+    'clean up' \\\n+    'git reset --hard'\n+\n+test_expect_success \\\n+    'moving to existing untracked target with trailing slash' \\\n+    'mkdir path1 &&\n+     git mv path0/ path1/ &&\n+     test_path_is_dir path1/path0/'\n+\n+test_expect_success \\\n+    'moving to existing tracked target with trailing slash' \\\n+    'mkdir path2 &&\n+     >path2/file && git add path2/file &&\n+     git mv path1/path0/ path2/ &&\n+     test_path_is_dir path2/path0/'\n \n test_expect_success \\\n     'clean up' \\\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"231506","messageId":"CACsJy8BdXkzzZpEaOvqkJ8ckubbQ0ArOMUT8qnMHMDscupfj_A@mail.gmail.com","threadId":"35459","inReplyTo":"vpqa9ghhunu.fsf@anie.imag.fr","subject":"Re: [PATCH] mv: let 'git mv file no-such-dir/' error out","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-04T13:10:05Z","receivedAt":"2013-12-04T13:10:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Dec 4, 2013 at 3:44 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Duy Nguyen <pclouds@gmail.com> writes:\n>\n>> On Tue, Dec 3, 2013 at 3:32 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:\n>> There's something we probably should check. In d78b0f3 ([PATCH]\n>> git-mv: add more path normalization - 2006-08-16), it mentions about\n>>\n>> git mv something/ somewhere/\n>>\n>> there's no test in that commit so I don't know the actual input and\n>> expected outcome.\n>\n> To me, the expected outcome is \"behave like Unix's mv\" (which works with\n> or without the trailing slash if somewhere exists).\n>\n>> If \"somewhere\" is a directory, it errors out with this patch and works\n>> without it.\n>\n> I can't reproduce. I've added this to my patch (indeed, the area wasn't\n> well tested), and the tests pass.\n\nNow I can't either. Probably some mis-setups or some local bugs in my\ntree. Good that we have more tests.\n-- \nDuy\n"},{"id":"231522","messageId":"1386178621-12822-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"35459","inReplyTo":"CACsJy8BdXkzzZpEaOvqkJ8ckubbQ0ArOMUT8qnMHMDscupfj_A@mail.gmail.com","subject":"[PATCH v2] mv: let 'git mv file no-such-dir/' error out","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-12-04T17:37:01Z","receivedAt":"2013-12-04T17:37:01Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Git used to trim the trailing slash, and make the command equivalent to\n'git mv file no-such-dir', which created the file no-such-dir (while the\ntrailing slash explicitly stated that it could only be a directory).\n\nThis patch skips the trailing slash removal for the destination path. The\npath with its trailing slash is passed to rename(2), which errors out\nwith the appropriate message:\n\n  $ git mv file no-such-dir/\n  fatal: renaming 'file' failed: Not a directory\n\nOriginal-patch-by: Duy Nguyen <pclouds@gmail.com>\nTests, tweaks and commit message by: Matthieu Moy <Matthieu.Moy@imag.fr>\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nSo, this patch adds more tests, as suggested by Duy. They all pass\nwithout modifying the code.\n\n builtin/mv.c  | 23 ++++++++++++++++-------\n t/t7001-mv.sh | 29 +++++++++++++++++++++++++++++\n 2 files changed, 45 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 2e0e61b..08fbc03 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -16,9 +16,12 @@ static const char * const builtin_mv_usage[] = {\n \tNULL\n };\n \n+#define DUP_BASENAME 1\n+#define KEEP_TRAILING_SLASH 2\n+\n static const char **internal_copy_pathspec(const char *prefix,\n \t\t\t\t\t   const char **pathspec,\n-\t\t\t\t\t   int count, int base_name)\n+\t\t\t\t\t   int count, unsigned flags)\n {\n \tint i;\n \tconst char **result = xmalloc((count + 1) * sizeof(const char *));\n@@ -27,11 +30,12 @@ static const char **internal_copy_pathspec(const char *prefix,\n \tfor (i = 0; i < count; i++) {\n \t\tint length = strlen(result[i]);\n \t\tint to_copy = length;\n-\t\twhile (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))\n+\t\twhile (!(flags & KEEP_TRAILING_SLASH) &&\n+\t\t       to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))\n \t\t\tto_copy--;\n-\t\tif (to_copy != length || base_name) {\n+\t\tif (to_copy != length || flags & DUP_BASENAME) {\n \t\t\tchar *it = xmemdupz(result[i], to_copy);\n-\t\t\tif (base_name) {\n+\t\t\tif (flags & DUP_BASENAME) {\n \t\t\t\tresult[i] = xstrdup(basename(it));\n \t\t\t\tfree(it);\n \t\t\t} else\n@@ -87,16 +91,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \n \tsource = internal_copy_pathspec(prefix, argv, argc, 0);\n \tmodes = xcalloc(argc, sizeof(enum update_mode));\n-\tdest_path = internal_copy_pathspec(prefix, argv + argc, 1, 0);\n+\t/*\n+\t * Keep trailing slash, needed to let\n+\t * \"git mv file no-such-dir/\" error out.\n+\t */\n+\tdest_path = internal_copy_pathspec(prefix, argv + argc, 1,\n+\t\t\t\t\t   KEEP_TRAILING_SLASH);\n \tsubmodule_gitfile = xcalloc(argc, sizeof(char *));\n \n \tif (dest_path[0][0] == '\\0')\n \t\t/* special case: \".\" was normalized to \"\" */\n-\t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, 1);\n+\t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);\n \telse if (!lstat(dest_path[0], &st) &&\n \t\t\tS_ISDIR(st.st_mode)) {\n \t\tdest_path[0] = add_slash(dest_path[0]);\n-\t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, 1);\n+\t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);\n \t} else {\n \t\tif (argc != 1)\n \t\t\tdie(\"destination '%s' is not a directory\", dest_path[0]);\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex b90e985..2f82478 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -70,6 +70,35 @@ test_expect_success \\\n rm -f idontexist untracked1 untracked2 \\\n      path0/idontexist path0/untracked1 path0/untracked2 \\\n      .git/index.lock\n+rmdir path1\n+\n+test_expect_success \\\n+    'moving to absent target with trailing slash' \\\n+    'test_must_fail git mv path0/COPYING no-such-dir/ &&\n+     test_must_fail git mv path0/COPYING no-such-dir// &&\n+     git mv path0/ no-such-dir/ &&\n+     test_path_is_dir no-such-dir/'\n+\n+test_expect_success \\\n+    'clean up' \\\n+    'git reset --hard'\n+\n+test_expect_success \\\n+    'moving to existing untracked target with trailing slash' \\\n+    'mkdir path1 &&\n+     git mv path0/ path1/ &&\n+     test_path_is_dir path1/path0/'\n+\n+test_expect_success \\\n+    'moving to existing tracked target with trailing slash' \\\n+    'mkdir path2 &&\n+     >path2/file && git add path2/file &&\n+     git mv path1/path0/ path2/ &&\n+     test_path_is_dir path2/path0/'\n+\n+test_expect_success \\\n+    'clean up' \\\n+    'git reset --hard'\n \n test_expect_success \\\n     'adding another file' \\\n-- \n1.8.5.rc3.4.g8bd3721\n"},{"id":"231524","messageId":"xmqqpppclde1.fsf@gitster.dls.corp.google.com","threadId":"35459","inReplyTo":"CACsJy8BdXkzzZpEaOvqkJ8ckubbQ0ArOMUT8qnMHMDscupfj_A@mail.gmail.com","subject":"Re: [PATCH] mv: let 'git mv file no-such-dir/' error out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-04T17:44:22Z","receivedAt":"2013-12-04T17:44:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Wed, Dec 4, 2013 at 3:44 PM, Matthieu Moy\n> <Matthieu.Moy@grenoble-inp.fr> wrote:\n>> Duy Nguyen <pclouds@gmail.com> writes:\n>>\n>>> On Tue, Dec 3, 2013 at 3:32 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:\n>>> There's something we probably should check. In d78b0f3 ([PATCH]\n>>> git-mv: add more path normalization - 2006-08-16), it mentions about\n>>>\n>>> git mv something/ somewhere/\n>>>\n>>> there's no test in that commit so I don't know the actual input and\n>>> expected outcome.\n>>\n>> To me, the expected outcome is \"behave like Unix's mv\" (which works with\n>> or without the trailing slash if somewhere exists).\n>>\n>>> If \"somewhere\" is a directory, it errors out with this patch and works\n>>> without it.\n>>\n>> I can't reproduce. I've added this to my patch (indeed, the area wasn't\n>> well tested), and the tests pass.\n>\n> Now I can't either. Probably some mis-setups or some local bugs in my\n> tree. Good that we have more tests.\n\nOK, I was also scratching my head after seeing your response.\n\nIt seems that t7001 needs some face-lifting, by the way.  Perhaps\nafter this patch solidifies.\n"},{"id":"231525","messageId":"vpqeh5s8q35.fsf@anie.imag.fr","threadId":"35459","inReplyTo":"xmqqpppclde1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] mv: let 'git mv file no-such-dir/' error out","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-12-04T17:48:30Z","receivedAt":"2013-12-04T17:48:30Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> It seems that t7001 needs some face-lifting, by the way.  Perhaps\n> after this patch solidifies.\n\nYes. I followed the style of surrounding code, but it could be\nreformatted to follow the current standard. I have no time for it now.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"232919","messageId":"52CD7DE8.6070101@kdbg.org","threadId":"35459","inReplyTo":"1386059524-14442-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-01-08T16:33:44Z","receivedAt":"2014-01-08T16:33:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)\nrelies on that rename(\"src\", \"dst/\") fails if directory dst does not\nexist (note the trailing slash). This does not work as expected on Windows:\nThis rename() call is successful. Insert an explicit check for this case.\n\nThis changes the error message from\n\n   $ git mv file no-such-dir/\n   fatal: renaming 'file' failed: Not a directory\n\nto\n\n   $ git mv file no-such-dir/\n   fatal: destination directory does not exist, source=file, destination=no-such-dir/\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n builtin/mv.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 08fbc03..21c46d1 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t} else if (string_list_has_string(&src_for_dst, dst))\n \t\t\tbad = _(\"multiple sources for the same target\");\n+\t\telse if (is_dir_sep(dst[strlen(dst) - 1]))\n+\t\t\tbad = _(\"destination directory does not exist\");\n \t\telse\n \t\t\tstring_list_insert(&src_for_dst, dst);\n \n-- \n1.8.5.2.1473.g6546919\n"},{"id":"232975","messageId":"xmqq7ga8aibn.fsf@gitster.dls.corp.google.com","threadId":"35459","inReplyTo":"52CD7DE8.6070101@kdbg.org","subject":"Re: [PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-01-09T22:42:04Z","receivedAt":"2014-01-09T22:42:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)\n> relies on that rename(\"src\", \"dst/\") fails if directory dst does not\n> exist (note the trailing slash). This does not work as expected on Windows:\n> This rename() call is successful. Insert an explicit check for this case.\n\nCould you care to explain \"Successful how\" a bit here?  Do we see\nno-such-dir mkdir'ed and then no-such-dir/file created?  Do we see\nfile moved to a new file whose name is no-such-dir/?  I am guessing\nthat it would be the latter, but if that is the case we would need\nat least an air-quote around \"successful\".\n\n> This changes the error message from\n>\n>    $ git mv file no-such-dir/\n>    fatal: renaming 'file' failed: Not a directory\n>\n> to\n>\n>    $ git mv file no-such-dir/\n>    fatal: destination directory does not exist, source=file, destination=no-such-dir/\n>\n> Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n> ---\n>  builtin/mv.c | 2 ++\n>  1 file changed, 2 insertions(+)\n>\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 08fbc03..21c46d1 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n>  \t\t\t}\n>  \t\t} else if (string_list_has_string(&src_for_dst, dst))\n>  \t\t\tbad = _(\"multiple sources for the same target\");\n> +\t\telse if (is_dir_sep(dst[strlen(dst) - 1]))\n> +\t\t\tbad = _(\"destination directory does not exist\");\n>  \t\telse\n>  \t\t\tstring_list_insert(&src_for_dst, dst);\n"},{"id":"233005","messageId":"52D04856.9070005@kdbg.org","threadId":"35459","inReplyTo":"xmqq7ga8aibn.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-01-10T19:21:58Z","receivedAt":"2014-01-10T19:21:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 09.01.2014 23:42, schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n>> The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)\n>> relies on that rename(\"src\", \"dst/\") fails if directory dst does not\n>> exist (note the trailing slash). This does not work as expected on Windows:\n>> This rename() call is successful. Insert an explicit check for this case.\n> \n> Could you care to explain \"Successful how\" a bit here?  Do we see\n> no-such-dir mkdir'ed and then no-such-dir/file created?  Do we see\n> file moved to a new file whose name is no-such-dir/?  I am guessing\n> that it would be the latter, but if that is the case we would need\n> at least an air-quote around \"successful\".\n\nThe file is renamed to no-such-dir without the slash at the end. The\nupdated commit message would be:\n\nmv: let 'git mv file no-such-dir/' error out on Windows, too\n\nThe previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)\nrelies on that rename(\"src\", \"dst/\") fails if directory dst does not\nexist (note the trailing slash). This does not work as expected on Windows:\nThe rename() call does not fail, but renames src to dst (without the\ntrailing slash). Insert an explicit check for this case to force an error.\n\nThis changes the error message from\n\n   $ git mv file no-such-dir/\n   fatal: renaming 'file' failed: Not a directory\n\nto\n\n   $ git mv file no-such-dir/\n   fatal: destination directory does not exist, source=file, destination=no-such-dir/\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n\n> \n>> This changes the error message from\n>>\n>>    $ git mv file no-such-dir/\n>>    fatal: renaming 'file' failed: Not a directory\n>>\n>> to\n>>\n>>    $ git mv file no-such-dir/\n>>    fatal: destination directory does not exist, source=file, destination=no-such-dir/\n>>\n>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n>> ---\n>>  builtin/mv.c | 2 ++\n>>  1 file changed, 2 insertions(+)\n>>\n>> diff --git a/builtin/mv.c b/builtin/mv.c\n>> index 08fbc03..21c46d1 100644\n>> --- a/builtin/mv.c\n>> +++ b/builtin/mv.c\n>> @@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n>>  \t\t\t}\n>>  \t\t} else if (string_list_has_string(&src_for_dst, dst))\n>>  \t\t\tbad = _(\"multiple sources for the same target\");\n>> +\t\telse if (is_dir_sep(dst[strlen(dst) - 1]))\n>> +\t\t\tbad = _(\"destination directory does not exist\");\n>>  \t\telse\n>>  \t\t\tstring_list_insert(&src_for_dst, dst);\n> \n"},{"id":"233007","messageId":"xmqqd2jz8wi9.fsf@gitster.dls.corp.google.com","threadId":"35459","inReplyTo":"52D04856.9070005@kdbg.org","subject":"Re: [PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-01-10T19:30:54Z","receivedAt":"2014-01-10T19:30:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> The file is renamed to no-such-dir without the slash at the end. The\n> updated commit message would be:\n>\n> mv: let 'git mv file no-such-dir/' error out on Windows, too\n>\n> The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)\n> relies on that rename(\"src\", \"dst/\") fails if directory dst does not\n> exist (note the trailing slash). This does not work as expected on Windows:\n> The rename() call does not fail, but renames src to dst (without the\n> trailing slash). Insert an explicit check for this case to force an error.\n>\n> This changes the error message from\n>\n>    $ git mv file no-such-dir/\n>    fatal: renaming 'file' failed: Not a directory\n>\n> to\n>\n>    $ git mv file no-such-dir/\n>    fatal: destination directory does not exist, source=file, destination=no-such-dir/\n>\n> Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n\nThanks.\n"}]}