{"thread":{"id":"36541","subject":"Bug: Case-insensitive filesystems can cause merge and checkout problems","startedAt":"2014-04-29T19:02:00Z","lastAt":"2014-05-08T20:55:57Z","messageCount":24,"participants":["David Turner","Junio C Hamano","dturner@twopensource.com","Johannes Sixt","Jonathan Nieder","brian m. carlson","Torsten Bögershausen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"240197","messageId":"CAE+yK_m_bPt2pS6MQOrpvVDuLAJf8NFxYOgM8i98tU6-gLcTDw@mail.gmail.com","threadId":"36541","inReplyTo":null,"subject":"Bug: Case-insensitive filesystems can cause merge and checkout problems","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-04-29T19:02:00Z","receivedAt":"2014-04-29T19:02:00Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"By default, git sets core.ignorecase=true when git init or git clone\nis run on a machine with a case-insensitive filesystem. Here's a\ntest-case for some problems that this causes:\n\ngit checkout master\ntouch TestCase\ngit add TestCase\ngit commit -m 'add TestCase'\ngit checkout -b with-camel\ntouch foo\ngit add foo\ngit commit -m 'intervening commit'\ngit checkout master\ngit rm TestCase\ntouch testcase\ngit add testcase\ngit commit -m 'rename to testcase'\ngit checkout with-camel\ngit merge master -m 'merge'\n\nOne would expect a clean working copy at this point, but in fact, the\nfile 'testcase' will be deleted.\n\nWith core.ignorecase=false, we get a different failure. The\npenultimate command fails with:\n$ git checkout with-camel\nerror: The following untracked working tree files would be overwritten\nby checkout:\n TestCase\nPlease move or remove them before you can switch branches.\nAborting\n\nOf course, there is no untracked working tree file by that name; there\nis a tracked working tree file named testcase (all-lowercase).\n"},{"id":"240501","messageId":"1398990069.19099.5.camel@stross","threadId":"36541","inReplyTo":"CAE+yK_m_bPt2pS6MQOrpvVDuLAJf8NFxYOgM8i98tU6-gLcTDw@mail.gmail.com","subject":"[PATCH] merge-recursive.c: Fix case-changing merge bug","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-02T00:21:09Z","receivedAt":"2014-05-02T00:21:09Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On a case-insensitive filesystem, when merging, a file would be\nwrongly deleted from the working tree if an incoming commit had\nrenamed it changing only its case. When merging a rename, the file\nwith the old name would be deleted -- but since the filesystem\nconsiders the old name to be the same as the new name, the new\nfile would in fact be deleted.\n\nWe avoid this by not deleting files that have a case-clone in the\nindex at stage 0.\n\nSigned-off-by: David Turner \n---\n merge-recursive.c | 6 ++++++\n t/t7063-merge-ignorecase.sh | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+)\n create mode 100755 t/t7063-merge-ignorecase.sh\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 4177092..cab16fa 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -589,6 +589,12 @@ static int remove_file(struct merge_options *o, int clean,\n \t\t\treturn -1;\n \t}\n \tif (update_working_directory) {\n+\t\tif (ignore_case) {\n+\t\t\tstruct cache_entry *ce;\n+\t\t\tce = cache_file_exists(path, strlen(path), ignore_case);\n+\t\t\tif (ce && ce_stage(ce) == 0)\n+\t\t\t\treturn 0;\n+\t\t}\n \t\tif (remove_path(path))\n \t\t\treturn -1;\n \t}\ndiff --git a/t/t7063-merge-ignorecase.sh b/t/t7063-merge-ignorecase.sh\nnew file mode 100755\nindex 0000000..6d4959f\n--- /dev/null\n+++ b/t/t7063-merge-ignorecase.sh\n@@ -0,0 +1,32 @@\n+#!/bin/sh\n+\n+test_description='git-merge with case-changing rename on case-insensitive file system'\n+\n+. ./test-lib.sh\n+\n+if ! test_have_prereq CASE_INSENSITIVE_FS\n+then\n+\tskip_all='skipping case insensitive tests - case sensitive file system'\n+\ttest_done\n+fi\n+\n+test_expect_success 'merge with case-changing rename with ignorecase=true' '\n+\ttest $(git config core.ignorecase) = true &&\n+\ttouch TestCase &&\n+\tgit add TestCase &&\n+\tgit commit -m \"add TestCase\" &&\n+\tgit checkout -b with-camel &&\n+\ttouch foo &&\n+\tgit add foo &&\n+\tgit commit -m \"intervening commit\" &&\n+\tgit checkout master &&\n+\tgit rm TestCase &&\n+\ttouch testcase &&\n+\tgit add testcase &&\n+\tgit commit -m \"rename to testcase\" &&\n+\tgit checkout with-camel &&\n+\tgit merge master -m \"merge\" &&\n+\ttest -e testcase\n+'\n+\n+test_done\n-- \n"},{"id":"240790","messageId":"xmqqwqdyg7jt.fsf@gitster.dls.corp.google.com","threadId":"36541","inReplyTo":"1398990069.19099.5.camel@stross","subject":"Re: [PATCH] merge-recursive.c: Fix case-changing merge bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-06T17:07:18Z","receivedAt":"2014-05-06T17:07:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> On a case-insensitive filesystem, when merging, a file would be\n> wrongly deleted from the working tree if an incoming commit had\n> renamed it changing only its case. When merging a rename, the file\n> with the old name would be deleted -- but since the filesystem\n> considers the old name to be the same as the new name, the new\n> file would in fact be deleted.\n>\n> We avoid this by not deleting files that have a case-clone in the\n> index at stage 0.\n>\n> Signed-off-by: David Turner \n> ---\n> merge-recursive.c | 6 ++++++\n> t/t7063-merge-ignorecase.sh | 32 ++++++++++++++++++++++++++++++++\n> 2 files changed, 38 insertions(+)\n> create mode 100755 t/t7063-merge-ignorecase.sh\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 4177092..cab16fa 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -589,6 +589,12 @@ static int remove_file(struct merge_options *o, int clean,\n> \t\t\treturn -1;\n> \t}\n> \tif (update_working_directory) {\n> +\t\tif (ignore_case) {\n> +\t\t\tstruct cache_entry *ce;\n> +\t\t\tce = cache_file_exists(path, strlen(path), ignore_case);\n> +\t\t\tif (ce && ce_stage(ce) == 0)\n> +\t\t\t\treturn 0;\n> +\t\t}\n> \t\tif (remove_path(path))\n> \t\t\treturn -1;\n> \t}\n\nThanks.\n\nI wonder what happens if you did the same merge using the resolve\nstrategy, though. If you see a similar issue, and the true reason\nof the breakage turns out to be because three-way merge performed by\nunpack_trees() (with opts.update set to 1) considers that these\npaths that only differ in case in \"our\" tree, in the index and in\nthe working tree are different paths (I am not saying that is the\ncase, but that was one of my first hunches after seeing the initial\nreport) then it may suggest that the above might not be the best\nchange to fix the issue.\n\n> diff --git a/t/t7063-merge-ignorecase.sh b/t/t7063-merge-ignorecase.sh\n> new file mode 100755\n> index 0000000..6d4959f\n> --- /dev/null\n> +++ b/t/t7063-merge-ignorecase.sh\n\nHmmm, did you really have to add a file dedicated for this single\ntest?\n\n> @@ -0,0 +1,32 @@\n> +#!/bin/sh\n> +\n> +test_description='git-merge with case-changing rename on case-insensitive file system'\n> +\n> +. ./test-lib.sh\n> +\n> +if ! test_have_prereq CASE_INSENSITIVE_FS\n> +then\n> +\tskip_all='skipping case insensitive tests - case sensitive file system'\n> +\ttest_done\n> +fi\n> +\n> +test_expect_success 'merge with case-changing rename with ignorecase=true' '\n> +\ttest $(git config core.ignorecase) = true &&\n> +\ttouch TestCase &&\n> +\tgit add TestCase &&\n> +\tgit commit -m \"add TestCase\" &&\n> +\tgit checkout -b with-camel &&\n> +\ttouch foo &&\n> +\tgit add foo &&\n> +\tgit commit -m \"intervening commit\" &&\n> +\tgit checkout master &&\n> +\tgit rm TestCase &&\n> +\ttouch testcase &&\n> +\tgit add testcase &&\n> +\tgit commit -m \"rename to testcase\" &&\n> +\tgit checkout with-camel &&\n> +\tgit merge master -m \"merge\" &&\n> +\ttest -e testcase\n> +'\n> +\n> +test_done\n"},{"id":"240797","messageId":"1399397774.11843.46.camel@stross","threadId":"36541","inReplyTo":"xmqqwqdyg7jt.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] merge-recursive.c: Fix case-changing merge bug","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-06T17:36:14Z","receivedAt":"2014-05-06T17:36:14Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2014-05-06 at 10:07 -0700, Junio C Hamano wrote:\n> David Turner writes:\n> \n> > On a case-insensitive filesystem, when merging, a file would be\n> > wrongly deleted from the working tree if an incoming commit had\n> > renamed it changing only its case. When merging a rename, the file\n> > with the old name would be deleted -- but since the filesystem\n> > considers the old name to be the same as the new name, the new\n> > file would in fact be deleted.\n> >\n> > We avoid this by not deleting files that have a case-clone in the\n> > index at stage 0.\n> >\n> > Signed-off-by: David Turner \n> > ---\n> > merge-recursive.c | 6 ++++++\n> > t/t7063-merge-ignorecase.sh | 32 ++++++++++++++++++++++++++++++++\n> > 2 files changed, 38 insertions(+)\n> > create mode 100755 t/t7063-merge-ignorecase.sh\n> >\n> > diff --git a/merge-recursive.c b/merge-recursive.c\n> > index 4177092..cab16fa 100644\n> > --- a/merge-recursive.c\n> > +++ b/merge-recursive.c\n> > @@ -589,6 +589,12 @@ static int remove_file(struct merge_options *o, int clean,\n> > \t\t\treturn -1;\n> > \t}\n> > \tif (update_working_directory) {\n> > +\t\tif (ignore_case) {\n> > +\t\t\tstruct cache_entry *ce;\n> > +\t\t\tce = cache_file_exists(path, strlen(path), ignore_case);\n> > +\t\t\tif (ce && ce_stage(ce) == 0)\n> > +\t\t\t\treturn 0;\n> > +\t\t}\n> > \t\tif (remove_path(path))\n> > \t\t\treturn -1;\n> > \t}\n> \n> Thanks.\n> \n> I wonder what happens if you did the same merge using the resolve\n> strategy, though. If you see a similar issue, and the true reason\n> of the breakage turns out to be because three-way merge performed by\n> unpack_trees() (with opts.update set to 1) considers that these\n> paths that only differ in case in \"our\" tree, in the index and in\n> the working tree are different paths (I am not saying that is the\n> case, but that was one of my first hunches after seeing the initial\n> report) then it may suggest that the above might not be the best\n> change to fix the issue.\n\nI changed the test to -s resolve, and it changed from failing to\npassing. So while I still don't know whether this is the right change,\nit's at least not wrong for that reason.\n\n> > diff --git a/t/t7063-merge-ignorecase.sh b/t/t7063-merge-ignorecase.sh\n> > new file mode 100755\n> > index 0000000..6d4959f\n> > --- /dev/null\n> > +++ b/t/t7063-merge-ignorecase.sh\n> \n> Hmmm, did you really have to add a file dedicated for this single\n> test?\n\nWould you prefer that I add it to t6022-merge-rename.sh? Or I could\nadd it to t7062-wtstatus-ignorecase.sh and rename that file to\nt7062-ignorecase.sh. \n"},{"id":"240817","messageId":"xmqqoazaelmi.fsf@gitster.dls.corp.google.com","threadId":"36541","inReplyTo":"1399397774.11843.46.camel@stross","subject":"Re: [PATCH] merge-recursive.c: Fix case-changing merge bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-06T19:46:13Z","receivedAt":"2014-05-06T19:46:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> Would you prefer that I add it to t6022-merge-rename.sh? Or I could\n> add it to t7062-wtstatus-ignorecase.sh and rename that file to\n> t7062-ignorecase.sh. \n\nIf I had only these two choices, t6022 would be it, as 6xxx series\nis where we have other tests for merge-recursive.\n\nI actually do not have a problem with adding a new file in t6xxx\nseries as you did in this patch, if a longer term direction is to\nadd more cases to it to make sure various paths that are only\ndifferent in their cases (not just combination where\none side renames, but things like combination where both\nsides rename, etc.) are handled correctly during a merge.\n\nThanks.\n\nBy the way, I see \"touch\" used to create a new file in the test,\nlike this:\n\n+\ttouch foo &&\n+\tgit add foo &&\n\nPlease don't. Instead, do it perhaps like this:\n\n\t>foo &&\n git add foo &&\n\nThe primary purpose to use \"touch\" is to update a file's timestamp,\nand using it to create a file is misleading to readers.\n"},{"id":"240854","messageId":"1399417144-24864-1-git-send-email-dturner@twopensource.com","threadId":"36541","inReplyTo":"xmqqoazaelmi.fsf@gitster.dls.corp.google.com","subject":"[PATCH 1/2] merge-recursive.c: Fix case-changing merge.","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-06T22:59:03Z","receivedAt":"2014-05-06T22:59:03Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: David Turner \n\nOn a case-insensitive filesystem, when merging, a file would be\nwrongly deleted from the working tree if an incoming commit had\nrenamed it changing only its case. When merging a rename, the file\nwith the old name would be deleted -- but since the filesystem\nconsiders the old name to be the same as the new name, the new\nfile would in fact be deleted.\n\nWe avoid this by not deleting files that have a case-clone in the\nindex at stage 0.\n\nSigned-off-by: David Turner \n---\n merge-recursive.c | 6 +++++\n t/t6039-merge-ignorecase.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 59 insertions(+)\n create mode 100755 t/t6039-merge-ignorecase.sh\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 4177092..cab16fa 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -589,6 +589,12 @@ static int remove_file(struct merge_options *o, int clean,\n \t\t\treturn -1;\n \t}\n \tif (update_working_directory) {\n+\t\tif (ignore_case) {\n+\t\t\tstruct cache_entry *ce;\n+\t\t\tce = cache_file_exists(path, strlen(path), ignore_case);\n+\t\t\tif (ce && ce_stage(ce) == 0)\n+\t\t\t\treturn 0;\n+\t\t}\n \t\tif (remove_path(path))\n \t\t\treturn -1;\n \t}\ndiff --git a/t/t6039-merge-ignorecase.sh b/t/t6039-merge-ignorecase.sh\nnew file mode 100755\nindex 0000000..ad06752\n--- /dev/null\n+++ b/t/t6039-merge-ignorecase.sh\n@@ -0,0 +1,53 @@\n+#!/bin/sh\n+\n+test_description='git-merge with case-changing rename on case-insensitive file system'\n+\n+. ./test-lib.sh\n+\n+if ! test_have_prereq CASE_INSENSITIVE_FS\n+then\n+\tskip_all='skipping case insensitive tests - case sensitive file system'\n+\ttest_done\n+fi\n+\n+test_expect_success 'merge with case-changing rename' '\n+\ttest $(git config core.ignorecase) = true &&\n+\t> TestCase &&\n+\tgit add TestCase &&\n+\tgit commit -m \"add TestCase\" &&\n+\tgit tag baseline\n+\tgit checkout -b with-camel &&\n+\t> foo &&\n+\tgit add foo &&\n+\tgit commit -m \"intervening commit\" &&\n+\tgit checkout master &&\n+\tgit rm TestCase &&\n+\t> testcase &&\n+\tgit add testcase &&\n+\tgit commit -m \"rename to testcase\" &&\n+\tgit checkout with-camel &&\n+\tgit merge master -m \"merge\" &&\n+\ttest -e testcase\n+'\n+\n+test_expect_success 'merge with case-changing rename on both sides' '\n+\tgit checkout master &&\n+\tgit reset --hard baseline &&\n+\tgit branch -D with-camel &&\n+\tgit checkout -b with-camel &&\n+\tgit mv --force TestCase testcase &&\n+\tgit commit -m \"recase on branch\" &&\n+\t> foo &&\n+\tgit add foo &&\n+\tgit commit -m \"intervening commit\" &&\n+\tgit checkout master &&\n+\tgit rm TestCase &&\n+\t> testcase &&\n+\tgit add testcase &&\n+\tgit commit -m \"rename to testcase\" &&\n+\tgit checkout with-camel &&\n+\tgit merge master -m \"merge\" &&\n+\ttest -e testcase\n+'\n+\n+test_done\n-- \n2.0.0.rc0.33.g27630aa\n"},{"id":"240855","messageId":"1399417144-24864-2-git-send-email-dturner@twopensource.com","threadId":"36541","inReplyTo":"1399417144-24864-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-06T22:59:04Z","receivedAt":"2014-05-06T22:59:04Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: David Turner \n\nMake it possible to change the case of a filename on a\ncase-insensitive filesystem using git mv. Change git mv to allow\nmoves where the destination file exists if either the destination file\nhas the same inode as the source file (for Mac) or the same name\nignoring case (for Win).\n\nSigned-off-by: David Turner \n---\n builtin/mv.c | 18 ++++++++++--------\n t/t6039-merge-ignorecase.sh | 2 +-\n 2 files changed, 11 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 45e57f3..8cead13 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -74,7 +74,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t};\n \tconst char **source, **destination, **dest_path, **submodule_gitfile;\n \tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n-\tstruct stat st;\n+\tstruct stat src_st,dst_st;\n \tstruct string_list src_for_dst = STRING_LIST_INIT_NODUP;\n \n \tgitmodules_config();\n@@ -102,8 +102,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\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, DUP_BASENAME);\n-\telse if (!lstat(dest_path[0], &st) &&\n-\t\t\tS_ISDIR(st.st_mode)) {\n+\telse if (!lstat(dest_path[0], &dst_st) &&\n+\t\t\tS_ISDIR(dst_st.st_mode)) {\n \t\tdest_path[0] = add_slash(dest_path[0]);\n \t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);\n \t} else {\n@@ -122,13 +122,13 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\tprintf(_(\"Checking rename of '%s' to '%s'\\n\"), src, dst);\n \n \t\tlength = strlen(src);\n-\t\tif (lstat(src, &st) < 0)\n+\t\tif (lstat(src, &src_st) < 0)\n \t\t\tbad = _(\"bad source\");\n \t\telse if (!strncmp(src, dst, length) &&\n \t\t\t\t(dst[length] == 0 || dst[length] == '/')) {\n \t\t\tbad = _(\"can not move directory into itself\");\n-\t\t} else if ((src_is_dir = S_ISDIR(st.st_mode))\n-\t\t\t\t&& lstat(dst, &st) == 0)\n+\t\t} else if ((src_is_dir = S_ISDIR(src_st.st_mode))\n+\t\t\t\t&& lstat(dst, &dst_st) == 0)\n \t\t\tbad = _(\"cannot move directory over file\");\n \t\telse if (src_is_dir) {\n \t\t\tint first = cache_name_pos(src, length);\n@@ -202,14 +202,16 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t}\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\telse if (lstat(dst, &dst_st) == 0 &&\n+\t\t\t (src_st.st_ino != dst_st.st_ino ||\n+\t\t\t (src_st.st_ino == 0 && strcasecmp(src, dst)))) {\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\tif (S_ISREG(dst_st.st_mode) || S_ISLNK(dst_st.st_mode)) {\n \t\t\t\t\tif (verbose)\n \t\t\t\t\t\twarning(_(\"overwriting '%s'\"), dst);\n \t\t\t\t\tbad = NULL;\ndiff --git a/t/t6039-merge-ignorecase.sh b/t/t6039-merge-ignorecase.sh\nindex ad06752..28cfb49 100755\n--- a/t/t6039-merge-ignorecase.sh\n+++ b/t/t6039-merge-ignorecase.sh\n@@ -35,7 +35,7 @@ test_expect_success 'merge with case-changing rename on both sides' '\n \tgit reset --hard baseline &&\n \tgit branch -D with-camel &&\n \tgit checkout -b with-camel &&\n-\tgit mv --force TestCase testcase &&\n+\tgit mv TestCase testcase &&\n \tgit commit -m \"recase on branch\" &&\n \t> foo &&\n \tgit add foo &&\n-- \n2.0.0.rc0.33.g27630aa\n"},{"id":"240871","messageId":"5369CFDE.2070207@viscovery.net","threadId":"36541","inReplyTo":"1399417144-24864-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-05-07T06:17:02Z","receivedAt":"2014-05-07T06:17:02Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/7/2014 0:59, schrieb dturner@twopensource.com:\n> From: David Turner \n> \n> Make it possible to change the case of a filename on a\n> case-insensitive filesystem using git mv. Change git mv to allow\n> moves where the destination file exists if either the destination file\n> has the same inode as the source file (for Mac) or the same name\n> ignoring case (for Win).\n> \n> Signed-off-by: David Turner \n> ---\n> builtin/mv.c | 18 ++++++++++--------\n> t/t6039-merge-ignorecase.sh | 2 +-\n> 2 files changed, 11 insertions(+), 9 deletions(-)\n> \n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 45e57f3..8cead13 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -74,7 +74,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> \t};\n> \tconst char **source, **destination, **dest_path, **submodule_gitfile;\n> \tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n> -\tstruct stat st;\n> +\tstruct stat src_st,dst_st;\n> \tstruct string_list src_for_dst = STRING_LIST_INIT_NODUP;\n> \n> \tgitmodules_config();\n> @@ -102,8 +102,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\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, DUP_BASENAME);\n> -\telse if (!lstat(dest_path[0], &st) &&\n> -\t\t\tS_ISDIR(st.st_mode)) {\n> +\telse if (!lstat(dest_path[0], &dst_st) &&\n> +\t\t\tS_ISDIR(dst_st.st_mode)) {\n> \t\tdest_path[0] = add_slash(dest_path[0]);\n> \t\tdestination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);\n> \t} else {\n> @@ -122,13 +122,13 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> \t\t\tprintf(_(\"Checking rename of '%s' to '%s'\\n\"), src, dst);\n> \n> \t\tlength = strlen(src);\n> -\t\tif (lstat(src, &st) < 0)\n> +\t\tif (lstat(src, &src_st) < 0)\n> \t\t\tbad = _(\"bad source\");\n> \t\telse if (!strncmp(src, dst, length) &&\n> \t\t\t\t(dst[length] == 0 || dst[length] == '/')) {\n> \t\t\tbad = _(\"can not move directory into itself\");\n> -\t\t} else if ((src_is_dir = S_ISDIR(st.st_mode))\n> -\t\t\t\t&& lstat(dst, &st) == 0)\n> +\t\t} else if ((src_is_dir = S_ISDIR(src_st.st_mode))\n> +\t\t\t\t&& lstat(dst, &dst_st) == 0)\n> \t\t\tbad = _(\"cannot move directory over file\");\n> \t\telse if (src_is_dir) {\n> \t\t\tint first = cache_name_pos(src, length);\n> @@ -202,14 +202,16 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> \t\t\t}\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\telse if (lstat(dst, &dst_st) == 0 &&\n> +\t\t\t (src_st.st_ino != dst_st.st_ino ||\n> +\t\t\t (src_st.st_ino == 0 && strcasecmp(src, dst)))) {\n\nDon't do that. st_ino is zero on Windows only because we do not spend time\nto fill in the field. Don't use it as an indicator for a case-insensitive\nfile system; zero may be a valid inode number on other systems.\n\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\tif (S_ISREG(dst_st.st_mode) || S_ISLNK(dst_st.st_mode)) {\n> \t\t\t\t\tif (verbose)\n> \t\t\t\t\t\twarning(_(\"overwriting '%s'\"), dst);\n> \t\t\t\t\tbad = NULL;\n> diff --git a/t/t6039-merge-ignorecase.sh b/t/t6039-merge-ignorecase.sh\n> index ad06752..28cfb49 100755\n> --- a/t/t6039-merge-ignorecase.sh\n> +++ b/t/t6039-merge-ignorecase.sh\n> @@ -35,7 +35,7 @@ test_expect_success 'merge with case-changing rename on both sides' '\n> \tgit reset --hard baseline &&\n> \tgit branch -D with-camel &&\n> \tgit checkout -b with-camel &&\n> -\tgit mv --force TestCase testcase &&\n> +\tgit mv TestCase testcase &&\n> \tgit commit -m \"recase on branch\" &&\n> \t> foo &&\n> \tgit add foo &&\n"},{"id":"240917","messageId":"1399480977.11843.62.camel@stross","threadId":"36541","inReplyTo":"5369CFDE.2070207@viscovery.net","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-07T16:42:57Z","receivedAt":"2014-05-07T16:42:57Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:\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\telse if (lstat(dst, &dst_st) == 0 &&\n> > +\t\t\t (src_st.st_ino != dst_st.st_ino ||\n> > +\t\t\t (src_st.st_ino == 0 && strcasecmp(src, dst)))) {\n> \n> Don't do that. st_ino is zero on Windows only because we do not spend time\n> to fill in the field. Don't use it as an indicator for a case-insensitive\n> file system; zero may be a valid inode number on other systems.\n\nI don't think it is a problem if zero is a valid inode. The only thing\nthat happens when there is a zero inode, is that we have to compare\nfilenames. The inode check is just an optimization to avoid doing a\nbunch of strcasecmp on systems that don't have to.\n"},{"id":"240923","messageId":"xmqqha51bhxg.fsf@gitster.dls.corp.google.com","threadId":"36541","inReplyTo":"1399480977.11843.62.camel@stross","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T17:46:35Z","receivedAt":"2014-05-07T17:46:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:\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\telse if (lstat(dst, &dst_st) == 0 &&\n>> > +\t\t\t (src_st.st_ino != dst_st.st_ino ||\n>> > +\t\t\t (src_st.st_ino == 0 && strcasecmp(src, dst)))) {\n>> \n>> Don't do that. st_ino is zero on Windows only because we do not spend time\n>> to fill in the field. Don't use it as an indicator for a case-insensitive\n>> file system; zero may be a valid inode number on other systems.\n>\n> I don't think it is a problem if zero is a valid inode. The only thing\n> that happens when there is a zero inode, is that we have to compare\n> filenames. The inode check is just an optimization to avoid doing a\n> bunch of strcasecmp on systems that don't have to.\n\nAm I correct to rephrase you that the code assumes that any\nfilesystem that cannot give unique inum to different files will use\n0 as the placeholder inum, so if src/dst share the same non-zero\ninum, it is guaranteed that is not a placeholder and we know they\nare different files without comparing the two paths?\n"},{"id":"240925","messageId":"xmqqd2fpbh9b.fsf@gitster.dls.corp.google.com","threadId":"36541","inReplyTo":"1399417144-24864-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH 1/2] merge-recursive.c: Fix case-changing merge.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T18:01:04Z","receivedAt":"2014-05-07T18:01:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"dturner@twopensource.com writes:\n\n> +if ! test_have_prereq CASE_INSENSITIVE_FS\n> +then\n> +\tskip_all='skipping case insensitive tests - case sensitive file system'\n> +\ttest_done\n> +fi\n> +\n> +test_expect_success 'merge with case-changing rename' '\n> +\ttest $(git config core.ignorecase) = true &&\n\nThis check seems a bit strange. You already know you are on a case\ninsensitive filesystem, so I would understand that if you assume it\nis set, or if you make sure it is set (if you are really paranoid).\n\nBut I'll let it pass, as it is not wrong per-se. Just looked strange.\n\n> +\t> TestCase &&\n\nPlease have no SP between redirection operator and its target.\n\n> +\ttest -e testcase\n\nPlease make it a habit to use \"test -f\" when you expect \"the path\nexists as a file\", not merely \"something exists there I do not care\nif it is a file or a directory\", for which \"test -e\" is perfectly\nappropriate.\n\nI'll fix up locally before queuing; no need to resend.\n\nThanks.\n"},{"id":"240926","messageId":"1399485666.11843.68.camel@stross","threadId":"36541","inReplyTo":"xmqqha51bhxg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-07T18:01:06Z","receivedAt":"2014-05-07T18:01:06Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2014-05-07 at 10:46 -0700, Junio C Hamano wrote:\n> David Turner writes:\n> \n> > On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:\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\telse if (lstat(dst, &dst_st) == 0 &&\n> >> > +\t\t\t (src_st.st_ino != dst_st.st_ino ||\n> >> > +\t\t\t (src_st.st_ino == 0 && strcasecmp(src, dst)))) {\n> >> \n> >> Don't do that. st_ino is zero on Windows only because we do not spend time\n> >> to fill in the field. Don't use it as an indicator for a case-insensitive\n> >> file system; zero may be a valid inode number on other systems.\n> >\n> > I don't think it is a problem if zero is a valid inode. The only thing\n> > that happens when there is a zero inode, is that we have to compare\n> > filenames. The inode check is just an optimization to avoid doing a\n> > bunch of strcasecmp on systems that don't have to.\n> \n> Am I correct to rephrase you that the code assumes that any\n> filesystem that cannot give unique inum to different files will use\n> 0 as the placeholder inum, so if src/dst share the same non-zero\n> inum, it is guaranteed that is not a placeholder and we know they\n> are different files without comparing the two paths?\n\nYes, this is indeed a fair rephrasing. In fact, the entire zero-check\nshould not be necessary, as POSIX requires that the st_ino field has a\n\"meaningful\" value. So in the case that this ever runs into a problem,\nwe ought to wrap the lstat call with a compatibility layer anyway. \n\nBut maybe there is an OS I'm not thinking of which fills in st_ino with\nsomething else?\n"},{"id":"240929","messageId":"20140507181355.GW9218@google.com","threadId":"36541","inReplyTo":"xmqqd2fpbh9b.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] merge-recursive.c: Fix case-changing merge.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-05-07T18:13:55Z","receivedAt":"2014-05-07T18:13:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> dturner@twopensource.com writes:\n\n>> +\ttest -e testcase\n>\n> Please make it a habit to use \"test -f\" when you expect \"the path\n> exists as a file\", not merely \"something exists there I do not care\n> if it is a file or a directory\", for which \"test -e\" is perfectly\n> appropriate.\n\nOr, better in tests,\n\n\ttest_path_is_file testcase\n\nwhich prints an error instead of just silently failing when\nthe path is not a file.\n\nThanks,\nJonathan\n"},{"id":"240960","messageId":"xmqqa9at8g56.fsf@gitster.dls.corp.google.com","threadId":"36541","inReplyTo":"20140507181355.GW9218@google.com","subject":"Re: [PATCH 1/2] merge-recursive.c: Fix case-changing merge.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T20:53:25Z","receivedAt":"2014-05-07T20:53:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n>> Please make it a habit to use \"test -f\" when you expect \"the path\n>> exists as a file\", not merely \"something exists there I do not care\n>> if it is a file or a directory\", for which \"test -e\" is perfectly\n>> appropriate.\n>\n> Or, better in tests,\n>\n> \ttest_path_is_file testcase\n>\n> which prints an error instead of just silently failing when\n> the path is not a file.\n\nThanks, will tweak.\n"},{"id":"240984","messageId":"20140508012238.GA25231@vauxhall.crustytoothpaste.net","threadId":"36541","inReplyTo":"1399417144-24864-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-08T01:22:38Z","receivedAt":"2014-05-08T01:22:38Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, May 06, 2014 at 03:59:04PM -0700, dturner@twopensource.com wrote:\n> @@ -74,7 +74,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> \t};\n> \tconst char **source, **destination, **dest_path, **submodule_gitfile;\n> \tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n> -\tstruct stat st;\n> +\tstruct stat src_st,dst_st;\n\nExtremely minor nit: we generally put a space after the comma.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"240994","messageId":"536B2637.2060808@viscovery.net","threadId":"36541","inReplyTo":"xmqqha51bhxg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-05-08T06:37:43Z","receivedAt":"2014-05-08T06:37:43Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/7/2014 19:46, schrieb Junio C Hamano:\n> David Turner writes:\n> \n>> On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:\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\telse if (lstat(dst, &dst_st) == 0 &&\n>>>> +\t\t\t (src_st.st_ino != dst_st.st_ino ||\n>>>> +\t\t\t (src_st.st_ino == 0 && strcasecmp(src, dst)))) {\n>>>\n>>> Don't do that. st_ino is zero on Windows only because we do not spend time\n>>> to fill in the field. Don't use it as an indicator for a case-insensitive\n>>> file system; zero may be a valid inode number on other systems.\n>>\n>> I don't think it is a problem if zero is a valid inode. The only thing\n>> that happens when there is a zero inode, is that we have to compare\n>> filenames. The inode check is just an optimization to avoid doing a\n>> bunch of strcasecmp on systems that don't have to.\n> \n> Am I correct to rephrase you that the code assumes that any\n> filesystem that cannot give unique inum to different files will use\n> 0 as the placeholder inum, so if src/dst share the same non-zero\n> inum, it is guaranteed that is not a placeholder and we know they\n> are different files without comparing the two paths?\n\n\"If src and dst share the same non-zero inum, it is guaranteed that it is\nnot a placeholder and we know they are the same file\" is more correct.\n\nWhat if both inums are zero? Can this happen on any sane POSIX system? I\ndon't know, but my gut feeling is that inode zero is too special to be\nallocated for files or directories.\n\nIn that case, it is safe to assume that the st_ino field is just a\nplaceholder when it is zero, and we have to compare the file name. Then we\ncan either assume that this happens only for our emulation layer on MinGW\n(and the comparison can be case-insensitive) or choose the comparison mode\nbased on core.ignorecase. This patch does the former, but I think we\nshould do the latter.\n\n-- Hannes\n"},{"id":"241002","messageId":"536B4680.1010806@web.de","threadId":"36541","inReplyTo":"536B2637.2060808@viscovery.net","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-05-08T08:55:28Z","receivedAt":"2014-05-08T08:55:28Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 05/08/2014 08:37 AM, Johannes Sixt wrote:\n> Am 5/7/2014 19:46, schrieb Junio C Hamano:\n>> David Turner writes:\n>>\n>>> On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:\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\telse if (lstat(dst, &dst_st) == 0 &&\n>>>>> +\t\t\t (src_st.st_ino != dst_st.st_ino ||\n>>>>> +\t\t\t (src_st.st_ino == 0 && strcasecmp(src, dst)))) {\n>>>> Don't do that. st_ino is zero on Windows only because we do not spend time\n>>>> to fill in the field. Don't use it as an indicator for a case-insensitive\n>>>> file system; zero may be a valid inode number on other systems.\n>>> I don't think it is a problem if zero is a valid inode. The only thing\n>>> that happens when there is a zero inode, is that we have to compare\n>>> filenames. The inode check is just an optimization to avoid doing a\n>>> bunch of strcasecmp on systems that don't have to.\n>> Am I correct to rephrase you that the code assumes that any\n>> filesystem that cannot give unique inum to different files will use\n>> 0 as the placeholder inum, so if src/dst share the same non-zero\n>> inum, it is guaranteed that is not a placeholder and we know they\n>> are different files without comparing the two paths?\n> \"If src and dst share the same non-zero inum, it is guaranteed that it is\n> not a placeholder and we know they are the same file\" is more correct.\n>\n> What if both inums are zero? Can this happen on any sane POSIX system? I\n> don't know, but my gut feeling is that inode zero is too special to be\n> allocated for files or directories.\n>\n> In that case, it is safe to assume that the st_ino field is just a\n> placeholder when it is zero, and we have to compare the file name. Then we\n> can either assume that this happens only for our emulation layer on MinGW\n> (and the comparison can be case-insensitive) or choose the comparison mode\n> based on core.ignorecase. This patch does the former, but I think we\n> should do the latter.\nWhatever we do, we should \"protect\" the strcasecmp() with ignore_case:\n\n!ignore_case || strcasecmp(src, dst)\n\n(And once that is done, you don't need to look at st_ino at all)\n"},{"id":"241011","messageId":"1399569814-20644-1-git-send-email-dturner@twopensource.com","threadId":"36541","inReplyTo":"536B4680.1010806@web.de","subject":"[PATCH 0/2] Re: ignorecase: Fix git mv on insensitive filesystems","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-08T17:23:32Z","receivedAt":"2014-05-08T17:23:32Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Torsten Bögershausen wrote:\n> On 05/08/2014 08:37 AM, Johannes Sixt wrote:\n> > What if both inums are zero? Can this happen on any sane POSIX system? I\n> > don't know, but my gut feeling is that inode zero is too special to be\n> > allocated for files or directories.\n> >\n> > In that case, it is safe to assume that the st_ino field is just a\n> > placeholder when it is zero, and we have to compare the file name. Then we\n> > can either assume that this happens only for our emulation layer on MinGW\n> > (and the comparison can be case-insensitive) or choose the comparison mode\n> > based on core.ignorecase. This patch does the former, but I think we\n> > should do the latter.\n> Whatever we do, we should \"protect\" the strcasecmp() with ignore_case:\n> \n> !ignore_case || strcasecmp(src, dst)\n> \n> (And once that is done, you don't need to look at st_ino at all)\n\nIf both st_inos are zero on POSIX systems, there are two\npossibilities: the two files are case-clones, or the two files are\nhard-links of one another. Checking st_ino was just an optimization\nto avoid the slow string comparison. I hadn't considered the\nhard-link case important, but I guess it's better to be correct. So\nhere's a patch without (as well as a cleaned-up version of the first\npatch, to keep the series together).\n"},{"id":"241012","messageId":"1399569814-20644-2-git-send-email-dturner@twopensource.com","threadId":"36541","inReplyTo":"1399569814-20644-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 1/2] merge-recursive.c: Fix case-changing merge.","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-08T17:23:33Z","receivedAt":"2014-05-08T17:23:33Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: David Turner \n\nOn a case-insensitive filesystem, when merging, a file would be\nwrongly deleted from the working tree if an incoming commit had\nrenamed it changing only its case. When merging a rename, the file\nwith the old name would be deleted -- but since the filesystem\nconsiders the old name to be the same as the new name, the new\nfile would in fact be deleted.\n\nWe avoid this by not deleting files that have a case-clone in the\nindex at stage 0.\n\nSigned-off-by: David Turner \n---\n merge-recursive.c | 6 +++++\n t/t6039-merge-ignorecase.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 59 insertions(+)\n create mode 100755 t/t6039-merge-ignorecase.sh\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 4177092..cab16fa 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -589,6 +589,12 @@ static int remove_file(struct merge_options *o, int clean,\n \t\t\treturn -1;\n \t}\n \tif (update_working_directory) {\n+\t\tif (ignore_case) {\n+\t\t\tstruct cache_entry *ce;\n+\t\t\tce = cache_file_exists(path, strlen(path), ignore_case);\n+\t\t\tif (ce && ce_stage(ce) == 0)\n+\t\t\t\treturn 0;\n+\t\t}\n \t\tif (remove_path(path))\n \t\t\treturn -1;\n \t}\ndiff --git a/t/t6039-merge-ignorecase.sh b/t/t6039-merge-ignorecase.sh\nnew file mode 100755\nindex 0000000..dfc9f17\n--- /dev/null\n+++ b/t/t6039-merge-ignorecase.sh\n@@ -0,0 +1,53 @@\n+#!/bin/sh\n+\n+test_description='git-merge with case-changing rename on case-insensitive file system'\n+\n+. ./test-lib.sh\n+\n+if ! test_have_prereq CASE_INSENSITIVE_FS\n+then\n+\tskip_all='skipping case insensitive tests - case sensitive file system'\n+\ttest_done\n+fi\n+\n+test_expect_success 'merge with case-changing rename' '\n+\ttest $(git config core.ignorecase) = true &&\n+\t>TestCase &&\n+\tgit add TestCase &&\n+\tgit commit -m \"add TestCase\" &&\n+\tgit tag baseline\n+\tgit checkout -b with-camel &&\n+\t>foo &&\n+\tgit add foo &&\n+\tgit commit -m \"intervening commit\" &&\n+\tgit checkout master &&\n+\tgit rm TestCase &&\n+\t>testcase &&\n+\tgit add testcase &&\n+\tgit commit -m \"rename to testcase\" &&\n+\tgit checkout with-camel &&\n+\tgit merge master -m \"merge\" &&\n+\ttest_path_is_file testcase\n+'\n+\n+test_expect_success 'merge with case-changing rename on both sides' '\n+\tgit checkout master &&\n+\tgit reset --hard baseline &&\n+\tgit branch -D with-camel &&\n+\tgit checkout -b with-camel &&\n+\tgit mv --force TestCase testcase &&\n+\tgit commit -m \"recase on branch\" &&\n+\t>foo &&\n+\tgit add foo &&\n+\tgit commit -m \"intervening commit\" &&\n+\tgit checkout master &&\n+\tgit rm TestCase &&\n+\t>testcase &&\n+\tgit add testcase &&\n+\tgit commit -m \"rename to testcase\" &&\n+\tgit checkout with-camel &&\n+\tgit merge master -m \"merge\" &&\n+\ttest_path_is_file testcase\n+'\n+\n+test_done\n-- \n2.0.0.rc0.33.g27630aa\n"},{"id":"241010","messageId":"1399569814-20644-3-git-send-email-dturner@twopensource.com","threadId":"36541","inReplyTo":"1399569814-20644-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-08T17:23:34Z","receivedAt":"2014-05-08T17:23:34Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: David Turner \n\nMake it possible to change the case of a filename on a\ncase-insensitive filesystem using git mv. Change git mv to allow\nmoves where the destination file exists if the destination file has\nthe same name as the source file ignoring case.\n\nSigned-off-by: David Turner \n---\n builtin/mv.c | 3 ++-\n t/t6039-merge-ignorecase.sh | 2 +-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 45e57f3..f4d89d0 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -202,7 +202,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t}\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\telse if (lstat(dst, &st) == 0 &&\n+\t\t\t (!ignore_case || strcasecmp(src, dst))) {\n \t\t\tbad = _(\"destination exists\");\n \t\t\tif (force) {\n \t\t\t\t/*\ndiff --git a/t/t6039-merge-ignorecase.sh b/t/t6039-merge-ignorecase.sh\nindex dfc9f17..a977653 100755\n--- a/t/t6039-merge-ignorecase.sh\n+++ b/t/t6039-merge-ignorecase.sh\n@@ -35,7 +35,7 @@ test_expect_success 'merge with case-changing rename on both sides' '\n \tgit reset --hard baseline &&\n \tgit branch -D with-camel &&\n \tgit checkout -b with-camel &&\n-\tgit mv --force TestCase testcase &&\n+\tgit mv TestCase testcase &&\n \tgit commit -m \"recase on branch\" &&\n \t>foo &&\n \tgit add foo &&\n-- \n2.0.0.rc0.33.g27630aa\n"},{"id":"241022","messageId":"xmqqlhuc2gxs.fsf@gitster.dls.corp.google.com","threadId":"36541","inReplyTo":"1399569814-20644-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH 1/2] merge-recursive.c: Fix case-changing merge.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-08T19:45:03Z","receivedAt":"2014-05-08T19:45:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; I think this is identical to what we already have on the\ndt/merge-recursive-case-insensitive topic.\n"},{"id":"241023","messageId":"xmqqha502ghc.fsf@gitster.dls.corp.google.com","threadId":"36541","inReplyTo":"1399569814-20644-3-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-08T19:54:55Z","receivedAt":"2014-05-08T19:54:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"dturner@twopensource.com writes:\n\n> From: David Turner \n>\n> Make it possible to change the case of a filename on a\n> case-insensitive filesystem using git mv. Change git mv to allow\n> moves where the destination file exists if the destination file has\n> the same name as the source file ignoring case.\n>\n> Signed-off-by: David Turner \n> ---\n> builtin/mv.c | 3 ++-\n> t/t6039-merge-ignorecase.sh | 2 +-\n> 2 files changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 45e57f3..f4d89d0 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -202,7 +202,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> \t\t\t}\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\telse if (lstat(dst, &st) == 0 &&\n> +\t\t\t (!ignore_case || strcasecmp(src, dst))) {\n\nHmm, I would find it easier to read if it were:\n\n\t\t... if (lstat(dst, &st) == 0 &&\n \t!(ignore_case && !strcasecmp(src, dst))) {\n\nThat is, \"it is an error for dst to exist, unless we are on a case\ninsensitive filesystem and src and dst refer to the same file.\", but\nmaybe it is just me.\n\nMore importantly, what is the end-user visible effect of this\nchange? Is it fair to summarize it like this?\n\n On a case-insensitive filesystem, \"mv hello.txt Hello.txt\"\n always trigger the \"dst already exists\" error, because both\n names refer to the same file to MS-DOS, requiring the user to\n pass the \"--force\" option. Allow it without \"--force\".\n\nOverwriting an existing file with \"mv hello.txt Hello.txt\" on a case\nsensitive filesystem *is* an unusual operation, and that is the\nreason why we require \"--force\" to make sure that the user means it.\nI have a slight suspicion that the same \"mv hello.txt Hello.txt\" on\na case insensitive filesystem, where two names are known (to the end\nuser of such a filesystem) to refer to the same path would equally\nbe a very unusual thing to do, and such an operation may deserve a\nsimilar safety precaution to make sure that the user really meant to\ndo so by requiring \"--force\".\n\nSo, I dunno.\n\n> \t\t\tbad = _(\"destination exists\");\n> \t\t\tif (force) {\n> \t\t\t\t/*\n> diff --git a/t/t6039-merge-ignorecase.sh b/t/t6039-merge-ignorecase.sh\n> index dfc9f17..a977653 100755\n> --- a/t/t6039-merge-ignorecase.sh\n> +++ b/t/t6039-merge-ignorecase.sh\n> @@ -35,7 +35,7 @@ test_expect_success 'merge with case-changing rename on both sides' '\n> \tgit reset --hard baseline &&\n> \tgit branch -D with-camel &&\n> \tgit checkout -b with-camel &&\n> -\tgit mv --force TestCase testcase &&\n> +\tgit mv TestCase testcase &&\n> \tgit commit -m \"recase on branch\" &&\n> \t>foo &&\n> \tgit add foo &&\n"},{"id":"241030","messageId":"1399581623.11843.105.camel@stross","threadId":"36541","inReplyTo":"xmqqha502ghc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-05-08T20:40:23Z","receivedAt":"2014-05-08T20:40:23Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-05-08 at 12:54 -0700, Junio C Hamano wrote:\n> dturner@twopensource.com writes:\n> \n> > From: David Turner \n> >\n> > Make it possible to change the case of a filename on a\n> > case-insensitive filesystem using git mv. Change git mv to allow\n> > moves where the destination file exists if the destination file has\n> > the same name as the source file ignoring case.\n> >\n> > Signed-off-by: David Turner \n> > ---\n> > builtin/mv.c | 3 ++-\n> > t/t6039-merge-ignorecase.sh | 2 +-\n> > 2 files changed, 3 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/builtin/mv.c b/builtin/mv.c\n> > index 45e57f3..f4d89d0 100644\n> > --- a/builtin/mv.c\n> > +++ b/builtin/mv.c\n> > @@ -202,7 +202,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> > \t\t\t}\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\telse if (lstat(dst, &st) == 0 &&\n> > +\t\t\t (!ignore_case || strcasecmp(src, dst))) {\n> \n> Hmm, I would find it easier to read if it were:\n> \n> \t\t... if (lstat(dst, &st) == 0 &&\n> \t!(ignore_case && !strcasecmp(src, dst))) {\n> \n> That is, \"it is an error for dst to exist, unless we are on a case\n> insensitive filesystem and src and dst refer to the same file.\", but\n> maybe it is just me.\n\nI personally dislike the double negative. I also considered breaking it\nout into a little function with a self-documenting name -- does that\nsound better?\n\n> More importantly, what is the end-user visible effect of this\n> change? Is it fair to summarize it like this?\n> \n> On a case-insensitive filesystem, \"mv hello.txt Hello.txt\"\n> always trigger the \"dst already exists\" error, because both\n> names refer to the same file to MS-DOS, requiring the user to\n ^^^^^^\n(I have not actually tested on Windows; I tested on the Mac since that's\nwhat I have handy)\n\n> pass the \"--force\" option. Allow it without \"--force\".\n\nYes.\n\n> Overwriting an existing file with \"mv hello.txt Hello.txt\" on a case\n> sensitive filesystem *is* an unusual operation, and that is the\n> reason why we require \"--force\" to make sure that the user means it.\n> I have a slight suspicion that the same \"mv hello.txt Hello.txt\" on\n> a case insensitive filesystem, where two names are known (to the end\n> user of such a filesystem) to refer to the same path would equally\n> be a very unusual thing to do, and such an operation may deserve a\n> similar safety precaution to make sure that the user really meant to\n> do so by requiring \"--force\".\n> \n> So, I dunno.\n\nThe argument against --force is that git's behavior should not\nsignificantly differ between sensitive and insensitive filesystems\n(where possible). I do not see a case-changing rename as unusual on a\ncase-insensitive filesystem; these filesystems typically preserve case,\nand a user might reasonably care about the case of a filename either for\naesthetic reasons or for functionality on sensible filesystems (e.g.\ndevelopers who work on Macs but deploy on GNU/Linux, as is quite\ncommon).\n\nThe Mac's interface itself provides conflicting evidence: on one hand,\nwe might expect git mv to work like plain mv: nothing special is needed\nto do a case-changing mv). On the other hand, in the Finder, attempting\na case-changing rename causes an error message (which there is no way to\nget around other than the two-rename dance). I read this as \"ordinary\nusers never intentionally change the case of files, but developers\nsometimes do\", but that's not the only possible reading.\n\nI myself am not actually a Mac user; I simply support a bunch of Mac\nusers (which is where the merge bug came from). So I don't know what\nMac users would prefer. Maybe there are some on the git mailing list?\n\nI also have not tried on Windows. I put in an email to the one\nWindows-using friend I can think of to ask her to give Windows Explorer\n(or whatever it's called these days) a try. My guess (based on a quick\nGoogle search) would be is that it works without error, but I will send\nan update if I hear otherwise.\n"},{"id":"241034","messageId":"xmqq4n102dnm.fsf@gitster.dls.corp.google.com","threadId":"36541","inReplyTo":"1399581623.11843.105.camel@stross","subject":"Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-08T20:55:57Z","receivedAt":"2014-05-08T20:55:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n>> Hmm, I would find it easier to read if it were:\n>> \n>> \t\t... if (lstat(dst, &st) == 0 &&\n>> \t!(ignore_case && !strcasecmp(src, dst))) {\n>> \n>> That is, \"it is an error for dst to exist, unless we are on a case\n>> insensitive filesystem and src and dst refer to the same file.\", but\n>> maybe it is just me.\n>\n> I personally dislike the double negative. I also considered breaking it\n> out into a little function with a self-documenting name -- does that\n> sound better?\n\nNo, it shows that it is just me. I did not say that the original is\nunreadable.\n\n>> More importantly, what is the end-user visible effect of this\n>> change? Is it fair to summarize it like this?\n>> \n>> On a case-insensitive filesystem, \"mv hello.txt Hello.txt\"\n>> always trigger the \"dst already exists\" error, because both\n>> names refer to the same file to MS-DOS, requiring the user to\n> ^^^^^^\n> (I have not actually tested on Windows; I tested on the Mac since that's\n> what I have handy)\n>\n>> pass the \"--force\" option. Allow it without \"--force\".\n>\n> Yes.\n>\n>> Overwriting an existing file with \"mv hello.txt Hello.txt\" on a case\n>> sensitive filesystem *is* an unusual operation, and that is the\n>> reason why we require \"--force\" to make sure that the user means it.\n>> I have a slight suspicion that the same \"mv hello.txt Hello.txt\" on\n>> a case insensitive filesystem, where two names are known (to the end\n>> user of such a filesystem) to refer to the same path would equally\n>> be a very unusual thing to do, and such an operation may deserve a\n>> similar safety precaution to make sure that the user really meant to\n>> do so by requiring \"--force\".\n>> \n>> So, I dunno.\n>\n> The argument against --force is that git's behavior should not\n> significantly differ between sensitive and insensitive filesystems\n> (where possible). I do not see a case-changing rename as unusual on a\n> case-insensitive filesystem; these filesystems typically preserve case,\n> and a user might reasonably care about the case of a filename either for\n> aesthetic reasons or for functionality on sensible filesystems (e.g.\n> developers who work on Macs but deploy on GNU/Linux, as is quite\n> common).\n>\n> The Mac's interface itself provides conflicting evidence: on one hand,\n> we might expect git mv to work like plain mv: nothing special is needed\n> to do a case-changing mv). On the other hand, in the Finder, attempting\n> a case-changing rename causes an error message (which there is no way to\n> get around other than the two-rename dance). I read this as \"ordinary\n> users never intentionally change the case of files, but developers\n> sometimes do\", but that's not the only possible reading.\n>\n> I myself am not actually a Mac user; I simply support a bunch of Mac\n> users (which is where the merge bug came from). So I don't know what\n> Mac users would prefer. Maybe there are some on the git mailing list?\n>\n> I also have not tried on Windows. I put in an email to the one\n> Windows-using friend I can think of to ask her to give Windows Explorer\n> (or whatever it's called these days) a try. My guess (based on a quick\n> Google search) would be is that it works without error, but I will send\n> an update if I hear otherwise.\n\nAlright. Thanks for sanity checking.\n\nI've already queued your patch as-is when I was composing the\nmessage you responded to and today's integration cycle is already\ngoing, so unless other people have ideas to convince us both that\nthis is a bad idea, all is well without any further action.\n\nThanks.\n"}]}