{"thread":{"id":"26790","subject":"[PATCH v2] Allow git mv FileA fILEa on case ignore file systems","startedAt":"2011-03-19T14:28:33Z","lastAt":"2011-04-12T06:16:12Z","messageCount":8,"participants":["Torsten Bögershausen","Erik Faye-Lund","Piotr Krukowiecki","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"163748","messageId":"201103191528.34646.tboegi@web.de","threadId":"26790","inReplyTo":null,"subject":"[PATCH v2] Allow git mv FileA fILEa on case ignore file systems","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2011-03-19T14:28:33Z","receivedAt":"2011-03-19T14:28:33Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"The typical use case is when a file \"FileA\" should be renamed into fILEa\nand we are on a case insenstive file system (system core.ignorecase = true).\nSource and destination are the same file, it can be accessed under both names.\nThis makes git think that the destination file exists.\nUnless used with --forced, git will refuse the \"git mv FileA fILEa\".\nThis change will allow \"git mv FileA fILEa\" under the following condition:\nOn Linux/Unix/Mac OS X the move is allowed when the inode of the source and\ndestination are equal (and they are on the same device).\nThis allows renames of MÄRCHEN into Märchen on Mac OS X.\n(As a side effect, a file can be renamed to a name which is already\nhard-linked to the same inode).\nOn Windows, the function win_is_same_file() from compat/win32/same-file.c\nis used.\nIt calls GetFileInformationByHandle() to check if both files are\n\"the same\".\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n Makefile                 |    8 +++++---\n builtin/mv.c             |    2 +-\n compat/win32/same-file.c |   26 ++++++++++++++++++++++++++\n git-compat-util.h        |   15 +++++++++++++++\n t/t7001-mv.sh            |   29 +++++++++++++++++++++++++++++\n 5 files changed, 76 insertions(+), 4 deletions(-)\n create mode 100644 compat/win32/same-file.c\n\ndiff --git a/Makefile b/Makefile\nindex 5c2b797..55b9a05 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -924,7 +924,7 @@ ifeq ($(uname_O),Cygwin)\n \t# Try commenting this out if you suspect MMAP is more efficient\n \tNO_MMAP = YesPlease\n \tX = .exe\n-\tCOMPAT_OBJS += compat/cygwin.o\n+\tCOMPAT_OBJS += compat/cygwin.o compat/win32/same-file.o\n \tUNRELIABLE_FSTAT = UnfortunatelyYes\n endif\n ifeq ($(uname_S),FreeBSD)\n@@ -1104,7 +1104,8 @@ ifeq ($(uname_S),Windows)\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n \tCOMPAT_OBJS = compat/msvc.o compat/winansi.o \\\n \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n-\t\tcompat/win32/sys/poll.o compat/win32/dirent.o\n+\t\tcompat/win32/sys/poll.o compat/win32/dirent.o \\\n+\t\tcompat/win32/same-file.o\n \tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib\n@@ -1177,7 +1178,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n \tCOMPAT_OBJS += compat/mingw.o compat/winansi.o \\\n \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n-\t\tcompat/win32/sys/poll.o compat/win32/dirent.o\n+\t\tcompat/win32/sys/poll.o compat/win32/dirent.o \\\n+\t\tcompat/win32/same-file.o\n \tEXTLIBS += -lws2_32\n \tPTHREAD_LIBS =\n \tX = .exe\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 93e8995..96792bd 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -166,7 +166,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\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\tif (force || is_same_file(src, dst)) {\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\ndiff --git a/compat/win32/same-file.c b/compat/win32/same-file.c\nnew file mode 100644\nindex 0000000..bb1a791\n--- /dev/null\n+++ b/compat/win32/same-file.c\n@@ -0,0 +1,26 @@\n+#include \"../../git-compat-util.h\"\n+#include \"../win32.h\"\n+\n+int win_is_same_file(const char *a, const char *b)\n+{\n+\tBY_HANDLE_FILE_INFORMATION hia, hib;\n+\tHANDLE h;\n+\n+\th = CreateFile(a, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);\n+\tif (INVALID_HANDLE_VALUE == h)\n+\t\treturn 0;\n+\tif (!(GetFileInformationByHandle(h,&hia)))\n+\t\treturn 0;\n+  CloseHandle(h);\n+\n+\th = CreateFile(b, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);\n+\tif (INVALID_HANDLE_VALUE == h)\n+\t\treturn 0;\n+\tif (!(GetFileInformationByHandle(h,&hib)))\n+\t\treturn 0;\n+  CloseHandle(h);\n+\n+\treturn hia.dwVolumeSerialNumber == hib.dwVolumeSerialNumber &&\n+\t       hia.nFileSizeLow == hib.nFileSizeLow &&\n+\t       hia.nFileSizeHigh == hib.nFileSizeHigh;\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 49b50ee..df95458 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -152,6 +152,21 @@\n #include \"compat/msvc.h\"\n #endif\n \n+#if defined (WIN32) || defined(__CYGWIN__)\n+/* MinGW or MSVC or cygwin */\n+int win_is_same_file(const char *a, const char *b);\n+#define is_same_file(a,b) win_is_same_file((a),(b))\n+#else\n+static inline int is_same_file(const char *a, const char *b)\n+{\n+\tstruct stat sta, stb;\n+\tif (lstat(a, &sta) ||\n+\t    lstat(b, &stb))\n+\t\treturn 0;\n+\treturn sta.st_ino && sta.st_dev == stb.st_dev && sta.st_ino == stb.st_ino;\n+}\n+#endif\n+\n #ifndef NO_LIBGEN_H\n #include <libgen.h>\n #else\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex a845b15..d0e73ee 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -255,4 +255,33 @@ test_expect_success SYMLINKS 'git mv should overwrite file with a symlink' '\n \n rm -f moved symlink\n \n+touch x\n+if ln x y 2>/dev/null; then\n+\thardlinks=1\n+fi\n+rm -f x y\n+\n+if test \"$(git config --bool core.ignorecase)\" = true -o \"$hardlinks\"; then\n+\ttest_expect_success 'git mv FileA fILEa' '\n+\n+\t\trm -fr .git * &&\n+\t\tgit init &&\n+\t\techo FileA > FileA &&\n+\t\tgit add FileA &&\n+\t\tgit commit -m add FileA &&\n+\t\t{\n+\t\t\tif ! test -f fILEa; then\n+\t\t\t\tln FileA fILEa\n+\t\t\tfi\n+\t\t} &&\n+\t\tgit mv FileA fILEa &&\n+\t\tgit commit -m \"mv FileA fILEa\" &&\n+\t\trm -f FileA fILEa &&\n+\t\tgit reset --hard &&\n+\t\ttest \"$(echo *)\" = fILEa\n+\t'\n+else\n+\tsay \"Neither ignorecase nor hardlinks, skipping git mv FileA fILEa\"\n+fi\n+\n test_done\n-- \n1.7.4\n"},{"id":"163769","messageId":"AANLkTimfk8bb-V8Br+KKpoLbA3G3y9HxyfjA=NrsgR0v@mail.gmail.com","threadId":"26790","inReplyTo":"201103191528.34646.tboegi@web.de","subject":"Re: [PATCH v2] Allow git mv FileA fILEa on case ignore file systems","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-03-19T18:20:38Z","receivedAt":"2011-03-19T18:20:38Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"2011/3/19 Torsten Bögershausen <tboegi@web.de>:\n> diff --git a/compat/win32/same-file.c b/compat/win32/same-file.c\n> new file mode 100644\n> index 0000000..bb1a791\n> --- /dev/null\n> +++ b/compat/win32/same-file.c\n> @@ -0,0 +1,26 @@\n> +#include \"../../git-compat-util.h\"\n> +#include \"../win32.h\"\n> +\n> +int win_is_same_file(const char *a, const char *b)\n\nThe compat/win32-folder is usually used to provide implementations of\nPOSIX APIs lacking on Windows. I don't think this is appropriate\nfunctionality to be put there...\n\n> +{\n> +       BY_HANDLE_FILE_INFORMATION hia, hib;\n> +       HANDLE h;\n> +\n> +       h = CreateFile(a, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);\n> +       if (INVALID_HANDLE_VALUE == h)\n> +               return 0;\n> +       if (!(GetFileInformationByHandle(h,&hia)))\n> +               return 0;\n> +  CloseHandle(h);\n\nIndentation-slip?\n\n> +\n> +       h = CreateFile(b, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);\n> +       if (INVALID_HANDLE_VALUE == h)\n> +               return 0;\n> +       if (!(GetFileInformationByHandle(h,&hib)))\n> +               return 0;\n> +  CloseHandle(h);\n> +\n\nSame as above...?\n\n> +#if defined (WIN32) || defined(__CYGWIN__)\n> +/* MinGW or MSVC or cygwin */\n> +int win_is_same_file(const char *a, const char *b);\n> +#define is_same_file(a,b) win_is_same_file((a),(b))\n> +#else\n> +static inline int is_same_file(const char *a, const char *b)\n> +{\n> +       struct stat sta, stb;\n> +       if (lstat(a, &sta) ||\n> +           lstat(b, &stb))\n> +               return 0;\n> +       return sta.st_ino && sta.st_dev == stb.st_dev && sta.st_ino == stb.st_ino;\n> +}\n> +#endif\n\nThis isn't how we usually do things like this. We usually define stuff\nin compat/mingw.h (which should really be called compat/win32.h\ninstead, since it's used by our MSVC builds as well. But that's a\ndifferent discussion), and check if it's defined near the bottom of\ngit-compat-util.h.\n"},{"id":"163772","messageId":"4D850466.8040909@gmail.com","threadId":"26790","inReplyTo":"AANLkTimfk8bb-V8Br+KKpoLbA3G3y9HxyfjA=NrsgR0v@mail.gmail.com","subject":"Re: [PATCH v2] Allow git mv FileA fILEa on case ignore file systems","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2011-03-19T19:30:46Z","receivedAt":"2011-03-19T19:30:46Z","isPatch":true,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"W dniu 19.03.2011 19:20, Erik Faye-Lund pisze:\n> 2011/3/19 Torsten Bögershausen <tboegi@web.de>:\n>> +       if (!(GetFileInformationByHandle(h,&hia)))\n>> +               return 0;\n>> +  CloseHandle(h);\n> \n> Indentation-slip?\n\nI suspect so. CloseHandle line does not use tabs.\n\nOut of curiosity I've roughly counted number of lines that have\ncorrect and incorrect indentation. \n\nAbout 8% (~12600 out of 151000 indented lines) are incorrectly indented.\nDon't know if that good or bad :)\n\n\nI have used following to get the numbers:\n\nBad indentation (includes \"Correctly indented comments\"):\ngit$ TAB=\"`echo -e '\\t'`\" && grep -E \"^[ $TAB]* [ $TAB]*[^ $TAB]\" *.sh *.[ch] */*.sh */*.[ch] | wc -l\n22495\n\nCorrectly indented comments:\ngit$ TAB=\"`echo -e '\\t'`\" && grep -E \"^[ $TAB]* [*]\" *.sh *.[ch] */*.sh */*.[ch] | wc -l\n9911\n\nGood indentation (does not include \"Correctly indented comments\"):\ngit$ TAB=\"`echo -e '\\t'`\" && grep -E \"^$TAB+[^ $TAB]\" *.sh *.[ch] */*.sh */*.[ch] | wc -l\n128173\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"163802","messageId":"7vsjuitk59.fsf@alter.siamese.dyndns.org","threadId":"26790","inReplyTo":"201103191528.34646.tboegi@web.de","subject":"Re: [PATCH v2] Allow git mv FileA fILEa on case ignore file systems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-20T05:50:58Z","receivedAt":"2011-03-20T05:50:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> The typical use case is when a file \"FileA\" should be renamed into fILEa\n> and we are on a case insenstive file system (system core.ignorecase = true).\n> Source and destination are the same file, it can be accessed under both names.\n> This makes git think that the destination file exists.\n> Unless used with --forced, git will refuse the \"git mv FileA fILEa\".\n> This change will allow \"git mv FileA fILEa\" under the following condition:\n> On Linux/Unix/Mac OS X the move is allowed when the inode of the source and\n> destination are equal (and they are on the same device).\n> This allows renames of MÄRCHEN into Märchen on Mac OS X.\n> (As a side effect, a file can be renamed to a name which is already\n> hard-linked to the same inode).\n> On Windows, the function win_is_same_file() from compat/win32/same-file.c\n> is used.\n> It calls GetFileInformationByHandle() to check if both files are\n> \"the same\".\n\nYeek; is it just me or the above single block of text too dense to be\nreadable? Can you use paragraph breaks?\n\n> The typical use case is when a file \"FileA\" should be renamed into fILEa\n> and we are on a case insenstive file system (system core.ignorecase = true).\n\nHuh? I don't think renaming \"FileA\" to \"fILEa\" is typical at all. It is\nvery rarely done.\n\n> (As a side effect, a file can be renamed to a name which is already\n> hard-linked to the same inode).\n\nIt is unclear \"a side effect\" means \"an added bonus\" or \"a regression\" in\nthis sentence. I think this is latter.\n\nAllowing filesystem specific logic to detect that two different \"names\"\nactually refer to a single \"file\" and help renaming succeed is a sane\napproach, but I think this particular implementation is flawed.\n\nThe important thing to notice is that \"names\" and \"file\" above refer to\nthe entities from the end user's point of view. Two files hardlinked\ntogether on a filesystem with sane pathname handling are never the same\n\"file\". I would probably have called it equivalent_filenames() to stress\nthe fact that two _different_ names alias to the same file. is_same_file()\nsounds more like you got two different filenames from the filesystem\n(i.e. readdir() gave you both at the same time) and you are trying to see\nif they are the same file, but that is not the case here.\n\nI also find it a bad taste to make this feature depend on win32; doesn't a\nLinux box mounting a vfat filesystem have the same issue that we should be\nable to solve with the same code?  Can't we instead have a configuration\nvariable that tells git that the working tree is on a filesystem with\nbroken pathname semantics, and what kind of workaround is needed?  Isn't\ncore.ignorecase already that configuration variable for case insensitve\nfilesystems [*1*]?\n\nI would imagine that the implementation of equivalent_filenames(a,b) may\nbe !strcmp(a,b) for a sane filesystem [*2*] and !strcasecmp(a,b) for a\ncase insensitive filesystem.  For a totally wacky filesystem, your\nlstat(2) based one might end up to be the best we could do [*3*].\n\nWhen two different _names_ \"A\" and \"a\" refer to a single file, the only\nthing that should happen for \"git mv A a\" is for the cache entry for \"A\"\nto be moved to cache entry for \"a\", and no rename(\"A\", \"a\") should be run,\nbut I don't see any such change in the code. It may happen to work (or be\na no-op) on Windows, but because builtin/mv.c is supposed to be generic\n(and that is the reason you introduced the is_same_file() abstraction in\nthe first place), I'd still see this as a breakage.\n\n\n[Footnote]\n\n*1* Off the top of my head, perhaps core.ignorecase may have to grow into\nboolean plus extra to cover \"this is not just case insensitive, but isn't\neven case preserving\" kind of broken filesystems like HFS+, but I didn't\nthink things through.\n\n*2* Incidentally, wouldn't \"!strcmp(a,b)\" solution suddenly start allowing\n\"git mv Makefile Makefile\" that we currently disallow? Is it a regression\n(less safety against an unexpected input) or a good change?\n\n*3* If we can find a solution that does not involve any calls to the\nfilesystem, it would be ideal, as we can reuse it later in codepaths where\nneither file \"a\" or file \"b\" exists on the filesystem yet (think: \"we are\nabout to create 'a' and 'b'---is that sensible, or will they overwrite\nwith each other on this filesystem?\").\n"},{"id":"165529","messageId":"4DA144A5.2080103@web.de","threadId":"26790","inReplyTo":"7vsjuitk59.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Allow git mv FileA fILEa on case ignore file systems","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2011-04-10T05:48:21Z","receivedAt":"2011-04-10T05:48:21Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 20.03.11 06:50, Junio C Hamano wrote:\n========================\n>Torsten Bögershausen <tboegi@web.de> writes:\n>\n>> > The typical use case is when a file \"FileA\" should be renamed into fILEa\n>> > and we are on a case insenstive file system (system core.ignorecase = true).\n>> > Source and destination are the same file, it can be accessed under both names.\n>> > This makes git think that the destination file exists.\n>> > Unless used with --forced, git will refuse the \"git mv FileA fILEa\".\n>> > This change will allow \"git mv FileA fILEa\" under the following condition:\n>> > On Linux/Unix/Mac OS X the move is allowed when the inode of the source and\n>> > destination are equal (and they are on the same device).\n>> > This allows renames of MÄRCHEN into Märchen on Mac OS X.\n>> > (As a side effect, a file can be renamed to a name which is already\n>> > hard-linked to the same inode).\n>> > On Windows, the function win_is_same_file() from compat/win32/same-file.c\n>> > is used.\n>> > It calls GetFileInformationByHandle() to check if both files are\n>> > \"the same\".\n>Yeek; is it just me or the above single block of text too dense to be\n>readable? Can you use paragraph breaks?\nYes\n>\n>> > The typical use case is when a file \"FileA\" should be renamed into fILEa\n>> > and we are on a case insenstive file system (system core.ignorecase = true).\n>Huh? I don't think renaming \"FileA\" to \"fILEa\" is typical at all. It is\n>very rarely done.\nProbably this is not a good example, I changed it to \"mv FILE File\"\n\n>\n>> > (As a side effect, a file can be renamed to a name which is already\n>> > hard-linked to the same inode).\n>It is unclear \"a side effect\" means \"an added bonus\" or \"a regression\" in\n>this sentence. I think this is latter.\nIf it is too much regression, we can check that the link count in struct stat\n\"if (1 == st_nlink)\" .\n\n\n>\n>Allowing filesystem specific logic to detect that two different \"names\"\n>actually refer to a single \"file\" and help renaming succeed is a sane\n>approach, but I think this particular implementation is flawed.\n>\n>The important thing to notice is that \"names\" and \"file\" above refer to\n>the entities from the end user's point of view. Two files hardlinked\n>together on a filesystem with sane pathname handling are never the same\n>\"file\". I would probably have called it equivalent_filenames() to stress\n>the fact that two _different_ names alias to the same file. is_same_file()\n>sounds more like you got two different filenames from the filesystem\n>(i.e. readdir() gave you both at the same time) and you are trying to see\n>if they are the same file, but that is not the case here.\nOk, I need to distinct beetwen file names and files.\n\n\n>I also find it a bad taste to make this feature depend on win32; doesn't a\n>Linux box mounting a vfat filesystem have the same issue that we should be\n>able to solve with the same code?  Can't we instead have a configuration\n>variable that tells git that the working tree is on a filesystem with\n>broken pathname semantics, and what kind of workaround is needed?  Isn't\n>core.ignorecase already that configuration variable for case insensitve\n>filesystems [*1*]?\nAgreed about the bad taste ;-)\nThe suggested patch works for Linux/UNIX/MAC OS X and Windows, \nbut the description wasn't to clear about it.\nThe same code is used for all OS, except Windows that needs some specific code \nwhich is located in cygwin.c and mingw.c\n\n>\n>I would imagine that the implementation of equivalent_filenames(a,b) may\n>be !strcmp(a,b) for a sane filesystem [*2*] and !strcasecmp(a,b) for a\n>case insensitive filesystem.  For a totally wacky filesystem, your\n>lstat(2) based one might end up to be the best we could do [*3*].\n>\n>When two different _names_ \"A\" and \"a\" refer to a single file, the only\n>thing that should happen for \"git mv A a\" is for the cache entry for \"A\"\n>to be moved to cache entry for \"a\", and no rename(\"A\", \"a\") should be run,\n>but I don't see any such change in the code. It may happen to work (or be\n>a no-op) on Windows, but because builtin/mv.c is supposed to be generic\n>(and that is the reason you introduced the is_same_file() abstraction in\n>the first place), I'd still see this as a breakage.\n>\nWhy shouldn't the rename() be done?\n\"git mv A B\" changes both the indes and the file system.\nIsn't it natural to have file name  \"a\" both in the index and in the \nfile system after \"git mv A a\"?\nNote: Windows and MAC OS X allow \"mv A a\" from command line, \nwhile Linux on VFAT gives an error \"'A' and 'a' are the same file\".\n\n>\n>[Footnote]\n>\n>*1* Off the top of my head, perhaps core.ignorecase may have to grow into\n>boolean plus extra to cover \"this is not just case insensitive, but isn't\n>even case preserving\" kind of broken filesystems like HFS+, but I didn't\n>think things through.\nIt is not that bad.\nMy HFS+ here is both case insenstive and case preserving,\nso that core.ignorecase can be used as it is.\n\nNote: HFS+ can be formated to be case sensitive (at least at MAC OS X 10.6),\nbut the default is case insensitive.\n\n>\n>*2* Incidentally, wouldn't \"!strcmp(a,b)\" solution suddenly start allowing\n>\"git mv Makefile Makefile\" that we currently disallow? Is it a regression\n>(less safety against an unexpected input) or a good change?\n\nWhen running \"git mv git.c git.c\", the current git says:\n\"fatal: can not move directory into itself, source=git.c, destination=git.c\"\nThat test is done earlier in the code path, \nso that this move is still not allowed.\nThe error message is somewhat confusing, as git.c is a file.\nIs it worth to send a separate patch for that?\n\n>\n>*3* If we can find a solution that does not involve any calls to the\n>filesystem, it would be ideal, as we can reuse it later in codepaths where\n>neither file \"a\" or file \"b\" exists on the filesystem yet (think: \"we are\n>about to create 'a' and 'b'---is that sensible, or will they overwrite\n>with each other on this filesystem?\").\n>\nThat would mean, that git needs to know the encoding of the local filesystem.\nBut we don't have that at the moment.\nWhat do you have in mind?\n\n\nI run a test script (see at the end of the mail, save it under test-mv.sh and\nexecute it with /bin/sh test-mv.sh).\nI tested Linux, MAC OS X, cygwin 1.5, cygwin 1.7 and current msys(git).\nShort summary: There are different flavors which filenames are equivalent.\n\nI send out a new patch, thank you for reading this long stuff.\n\n/Torsten\n\n\n=======================\nLinux 2.6.34.7-0.7-default\n/dev/sda6 on /D type vfat (rw,noexec,nosuid,nodev,gid=100,umask=0002,utf8=true)\nenc=UTF-8\na=A    Case ignored\nmv: `a' and `A' are the same file\n\næ=Æ    Case ignored\nmv: `æ' and `Æ' are the same file\n\nø=ø    Case sensitive\nmv:  ø  -->  Ø  OK\n\nä=Ä    Case ignored\nmv: `ä' and `Ä' are the same file\n==================================================================\nLinux 2.6.34.7-0.7-default\n/dev/sda6 on /D type vfat (rw)\nenc=UTF-8\na=A    Case ignored\nmv: `a' and `A' are the same file\n\næ=æ    Case sensitive\nmv:  æ  -->  Æ  OK\n\nø=ø    Case sensitive\nmv:  ø  -->  Ø  OK\n\nä=ä    Case sensitive\nmv:  ä  -->  Ä  OK\n==================================================================\nLinux 2.6.34.7-0.7-default\n//birne/tb on /birne/tb type cifs (rw)\nenc=UTF-8\na=A    Case ignored\nmv:  a  -->  A  OK\n\næ=Æ    Case ignored\nmv:  æ  -->  Æ  OK\n\nø=Ø    Case ignored\nmv:  ø  -->  Ø  OK\n\nä=Ä    Case ignored\nmv:  ä  -->  Ä  OK\n==================================================================\nDarwin 10.7.0\n/dev/disk0s2 on / (hfs, local, journaled)\nenc=UTF-8\na=A    Case ignored\nmv:  a  -->  A  OK\n\næ=Æ    Case ignored\nmv:  æ  -->  Æ  OK\n\nø=Ø    Case ignored\nmv:  ø  -->  Ø  OK\n\nä=Ä    Case ignored\nmv:  ä  -->  Ä  OK\n==================================================================\nDarwin 10.7.0\n/dev/disk1s3 on /Volumes/LACIEFAT (msdos, local, nodev, nosuid, noowners)\nenc=UTF-8\na=A    Case ignored\nmv:  a  -->  A  OK\n\næ=Æ    Case ignored\nmv:  æ  -->  Æ  OK\n\nø=Ø    Case ignored\nmv:  ø  -->  Ø  OK\n\nä=Ä    Case ignored\nmv:  ä  -->  Ä  OK\n==================================================================\nDarwin 10.7.0\n/dev/disk1s2 on /Volumes/LacieMacOS (hfs, local, nodev, nosuid, journaled)\nenc=UTF-8\na=a    Case sensitive\nmv:  a  -->  A  OK\n\næ=æ    Case sensitive\nmv:  æ  -->  Æ  OK\n\nø=ø    Case sensitive\nmv:  ø  -->  Ø  OK\n\nä=ä    Case sensitive\nmv:  ä  -->  Ä  OK\n==================================================================\nCYGWIN_NT-5.1 1.5.25(0.156/4/2)\nC:\\cygwin on / type system (binmode)\nenc=ISO-8859-1\na=A    Case ignored\nmv:  a  -->  A  OK\n\næ=Æ    Case ignored\nmv: `æ' and `Æ' are the same file\n\nø=Ø    Case ignored\nmv: `ø' and `Ø' are the same file\n\nä=Ä    Case ignored\nmv: `ä' and `Ä' are the same file\n==================================================================\nCYGWIN_NT-5.1 1.7.8(0.236/5/3)\nC:/cygwin on / type ntfs (binary,auto)\nenc=UTF-8\na=A    Case ignored\nmv:  a  -->  A  OK\n\næ=Æ    Case ignored\nmv: `æ' and `Æ' are the same file\n\nø=Ø    Case ignored\nmv: `ø' and `Ø' are the same file\n\nä=Ä    Case ignored\nmv: `ä' and `Ä' are the same file\n==================================================================\nMINGW32_NT-5.1 1.0.12(0.46/3/2)\nC:\\msysgit\\msysgit on / type user (binmode,noumount)\nenc=ISO-8859-1\na=A    Case ignored\nmv:  a  -->  A  OK\n\næ=Æ    Case ignored\nmv:  æ  -->  Æ  OK\n\nø=Ø    Case ignored\nmv:  ø  -->  Ø  OK\n\nä=Ä    Case ignored\nmv:  ä  -->  Ä  OK\n\n\n\n\n\n==========================================================\n#!/bin/sh\ntestmv() {\n  (echo $1 > $1 && echo $2 > $2) || {\n    echo >&2 \"Wrong encoding $enc\"\n    cd ..\n    rm -rf $$.trash\n    exit 1\n  }\n  a=$(cat $1)\n  printf \"$1=$a\"\n  if test $(cat $1) != $1; then\n    echo \"    Case ignored\"\n  else\n    echo \"    Case sensitive\"\n  fi\n  rm -f $1 $2\n\n  echo $1 > $1\n  if mv $1 $2; then\n    echo \"mv:  $1  -->  $2  OK\"\n  fi\n  rm -f $1 $2\n  echo\n  return 0\n}\n\nrm -rf $$.trash\nmkdir $$.trash || {\n  echo >&2 \"mkdir $$.trash failed\"\n  exit 1\n}\n\ncd $$.trash || {\n  echo >&2 \"cd $$.trash failed\"\n  exit 1\n}\nuname -sr\n# get root dir\nrdir=$PWD\nwhile test \"$rdir\" != \"\"\ndo\n  if mount | grep $rdir; then\n    break\n  fi\n  rdir=${rdir%/*}\ndone\nif test -z $rdir; then\n  mount | grep \" / \"\nfi\n\nenc=ISO-8859-1\n#Find out if utf-8 is used\ncase \"$LANG\" in\n  *[uU][tT][fF]*8)\n  enc=UTF-8\n  ;;\nesac\ncase \"$LC_CTYPE\" in\n  *[uU][tT][fF]*8)\n  enc=UTF-8\n  ;;\nesac\n\ncase $(uname) in\n  Darwin)\n  enc=UTF-8\n  ;;\nesac\n\necho enc=$enc\n\ncase \"$enc\" in\n  UTF-8)\n  ae=$(printf '\\303\\246')\n  AE=$(printf '\\303\\206')\n  oe=$(printf '\\303\\270')\n  OE=$(printf '\\303\\230')\n  auml=$(printf '\\303\\244')\n  Auml=$(printf '\\303\\204')\n  ;;\n  *)\n  ae=$(printf '\\346')\n  AE=$(printf '\\306')\n  oe=$(printf '\\370')\n  OE=$(printf '\\330')\n  auml=$(printf '\\344')\n  Auml=$(printf '\\304')\n  ;;\nesac\n\ntestmv a A\ntestmv $ae $AE\ntestmv $oe $OE\ntestmv $auml $Auml\n\ncd ..\nrm -rf $$.trash\n"},{"id":"165629","messageId":"7vd3ksybfr.fsf@alter.siamese.dyndns.org","threadId":"26790","inReplyTo":"4DA144A5.2080103@web.de","subject":"Re: [PATCH v2] Allow git mv FileA fILEa on case ignore file systems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-11T16:55:52Z","receivedAt":"2011-04-11T16:55:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n>>When two different _names_ \"A\" and \"a\" refer to a single file, the only\n>>thing that should happen for \"git mv A a\" is for the cache entry for \"A\"\n>>to be moved to cache entry for \"a\", and no rename(\"A\", \"a\") should be run,\n>>but I don't see any such change in the code. It may happen to work (or be\n>>a no-op) on Windows, but because builtin/mv.c is supposed to be generic\n>>(and that is the reason you introduced the is_same_file() abstraction in\n>>the first place), I'd still see this as a breakage.\n>>\n> Why shouldn't the rename() be done?\n> \"git mv A B\" changes both the indes and the file system.\n> Isn't it natural to have file name  \"a\" both in the index and in the \n> file system after \"git mv A a\"?\n> Note: Windows and MAC OS X allow \"mv A a\" from command line, \n> while Linux on VFAT gives an error \"'A' and 'a' are the same file\".\n\nYeah, I forgot about the primary thing we are trying to do in this\ndiscussion.  Sorry about that.  My thinking stopped at 'if we rename \"A\"\nto \"a in the index, that is sufficient.  We already know that we can still\nopen(\"A\") because the filesystem is case insensitive.'\n\nIn fact, we want both the index entry \"A\" renamed to \"a\" _and_ also we\nwant to see next \"/bin/ls\" to show \"a\", not \"A\".  For the latter, we do\nwant to run rename(2) on them.\n\nThere was another thing that made me worry about running rename(2) on two\nequivalent filenames.  You said on Linux VFAT \"mv A a\" fails.  Does the\nunderlying rename(2) fail when you do this?\n\n\tstatus = rename(\"A\", \"a\");\n\nIf old and new resolve to the same existing directory entry or different\ndirectory entries for the same existing file, POSIX says that rename(old,\nnew) should succeed and perform no other action, so the above should\nsucceed on correctly implemented case insensitive filesystems.\n\nBut we know not all FS implementations are perfect.  If this can result in\nan unnecessary failure, it would be far better if we are careful to avoid\nrunning rename(\"A\", \"a\") and just rename the index entry to make sure \"git\nmv A a\" succeeds, than trying to fulfil the \"we want to see next '/bin/ls'\nto show 'a' not 'A'\" wish and make the whole \"git mv A a\" fail.\n\nBut of course we can always blame the breakage on filesystems.  I am\nleaning to prefer running rename(\"A\", \"a\") unconditionally.\n\nThanks.\n"},{"id":"165645","messageId":"4DA35F22.7090006@web.de","threadId":"26790","inReplyTo":"7vd3ksybfr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Allow git mv FileA fILEa on case ignore file systems","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2011-04-11T20:05:54Z","receivedAt":"2011-04-11T20:05:54Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 04/11/2011 06:55 PM, Junio C Hamano wrote:\n> Torsten Bögershausen<tboegi@web.de>  writes:\n>\n>>> When two different _names_ \"A\" and \"a\" refer to a single file, the only\n>>> thing that should happen for \"git mv A a\" is for the cache entry for \"A\"\n>>> to be moved to cache entry for \"a\", and no rename(\"A\", \"a\") should be run,\n>>> but I don't see any such change in the code. It may happen to work (or be\n>>> a no-op) on Windows, but because builtin/mv.c is supposed to be generic\n>>> (and that is the reason you introduced the is_same_file() abstraction in\n>>> the first place), I'd still see this as a breakage.\n>>>\n>> Why shouldn't the rename() be done?\n>> \"git mv A B\" changes both the indes and the file system.\n>> Isn't it natural to have file name  \"a\" both in the index and in the\n>> file system after \"git mv A a\"?\n>> Note: Windows and MAC OS X allow \"mv A a\" from command line,\n>> while Linux on VFAT gives an error \"'A' and 'a' are the same file\".\n> Yeah, I forgot about the primary thing we are trying to do in this\n> discussion.  Sorry about that.  My thinking stopped at 'if we rename \"A\"\n> to \"a in the index, that is sufficient.  We already know that we can still\n> open(\"A\") because the filesystem is case insensitive.'\n>\n> In fact, we want both the index entry \"A\" renamed to \"a\" _and_ also we\n> want to see next \"/bin/ls\" to show \"a\", not \"A\".  For the latter, we do\n> want to run rename(2) on them.\n>\n> There was another thing that made me worry about running rename(2) on two\n> equivalent filenames.  You said on Linux VFAT \"mv A a\" fails.  Does the\n> underlying rename(2) fail when you do this?\n>\n> \tstatus = rename(\"A\", \"a\");\n>\n> If old and new resolve to the same existing directory entry or different\n> directory entries for the same existing file, POSIX says that rename(old,\n> new) should succeed and perform no other action, so the above should\n> succeed on correctly implemented case insensitive filesystems.\n>\n> But we know not all FS implementations are perfect.  If this can result in\n> an unnecessary failure, it would be far better if we are careful to avoid\n> running rename(\"A\", \"a\") and just rename the index entry to make sure \"git\n> mv A a\" succeeds, than trying to fulfil the \"we want to see next '/bin/ls'\n> to show 'a' not 'A'\" wish and make the whole \"git mv A a\" fail.\n>\n> But of course we can always blame the breakage on filesystems.  I am\n> leaning to prefer running rename(\"A\", \"a\") unconditionally.\n>\n> Thanks.\nThanks for reading and for the reply.\n\n>You said on Linux VFAT \"mv A a\" fails\n\nMy excuses for being unclear, as \"fails\" often means returning -1 and \nsetting errno.\nBut this is not the case here. I should have said:\nrename(\"A\", \"a\") returns 0 but does not rename the file.\n\nHere comes the  long version. I wrote a short program called \"rename\",  \nand run some tests.\nThe /D is a VFAT partition, mounted writable for root only.\n-----------------------------------\ntb@wanderer:/D/test> ls\nA  B  x\ntb@wanderer:/D/test> rename A A\nrename A A res=0 Success\ntb@wanderer:/D/test> ls\nA  B  x\ntb@wanderer:/D/test> rename A a\nrename A a res=0 Success\ntb@wanderer:/D/test> ls\nA  B  x\ntb@wanderer:/D/test> rename A D\nrename A D res=-1 Permission denied\ntb@wanderer:/D/test>\n------------------------------------\n\nI had a short look into the Linux kernel. A comment in namei_vfat.c \nindicates that\nrename \"filename\" \"FILENAME\" is not supported for now.\n(and all respect and thanks from my side to the people who made VFAT \nworking).\n(and of course to the git people)\n"},{"id":"165664","messageId":"4DA3EE2C.2010702@viscovery.net","threadId":"26790","inReplyTo":"7vd3ksybfr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Allow git mv FileA fILEa on case ignore file systems","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-04-12T06:16:12Z","receivedAt":"2011-04-12T06:16:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 4/11/2011 18:55, schrieb Junio C Hamano:\n> Yeah, I forgot about the primary thing we are trying to do in this\n> discussion.  Sorry about that.  My thinking stopped at 'if we rename \"A\"\n> to \"a in the index, that is sufficient.  We already know that we can still\n> open(\"A\") because the filesystem is case insensitive.'\n> \n> In fact, we want both the index entry \"A\" renamed to \"a\" _and_ also we\n> want to see next \"/bin/ls\" to show \"a\", not \"A\".  For the latter, we do\n> want to run rename(2) on them.\n\nI wonder why we need this at all. You can always break this into two\noperations:\n\n  git mv A a1\n  git mv a1 a\n\nIs 'git mv A a' on a case-insensitive filesystem such an every-day\noperation that we have to provide special support for it in git?\n\n-- Hannes\n"}]}