{"thread":{"id":"35448","subject":"[BUG] git mv file directory/ creates the file directory","startedAt":"2013-12-02T10:04:06Z","lastAt":"2013-12-02T17:07:54Z","messageCount":3,"participants":["Matthieu Moy","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"231359","messageId":"vpqli03sh61.fsf@anie.imag.fr","threadId":"35448","inReplyTo":null,"subject":"[BUG] git mv file directory/ creates the file directory","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-12-02T10:04:06Z","receivedAt":"2013-12-02T10:04:06Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Hi,\n\nWhen directory/ does not exist, I'd expect this to fail:\n\n  git mv existing-file directory/\n\n(note the trailing slash, to make it clear that directory/ is a\ndirectory). Unix's mv does fail:\n\n  $ mv existing-file directory/\n  mv: cannot move `existing-file' to `directory/': Not a directory\n\nInstead, \"git mv\" seems to do the equivalent of\n\n  git mv existing-file directory # without trailing slash\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"231367","messageId":"20131202133544.GA8755@lanh","threadId":"35448","inReplyTo":"vpqli03sh61.fsf@anie.imag.fr","subject":"Re: [BUG] git mv file directory/ creates the file directory","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-02T13:35:44Z","receivedAt":"2013-12-02T13:35:44Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 02, 2013 at 11:04:06AM +0100, Matthieu Moy wrote:\n> Hi,\n> \n> When directory/ does not exist, I'd expect this to fail:\n> \n>   git mv existing-file directory/\n> \n> (note the trailing slash, to make it clear that directory/ is a\n> directory). Unix's mv does fail:\n> \n>   $ mv existing-file directory/\n>   mv: cannot move `existing-file' to `directory/': Not a directory\n> \n> Instead, \"git mv\" seems to do the equivalent of\n> \n>   git mv existing-file directory # without trailing slash\n\nThis may be a start. Does not seem to break anything..\n\n-- 8< --\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 2e0e61b..0fcccd5 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,16 @@ 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+\tdest_path = internal_copy_pathspec(prefix, argv + argc, 1, 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]);\n-- 8< --\n"},{"id":"231375","messageId":"vpq61r7npud.fsf@anie.imag.fr","threadId":"35448","inReplyTo":"20131202133544.GA8755@lanh","subject":"Re: [BUG] git mv file directory/ creates the file directory","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-12-02T17:07:54Z","receivedAt":"2013-12-02T17:07:54Z","isPatch":false,"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> This may be a start. Does not seem to break anything..\n\nI did not thoroughly review/test, but it does fix my case. Below is the\nsame patch with one test case. No time to do more right now.\n\nThanks,\n\n>From 99985341ed1312cf6a7b63e14be7da0d51c701b4 Mon Sep 17 00:00:00 2001\nFrom: Matthieu Moy <Matthieu.Moy@imag.fr>\nDate: Mon, 2 Dec 2013 18:03:20 +0100\nSubject: [PATCH] WIP: error out on git mv file no-such-dir/\n\n---\n builtin/mv.c  | 18 +++++++++++-------\n t/t7001-mv.sh |  9 +++++++++\n 2 files changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 2e0e61b..0fcccd5 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,16 @@ 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+\tdest_path = internal_copy_pathspec(prefix, argv + argc, 1, 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..7e74bf3 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -72,6 +72,15 @@ 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+     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\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}