{"thread":{"id":"26283","subject":"[PATCH] handle rename of case only, for windows","startedAt":"2011-01-14T13:44:04Z","lastAt":"2011-01-14T14:17:27Z","messageCount":2,"participants":["Tim Abell","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"159485","messageId":"1295012644.7883.1415296115@webmail.messagingengine.com","threadId":"26283","inReplyTo":null,"subject":"[PATCH] handle rename of case only, for windows","fromName":"Tim Abell","fromEmail":"tim@timwise.co.uk","sentAt":"2011-01-14T13:44:04Z","receivedAt":"2011-01-14T13:44:04Z","isPatch":true,"sender":{"key":"tim@timwise.co.uk","avatar":"https://gravatar.com/avatar/0d3f826b8cc4813b0a373fbfaf402be87a89e01328c16065e5762406b8066fee?d=mp&s=160"},"body":"Hi folks,\n\nI've never contributed to git before so be gentle :-)\n\nWould someone have the time to help me get this patch into mailine git?\n\nI tripped over a failure to rename files on windows when only the case\nhas changed. I've created a patch which fixes it for me and doesn't seem\nto break on linux or windows. I also created a test to demonstrate the\nissue (part of this patch). This test passes on linux and fails on\nwindows before my patch for mv.c is applied, and passes on both windows\nand linux for me after my patch is applied.\n\nThe problem with changing the case of a file happens because git mv\nchecks if the destination filename exists before performing a\nmove/rename, and on windows lstat reports that the destination file\n*does* already exist because it ignores case for this check and\nsemi-erroneously finds the source file.\n\nThe way I've attempted to fix it in my patch is by checking if the inode\nof the source and destination are the same before deciding to fail with\na \"destination exists\" error.\n\nWhen using \"git mv\" it is possible to work around the error by using\n--force.\n\nI've also seen a problem with windows users pulling from remotes where a\ncase only rename has been performed which is more problematic as you\nhave to tell every user how to handle the issue. I'm not sure if I've\nmanaged to fix that case or not.\n\nThe fault exists in both the current cygwin git and the current msysgit,\nso I figured it would be good to get a patch to upstream (you) so that\nit could work everywhere. \n\nI found an email from Linus relating to this issue here:\nhttp://marc.info/?l=git&m=120612172706823 so it's a known problem.\n\nThanks\n\nTim Abell\n\n---\n builtin/mv.c  |   33 ++++++++++++++++++++++-----------\n t/t7001-mv.sh |    9 +++++++++\n 2 files changed, 31 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 93e8995..6bb262e 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -63,6 +63,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \tconst char **source, **destination, **dest_path;\n \tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n \tstruct stat st;\n+\tstruct stat src_st;\n \tstruct string_list src_for_dst = STRING_LIST_INIT_NODUP;\n \n \tgit_config(git_default_config, NULL);\n@@ -165,17 +166,27 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t} else if (cache_name_pos(src, length) < 0)\n \t\t\tbad = \"not under version control\";\n \t\telse if (lstat(dst, &st) == 0) {\n-\t\t\tbad = \"destination exists\";\n-\t\t\tif (force) {\n-\t\t\t\t/*\n-\t\t\t\t * only files can overwrite each other:\n-\t\t\t\t * check both source and destination\n-\t\t\t\t */\n-\t\t\t\tif (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n-\t\t\t\t\twarning(\"%s; will overwrite!\", bad);\n-\t\t\t\t\tbad = NULL;\n-\t\t\t\t} else\n-\t\t\t\t\tbad = \"Cannot overwrite\";\n+\t\t\t/* If we are on a case insensitive files= system (windows) http://is.gd/kyxgg\n+\t\t\t * and we are only changing the case of the file then lstat for the\n+\t\t\t * destination will return != 0 because it sees the source file.\n+\t\t\t * To prevent this causing failure, lstat is used to get the inode of the src\n+\t\t\t * and see if it's actually the same file.\n+\t\t\t */\n+\t\t\tlstat(src, &src_st); //get file serial number (inode) for source\n+\t\t\t#warning(\"src inode: %s, dst inode: %s\", src_st.st_ino, st.st_ino);\n+\t\t\tif (src_st.st_ino != st.st_ino) {\n+\t\t\t\tbad = \"destination exists\";\n+\t\t\t\tif (force) {\n+\t\t\t\t\t/*\n+\t\t\t\t\t * only files can overwrite each other:\n+\t\t\t\t\t * check both source and destination\n+\t\t\t\t\t */\n+\t\t\t\t\tif (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n+\t\t\t\t\t\twarning(\"%s; will overwrite!\", bad);\n+\t\t\t\t\t\tbad = NULL;\n+\t\t\t\t\t} else\n+\t\t\t\t\t\tbad = \"Cannot overwrite\";\n+\t\t\t\t}\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\";\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex a845b15..95146bf 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -255,4 +255,13 @@ test_expect_success SYMLINKS 'git mv should overwrite file with a symlink' '\n \n rm -f moved symlink\n \n+test_expect_success 'git mv should not fail when only changing case' '\n+\n+\trm -fr .git &&\n+\tgit init &&\n+\t>foo.txt &&\n+\tgit add foo.txt &&\n+\tgit mv foo.txt Foo.txt\n+'\n+\n test_done\n-- \n1.5.6.5\n"},{"id":"159487","messageId":"AANLkTinbnYzTqP5-vA4hJqSgSBBjJrx8LCsq5G3nVmFW@mail.gmail.com","threadId":"26283","inReplyTo":"1295012644.7883.1415296115@webmail.messagingengine.com","subject":"Re: [PATCH] handle rename of case only, for windows","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-01-14T14:17:27Z","receivedAt":"2011-01-14T14:17:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jan 14, 2011 at 8:44 PM, Tim Abell <tim@timwise.co.uk> wrote:\n>                else if (lstat(dst, &st) == 0) {\n> -                       bad = \"destination exists\";\n> -                       if (force) {\n> -                               /*\n> -                                * only files can overwrite each other:\n> -                                * check both source and destination\n> -                                */\n> -                               if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n> -                                       warning(\"%s; will overwrite!\", bad);\n> -                                       bad = NULL;\n> -                               } else\n> -                                       bad = \"Cannot overwrite\";\n> +                       /* If we are on a case insensitive files= system (windows) http://is.gd/kyxgg\n> +                        * and we are only changing the case of the file then lstat for the\n> +                        * destination will return != 0 because it sees the source file.\n> +                        * To prevent this causing failure, lstat is used to get the inode of the src\n> +                        * and see if it's actually the same file.\n> +                        */\n> +                       lstat(src, &src_st); //get file serial number (inode) for source\n> +                       #warning(\"src inode: %s, dst inode: %s\", src_st.st_ino, st.st_ino);\n> +                       if (src_st.st_ino != st.st_ino) {\n> +                               bad = \"destination exists\";\n> +                               if (force) {\n> +                                       /*\n> +                                        * only files can overwrite each other:\n> +                                        * check both source and destination\n> +                                        */\n> +                                       if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n> +                                               warning(\"%s; will overwrite!\", bad);\n> +                                               bad = NULL;\n> +                                       } else\n> +                                               bad = \"Cannot overwrite\";\n> +                               }\n>                        }\n>                } else if (string_list_has_string(&src_for_dst, dst))\n>                        bad = \"multiple sources for the same target\";\n\nI wonder if we can make lstat_case() that would only return 0 if it\nmatches exactly the filename, even on FAT. FindFirstFile/FindNextFile\nshould return true file name, I think. If not, we can make\nlstat_case() take two paths (src and dst) and move all inode\ncomparison code in there. Much cleaner.\n-- \nDuy\n"}]}