{"thread":{"id":"57986","subject":"[PATCH] apply: support case-only renames in case-insensitive filesystems","startedAt":"2022-06-11T17:04:06Z","lastAt":"2023-05-28T10:00:14Z","messageCount":21,"participants":["Tao Klerks via GitGitGadget","Junio C Hamano","Tao Klerks","Junio C Hamano via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"457067","messageId":"pull.1257.git.1654967038802.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":null,"subject":"[PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-11T17:03:58Z","receivedAt":"2022-06-11T17:04:06Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"From: Tao Klerks \n\n\"git apply\" checks, when validating a patch, to ensure that any files\nbeing added aren't already in the worktree.\n\nWhen this check runs on a case-only rename, in a case-insensitive\nfilesystem, this leads to a false positive - the command fails with an\nerror like:\nerror: File1: already exists in working directory\n\nFix this existence check to allow the file to exist, for a case-only\nrename when config core.ignorecase is set.\n\nAlso add a test for this case, while verifying that conflicting file\nconditions are still caught correctly, including case-only conflicts on\ncase-sensitive filesystems.\n\nSigned-off-by: Tao Klerks \n---\n apply: support case-only renames in case-insensitive filesystems\n \n As suggested recently in thread\n CAPMMpojwV+f=z9sgc_GaUOTFBCUVdbrGW8WjatWWmC3WTcsoXw@mail.gmail.com,\n proposing a fix to git-apply for case-only renames on case-insensitive\n filesystems.\n \n Also including tests to check both the corrected behavior and the\n corresponding legitimate errors.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1257%2FTaoK%2Ftao-apply-case-insensitive-renames-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1257/TaoK/tao-apply-case-insensitive-renames-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1257\n\n apply.c | 2 +\n t/t4141-apply-case-insensitive-rename.sh | 50 ++++++++++++++++++++++++\n 2 files changed, 52 insertions(+)\n create mode 100755 t/t4141-apply-case-insensitive-rename.sh\n\ndiff --git a/apply.c b/apply.c\nindex 2b7cd930efa..ccba7f90393 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -3942,6 +3942,8 @@ static int check_patch(struct apply_state *state, struct patch *patch)\n \tif ((tpatch = in_fn_table(state, new_name)) &&\n \t (was_deleted(tpatch) || to_be_deleted(tpatch)))\n \t\tok_if_exists = 1;\n+\telse if (ignore_case && !strcasecmp(old_name, new_name))\n+\t\tok_if_exists = 1;\n \telse\n \t\tok_if_exists = 0;\n \ndiff --git a/t/t4141-apply-case-insensitive-rename.sh b/t/t4141-apply-case-insensitive-rename.sh\nnew file mode 100755\nindex 00000000000..6b394252ff8\n--- /dev/null\n+++ b/t/t4141-apply-case-insensitive-rename.sh\n@@ -0,0 +1,50 @@\n+#!/bin/sh\n+\n+test_description='git apply should handle case-only renames on case-insensitive filesystems'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n+\n+# Please note, this test assumes that core.ignorecase is set appropriately for the filesystem,\n+# as tested in t0050. Case-only rename conflicts are only tested in case-sensitive filesystems.\n+\n+if ! test_have_prereq CASE_INSENSITIVE_FS\n+then\n+\ttest_set_prereq CASE_SENSITIVE_FS\n+\techo nuts\n+fi\n+\n+test_expect_success setup '\n+\techo \"This is some content in the file.\" > file1 &&\n+\techo \"A completely different file.\" > file2 &&\n+\tgit update-index --add file1 &&\n+\tgit update-index --add file2 &&\n+\tcat >case_only_rename_patch <<-\\EOF\n+\tdiff --git a/file1 b/File1\n+\tsimilarity index 100%\n+\trename from file1\n+\trename to File1\n+\tEOF\n+'\n+\n+test_expect_success 'refuse to apply rename patch with conflict' '\n+\tcat >conflict_patch <<-\\EOF &&\n+\tdiff --git a/file1 b/file2\n+\tsimilarity index 100%\n+\trename from file1\n+\trename to file2\n+\tEOF\n+\ttest_must_fail git apply --index conflict_patch\n+'\n+\n+test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '\n+\ttest_when_finished \"git mv File1 file2\" &&\n+\tgit mv file2 File1 &&\n+\ttest_must_fail git apply --index case_only_rename_patch\n+'\n+\n+test_expect_success 'apply case-only rename patch without conflict' '\n+\tgit apply --index case_only_rename_patch\n+'\n+\n+test_done\n\nbase-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64\n-- \ngitgitgadget\n"},{"id":"457069","messageId":"xmqqleu3au2n.fsf@gitster.g","threadId":"57986","inReplyTo":"pull.1257.git.1654967038802.gitgitgadget@gmail.com","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-11T19:17:20Z","receivedAt":"2022-06-11T19:17:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Tao Klerks via GitGitGadget\" writes:\n\n> From: Tao Klerks \n>\n> \"git apply\" checks, when validating a patch, to ensure that any files\n> being added aren't already in the worktree.\n>\n> When this check runs on a case-only rename, in a case-insensitive\n> filesystem, this leads to a false positive - the command fails with an\n> error like:\n> error: File1: already exists in working directory\n>\n> Fix this existence check to allow the file to exist, for a case-only\n> rename when config core.ignorecase is set.\n\nHmph, close, but the patch as-posted may be fatally buggy, I think.\n\nAt the beginning of the function there is this block:\n\n\tconst char *old_name = patch->old_name;\n\tconst char *new_name = patch->new_name;\n\tconst char *name = old_name ? old_name : new_name;\n\nwhich makes us realize that old_name CAN legitimately be NULL. That\nis true for a creation patch. new_name can also be NULL for a\ndeletion patch.\n\n> \tif ((tpatch = in_fn_table(state, new_name)) &&\n> \t (was_deleted(tpatch) || to_be_deleted(tpatch)))\n> \t\tok_if_exists = 1;\n> +\telse if (ignore_case && !strcasecmp(old_name, new_name))\n> +\t\tok_if_exists = 1;\n\nYou'd get a segfault when the patch is creating a file at new_name,\nor deleting a file at old_name, wouldn't you?\n\nWe need a new test or two to see if a straight creation or deletion\npatch does work correctly with icase set, before we even dream of\nhandling rename patches. Not having tests for such basic cases is\nquite surprising, but apparently the above line passed the CI.\n\n> \telse\n> \t\tok_if_exists = 0;\n\nHaving said that, I wonder what the existing check before the new\ncondition is doing? Especially, what is in_fn_table() for and how\ndoes it try to do its work?\n\nReading the big comment before it, it seems that it tries to deal\nwith tricky delete/create case already. With a typechange patch\nthat first removes a regular file \"hello.txt\" and then creates a\nsymbolic link \"hello.txt\" is exempted from the \"what you are\ncreating should not exist already\" rule by using in_fn_table()\ncheck. If it tries to create a symlink \"Hello.txt\" instead,\nshouldn't we allow it the same way on case-insensitive systems? I\ndo not think in_fn_table() pays attention to \"ignore_case\" option,\nso there may be an existing bug there already, regardless of the\nproblem you are trying to address with your patch.\n\nAnd I wonder if doing case-insensitive match in in_fn_table() lets\nus cover this new case as well as \"fixing\" the existing issue.\n\nIn any case, here are such two tests to make sure creation and\ndeletion patches on icase systems are not broken by careless\nmistakes like the one in this patch.\n\n t/t4114-apply-typechange.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 40 insertions(+)\n\ndiff --git c/t/t4114-apply-typechange.sh w/t/t4114-apply-typechange.sh\nindex da3e64f811..e565ad8da1 100755\n--- c/t/t4114-apply-typechange.sh\n+++ w/t/t4114-apply-typechange.sh\n@@ -126,4 +126,44 @@ test_expect_success 'directory becomes symlink' '\n test_debug 'cat patch'\n \n \n+test_expect_success 'file becomes nothing' '\n+\tgit checkout -f initial &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\t# prepare a patch to remove path \"foo\"\n+\tgit rm --cached foo &&\n+\tgit diff-index -p --cached HEAD >patch &&\n+\n+\t# such a patch should apply cleanly to the index\n+\tgit reset HEAD &&\n+\tgit apply --cached patch &&\n+\n+\t# and even with icase set.\n+\tgit reset HEAD &&\n+\tgit -c core.ignorecase=true apply --cached patch\n+'\n+\n+test_debug 'cat patch'\n+\n+test_expect_success 'nothing becomes a file' '\n+\tgit checkout -f initial &&\n+\ttest_when_finished \"git reset --hard HEAD\" &&\n+\n+\t# prepare a patch to add path \"foo\"\n+\tgit rm --cached foo &&\n+\tgit diff-index -p --cached -R HEAD >patch &&\n+\n+\t# such a patch should apply cleanly to the index without \"foo\"\n+\tgit reset HEAD &&\n+\tgit rm --cached foo &&\n+\tgit apply --cached patch &&\n+\n+\t# and even with icase set.\n+\tgit reset HEAD &&\n+\tgit rm --cached foo &&\n+\tgit -c core.ignorecase=true apply --cached patch\n+'\n+\n+test_debug 'cat patch'\n+\n test_done\n"},{"id":"457091","messageId":"xmqqr13t8np7.fsf@gitster.g","threadId":"57986","inReplyTo":"pull.1257.git.1654967038802.gitgitgadget@gmail.com","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-12T23:30:12Z","receivedAt":"2022-06-12T23:30:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Tao Klerks via GitGitGadget\" writes:\n\n> +if ! test_have_prereq CASE_INSENSITIVE_FS\n> +then\n> +\ttest_set_prereq CASE_SENSITIVE_FS\n> +\techo nuts\n> +fi\n\nYou can easily say !CASE_INSENSITIVE_FS as the prerequiste, so I do\nnot see the point of this. I do not see the point of \"nuts\", either.\n\nBut it probably is a moot point as I do not think you should do the\nprerequisite at all.\n\nInstead, you can explicitly set the core.ignorecase configuration,\ni.e. \"git -c core.ignorecase=yes/no\", and possibly \"apply --cached\"\nso that you do not have to worry about the case sensitivity of the\nfilesystem at all.\n\n> +test_expect_success setup '\n> +\techo \"This is some content in the file.\" > file1 &&\n\nStyle. Redirection operator \">\" sticks to its operand, i.e.\n\n\techo \"This is some content in the file.\" >file1 &&\n\n> +\techo \"A completely different file.\" > file2 &&\n> +\tgit update-index --add file1 &&\n> +\tgit update-index --add file2 &&\n> +\tcat >case_only_rename_patch <<-\\EOF\n> +\tdiff --git a/file1 b/File1\n> +\tsimilarity index 100%\n> +\trename from file1\n> +\trename to File1\n> +\tEOF\n\nYou are better off not writing the diff output manually. Instead,\nyou can let the test write it for you, e.g.\n\n\techo \"This is some content in the file.\" >file1 &&\n\tgit update-index --add file1 &&\n file1blob=$(git rev-parse :file1) &&\n\tgit commit -m \"Initial - file1\" &&\n\tgit update-index --add --cacheinfo 100644,$file1blob,File1 &&\n\tgit rm --cached file1 &&\n\tgit diff --cached -M HEAD >case-only-rename-patch\n\nIf you want to be extra careful not to rely on your filesystem\ncorrupting the pathnames you feed (e.g. the redireciton to \"file1\"\nmight create file FILE1 on MS-DOS ;-), you could even do:\n\n\tfile1blob=$(echo \"This is some content in the file.\" |\n\t\t git hash-object -w --stdin) &&\n\tfile2blob=$(echo \"A completeloy different contents.\" |\n\t\t git hash-object -w --stdin) &&\n\tgit update-index --add --cacheinfo 100644,$file1blob,file1 &&\n\n\tgit commit -m \"Initial - file1\" &&\n\tgit update-index --add --cacheinfo 100644,$file1blob,File1 &&\n\tgit rm --cached file1 &&\n\tgit diff --cached -M HEAD >rename-file1-to-File2 &&\n\n\tgit reset --hard HEAD &&\n git update-index --add --cacheinfo 100644,$file1blob,file2 &&\n\tgit rm --cached file1 &&\n\tgit diff --cached -M HEAD >rename-file1-to-file2 &&\n\n\t# from here on, HEAD has file1 and file2\n\tgit reset --hard HEAD &&\n\tgit update-index --add --cacheinfo 100644,$file2blob,file2 &&\n\tgit commit -m 'file1 and file2'\n\n> +'\n> +\n> +test_expect_success 'refuse to apply rename patch with conflict' '\n> +\tcat >conflict_patch <<-\\EOF &&\n> +\tdiff --git a/file1 b/file2\n> +\tsimilarity index 100%\n> +\trename from file1\n> +\trename to file2\n> +\tEOF\n> +\ttest_must_fail git apply --index conflict_patch\n\nAnd then, you could use --cached (not --index) to bypass the working\ntree altogether, which is a good way to test the feature without\ngetting affected by the underlying filesystem. Check both case\nsensitive and case insensitive cases:\n\n\t# Start from a known state\n\tgit reset --hard HEAD &&\n\ttest_must_fail git -c core.ignorecase=no apply --cached rename-file1-to-file2 &&\n\n\t# Start from a known state\n\tgit reset --hard HEAD &&\n\ttest_must_fail git -c core.ignorecase=yes apply --cached rename-file1-to-file2 &&\n\n> +'\n> +\n> +test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '\n\nLose the prerequisite, replace --index with --cached, and force core.ignorecase\nto both case insensitive and sensitive to check the behaviour.\n\n> +\ttest_when_finished \"git mv File1 file2\" &&\n> +\tgit mv file2 File1 &&\n> +\ttest_must_fail git apply --index case_only_rename_patch\n> +'\n> +\n> +test_expect_success 'apply case-only rename patch without conflict' '\n\nLikewise, try both sensitive and insensitive one.\n\n> +\tgit apply --index case_only_rename_patch\n> +'\n> +\n> +test_done\n>\n> base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64\n\nThanks.\n\n"},{"id":"457093","messageId":"xmqqedzt8nfq.fsf@gitster.g","threadId":"57986","inReplyTo":"xmqqleu3au2n.fsf@gitster.g","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-12T23:35:53Z","receivedAt":"2022-06-12T23:35:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> ... I\n> do not think in_fn_table() pays attention to \"ignore_case\" option,\n> so there may be an existing bug there already, regardless of the\n> problem you are trying to address with your patch.\n>\n> And I wonder if doing case-insensitive match in in_fn_table() lets\n> us cover this new case as well as \"fixing\" the existing issue.\n\nWhile I still think in_fn_table() should be looked into for an\nexisting case sensitivity bug, I think this one is different enough\nthat in_fn_table() logic wouild not trigger for it, and a patch to\nadd an extra piece of logic for renames is probably needed.\n\nIt might be sufficient to tighten the condition so that it triggers\nonly to the case you wanted to handle, i.e. a rename between the\nsame name.\n\n\telse if (ignore_case && old_name && new_name &&\n\t\t !strcasecmp(old_name, new_name))\n\n(the \"both names must be non-NULL\" check is new).\n\nThanks.\n"},{"id":"457111","messageId":"xmqqo7yw77qo.fsf@gitster.g","threadId":"57986","inReplyTo":"xmqqr13t8np7.fsf@gitster.g","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-13T18:12:31Z","receivedAt":"2022-06-13T19:42:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> And then, you could use --cached (not --index) to bypass the working\n> tree altogether, which is a good way to test the feature without\n> getting affected by the underlying filesystem. Check both case\n> sensitive and case insensitive cases:\n> ...\n> Likewise, try both sensitive and insensitive one.\n\nAs I already wrote tests for basic cases, I'm sending them out,\n\nso that you may extend them with your new cases so that new code you\nwrite can be checked.\n\nThanks.\n\n----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----\nFrom: Junio C Hamano \nDate: Mon, 13 Jun 2022 11:05:54 -0700\nSubject: [PATCH] t4141: test \"git apply\" with core.ignorecase\n\nSigned-off-by: Junio C Hamano \n---\n t/t4141-apply-icase.sh | 128 +++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 128 insertions(+)\n create mode 100755 t/t4141-apply-icase.sh\n\ndiff --git a/t/t4141-apply-icase.sh b/t/t4141-apply-icase.sh\nnew file mode 100755\nindex 0000000000..9b70ff82c3\n--- /dev/null\n+++ b/t/t4141-apply-icase.sh\n@@ -0,0 +1,128 @@\n+#!/bin/sh\n+\n+test_description='git apply with core.ignorecase'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t# initial commit has file0 only\n+\ttest_commit \"initial\" file0 \"initial commit with file0\" initial &&\n+\n+\t# current commit has file1 as well\n+\ttest_commit \"current\" file1 \"initial content of file1\" current &&\n+\tfile0blob=$(git rev-parse :file0) &&\n+\tfile1blob=$(git rev-parse :file1) &&\n+\n+\t# prepare sample patches\n+\t# file0 is modified\n+\techo modification to file0 >file0 &&\n+\tgit add file0 &&\n+\tmodifiedfile0blob=$(git rev-parse :file0) &&\n+\n+\t# file1 is removed and then ...\n+\tgit rm --cached file1 &&\n+\t# ... identical copies are placed at File1 and file2\n+\tgit update-index --add --cacheinfo 100644,$file1blob,file2 &&\n+\tgit update-index --add --cacheinfo 100644,$file1blob,File1 &&\n+\n+\t# then various patches to do basic things\n+\tgit diff HEAD^ HEAD -- file1 >creation-patch &&\n+\tgit diff HEAD HEAD^ -- file1 >deletion-patch &&\n+\tgit diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&\n+\tgit diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&\n+\tgit diff --cached HEAD -- file0 >modify-file0-patch\n+'\n+\n+# Basic creation, deletion, modification and renaming.\n+test_expect_success 'creation and deletion' '\n+\t# start at \"initial\" with file0 only\n+\tgit reset --hard initial &&\n+\n+\t# add file1\n+\tgit -c core.ignorecase=false apply --cached creation-patch &&\n+\ttest_cmp_rev :file1 \"$file1blob\" &&\n+\n+\t# remove file1\n+\tgit -c core.ignorecase=false apply --cached deletion-patch &&\n+\ttest_must_fail git rev-parse --verify :file1 &&\n+\n+\t# do the same with ignorecase\n+\tgit -c core.ignorecase=true apply --cached creation-patch &&\n+\ttest_cmp_rev :file1 \"$file1blob\" &&\n+\tgit -c core.ignorecase=true apply --cached deletion-patch &&\n+\ttest_must_fail git rev-parse --verify :file1\n+'\n+\n+test_expect_success 'modificaiton' '\n+\t# start at \"initial\" with file0 only\n+\tgit reset --hard initial &&\n+\n+\t# modify file0\n+\tgit -c core.ignorecase=false apply --cached modify-file0-patch &&\n+\ttest_cmp_rev :file0 \"$modifiedfile0blob\" &&\n+\tgit -c core.ignorecase=false apply --cached -R modify-file0-patch &&\n+\ttest_cmp_rev :file0 \"$file0blob\" &&\n+\n+\t# do the same with ignorecase\n+\tgit -c core.ignorecase=true apply --cached modify-file0-patch &&\n+\ttest_cmp_rev :file0 \"$modifiedfile0blob\" &&\n+\tgit -c core.ignorecase=true apply --cached -R modify-file0-patch &&\n+\ttest_cmp_rev :file0 \"$file0blob\"\n+'\n+\n+test_expect_success 'rename file1 to file2' '\n+\t# start from file0 and file1\n+\tgit reset --hard current &&\n+\n+\t# rename file1 to file2\n+\tgit -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&\n+\ttest_must_fail git rev-parse --verify :file1 &&\n+\ttest_cmp_rev :file2 \"$file1blob\" &&\n+\tgit -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&\n+\ttest_must_fail git rev-parse --verify :file2 &&\n+\ttest_cmp_rev :file1 \"$file1blob\" &&\n+\n+\t# do the same with ignorecase\n+\tgit -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&\n+\ttest_must_fail git rev-parse --verify :file1 &&\n+\ttest_cmp_rev :file2 \"$file1blob\" &&\n+\tgit -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&\n+\ttest_must_fail git rev-parse --verify :file2 &&\n+\ttest_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success 'rename file1 to file2' '\n+\t# start from file0 and file1\n+\tgit reset --hard current &&\n+\n+\t# rename file1 to File1\n+\tgit -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&\n+\ttest_must_fail git rev-parse --verify :file1 &&\n+\ttest_cmp_rev :File1 \"$file1blob\" &&\n+\tgit -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&\n+\ttest_must_fail git rev-parse --verify :File1 &&\n+\ttest_cmp_rev :file1 \"$file1blob\" &&\n+\n+\t# do the same with ignorecase\n+\tgit -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&\n+\ttest_must_fail git rev-parse --verify :file1 &&\n+\ttest_cmp_rev :File1 \"$file1blob\" &&\n+\tgit -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&\n+\ttest_must_fail git rev-parse --verify :File1 &&\n+\ttest_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+# We may want to add tests with working tree here, without \"--cached\" and\n+# with and without \"--index\" here. For example, should modify-file0-patch\n+# apply cleanly if we have File0 with $file0blob in the index and the working\n+# tree if core.icase is set?\n+\n+test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '\n+\t: sample\n+'\n+\n+test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '\n+\t: sample\n+'\n+\n+test_done\n-- \n2.36.1-513-gd2306e2395\n\n"},{"id":"457168","messageId":"CAPMMpojdnAMnczJAevqL8GSOb8gvddcSiYfbz0c51oPpn4U0wA@mail.gmail.com","threadId":"57986","inReplyTo":"xmqqleu3au2n.fsf@gitster.g","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-06-14T05:13:31Z","receivedAt":"2022-06-14T05:13:47Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Sat, Jun 11, 2022 at 9:17 PM Junio C Hamano wrote:\n>\n> \"Tao Klerks via GitGitGadget\" writes:\n>\n> > From: Tao Klerks \n> >\n> > \"git apply\" checks, when validating a patch, to ensure that any files\n> > being added aren't already in the worktree.\n> >\n> > When this check runs on a case-only rename, in a case-insensitive\n> > filesystem, this leads to a false positive - the command fails with an\n> > error like:\n> > error: File1: already exists in working directory\n> >\n> > Fix this existence check to allow the file to exist, for a case-only\n> > rename when config core.ignorecase is set.\n>\n> Hmph, close, but the patch as-posted may be fatally buggy, I think.\n>\n\nYes indeed, very much so.\n\n> At the beginning of the function there is this block:\n>\n> const char *old_name = patch->old_name;\n> const char *new_name = patch->new_name;\n> const char *name = old_name ? old_name : new_name;\n>\n> which makes us realize that old_name CAN legitimately be NULL. That\n> is true for a creation patch. new_name can also be NULL for a\n> deletion patch.\n>\n\nYep, I was aware of the nulls, but I was unaware that passing nulls\ninto \"strcasecmp()\" was a bad thing to do. I just assumed a non-zero\ncomparison result would ensue.\n\n> > if ((tpatch = in_fn_table(state, new_name)) &&\n> > (was_deleted(tpatch) || to_be_deleted(tpatch)))\n> > ok_if_exists = 1;\n> > + else if (ignore_case && !strcasecmp(old_name, new_name))\n> > + ok_if_exists = 1;\n>\n> You'd get a segfault when the patch is creating a file at new_name,\n> or deleting a file at old_name, wouldn't you?\n>\n\nIndeed you do (when ignorecase is true of course).\n\n> We need a new test or two to see if a straight creation or deletion\n> patch does work correctly with icase set, before we even dream of\n> handling rename patches. Not having tests for such basic cases is\n> quite surprising, but apparently the above line passed the CI.\n>\n\nThis is where I made some very bad assumptions: I only manually ran\nthe new \"t4141-apply-case-insensitive-rename.sh\" test, and assumed\nthat the test suite ran against linux, windows, and OSX, with the\nlatter two running on case-insensitive filesystems. I assumed that\nboth case-sensitive and case-insensitive code paths would be tested by\nthe complete CI suite.\n\nThe OSX tests were not running for me at all in GitGitGadget (seems to\nbe an ongoing struggle), but I assumed that everything was still\ntested in case-insensitive mode because of the windows suite. It looks\nlike that was wrong, although I still don´t know how/why.\n\nHad I run \"t4114-apply-typechange.sh\" (or probably some others in the\n41XX range) on the OSX environment where I happen to have developed\nthis weekend, I would have seen the failures immediately.\n\n> > else\n> > ok_if_exists = 0;\n>\n> Having said that, I wonder what the existing check before the new\n> condition is doing? Especially, what is in_fn_table() for and how\n> does it try to do its work?\n>\n> Reading the big comment before it, it seems that it tries to deal\n> with tricky delete/create case already. With a typechange patch\n> that first removes a regular file \"hello.txt\" and then creates a\n> symbolic link \"hello.txt\" is exempted from the \"what you are\n> creating should not exist already\" rule by using in_fn_table()\n> check. If it tries to create a symlink \"Hello.txt\" instead,\n> shouldn't we allow it the same way on case-insensitive systems? I\n> do not think in_fn_table() pays attention to \"ignore_case\" option,\n> so there may be an existing bug there already, regardless of the\n> problem you are trying to address with your patch.\n>\n> And I wonder if doing case-insensitive match in in_fn_table() lets\n> us cover this new case as well as \"fixing\" the existing issue.\n>\n\nYep, I confirmed that as you expect, it does fix the issue I set out\nto fix, and as you noted also fixes other (slightly more obscure?)\nexisting issues with \"git apply\" on case-insensitive filesystems.\n\nThis time I tested all of t41XX on a case-insensitive system, and the\nCI process ran in GitLab, presumably on case-sensitive filesystems\nonly.\n\nI'm not sure what more to look out for, and will note as much in the\npatch v2 comments.\n\n> In any case, here are such two tests to make sure creation and\n> deletion patches on icase systems are not broken by careless\n> mistakes like the one in this patch.\n\nI have a question related to this:\n\n*Do* we expect to run the full test suite on case-insensitive systems\nin gitlab, or do we expect to need to add explicit \"-C\ncore.ignorecase\" tests as you have done here? The latter seems risky\nboth because the behavior is not representatively tested (because it's\nstill actually running in a case-sensitive filesystem), and because\nit's hard to predict all the things that should be explicitly retested\nin this way.\n\nI don't think these specific tests were necessary, and I guess they\nare replaced by later ones in this thread, so I will ignore this bit\nspecifically.\n\nThanks for the careful review, my apologies for the careless patch.\n"},{"id":"457170","messageId":"CAPMMpogTcKqw6SHon9soj_CqPf-E8SmHpJ1FRRBKaCcOVnyHRg@mail.gmail.com","threadId":"57986","inReplyTo":"xmqqr13t8np7.fsf@gitster.g","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-06-14T06:16:44Z","receivedAt":"2022-06-14T06:17:00Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Mon, Jun 13, 2022 at 1:30 AM Junio C Hamano wrote:\n>\n> \"Tao Klerks via GitGitGadget\" writes:\n>\n> > +if ! test_have_prereq CASE_INSENSITIVE_FS\n> > +then\n> > + test_set_prereq CASE_SENSITIVE_FS\n> > + echo nuts\n> > +fi\n>\n> You can easily say !CASE_INSENSITIVE_FS as the prerequiste, so I do\n> not see the point of this. I do not see the point of \"nuts\", either.\n\nI was not aware of negated prerequisite support (I did not see it in\nthe README nor in any examples I scanned), but I agree this is much\ncleaner of course!\n\n\"nuts\" was a debugging leak, my apologies.\n\n>\n> But it probably is a moot point as I do not think you should do the\n> prerequisite at all.\n>\n> Instead, you can explicitly set the core.ignorecase configuration,\n> i.e. \"git -c core.ignorecase=yes/no\", and possibly \"apply --cached\"\n> so that you do not have to worry about the case sensitivity of the\n> filesystem at all.\n\nSure, I can see how we can test most of the case-sensitive logic, even\non a case-insensitive filesystem, with \"--cached\" and \"-c\ncore.ignorecase=no\". I'm not sure whether there's a need to test the\nsame things against the actual file system or not (certainly in the\ncase-insensitive path there is, as this is where the errors/conflicts\nactually occur).\n\n>\n> > +test_expect_success setup '\n> > + echo \"This is some content in the file.\" > file1 &&\n>\n> Style. Redirection operator \">\" sticks to its operand, i.e.\n>\n> echo \"This is some content in the file.\" >file1 &&\n>\n\nThx.\n\n> > + echo \"A completely different file.\" > file2 &&\n> > + git update-index --add file1 &&\n> > + git update-index --add file2 &&\n> > + cat >case_only_rename_patch <<-\\EOF\n> > + diff --git a/file1 b/File1\n> > + similarity index 100%\n> > + rename from file1\n> > + rename to File1\n> > + EOF\n>\n> You are better off not writing the diff output manually. Instead,\n> you can let the test write it for you, e.g.\n>\n> echo \"This is some content in the file.\" >file1 &&\n> git update-index --add file1 &&\n> file1blob=$(git rev-parse :file1) &&\n> git commit -m \"Initial - file1\" &&\n> git update-index --add --cacheinfo 100644,$file1blob,File1 &&\n> git rm --cached file1 &&\n> git diff --cached -M HEAD >case-only-rename-patch\n>\n\nMakes sense, thx.\n\n> If you want to be extra careful not to rely on your filesystem\n> corrupting the pathnames you feed (e.g. the redireciton to \"file1\"\n> might create file FILE1 on MS-DOS ;-), you could even do:\n>\n> file1blob=$(echo \"This is some content in the file.\" |\n> git hash-object -w --stdin) &&\n> file2blob=$(echo \"A completeloy different contents.\" |\n> git hash-object -w --stdin) &&\n> git update-index --add --cacheinfo 100644,$file1blob,file1 &&\n>\n> git commit -m \"Initial - file1\" &&\n> git update-index --add --cacheinfo 100644,$file1blob,File1 &&\n> git rm --cached file1 &&\n> git diff --cached -M HEAD >rename-file1-to-File2 &&\n>\n> git reset --hard HEAD &&\n> git update-index --add --cacheinfo 100644,$file1blob,file2 &&\n> git rm --cached file1 &&\n> git diff --cached -M HEAD >rename-file1-to-file2 &&\n>\n> # from here on, HEAD has file1 and file2\n> git reset --hard HEAD &&\n> git update-index --add --cacheinfo 100644,$file2blob,file2 &&\n> git commit -m 'file1 and file2'\n>\n\nCool, but probably excessive? (do we support MS-DOS??)\n\n> > +'\n> > +\n> > +test_expect_success 'refuse to apply rename patch with conflict' '\n> > + cat >conflict_patch <<-\\EOF &&\n> > + diff --git a/file1 b/file2\n> > + similarity index 100%\n> > + rename from file1\n> > + rename to file2\n> > + EOF\n> > + test_must_fail git apply --index conflict_patch\n>\n> And then, you could use --cached (not --index) to bypass the working\n> tree altogether, which is a good way to test the feature without\n> getting affected by the underlying filesystem. Check both case\n> sensitive and case insensitive cases:\n>\n> # Start from a known state\n> git reset --hard HEAD &&\n> test_must_fail git -c core.ignorecase=no apply --cached rename-file1-to-file2 &&\n>\n> # Start from a known state\n> git reset --hard HEAD &&\n> test_must_fail git -c core.ignorecase=yes apply --cached rename-file1-to-file2 &&\n>\n\nMakes sense, understanding that this tests \"happy paths\" - it doesn't\nfail even if talking to the (case-insensitive) filesystem actually\nwould (which here it wouldn't of course).\n\n> > +'\n> > +\n> > +test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '\n>\n> Lose the prerequisite, replace --index with --cached, and force core.ignorecase\n> to both case insensitive and sensitive to check the behaviour.\n>\n\nSure, makes sense - you can test case-sensitive behaviors in git\nwithout needing a case-sensitive FS.\n\n> > + test_when_finished \"git mv File1 file2\" &&\n> > + git mv file2 File1 &&\n> > + test_must_fail git apply --index case_only_rename_patch\n> > +'\n> > +\n> > +test_expect_success 'apply case-only rename patch without conflict' '\n>\n> Likewise, try both sensitive and insensitive one.\n>\n\nThis one will fail on a case-insensitive filesystem if you disable\ncore.ignorecase, so explicitly trying with both settings in a single\ntest, without prerequisites, presumably isn't the right thing. I\nassume the right thing is to have 2 versions of the same test, one\nwhich expects success in all cases on a case-sensitive filesystem, and\none which expects failure when case-insensitivity is disabled on a\ncase-insensitive filesystem?\n\n> > + git apply --index case_only_rename_patch\n> > +'\n> > +\n> > +test_done\n> >\n> > base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64\n>\n> Thanks.\n>\n\nThank you!\n"},{"id":"457171","messageId":"CAPMMpogcm36pd7fjvG64G7Vg29arukF-wzOKYbNYG9NOpVCXvQ@mail.gmail.com","threadId":"57986","inReplyTo":"xmqqedzt8nfq.fsf@gitster.g","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-06-14T06:22:37Z","receivedAt":"2022-06-14T06:22:54Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Mon, Jun 13, 2022 at 1:35 AM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > ... I\n> > do not think in_fn_table() pays attention to \"ignore_case\" option,\n> > so there may be an existing bug there already, regardless of the\n> > problem you are trying to address with your patch.\n> >\n> > And I wonder if doing case-insensitive match in in_fn_table() lets\n> > us cover this new case as well as \"fixing\" the existing issue.\n>\n> While I still think in_fn_table() should be looked into for an\n> existing case sensitivity bug, I think this one is different enough\n> that in_fn_table() logic wouild not trigger for it, and a patch to\n> add an extra piece of logic for renames is probably needed.\n>\n\nHaving spent some time with this yesterday and today, I'm quite\nconfident not only that you were right about the general\ncase-sensitivity fix required here, but also that it fixes the\ncase-only rename issue. It turns it (on case-insensitive filesystems)\ninto a similar issue to the mode change, which is treated as a remove\nand add of the same file.\n\nAs you suggested, it is possible to construct \"case-differing file\nswaps\" which are not file swaps on a case-sensitive FS but are on a\ncase-insensitive one, and without a fix these fail. The same (very\nsmall) change fixes both issues.\n\n> It might be sufficient to tighten the condition so that it triggers\n> only to the case you wanted to handle, i.e. a rename between the\n> same name.\n>\n> else if (ignore_case && old_name && new_name &&\n> !strcasecmp(old_name, new_name))\n>\n> (the \"both names must be non-NULL\" check is new).\n>\n\nThis was my original tack, but I think it makes more sense to make the\ngeneral fix and explain how it also handles this case.\n\nNew patch coming.\n\nThanks!\n"},{"id":"457172","messageId":"CAPMMpoi_XgJyEvKvLZ5qk69G_dBs+R0rM359HH90+RY-OGbi-Q@mail.gmail.com","threadId":"57986","inReplyTo":"xmqqo7yw77qo.fsf@gitster.g","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-06-14T06:26:50Z","receivedAt":"2022-06-14T06:27:08Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Mon, Jun 13, 2022 at 8:12 PM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > And then, you could use --cached (not --index) to bypass the working\n> > tree altogether, which is a good way to test the feature without\n> > getting affected by the underlying filesystem. Check both case\n> > sensitive and case insensitive cases:\n> > ...\n> > Likewise, try both sensitive and insensitive one.\n>\n> As I already wrote tests for basic cases, I'm sending them out,\n>\n> so that you may extend them with your new cases so that new code you\n> write can be checked.\n>\n\nThanks! I'm not sure how to handle this procedurally, especially as\nI'm using GitGitGadget, so my patches must always be anchored on a\ncommit found in the public mirror.\n\nFor now I'll add this as a new first commit in my patch, which becomes\nmy patch series I guess (but keeping your commit metadata\nas-specified). Please let me know if there's a better way.\n"},{"id":"457270","messageId":"CAPMMpohZbcK1a8T+eoTdG2wjaOvLun1E0QZEEVv6QTxhHb8r5w@mail.gmail.com","threadId":"57986","inReplyTo":"CAPMMpogcm36pd7fjvG64G7Vg29arukF-wzOKYbNYG9NOpVCXvQ@mail.gmail.com","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-06-15T11:24:05Z","receivedAt":"2022-06-15T11:24:33Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Tue, Jun 14, 2022 at 8:22 AM Tao Klerks wrote:\n>\n>\n> New patch coming.\n>\n\nQuick update on this - while exploring test scenarios I found an\nedge-case where these changes *seem* to introduce a regression. The\nprobably-problematic behavior is that if there is a\ndiffering-in-case-only (case-insensitive duplicate) file (with\ndifferent content) in the index, but obviously not on the filesystem\nas the case-insensitive filesystem can't allow both files to exist,\nthe \"git apply --index\" of a case-only rename from the original\nfilename to the \"duplicate\" filename will replace that duplicate file\nin the index, replacing its original content. With these changes, the\nrename of a file can, under very specific circumstances, cause a\n\"rename\" patch to replace/delete unrelated data in a staged-only (and\nmaybe also committed) file.\n\nI need to do more testing to understand the relationship between this\nbehavior and similar scenarios in, for example, git checkout.\n\nThe current patch can be found at\nhttps://github.com/gitgitgadget/git/pull/1257, but I'm not sure when\nI'll have time to investigate further, settle whether this is a\n(meaningful) regression or not, think about how to address it if so,\nand submit a v2 :(\n"},{"id":"457486","messageId":"xmqqiloyn6j5.fsf@gitster.g","threadId":"57986","inReplyTo":"CAPMMpojdnAMnczJAevqL8GSOb8gvddcSiYfbz0c51oPpn4U0wA@mail.gmail.com","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-18T00:45:34Z","receivedAt":"2022-06-18T00:45:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tao Klerks writes:\n\n>> We need a new test or two to see if a straight creation or deletion\n>> patch does work correctly with icase set, before we even dream of\n>> handling rename patches. Not having tests for such basic cases is\n>> quite surprising, but apparently the above line passed the CI.\n>\n> This is where I made some very bad assumptions: I only manually ran\n> the new \"t4141-apply-case-insensitive-rename.sh\" test, and assumed\n> that the test suite ran against linux, windows, and OSX, with the\n> latter two running on case-insensitive filesystems. I assumed that\n> both case-sensitive and case-insensitive code paths would be tested by\n> the complete CI suite.\n\nApparently we were surprised the same way ;-)\n\n> *Do* we expect to run the full test suite on case-insensitive systems\n> in gitlab, or do we expect to need to add explicit \"-C\n> core.ignorecase\" tests as you have done here?\n\nRunning all tests on case-insensitive systems and expect them to\npass is reasonable; we need to sprinkle !CASE_INSENSITIVE_FS\nprerequiste to skip certain tests that exercise functionalities that\ncase insensitive filesystem will never be able to support (e.g. you\ncannot by their design have file1.txt and File1.txt at the same time\non the filesystem, so any test with \"test_cmp file1.txt FIle1.txt\"\nmust be marked with !CASE_INSENSITIVE_FS prerequisite).\n\nWhen the system I am primary owrking on is case sensitive, it is\nalways nice to be able to discover that I broke something on case\nINsensitive system before I conclude my WIP into a commit and throw\nit at CI. We may have to case-insensitively treat the paths in the\nindex in order to match what the working tree would do to make \"git\ncheckout -- \" work case-insentively, and doing in-index-only\nmode of operation with core.ignorecase=yes on case-sensitive system\nmay be a way to \"emulate\" some of the requirement case-insentive\nsystems have with these \"-c core.ignorecase\" trick, but of course\nnot all scenarios can be tested without being on case-insensitive\nsystems.\n\nSo we need both, I think.\n\n"},{"id":"457502","messageId":"CAPMMpohkEDwdDoDZ9nQkD71FbDZU6a9Ut0WLUSqBp-oqFLOr5g@mail.gmail.com","threadId":"57986","inReplyTo":"xmqqiloyn6j5.fsf@gitster.g","subject":"Re: [PATCH] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-06-18T15:34:58Z","receivedAt":"2022-06-18T15:35:17Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Sat, Jun 18, 2022 at 2:45 AM Junio C Hamano wrote:\n>\n> Tao Klerks writes:\n>\n> > I assumed that\n> > both case-sensitive and case-insensitive code paths would be tested by\n> > the complete CI suite.\n>\n>\n> When the system I am primary owrking on is case sensitive, it is\n> always nice to be able to discover that I broke something on case\n> INsensitive system before I conclude my WIP into a commit and throw\n> it at CI. We may have to case-insensitively treat the paths in the\n> index in order to match what the working tree would do to make \"git\n> checkout -- \" work case-insentively, and doing in-index-only\n> mode of operation with core.ignorecase=yes on case-sensitive system\n> may be a way to \"emulate\" some of the requirement case-insentive\n> systems have with these \"-c core.ignorecase\" trick, but of course\n> not all scenarios can be tested without being on case-insensitive\n> systems.\n>\n> So we need both, I think.\n>\n\nUnderstood, makes sense, thank you.\n\nI made some changes that seem to resolve the regression that I had\npreviously noted, but I'm not sure the approach makes sense, it feels\nlike there must be a better way. I will submit an RFC series at this\npoint I think.\n"},{"id":"457534","messageId":"pull.1257.v2.git.1655655027.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":"pull.1257.git.1654967038802.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] RFC: apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-19T16:10:23Z","receivedAt":"2022-06-19T16:10:35Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"As suggested recently in thread\nCAPMMpojwV+f=z9sgc_GaUOTFBCUVdbrGW8WjatWWmC3WTcsoXw@mail.gmail.com,\nproposing a fix to git-apply for case-only renames on case-insensitive\nfilesystems.\n\nChanges in V2:\n\n * Prepended a commit from Junio, with new apply tests to build on\n * Added a largely-unrelated new known failing test, concerning reset --hard\n in the presence of index case conflicts on a case-insensitive filesystem,\n which we later need to work around in a corner-case test\n * Moved test cases to build on Junio's new test file\n * Switched fix approach from \"allow same-name commit explicitly\" to \"track\n files marked for deletion case-insensitively for filesystem checks\",\n which addresses the issue noted and other more obscure ones like \"rename\n swap with case change\"\n * Added a test case for \"rename swap with case change\"\n * Added test cases setting \"core.ignorecase\" on and off explicitly\n * Added a test case exposing one remaining surprising behavior\n\nPOSSIBLE CONCERN:\n\nThis fix was originally much simpler - it just made the \"fn_table\" string\nlist use a case-insensitive string comparison - using case-insensitive\ncomparisons when dealing with all replacement checks, both on the index and\non the filesystem.\n\nHowever, with that simple implementation, there was at least one edge-case\nwhere data loss could result: If the index contained two files differing\nonly by case, with different content, and we were doing a case-only rename,\na swap, or some other operation involving the deletion and creation of a\nfile with that name (ignoring case), then both of the files with that name\nin the index would be overwritten - even though only one of them had the\nexpected content, and even though the one deleted might never have been\ncommitted.\n\nIt seems as though the core.ignorecase option should typically only apply to\nfilesystem checks - that the index is always case-sensitive.\n\nThe current fix proposal therefore splits the string list used for \"can I\ncreate a file that already exists?\" checks into two such structures - one\nstring list used for filesystem checks, which is case-insensitive when\nspecified by core.ignorecase, and one used for index checks, which is always\ncase-sensitive.\n\nThe resulting duplication is not appealing, but I'm not sure how to address\nit / how to do this more elegantly. I'm also still not completely certain\nthat my rule of thumb about the index always being case-sensitive is the\nright way of thinking of things.\n\nJunio C Hamano (1):\n t4141: test \"git apply\" with core.ignorecase\n\nTao Klerks (2):\n reset: new failing test for reset of case-insensitive duplicate in\n index\n apply: support case-only renames in case-insensitive filesystems\n\n apply.c | 81 +++++++++----\n apply.h | 5 +-\n t/t4141-apply-icase.sh | 258 +++++++++++++++++++++++++++++++++++++++++\n t/t7104-reset-hard.sh | 11 ++\n 4 files changed, 334 insertions(+), 21 deletions(-)\n create mode 100755 t/t4141-apply-icase.sh\n\n\nbase-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1257%2FTaoK%2Ftao-apply-case-insensitive-renames-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1257/TaoK/tao-apply-case-insensitive-renames-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1257\n\nRange-diff vs v1:\n\n 1: b8bd612aa1e ! 1: efd3bd4cdda apply: support case-only renames in case-insensitive filesystems\n @@\n ## Metadata ##\n -Author: Tao Klerks \n +Author: Junio C Hamano \n \n ## Commit message ##\n - apply: support case-only renames in case-insensitive filesystems\n + t4141: test \"git apply\" with core.ignorecase\n \n - \"git apply\" checks, when validating a patch, to ensure that any files\n - being added aren't already in the worktree.\n + Signed-off-by: Junio C Hamano \n \n - When this check runs on a case-only rename, in a case-insensitive\n - filesystem, this leads to a false positive - the command fails with an\n - error like:\n - error: File1: already exists in working directory\n -\n - Fix this existence check to allow the file to exist, for a case-only\n - rename when config core.ignorecase is set.\n -\n - Also add a test for this case, while verifying that conflicting file\n - conditions are still caught correctly, including case-only conflicts on\n - case-sensitive filesystems.\n -\n - Signed-off-by: Tao Klerks \n -\n - ## apply.c ##\n -@@ apply.c: static int check_patch(struct apply_state *state, struct patch *patch)\n - \tif ((tpatch = in_fn_table(state, new_name)) &&\n - \t (was_deleted(tpatch) || to_be_deleted(tpatch)))\n - \t\tok_if_exists = 1;\n -+\telse if (ignore_case && !strcasecmp(old_name, new_name))\n -+\t\tok_if_exists = 1;\n - \telse\n - \t\tok_if_exists = 0;\n - \n -\n - ## t/t4141-apply-case-insensitive-rename.sh (new) ##\n + ## t/t4141-apply-icase.sh (new) ##\n @@\n +#!/bin/sh\n +\n -+test_description='git apply should handle case-only renames on case-insensitive filesystems'\n ++test_description='git apply with core.ignorecase'\n +\n -+TEST_PASSES_SANITIZE_LEAK=true\n +. ./test-lib.sh\n +\n -+# Please note, this test assumes that core.ignorecase is set appropriately for the filesystem,\n -+# as tested in t0050. Case-only rename conflicts are only tested in case-sensitive filesystems.\n ++test_expect_success setup '\n ++ # initial commit has file0 only\n ++ test_commit \"initial\" file0 \"initial commit with file0\" initial &&\n +\n -+if ! test_have_prereq CASE_INSENSITIVE_FS\n -+then\n -+\ttest_set_prereq CASE_SENSITIVE_FS\n -+\techo nuts\n -+fi\n ++ # current commit has file1 as well\n ++ test_commit \"current\" file1 \"initial content of file1\" current &&\n ++ file0blob=$(git rev-parse :file0) &&\n ++ file1blob=$(git rev-parse :file1) &&\n +\n -+test_expect_success setup '\n -+\techo \"This is some content in the file.\" > file1 &&\n -+\techo \"A completely different file.\" > file2 &&\n -+\tgit update-index --add file1 &&\n -+\tgit update-index --add file2 &&\n -+\tcat >case_only_rename_patch <<-\\EOF\n -+\tdiff --git a/file1 b/File1\n -+\tsimilarity index 100%\n -+\trename from file1\n -+\trename to File1\n -+\tEOF\n ++ # prepare sample patches\n ++ # file0 is modified\n ++ echo modification to file0 >file0 &&\n ++ git add file0 &&\n ++ modifiedfile0blob=$(git rev-parse :file0) &&\n ++\n ++ # file1 is removed and then ...\n ++ git rm --cached file1 &&\n ++ # ... identical copies are placed at File1 and file2\n ++ git update-index --add --cacheinfo 100644,$file1blob,file2 &&\n ++ git update-index --add --cacheinfo 100644,$file1blob,File1 &&\n ++\n ++ # then various patches to do basic things\n ++ git diff HEAD^ HEAD -- file1 >creation-patch &&\n ++ git diff HEAD HEAD^ -- file1 >deletion-patch &&\n ++ git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&\n ++ git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&\n ++ git diff --cached HEAD -- file0 >modify-file0-patch\n +'\n +\n -+test_expect_success 'refuse to apply rename patch with conflict' '\n -+\tcat >conflict_patch <<-\\EOF &&\n -+\tdiff --git a/file1 b/file2\n -+\tsimilarity index 100%\n -+\trename from file1\n -+\trename to file2\n -+\tEOF\n -+\ttest_must_fail git apply --index conflict_patch\n ++# Basic creation, deletion, modification and renaming.\n ++test_expect_success 'creation and deletion' '\n ++ # start at \"initial\" with file0 only\n ++ git reset --hard initial &&\n ++\n ++ # add file1\n ++ git -c core.ignorecase=false apply --cached creation-patch &&\n ++ test_cmp_rev :file1 \"$file1blob\" &&\n ++\n ++ # remove file1\n ++ git -c core.ignorecase=false apply --cached deletion-patch &&\n ++ test_must_fail git rev-parse --verify :file1 &&\n ++\n ++ # do the same with ignorecase\n ++ git -c core.ignorecase=true apply --cached creation-patch &&\n ++ test_cmp_rev :file1 \"$file1blob\" &&\n ++ git -c core.ignorecase=true apply --cached deletion-patch &&\n ++ test_must_fail git rev-parse --verify :file1\n +'\n +\n -+test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '\n -+\ttest_when_finished \"git mv File1 file2\" &&\n -+\tgit mv file2 File1 &&\n -+\ttest_must_fail git apply --index case_only_rename_patch\n ++test_expect_success 'modificaiton' '\n ++ # start at \"initial\" with file0 only\n ++ git reset --hard initial &&\n ++\n ++ # modify file0\n ++ git -c core.ignorecase=false apply --cached modify-file0-patch &&\n ++ test_cmp_rev :file0 \"$modifiedfile0blob\" &&\n ++ git -c core.ignorecase=false apply --cached -R modify-file0-patch &&\n ++ test_cmp_rev :file0 \"$file0blob\" &&\n ++\n ++ # do the same with ignorecase\n ++ git -c core.ignorecase=true apply --cached modify-file0-patch &&\n ++ test_cmp_rev :file0 \"$modifiedfile0blob\" &&\n ++ git -c core.ignorecase=true apply --cached -R modify-file0-patch &&\n ++ test_cmp_rev :file0 \"$file0blob\"\n ++'\n ++\n ++test_expect_success 'rename file1 to file2' '\n ++ # start from file0 and file1\n ++ git reset --hard current &&\n ++\n ++ # rename file1 to file2\n ++ git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&\n ++ test_must_fail git rev-parse --verify :file1 &&\n ++ test_cmp_rev :file2 \"$file1blob\" &&\n ++ git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&\n ++ test_must_fail git rev-parse --verify :file2 &&\n ++ test_cmp_rev :file1 \"$file1blob\" &&\n ++\n ++ # do the same with ignorecase\n ++ git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&\n ++ test_must_fail git rev-parse --verify :file1 &&\n ++ test_cmp_rev :file2 \"$file1blob\" &&\n ++ git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&\n ++ test_must_fail git rev-parse --verify :file2 &&\n ++ test_cmp_rev :file1 \"$file1blob\"\n ++'\n ++\n ++test_expect_success 'rename file1 to file2' '\n ++ # start from file0 and file1\n ++ git reset --hard current &&\n ++\n ++ # rename file1 to File1\n ++ git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&\n ++ test_must_fail git rev-parse --verify :file1 &&\n ++ test_cmp_rev :File1 \"$file1blob\" &&\n ++ git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&\n ++ test_must_fail git rev-parse --verify :File1 &&\n ++ test_cmp_rev :file1 \"$file1blob\" &&\n ++\n ++ # do the same with ignorecase\n ++ git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&\n ++ test_must_fail git rev-parse --verify :file1 &&\n ++ test_cmp_rev :File1 \"$file1blob\" &&\n ++ git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&\n ++ test_must_fail git rev-parse --verify :File1 &&\n ++ test_cmp_rev :file1 \"$file1blob\"\n ++'\n ++\n ++# We may want to add tests with working tree here, without \"--cached\" and\n ++# with and without \"--index\" here. For example, should modify-file0-patch\n ++# apply cleanly if we have File0 with $file0blob in the index and the working\n ++# tree if core.icase is set?\n ++\n ++test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '\n ++ : sample\n +'\n +\n -+test_expect_success 'apply case-only rename patch without conflict' '\n -+\tgit apply --index case_only_rename_patch\n ++test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '\n ++ : sample\n +'\n +\n +test_done\n -: ----------- > 2: 1226fbd3caf reset: new failing test for reset of case-insensitive duplicate in index\n -: ----------- > 3: 04d83283716 apply: support case-only renames in case-insensitive filesystems\n\n-- \ngitgitgadget\n"},{"id":"457535","messageId":"1226fbd3cafbe5202b576a2ca74f3cbf1f603f02.1655655027.git.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":"pull.1257.v2.git.1655655027.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] reset: new failing test for reset of case-insensitive duplicate in index","fromName":"Tao Klerks via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-19T16:10:25Z","receivedAt":"2022-06-19T16:10:42Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"From: Tao Klerks \n\nOn case-insensitive filesystems, where core.ignorecase is normally set,\nthe index is still case-sensitive, and surprising outcomes are possible\nwhen the index contains states that cannot be represented on the file\nsystem.\n\nAdd an \"expect_failure\" test to illustrate one such situation, where two\nfiles differing only in case are in the index, and a \"reset --hard\" ends\nup creating an unexpected worktree change.\n\nSigned-off-by: Tao Klerks \n---\n t/t7104-reset-hard.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh\nindex cf9697eba9a..55050ac831e 100755\n--- a/t/t7104-reset-hard.sh\n+++ b/t/t7104-reset-hard.sh\n@@ -44,4 +44,15 @@ test_expect_success 'reset --hard did not corrupt index or cache-tree' '\n \n '\n \n+test_expect_failure CASE_INSENSITIVE_FS 'reset --hard handles index-only case-insensitive duplicate' '\n+\ttest_commit \"initial\" file1 \"initial commit with file1\" initial &&\n+\tfile1blob=$(git rev-parse :file1) &&\n+\tgit update-index --add --cacheinfo 100644,$file1blob,File1 &&\n+\n+\t# reset --hard accidentally leaves the working tree with a deleted file.\n+\tgit reset --hard &&\n+\tgit status --porcelain -uno >wt_changes_remaining &&\n+\ttest_must_be_empty wt_changes_remaining\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"457536","messageId":"efd3bd4cdda815a1b7dec35de6569cd4ab0817f3.1655655027.git.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":"pull.1257.v2.git.1655655027.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] t4141: test \"git apply\" with core.ignorecase","fromName":"Junio C Hamano via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-19T16:10:24Z","receivedAt":"2022-06-19T16:10:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Junio C Hamano \n\nSigned-off-by: Junio C Hamano \n---\n t/t4141-apply-icase.sh | 128 +++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 128 insertions(+)\n create mode 100755 t/t4141-apply-icase.sh\n\ndiff --git a/t/t4141-apply-icase.sh b/t/t4141-apply-icase.sh\nnew file mode 100755\nindex 00000000000..17eb023a437\n--- /dev/null\n+++ b/t/t4141-apply-icase.sh\n@@ -0,0 +1,128 @@\n+#!/bin/sh\n+\n+test_description='git apply with core.ignorecase'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+ # initial commit has file0 only\n+ test_commit \"initial\" file0 \"initial commit with file0\" initial &&\n+\n+ # current commit has file1 as well\n+ test_commit \"current\" file1 \"initial content of file1\" current &&\n+ file0blob=$(git rev-parse :file0) &&\n+ file1blob=$(git rev-parse :file1) &&\n+\n+ # prepare sample patches\n+ # file0 is modified\n+ echo modification to file0 >file0 &&\n+ git add file0 &&\n+ modifiedfile0blob=$(git rev-parse :file0) &&\n+\n+ # file1 is removed and then ...\n+ git rm --cached file1 &&\n+ # ... identical copies are placed at File1 and file2\n+ git update-index --add --cacheinfo 100644,$file1blob,file2 &&\n+ git update-index --add --cacheinfo 100644,$file1blob,File1 &&\n+\n+ # then various patches to do basic things\n+ git diff HEAD^ HEAD -- file1 >creation-patch &&\n+ git diff HEAD HEAD^ -- file1 >deletion-patch &&\n+ git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&\n+ git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&\n+ git diff --cached HEAD -- file0 >modify-file0-patch\n+'\n+\n+# Basic creation, deletion, modification and renaming.\n+test_expect_success 'creation and deletion' '\n+ # start at \"initial\" with file0 only\n+ git reset --hard initial &&\n+\n+ # add file1\n+ git -c core.ignorecase=false apply --cached creation-patch &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+\n+ # remove file1\n+ git -c core.ignorecase=false apply --cached deletion-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --cached creation-patch &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ git -c core.ignorecase=true apply --cached deletion-patch &&\n+ test_must_fail git rev-parse --verify :file1\n+'\n+\n+test_expect_success 'modificaiton' '\n+ # start at \"initial\" with file0 only\n+ git reset --hard initial &&\n+\n+ # modify file0\n+ git -c core.ignorecase=false apply --cached modify-file0-patch &&\n+ test_cmp_rev :file0 \"$modifiedfile0blob\" &&\n+ git -c core.ignorecase=false apply --cached -R modify-file0-patch &&\n+ test_cmp_rev :file0 \"$file0blob\" &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --cached modify-file0-patch &&\n+ test_cmp_rev :file0 \"$modifiedfile0blob\" &&\n+ git -c core.ignorecase=true apply --cached -R modify-file0-patch &&\n+ test_cmp_rev :file0 \"$file0blob\"\n+'\n+\n+test_expect_success 'rename file1 to file2' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # rename file1 to file2\n+ git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :file2 \"$file1blob\" &&\n+ git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file2 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :file2 \"$file1blob\" &&\n+ git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file2 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success 'rename file1 to file2' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # rename file1 to File1\n+ git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :File1 \"$file1blob\" &&\n+ git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :File1 \"$file1blob\" &&\n+ git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+# We may want to add tests with working tree here, without \"--cached\" and\n+# with and without \"--index\" here. For example, should modify-file0-patch\n+# apply cleanly if we have File0 with $file0blob in the index and the working\n+# tree if core.icase is set?\n+\n+test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '\n+ : sample\n+'\n+\n+test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '\n+ : sample\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"457537","messageId":"04d83283716b6048cb89f8485c818dae24921405.1655655027.git.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":"pull.1257.v2.git.1655655027.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-19T16:10:26Z","receivedAt":"2022-06-19T16:10:43Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"From: Tao Klerks \n\n\"git apply\" checks, when validating a patch, to ensure that any files\nbeing added aren't already in the worktree.\n\nWhen this check runs on a case-only rename, in a case-insensitive\nfilesystem, this leads to a false positive - the command fails with an\nerror like:\nerror: File1: already exists in working directory\n\nThere is a mechanism to ensure that \"seemingly conflicting\" files are\nhandled correctly - for example overlapping rename pairs or swaps -\nthis mechanism treats renames as add/remove pairs, and would end up\ntreating a case-only rename as a \"self-swap\"... Except it does not\naccount for case-insensitive filesystems yet.\n\nBecause the index is inherently case-sensitive even on a\ncase-insensitive filesystem, we actually need this mechanism to be\nhandle both requirements, lest we fail to account for conflicting\nfiles only in the index.\n\nFix the \"rename chain\" existence exemption mechanism to account for\ncase-insensitive config, fixing case-only-rename-handling as a\n\"self-swap\" and also fixing less-common \"case-insensitive rename\npairs\" when config core.ignorecase is set, but keep the index checks\nfile-sensitive.\n\nAlso add test cases around these behaviors - verifying that conflicting\nfile conditions are still caught correctly, including case-only\nconflicts on case-sensitive filesystems, and edge cases around\ncase-sensitive index behaviors on a case-insensitive filesystem.\n\nSigned-off-by: Tao Klerks \n---\n apply.c | 81 ++++++++++++++++------\n apply.h | 5 +-\n t/t4141-apply-icase.sh | 154 +++++++++++++++++++++++++++++++++++++----\n 3 files changed, 207 insertions(+), 33 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 2b7cd930efa..2bd59b63edd 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -101,7 +101,9 @@ int init_apply_state(struct apply_state *state,\n \tstate->ws_error_action = warn_on_ws_error;\n \tstate->ws_ignore_action = ignore_ws_none;\n \tstate->linenr = 1;\n-\tstring_list_init_nodup(&state->fn_table);\n+\tstring_list_init_nodup(&state->fs_fn_table);\n+\tstate->fs_fn_table.cmp = fspathcmp;\n+\tstring_list_init_nodup(&state->index_fn_table);\n \tstring_list_init_nodup(&state->limit_by_name);\n \tstrset_init(&state->removed_symlinks);\n \tstrset_init(&state->kept_symlinks);\n@@ -122,7 +124,10 @@ void clear_apply_state(struct apply_state *state)\n \tstrset_clear(&state->kept_symlinks);\n \tstrbuf_release(&state->root);\n \n-\t/* &state->fn_table is cleared at the end of apply_patch() */\n+\t/*\n+\t * &state->fs_fn_table and &state->index_fn_table are cleared at the\n+\t * end of apply_patch()\n+\t */\n }\n \n static void mute_routine(const char *msg, va_list params)\n@@ -3270,14 +3275,28 @@ static int read_file_or_gitlink(const struct cache_entry *ce, struct strbuf *buf\n \treturn read_blob_object(buf, &ce->oid, ce->ce_mode);\n }\n \n-static struct patch *in_fn_table(struct apply_state *state, const char *name)\n+static struct patch *in_fs_fn_table(struct apply_state *state, const char *name)\n {\n \tstruct string_list_item *item;\n \n \tif (!name)\n \t\treturn NULL;\n \n-\titem = string_list_lookup(&state->fn_table, name);\n+\titem = string_list_lookup(&state->fs_fn_table, name);\n+\tif (item)\n+\t\treturn (struct patch *)item->util;\n+\n+\treturn NULL;\n+}\n+\n+static struct patch *in_index_fn_table(struct apply_state *state, const char *name)\n+{\n+\tstruct string_list_item *item;\n+\n+\tif (!name)\n+\t\treturn NULL;\n+\n+\titem = string_list_lookup(&state->index_fn_table, name);\n \tif (item)\n \t\treturn (struct patch *)item->util;\n \n@@ -3309,7 +3328,7 @@ static int was_deleted(struct patch *patch)\n \treturn patch == PATH_WAS_DELETED;\n }\n \n-static void add_to_fn_table(struct apply_state *state, struct patch *patch)\n+static void add_to_fn_tables(struct apply_state *state, struct patch *patch)\n {\n \tstruct string_list_item *item;\n \n@@ -3319,7 +3338,9 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)\n \t * file creations and copies\n \t */\n \tif (patch->new_name) {\n-\t\titem = string_list_insert(&state->fn_table, patch->new_name);\n+\t\titem = string_list_insert(&state->fs_fn_table, patch->new_name);\n+\t\titem->util = patch;\n+\t\titem = string_list_insert(&state->index_fn_table, patch->new_name);\n \t\titem->util = patch;\n \t}\n \n@@ -3328,7 +3349,9 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)\n \t * later chunks shouldn't patch old names\n \t */\n \tif ((patch->new_name == NULL) || (patch->is_rename)) {\n-\t\titem = string_list_insert(&state->fn_table, patch->old_name);\n+\t\titem = string_list_insert(&state->fs_fn_table, patch->old_name);\n+\t\titem->util = PATH_WAS_DELETED;\n+\t\titem = string_list_insert(&state->index_fn_table, patch->old_name);\n \t\titem->util = PATH_WAS_DELETED;\n \t}\n }\n@@ -3341,7 +3364,9 @@ static void prepare_fn_table(struct apply_state *state, struct patch *patch)\n \twhile (patch) {\n \t\tif ((patch->new_name == NULL) || (patch->is_rename)) {\n \t\t\tstruct string_list_item *item;\n-\t\t\titem = string_list_insert(&state->fn_table, patch->old_name);\n+\t\t\titem = string_list_insert(&state->fs_fn_table, patch->old_name);\n+\t\t\titem->util = PATH_TO_BE_DELETED;\n+\t\t\titem = string_list_insert(&state->index_fn_table, patch->old_name);\n \t\t\titem->util = PATH_TO_BE_DELETED;\n \t\t}\n \t\tpatch = patch->next;\n@@ -3371,7 +3396,7 @@ static struct patch *previous_patch(struct apply_state *state,\n \tif (patch->is_copy || patch->is_rename)\n \t\treturn NULL; /* \"git\" patches do not depend on the order */\n \n-\tprevious = in_fn_table(state, patch->old_name);\n+\tprevious = in_index_fn_table(state, patch->old_name);\n \tif (!previous)\n \t\treturn NULL;\n \n@@ -3681,7 +3706,7 @@ static int apply_data(struct apply_state *state, struct patch *patch,\n \t}\n \tpatch->result = image.buf;\n \tpatch->resultsize = image.len;\n-\tadd_to_fn_table(state, patch);\n+\tadd_to_fn_tables(state, patch);\n \tfree(image.line_allocated);\n \n \tif (0 < patch->is_delete && patch->resultsize)\n@@ -3780,11 +3805,12 @@ static int check_preimage(struct apply_state *state,\n \n static int check_to_create(struct apply_state *state,\n \t\t\t const char *new_name,\n-\t\t\t int ok_if_exists)\n+\t\t\t int ok_if_exists_in_fs,\n+\t\t\t int ok_if_exists_in_index)\n {\n \tstruct stat nst;\n \n-\tif (state->check_index && (!ok_if_exists || !state->cached)) {\n+\tif (state->check_index && (!ok_if_exists_in_index || !state->cached)) {\n \t\tint pos;\n \n \t\tpos = index_name_pos(state->repo->index, new_name, strlen(new_name));\n@@ -3792,7 +3818,7 @@ static int check_to_create(struct apply_state *state,\n \t\t\tstruct cache_entry *ce = state->repo->index->cache[pos];\n \n \t\t\t/* allow ITA, as they do not yet exist in the index */\n-\t\t\tif (!ok_if_exists && !(ce->ce_flags & CE_INTENT_TO_ADD))\n+\t\t\tif (!ok_if_exists_in_index && !(ce->ce_flags & CE_INTENT_TO_ADD))\n \t\t\t\treturn EXISTS_IN_INDEX;\n \n \t\t\t/* ITA entries can never match working tree files */\n@@ -3805,7 +3831,7 @@ static int check_to_create(struct apply_state *state,\n \t\treturn 0;\n \n \tif (!lstat(new_name, &nst)) {\n-\t\tif (S_ISDIR(nst.st_mode) || ok_if_exists)\n+\t\tif (S_ISDIR(nst.st_mode) || ok_if_exists_in_fs)\n \t\t\treturn 0;\n \t\t/*\n \t\t * A leading component of new_name might be a symlink\n@@ -3915,7 +3941,8 @@ static int check_patch(struct apply_state *state, struct patch *patch)\n \tconst char *name = old_name ? old_name : new_name;\n \tstruct cache_entry *ce = NULL;\n \tstruct patch *tpatch;\n-\tint ok_if_exists;\n+\tint ok_if_exists_in_fs;\n+\tint ok_if_exists_in_index;\n \tint status;\n \n \tpatch->rejected = 1; /* we will drop this after we succeed */\n@@ -3938,16 +3965,29 @@ static int check_patch(struct apply_state *state, struct patch *patch)\n \t * B; ask to_be_deleted() about the later rename. Removal of\n \t * B and rename from A to B is handled the same way by asking\n \t * was_deleted().\n+\t *\n+\t * These exemptions account for the core.ignorecase config -\n+\t * a file that differs only by case is also considered \"deleted\"\n+\t * if git is configured to ignore case. This means a case-only\n+\t * rename, in a case-insensitive filesystem, is treated here as\n+\t * a \"self-swap\" or mode change.\n \t */\n-\tif ((tpatch = in_fn_table(state, new_name)) &&\n+\tif ((tpatch = in_fs_fn_table(state, new_name)) &&\n+\t (was_deleted(tpatch) || to_be_deleted(tpatch)))\n+\t\tok_if_exists_in_fs = 1;\n+\telse\n+\t\tok_if_exists_in_fs = 0;\n+\n+\tif ((tpatch = in_index_fn_table(state, new_name)) &&\n \t (was_deleted(tpatch) || to_be_deleted(tpatch)))\n-\t\tok_if_exists = 1;\n+\t\tok_if_exists_in_index = 1;\n \telse\n-\t\tok_if_exists = 0;\n+\t\tok_if_exists_in_index = 0;\n \n \tif (new_name &&\n \t ((0 < patch->is_new) || patch->is_rename || patch->is_copy)) {\n-\t\tint err = check_to_create(state, new_name, ok_if_exists);\n+\t\tint err = check_to_create(state, new_name, ok_if_exists_in_fs,\n+\t\t\t\t\t ok_if_exists_in_index);\n \n \t\tif (err && state->threeway) {\n \t\t\tpatch->direct_to_threeway = 1;\n@@ -4808,7 +4848,8 @@ static int apply_patch(struct apply_state *state,\n end:\n \tfree_patch_list(list);\n \tstrbuf_release(&buf);\n-\tstring_list_clear(&state->fn_table, 0);\n+\tstring_list_clear(&state->fs_fn_table, 0);\n+\tstring_list_clear(&state->index_fn_table, 0);\n \treturn res;\n }\n \ndiff --git a/apply.h b/apply.h\nindex b9f18ce87d1..b520ce8c40a 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -95,8 +95,11 @@ struct apply_state {\n \t/*\n \t * Records filenames that have been touched, in order to handle\n \t * the case where more than one patches touch the same file.\n+\t * Two separate structures because with ignorecase, one of them\n+\t * needs to be case-insensitive and the other not.\n \t */\n-\tstruct string_list fn_table;\n+\tstruct string_list fs_fn_table;\n+\tstruct string_list index_fn_table;\n \n \t/*\n \t * This is to save reporting routines before using\ndiff --git a/t/t4141-apply-icase.sh b/t/t4141-apply-icase.sh\nindex 17eb023a437..1c785133d16 100755\n--- a/t/t4141-apply-icase.sh\n+++ b/t/t4141-apply-icase.sh\n@@ -30,7 +30,16 @@ test_expect_success setup '\n git diff HEAD HEAD^ -- file1 >deletion-patch &&\n git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&\n git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&\n- git diff --cached HEAD -- file0 >modify-file0-patch\n+ git diff --cached HEAD -- file0 >modify-file0-patch &&\n+\n+ # then set up for swap\n+ git reset --hard current &&\n+ test_commit \"swappable\" file3 \"different content for file3\" swappable &&\n+ file3blob=$(git rev-parse :file3) &&\n+ git rm --cached file1 file3 &&\n+ git update-index --add --cacheinfo 100644,$file1blob,File3 &&\n+ git update-index --add --cacheinfo 100644,$file3blob,File1 &&\n+ git diff --cached HEAD -- file1 file3 File1 File3 >swap-file1-and-file3-to-File3-and-File1-patch\n '\n \n # Basic creation, deletion, modification and renaming.\n@@ -53,7 +62,7 @@ test_expect_success 'creation and deletion' '\n test_must_fail git rev-parse --verify :file1\n '\n \n-test_expect_success 'modificaiton' '\n+test_expect_success 'modification (index-only)' '\n # start at \"initial\" with file0 only\n git reset --hard initial &&\n \n@@ -70,7 +79,7 @@ test_expect_success 'modificaiton' '\n test_cmp_rev :file0 \"$file0blob\"\n '\n \n-test_expect_success 'rename file1 to file2' '\n+test_expect_success 'rename file1 to file2 (index-only)' '\n # start from file0 and file1\n git reset --hard current &&\n \n@@ -91,7 +100,7 @@ test_expect_success 'rename file1 to file2' '\n test_cmp_rev :file1 \"$file1blob\"\n '\n \n-test_expect_success 'rename file1 to file2' '\n+test_expect_success 'rename file1 to File1 (index-only)' '\n # start from file0 and file1\n git reset --hard current &&\n \n@@ -112,17 +121,138 @@ test_expect_success 'rename file1 to file2' '\n test_cmp_rev :file1 \"$file1blob\"\n '\n \n-# We may want to add tests with working tree here, without \"--cached\" and\n-# with and without \"--index\" here. For example, should modify-file0-patch\n-# apply cleanly if we have File0 with $file0blob in the index and the working\n-# tree if core.icase is set?\n+# involve filesystem on renames\n+test_expect_success 'rename file1 to File1 (with ignorecase, working tree)' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --index rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :File1 \"$file1blob\" &&\n+ git -c core.ignorecase=true apply --index -R rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success CASE_INSENSITIVE_FS 'rename file1 to File1 (without ignorecase, case-insensitive FS)' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # rename file1 to File1 without ignorecase (fails as expected)\n+ test_must_fail git -c core.ignorecase=false apply --index rename-file1-to-File1-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success !CASE_INSENSITIVE_FS 'rename file1 to File1 (without ignorecase, case-sensitive FS)' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # rename file1 to File1 without ignorecase\n+ git -c core.ignorecase=false apply --index rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :File1 \"$file1blob\" &&\n+ git -c core.ignorecase=false apply --index -R rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success 'rename file1 to file2 with working tree conflict' '\n+ # start from file0 and file1, and file2 untracked\n+ git reset --hard current &&\n+ test_when_finished \"rm file2\" &&\n+ touch file2 &&\n+\n+ # rename file1 to file2 with conflict\n+ test_must_fail git -c core.ignorecase=false apply --index rename-file1-to-file2-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n \n-test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '\n- : sample\n+ # do the same with ignorecase\n+ test_must_fail git -c core.ignorecase=true apply --index rename-file1-to-file2-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n '\n \n-test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '\n- : sample\n+test_expect_success 'rename file1 to file2 with case-insensitive conflict (index-only - ignorecase disabled)' '\n+ # start from file0 and file1, and File2 in index\n+ git reset --hard current &&\n+ git update-index --add --cacheinfo 100644,$file3blob,File2 &&\n+\n+ # rename file1 to file2 without ignorecase\n+ git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :file2 \"$file1blob\" &&\n+ git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file2 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :File2 \"$file3blob\"\n+'\n+\n+test_expect_failure 'rename file1 to file2 with case-insensitive conflict (index-only - ignorecase enabled)' '\n+ # start from file0 and file1, and File2 in index\n+ git reset --hard current &&\n+ git update-index --add --cacheinfo 100644,$file3blob,File2 &&\n+\n+ # rename file1 to file2 with ignorecase, with a \"File2\" conflicting file in place - expect failure.\n+ # instead of failure, we get success with \"File1\" and \"file1\" both existing in the index, despite\n+ # the ignorecase configuration.\n+ test_must_fail git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :File2 \"$file3blob\"\n+'\n+\n+test_expect_success 'rename file1 to File1 with case-sensitive conflict (index-only)' '\n+ # start from file0 and file1, and File1 in index\n+ git reset --hard current &&\n+ git update-index --add --cacheinfo 100644,$file3blob,File1 &&\n+\n+ # On a case-insensitive filesystem with core.ignorecase on, a single git\n+ # \"reset --hard\" will actually leave things wrong because of the\n+ # index-to-working-tree discrepancy - see \"reset --hard handles\n+ # index-only case-insensitive duplicate\" under t7104-reset-hard.sh.\n+ # We are creating this unexpected state, so we should explicitly queue\n+ # an extra reset. If reset ever starts to handle this case, this will\n+ # become unnecessary but also not harmful.\n+ test_when_finished \"git reset --hard\" &&\n+\n+ # rename file1 to File1 when File1 is already in index (fails with conflict)\n+ test_must_fail git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :File1 \"$file3blob\" &&\n+\n+ # do the same with ignorecase\n+ test_must_fail git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :File1 \"$file3blob\"\n+'\n+\n+test_expect_success CASE_INSENSITIVE_FS 'case-insensitive swap - file1 to File2 and file2 to File1 (working tree)' '\n+ # start from file0, file1, and file3\n+ git reset --hard swappable &&\n+\n+ # \"swap\" file1 and file3 to case-insensitive versions without ignorecase on case-insensitive FS (fails as expected)\n+ test_must_fail git -c core.ignorecase=false apply --index swap-file1-and-file3-to-File3-and-File1-patch &&\n+ git rev-parse --verify :file1 &&\n+ git rev-parse --verify :file3 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :file3 \"$file3blob\" &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --index swap-file1-and-file3-to-File3-and-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_must_fail git rev-parse --verify :file3 &&\n+ test_cmp_rev :File3 \"$file1blob\" &&\n+ test_cmp_rev :File1 \"$file3blob\" &&\n+ git -c core.ignorecase=true apply --index -R swap-file1-and-file3-to-File3-and-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_must_fail git rev-parse --verify :File3 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :file3 \"$file3blob\"\n '\n \n test_done\n-- \ngitgitgadget\n"},{"id":"464468","messageId":"CAPMMpogQG_ggwQdhwEUtUmhS7JMmf5Ke72+f=+Xox-J6iMpfOQ@mail.gmail.com","threadId":"57986","inReplyTo":"pull.1257.v2.git.1655655027.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] RFC: apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-10-10T04:09:13Z","receivedAt":"2022-10-10T04:09:29Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"Hi folks,\n\nThese patches are a few months old now, but they still apply cleanly\nand I'm not sure how to improve them.\n\nI'd appreciate any feedback on both the approach, and the detailed\ncode changes themselves, that could help me make this a viable\npatchset to fix case-only renames on file-insensitive filesystems\nusing \"git apply\".\n\nThanks,\nTao\n\nOn Sun, Jun 19, 2022 at 6:10 PM Tao Klerks via GitGitGadget\n wrote:\n>\n> As suggested recently in thread\n> CAPMMpojwV+f=z9sgc_GaUOTFBCUVdbrGW8WjatWWmC3WTcsoXw@mail.gmail.com,\n> proposing a fix to git-apply for case-only renames on case-insensitive\n> filesystems.\n>\n> Changes in V2:\n>\n> * Prepended a commit from Junio, with new apply tests to build on\n> * Added a largely-unrelated new known failing test, concerning reset --hard\n> in the presence of index case conflicts on a case-insensitive filesystem,\n> which we later need to work around in a corner-case test\n> * Moved test cases to build on Junio's new test file\n> * Switched fix approach from \"allow same-name commit explicitly\" to \"track\n> files marked for deletion case-insensitively for filesystem checks\",\n> which addresses the issue noted and other more obscure ones like \"rename\n> swap with case change\"\n> * Added a test case for \"rename swap with case change\"\n> * Added test cases setting \"core.ignorecase\" on and off explicitly\n> * Added a test case exposing one remaining surprising behavior\n>\n> POSSIBLE CONCERN:\n>\n> This fix was originally much simpler - it just made the \"fn_table\" string\n> list use a case-insensitive string comparison - using case-insensitive\n> comparisons when dealing with all replacement checks, both on the index and\n> on the filesystem.\n>\n> However, with that simple implementation, there was at least one edge-case\n> where data loss could result: If the index contained two files differing\n> only by case, with different content, and we were doing a case-only rename,\n> a swap, or some other operation involving the deletion and creation of a\n> file with that name (ignoring case), then both of the files with that name\n> in the index would be overwritten - even though only one of them had the\n> expected content, and even though the one deleted might never have been\n> committed.\n>\n> It seems as though the core.ignorecase option should typically only apply to\n> filesystem checks - that the index is always case-sensitive.\n>\n> The current fix proposal therefore splits the string list used for \"can I\n> create a file that already exists?\" checks into two such structures - one\n> string list used for filesystem checks, which is case-insensitive when\n> specified by core.ignorecase, and one used for index checks, which is always\n> case-sensitive.\n>\n> The resulting duplication is not appealing, but I'm not sure how to address\n> it / how to do this more elegantly. I'm also still not completely certain\n> that my rule of thumb about the index always being case-sensitive is the\n> right way of thinking of things.\n>\n> Junio C Hamano (1):\n> t4141: test \"git apply\" with core.ignorecase\n>\n> Tao Klerks (2):\n> reset: new failing test for reset of case-insensitive duplicate in\n> index\n> apply: support case-only renames in case-insensitive filesystems\n>\n> apply.c | 81 +++++++++----\n> apply.h | 5 +-\n> t/t4141-apply-icase.sh | 258 +++++++++++++++++++++++++++++++++++++++++\n> t/t7104-reset-hard.sh | 11 ++\n> 4 files changed, 334 insertions(+), 21 deletions(-)\n> create mode 100755 t/t4141-apply-icase.sh\n>\n>\n> base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1257%2FTaoK%2Ftao-apply-case-insensitive-renames-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1257/TaoK/tao-apply-case-insensitive-renames-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1257\n>\n> Range-diff vs v1:\n>\n> 1: b8bd612aa1e ! 1: efd3bd4cdda apply: support case-only renames in case-insensitive filesystems\n> @@\n> ## Metadata ##\n> -Author: Tao Klerks \n> +Author: Junio C Hamano \n>\n> ## Commit message ##\n> - apply: support case-only renames in case-insensitive filesystems\n> + t4141: test \"git apply\" with core.ignorecase\n>\n> - \"git apply\" checks, when validating a patch, to ensure that any files\n> - being added aren't already in the worktree.\n> + Signed-off-by: Junio C Hamano \n>\n> - When this check runs on a case-only rename, in a case-insensitive\n> - filesystem, this leads to a false positive - the command fails with an\n> - error like:\n> - error: File1: already exists in working directory\n> -\n> - Fix this existence check to allow the file to exist, for a case-only\n> - rename when config core.ignorecase is set.\n> -\n> - Also add a test for this case, while verifying that conflicting file\n> - conditions are still caught correctly, including case-only conflicts on\n> - case-sensitive filesystems.\n> -\n> - Signed-off-by: Tao Klerks \n> -\n> - ## apply.c ##\n> -@@ apply.c: static int check_patch(struct apply_state *state, struct patch *patch)\n> - if ((tpatch = in_fn_table(state, new_name)) &&\n> - (was_deleted(tpatch) || to_be_deleted(tpatch)))\n> - ok_if_exists = 1;\n> -+ else if (ignore_case && !strcasecmp(old_name, new_name))\n> -+ ok_if_exists = 1;\n> - else\n> - ok_if_exists = 0;\n> -\n> -\n> - ## t/t4141-apply-case-insensitive-rename.sh (new) ##\n> + ## t/t4141-apply-icase.sh (new) ##\n> @@\n> +#!/bin/sh\n> +\n> -+test_description='git apply should handle case-only renames on case-insensitive filesystems'\n> ++test_description='git apply with core.ignorecase'\n> +\n> -+TEST_PASSES_SANITIZE_LEAK=true\n> +. ./test-lib.sh\n> +\n> -+# Please note, this test assumes that core.ignorecase is set appropriately for the filesystem,\n> -+# as tested in t0050. Case-only rename conflicts are only tested in case-sensitive filesystems.\n> ++test_expect_success setup '\n> ++ # initial commit has file0 only\n> ++ test_commit \"initial\" file0 \"initial commit with file0\" initial &&\n> +\n> -+if ! test_have_prereq CASE_INSENSITIVE_FS\n> -+then\n> -+ test_set_prereq CASE_SENSITIVE_FS\n> -+ echo nuts\n> -+fi\n> ++ # current commit has file1 as well\n> ++ test_commit \"current\" file1 \"initial content of file1\" current &&\n> ++ file0blob=$(git rev-parse :file0) &&\n> ++ file1blob=$(git rev-parse :file1) &&\n> +\n> -+test_expect_success setup '\n> -+ echo \"This is some content in the file.\" > file1 &&\n> -+ echo \"A completely different file.\" > file2 &&\n> -+ git update-index --add file1 &&\n> -+ git update-index --add file2 &&\n> -+ cat >case_only_rename_patch <<-\\EOF\n> -+ diff --git a/file1 b/File1\n> -+ similarity index 100%\n> -+ rename from file1\n> -+ rename to File1\n> -+ EOF\n> ++ # prepare sample patches\n> ++ # file0 is modified\n> ++ echo modification to file0 >file0 &&\n> ++ git add file0 &&\n> ++ modifiedfile0blob=$(git rev-parse :file0) &&\n> ++\n> ++ # file1 is removed and then ...\n> ++ git rm --cached file1 &&\n> ++ # ... identical copies are placed at File1 and file2\n> ++ git update-index --add --cacheinfo 100644,$file1blob,file2 &&\n> ++ git update-index --add --cacheinfo 100644,$file1blob,File1 &&\n> ++\n> ++ # then various patches to do basic things\n> ++ git diff HEAD^ HEAD -- file1 >creation-patch &&\n> ++ git diff HEAD HEAD^ -- file1 >deletion-patch &&\n> ++ git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&\n> ++ git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&\n> ++ git diff --cached HEAD -- file0 >modify-file0-patch\n> +'\n> +\n> -+test_expect_success 'refuse to apply rename patch with conflict' '\n> -+ cat >conflict_patch <<-\\EOF &&\n> -+ diff --git a/file1 b/file2\n> -+ similarity index 100%\n> -+ rename from file1\n> -+ rename to file2\n> -+ EOF\n> -+ test_must_fail git apply --index conflict_patch\n> ++# Basic creation, deletion, modification and renaming.\n> ++test_expect_success 'creation and deletion' '\n> ++ # start at \"initial\" with file0 only\n> ++ git reset --hard initial &&\n> ++\n> ++ # add file1\n> ++ git -c core.ignorecase=false apply --cached creation-patch &&\n> ++ test_cmp_rev :file1 \"$file1blob\" &&\n> ++\n> ++ # remove file1\n> ++ git -c core.ignorecase=false apply --cached deletion-patch &&\n> ++ test_must_fail git rev-parse --verify :file1 &&\n> ++\n> ++ # do the same with ignorecase\n> ++ git -c core.ignorecase=true apply --cached creation-patch &&\n> ++ test_cmp_rev :file1 \"$file1blob\" &&\n> ++ git -c core.ignorecase=true apply --cached deletion-patch &&\n> ++ test_must_fail git rev-parse --verify :file1\n> +'\n> +\n> -+test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '\n> -+ test_when_finished \"git mv File1 file2\" &&\n> -+ git mv file2 File1 &&\n> -+ test_must_fail git apply --index case_only_rename_patch\n> ++test_expect_success 'modificaiton' '\n> ++ # start at \"initial\" with file0 only\n> ++ git reset --hard initial &&\n> ++\n> ++ # modify file0\n> ++ git -c core.ignorecase=false apply --cached modify-file0-patch &&\n> ++ test_cmp_rev :file0 \"$modifiedfile0blob\" &&\n> ++ git -c core.ignorecase=false apply --cached -R modify-file0-patch &&\n> ++ test_cmp_rev :file0 \"$file0blob\" &&\n> ++\n> ++ # do the same with ignorecase\n> ++ git -c core.ignorecase=true apply --cached modify-file0-patch &&\n> ++ test_cmp_rev :file0 \"$modifiedfile0blob\" &&\n> ++ git -c core.ignorecase=true apply --cached -R modify-file0-patch &&\n> ++ test_cmp_rev :file0 \"$file0blob\"\n> ++'\n> ++\n> ++test_expect_success 'rename file1 to file2' '\n> ++ # start from file0 and file1\n> ++ git reset --hard current &&\n> ++\n> ++ # rename file1 to file2\n> ++ git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&\n> ++ test_must_fail git rev-parse --verify :file1 &&\n> ++ test_cmp_rev :file2 \"$file1blob\" &&\n> ++ git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&\n> ++ test_must_fail git rev-parse --verify :file2 &&\n> ++ test_cmp_rev :file1 \"$file1blob\" &&\n> ++\n> ++ # do the same with ignorecase\n> ++ git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&\n> ++ test_must_fail git rev-parse --verify :file1 &&\n> ++ test_cmp_rev :file2 \"$file1blob\" &&\n> ++ git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&\n> ++ test_must_fail git rev-parse --verify :file2 &&\n> ++ test_cmp_rev :file1 \"$file1blob\"\n> ++'\n> ++\n> ++test_expect_success 'rename file1 to file2' '\n> ++ # start from file0 and file1\n> ++ git reset --hard current &&\n> ++\n> ++ # rename file1 to File1\n> ++ git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&\n> ++ test_must_fail git rev-parse --verify :file1 &&\n> ++ test_cmp_rev :File1 \"$file1blob\" &&\n> ++ git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&\n> ++ test_must_fail git rev-parse --verify :File1 &&\n> ++ test_cmp_rev :file1 \"$file1blob\" &&\n> ++\n> ++ # do the same with ignorecase\n> ++ git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&\n> ++ test_must_fail git rev-parse --verify :file1 &&\n> ++ test_cmp_rev :File1 \"$file1blob\" &&\n> ++ git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&\n> ++ test_must_fail git rev-parse --verify :File1 &&\n> ++ test_cmp_rev :file1 \"$file1blob\"\n> ++'\n> ++\n> ++# We may want to add tests with working tree here, without \"--cached\" and\n> ++# with and without \"--index\" here. For example, should modify-file0-patch\n> ++# apply cleanly if we have File0 with $file0blob in the index and the working\n> ++# tree if core.icase is set?\n> ++\n> ++test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '\n> ++ : sample\n> +'\n> +\n> -+test_expect_success 'apply case-only rename patch without conflict' '\n> -+ git apply --index case_only_rename_patch\n> ++test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '\n> ++ : sample\n> +'\n> +\n> +test_done\n> -: ----------- > 2: 1226fbd3caf reset: new failing test for reset of case-insensitive duplicate in index\n> -: ----------- > 3: 04d83283716 apply: support case-only renames in case-insensitive filesystems\n>\n> --\n> gitgitgadget\n"},{"id":"477765","messageId":"pull.1257.v3.git.1685267999.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":"pull.1257.v2.git.1655655027.gitgitgadget@gmail.com","subject":"[PATCH v3 0/3] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-05-28T09:59:56Z","receivedAt":"2023-05-28T10:00:08Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"As suggested almost a year ago in thread\nCAPMMpojwV+f=z9sgc_GaUOTFBCUVdbrGW8WjatWWmC3WTcsoXw@mail.gmail.com,\nproposing a fix to git-apply for case-only renames on case-insensitive\nfilesystems.\n\nChanges in V3:\n\n * Rebased onto recent main\n * Renumbered now-duplicate-number test t4141 to t4142\n * Removed \"RFC\" prefix to officially submit; I don't see a better\n direction, and haven't received any corresponding feedback\n\nAs mentioned in V2, I'm not super-happy with the duplication of filename\ntracking tables, but I do think this bug needs to be fixed, and I don't see\nany other way to do so. The fundamental rule this change implements is that\nfilesystem filename duplication checks should respect the core.ignorecase\noption, but index filename duplication checks should not.\n\nJunio C Hamano (1):\n t4142: test \"git apply\" with core.ignorecase\n\nTao Klerks (2):\n reset: new failing test for reset of case-insensitive duplicate in\n index\n apply: support case-only renames in case-insensitive filesystems\n\n apply.c | 81 +++++++++----\n apply.h | 5 +-\n t/t4142-apply-icase.sh | 258 +++++++++++++++++++++++++++++++++++++++++\n t/t7104-reset-hard.sh | 11 ++\n 4 files changed, 334 insertions(+), 21 deletions(-)\n create mode 100755 t/t4142-apply-icase.sh\n\n\nbase-commit: 4a714b37029a4b63dbd22f7d7ed81f7a0d693680\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1257%2FTaoK%2Ftao-apply-case-insensitive-renames-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1257/TaoK/tao-apply-case-insensitive-renames-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1257\n\nRange-diff vs v2:\n\n 1: efd3bd4cdda ! 1: 8ad60943c66 t4141: test \"git apply\" with core.ignorecase\n @@ Metadata\n Author: Junio C Hamano \n \n ## Commit message ##\n - t4141: test \"git apply\" with core.ignorecase\n + t4142: test \"git apply\" with core.ignorecase\n \n Signed-off-by: Junio C Hamano \n \n - ## t/t4141-apply-icase.sh (new) ##\n + ## t/t4142-apply-icase.sh (new) ##\n @@\n +#!/bin/sh\n +\n 2: 1226fbd3caf = 2: ab1cdd95e03 reset: new failing test for reset of case-insensitive duplicate in index\n 3: 04d83283716 ! 3: 52359738532 apply: support case-only renames in case-insensitive filesystems\n @@ Commit message\n account for case-insensitive filesystems yet.\n \n Because the index is inherently case-sensitive even on a\n - case-insensitive filesystem, we actually need this mechanism to be\n + case-insensitive filesystem, we actually need this mechanism to\n handle both requirements, lest we fail to account for conflicting\n files only in the index.\n \n @@ apply.c: void clear_apply_state(struct apply_state *state)\n +\t */\n }\n \n - static void mute_routine(const char *msg, va_list params)\n + static void mute_routine(const char *msg UNUSED, va_list params UNUSED)\n @@ apply.c: static int read_file_or_gitlink(const struct cache_entry *ce, struct strbuf *buf\n \treturn read_blob_object(buf, &ce->oid, ce->ce_mode);\n }\n @@ apply.h: struct apply_state {\n \t/*\n \t * This is to save reporting routines before using\n \n - ## t/t4141-apply-icase.sh ##\n -@@ t/t4141-apply-icase.sh: test_expect_success setup '\n + ## t/t4142-apply-icase.sh ##\n +@@ t/t4142-apply-icase.sh: test_expect_success setup '\n git diff HEAD HEAD^ -- file1 >deletion-patch &&\n git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&\n git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&\n @@ t/t4141-apply-icase.sh: test_expect_success setup '\n '\n \n # Basic creation, deletion, modification and renaming.\n -@@ t/t4141-apply-icase.sh: test_expect_success 'creation and deletion' '\n +@@ t/t4142-apply-icase.sh: test_expect_success 'creation and deletion' '\n test_must_fail git rev-parse --verify :file1\n '\n \n @@ t/t4141-apply-icase.sh: test_expect_success 'creation and deletion' '\n # start at \"initial\" with file0 only\n git reset --hard initial &&\n \n -@@ t/t4141-apply-icase.sh: test_expect_success 'modificaiton' '\n +@@ t/t4142-apply-icase.sh: test_expect_success 'modificaiton' '\n test_cmp_rev :file0 \"$file0blob\"\n '\n \n @@ t/t4141-apply-icase.sh: test_expect_success 'modificaiton' '\n # start from file0 and file1\n git reset --hard current &&\n \n -@@ t/t4141-apply-icase.sh: test_expect_success 'rename file1 to file2' '\n +@@ t/t4142-apply-icase.sh: test_expect_success 'rename file1 to file2' '\n test_cmp_rev :file1 \"$file1blob\"\n '\n \n @@ t/t4141-apply-icase.sh: test_expect_success 'rename file1 to file2' '\n # start from file0 and file1\n git reset --hard current &&\n \n -@@ t/t4141-apply-icase.sh: test_expect_success 'rename file1 to file2' '\n +@@ t/t4142-apply-icase.sh: test_expect_success 'rename file1 to file2' '\n test_cmp_rev :file1 \"$file1blob\"\n '\n \n\n-- \ngitgitgadget\n"},{"id":"477766","messageId":"ab1cdd95e03c8f0a9896898b34ceab7b67bee3b5.1685267999.git.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":"pull.1257.v3.git.1685267999.gitgitgadget@gmail.com","subject":"[PATCH v3 2/3] reset: new failing test for reset of case-insensitive duplicate in index","fromName":"Tao Klerks via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-05-28T09:59:58Z","receivedAt":"2023-05-28T10:00:11Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"From: Tao Klerks \n\nOn case-insensitive filesystems, where core.ignorecase is normally set,\nthe index is still case-sensitive, and surprising outcomes are possible\nwhen the index contains states that cannot be represented on the file\nsystem.\n\nAdd an \"expect_failure\" test to illustrate one such situation, where two\nfiles differing only in case are in the index, and a \"reset --hard\" ends\nup creating an unexpected worktree change.\n\nSigned-off-by: Tao Klerks \n---\n t/t7104-reset-hard.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh\nindex cf9697eba9a..55050ac831e 100755\n--- a/t/t7104-reset-hard.sh\n+++ b/t/t7104-reset-hard.sh\n@@ -44,4 +44,15 @@ test_expect_success 'reset --hard did not corrupt index or cache-tree' '\n \n '\n \n+test_expect_failure CASE_INSENSITIVE_FS 'reset --hard handles index-only case-insensitive duplicate' '\n+\ttest_commit \"initial\" file1 \"initial commit with file1\" initial &&\n+\tfile1blob=$(git rev-parse :file1) &&\n+\tgit update-index --add --cacheinfo 100644,$file1blob,File1 &&\n+\n+\t# reset --hard accidentally leaves the working tree with a deleted file.\n+\tgit reset --hard &&\n+\tgit status --porcelain -uno >wt_changes_remaining &&\n+\ttest_must_be_empty wt_changes_remaining\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"477767","messageId":"8ad60943c6670a041e854f574aae98f8ddb38c70.1685267999.git.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":"pull.1257.v3.git.1685267999.gitgitgadget@gmail.com","subject":"[PATCH v3 1/3] t4142: test \"git apply\" with core.ignorecase","fromName":"Junio C Hamano via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-05-28T09:59:57Z","receivedAt":"2023-05-28T10:00:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Junio C Hamano \n\nSigned-off-by: Junio C Hamano \n---\n t/t4142-apply-icase.sh | 128 +++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 128 insertions(+)\n create mode 100755 t/t4142-apply-icase.sh\n\ndiff --git a/t/t4142-apply-icase.sh b/t/t4142-apply-icase.sh\nnew file mode 100755\nindex 00000000000..17eb023a437\n--- /dev/null\n+++ b/t/t4142-apply-icase.sh\n@@ -0,0 +1,128 @@\n+#!/bin/sh\n+\n+test_description='git apply with core.ignorecase'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+ # initial commit has file0 only\n+ test_commit \"initial\" file0 \"initial commit with file0\" initial &&\n+\n+ # current commit has file1 as well\n+ test_commit \"current\" file1 \"initial content of file1\" current &&\n+ file0blob=$(git rev-parse :file0) &&\n+ file1blob=$(git rev-parse :file1) &&\n+\n+ # prepare sample patches\n+ # file0 is modified\n+ echo modification to file0 >file0 &&\n+ git add file0 &&\n+ modifiedfile0blob=$(git rev-parse :file0) &&\n+\n+ # file1 is removed and then ...\n+ git rm --cached file1 &&\n+ # ... identical copies are placed at File1 and file2\n+ git update-index --add --cacheinfo 100644,$file1blob,file2 &&\n+ git update-index --add --cacheinfo 100644,$file1blob,File1 &&\n+\n+ # then various patches to do basic things\n+ git diff HEAD^ HEAD -- file1 >creation-patch &&\n+ git diff HEAD HEAD^ -- file1 >deletion-patch &&\n+ git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&\n+ git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&\n+ git diff --cached HEAD -- file0 >modify-file0-patch\n+'\n+\n+# Basic creation, deletion, modification and renaming.\n+test_expect_success 'creation and deletion' '\n+ # start at \"initial\" with file0 only\n+ git reset --hard initial &&\n+\n+ # add file1\n+ git -c core.ignorecase=false apply --cached creation-patch &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+\n+ # remove file1\n+ git -c core.ignorecase=false apply --cached deletion-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --cached creation-patch &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ git -c core.ignorecase=true apply --cached deletion-patch &&\n+ test_must_fail git rev-parse --verify :file1\n+'\n+\n+test_expect_success 'modificaiton' '\n+ # start at \"initial\" with file0 only\n+ git reset --hard initial &&\n+\n+ # modify file0\n+ git -c core.ignorecase=false apply --cached modify-file0-patch &&\n+ test_cmp_rev :file0 \"$modifiedfile0blob\" &&\n+ git -c core.ignorecase=false apply --cached -R modify-file0-patch &&\n+ test_cmp_rev :file0 \"$file0blob\" &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --cached modify-file0-patch &&\n+ test_cmp_rev :file0 \"$modifiedfile0blob\" &&\n+ git -c core.ignorecase=true apply --cached -R modify-file0-patch &&\n+ test_cmp_rev :file0 \"$file0blob\"\n+'\n+\n+test_expect_success 'rename file1 to file2' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # rename file1 to file2\n+ git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :file2 \"$file1blob\" &&\n+ git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file2 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :file2 \"$file1blob\" &&\n+ git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file2 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success 'rename file1 to file2' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # rename file1 to File1\n+ git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :File1 \"$file1blob\" &&\n+ git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :File1 \"$file1blob\" &&\n+ git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+# We may want to add tests with working tree here, without \"--cached\" and\n+# with and without \"--index\" here. For example, should modify-file0-patch\n+# apply cleanly if we have File0 with $file0blob in the index and the working\n+# tree if core.icase is set?\n+\n+test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '\n+ : sample\n+'\n+\n+test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '\n+ : sample\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"477768","messageId":"52359738532ab89c4612e4b83f08ab50fa169dd4.1685267999.git.gitgitgadget@gmail.com","threadId":"57986","inReplyTo":"pull.1257.v3.git.1685267999.gitgitgadget@gmail.com","subject":"[PATCH v3 3/3] apply: support case-only renames in case-insensitive filesystems","fromName":"Tao Klerks via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-05-28T09:59:59Z","receivedAt":"2023-05-28T10:00:14Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"From: Tao Klerks \n\n\"git apply\" checks, when validating a patch, to ensure that any files\nbeing added aren't already in the worktree.\n\nWhen this check runs on a case-only rename, in a case-insensitive\nfilesystem, this leads to a false positive - the command fails with an\nerror like:\nerror: File1: already exists in working directory\n\nThere is a mechanism to ensure that \"seemingly conflicting\" files are\nhandled correctly - for example overlapping rename pairs or swaps -\nthis mechanism treats renames as add/remove pairs, and would end up\ntreating a case-only rename as a \"self-swap\"... Except it does not\naccount for case-insensitive filesystems yet.\n\nBecause the index is inherently case-sensitive even on a\ncase-insensitive filesystem, we actually need this mechanism to\nhandle both requirements, lest we fail to account for conflicting\nfiles only in the index.\n\nFix the \"rename chain\" existence exemption mechanism to account for\ncase-insensitive config, fixing case-only-rename-handling as a\n\"self-swap\" and also fixing less-common \"case-insensitive rename\npairs\" when config core.ignorecase is set, but keep the index checks\nfile-sensitive.\n\nAlso add test cases around these behaviors - verifying that conflicting\nfile conditions are still caught correctly, including case-only\nconflicts on case-sensitive filesystems, and edge cases around\ncase-sensitive index behaviors on a case-insensitive filesystem.\n\nSigned-off-by: Tao Klerks \n---\n apply.c | 81 ++++++++++++++++------\n apply.h | 5 +-\n t/t4142-apply-icase.sh | 154 +++++++++++++++++++++++++++++++++++++----\n 3 files changed, 207 insertions(+), 33 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 6212ab3a1b3..a2e2f6b531d 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -113,7 +113,9 @@ int init_apply_state(struct apply_state *state,\n \tstate->ws_error_action = warn_on_ws_error;\n \tstate->ws_ignore_action = ignore_ws_none;\n \tstate->linenr = 1;\n-\tstring_list_init_nodup(&state->fn_table);\n+\tstring_list_init_nodup(&state->fs_fn_table);\n+\tstate->fs_fn_table.cmp = fspathcmp;\n+\tstring_list_init_nodup(&state->index_fn_table);\n \tstring_list_init_nodup(&state->limit_by_name);\n \tstrset_init(&state->removed_symlinks);\n \tstrset_init(&state->kept_symlinks);\n@@ -134,7 +136,10 @@ void clear_apply_state(struct apply_state *state)\n \tstrset_clear(&state->kept_symlinks);\n \tstrbuf_release(&state->root);\n \n-\t/* &state->fn_table is cleared at the end of apply_patch() */\n+\t/*\n+\t * &state->fs_fn_table and &state->index_fn_table are cleared at the\n+\t * end of apply_patch()\n+\t */\n }\n \n static void mute_routine(const char *msg UNUSED, va_list params UNUSED)\n@@ -3294,14 +3299,28 @@ static int read_file_or_gitlink(const struct cache_entry *ce, struct strbuf *buf\n \treturn read_blob_object(buf, &ce->oid, ce->ce_mode);\n }\n \n-static struct patch *in_fn_table(struct apply_state *state, const char *name)\n+static struct patch *in_fs_fn_table(struct apply_state *state, const char *name)\n {\n \tstruct string_list_item *item;\n \n \tif (!name)\n \t\treturn NULL;\n \n-\titem = string_list_lookup(&state->fn_table, name);\n+\titem = string_list_lookup(&state->fs_fn_table, name);\n+\tif (item)\n+\t\treturn (struct patch *)item->util;\n+\n+\treturn NULL;\n+}\n+\n+static struct patch *in_index_fn_table(struct apply_state *state, const char *name)\n+{\n+\tstruct string_list_item *item;\n+\n+\tif (!name)\n+\t\treturn NULL;\n+\n+\titem = string_list_lookup(&state->index_fn_table, name);\n \tif (item)\n \t\treturn (struct patch *)item->util;\n \n@@ -3333,7 +3352,7 @@ static int was_deleted(struct patch *patch)\n \treturn patch == PATH_WAS_DELETED;\n }\n \n-static void add_to_fn_table(struct apply_state *state, struct patch *patch)\n+static void add_to_fn_tables(struct apply_state *state, struct patch *patch)\n {\n \tstruct string_list_item *item;\n \n@@ -3343,7 +3362,9 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)\n \t * file creations and copies\n \t */\n \tif (patch->new_name) {\n-\t\titem = string_list_insert(&state->fn_table, patch->new_name);\n+\t\titem = string_list_insert(&state->fs_fn_table, patch->new_name);\n+\t\titem->util = patch;\n+\t\titem = string_list_insert(&state->index_fn_table, patch->new_name);\n \t\titem->util = patch;\n \t}\n \n@@ -3352,7 +3373,9 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)\n \t * later chunks shouldn't patch old names\n \t */\n \tif ((patch->new_name == NULL) || (patch->is_rename)) {\n-\t\titem = string_list_insert(&state->fn_table, patch->old_name);\n+\t\titem = string_list_insert(&state->fs_fn_table, patch->old_name);\n+\t\titem->util = PATH_WAS_DELETED;\n+\t\titem = string_list_insert(&state->index_fn_table, patch->old_name);\n \t\titem->util = PATH_WAS_DELETED;\n \t}\n }\n@@ -3365,7 +3388,9 @@ static void prepare_fn_table(struct apply_state *state, struct patch *patch)\n \twhile (patch) {\n \t\tif ((patch->new_name == NULL) || (patch->is_rename)) {\n \t\t\tstruct string_list_item *item;\n-\t\t\titem = string_list_insert(&state->fn_table, patch->old_name);\n+\t\t\titem = string_list_insert(&state->fs_fn_table, patch->old_name);\n+\t\t\titem->util = PATH_TO_BE_DELETED;\n+\t\t\titem = string_list_insert(&state->index_fn_table, patch->old_name);\n \t\t\titem->util = PATH_TO_BE_DELETED;\n \t\t}\n \t\tpatch = patch->next;\n@@ -3395,7 +3420,7 @@ static struct patch *previous_patch(struct apply_state *state,\n \tif (patch->is_copy || patch->is_rename)\n \t\treturn NULL; /* \"git\" patches do not depend on the order */\n \n-\tprevious = in_fn_table(state, patch->old_name);\n+\tprevious = in_index_fn_table(state, patch->old_name);\n \tif (!previous)\n \t\treturn NULL;\n \n@@ -3706,7 +3731,7 @@ static int apply_data(struct apply_state *state, struct patch *patch,\n \t}\n \tpatch->result = image.buf;\n \tpatch->resultsize = image.len;\n-\tadd_to_fn_table(state, patch);\n+\tadd_to_fn_tables(state, patch);\n \tfree(image.line_allocated);\n \n \tif (0 < patch->is_delete && patch->resultsize)\n@@ -3805,11 +3830,12 @@ static int check_preimage(struct apply_state *state,\n \n static int check_to_create(struct apply_state *state,\n \t\t\t const char *new_name,\n-\t\t\t int ok_if_exists)\n+\t\t\t int ok_if_exists_in_fs,\n+\t\t\t int ok_if_exists_in_index)\n {\n \tstruct stat nst;\n \n-\tif (state->check_index && (!ok_if_exists || !state->cached)) {\n+\tif (state->check_index && (!ok_if_exists_in_index || !state->cached)) {\n \t\tint pos;\n \n \t\tpos = index_name_pos(state->repo->index, new_name, strlen(new_name));\n@@ -3817,7 +3843,7 @@ static int check_to_create(struct apply_state *state,\n \t\t\tstruct cache_entry *ce = state->repo->index->cache[pos];\n \n \t\t\t/* allow ITA, as they do not yet exist in the index */\n-\t\t\tif (!ok_if_exists && !(ce->ce_flags & CE_INTENT_TO_ADD))\n+\t\t\tif (!ok_if_exists_in_index && !(ce->ce_flags & CE_INTENT_TO_ADD))\n \t\t\t\treturn EXISTS_IN_INDEX;\n \n \t\t\t/* ITA entries can never match working tree files */\n@@ -3830,7 +3856,7 @@ static int check_to_create(struct apply_state *state,\n \t\treturn 0;\n \n \tif (!lstat(new_name, &nst)) {\n-\t\tif (S_ISDIR(nst.st_mode) || ok_if_exists)\n+\t\tif (S_ISDIR(nst.st_mode) || ok_if_exists_in_fs)\n \t\t\treturn 0;\n \t\t/*\n \t\t * A leading component of new_name might be a symlink\n@@ -3940,7 +3966,8 @@ static int check_patch(struct apply_state *state, struct patch *patch)\n \tconst char *name = old_name ? old_name : new_name;\n \tstruct cache_entry *ce = NULL;\n \tstruct patch *tpatch;\n-\tint ok_if_exists;\n+\tint ok_if_exists_in_fs;\n+\tint ok_if_exists_in_index;\n \tint status;\n \n \tpatch->rejected = 1; /* we will drop this after we succeed */\n@@ -3963,16 +3990,29 @@ static int check_patch(struct apply_state *state, struct patch *patch)\n \t * B; ask to_be_deleted() about the later rename. Removal of\n \t * B and rename from A to B is handled the same way by asking\n \t * was_deleted().\n+\t *\n+\t * These exemptions account for the core.ignorecase config -\n+\t * a file that differs only by case is also considered \"deleted\"\n+\t * if git is configured to ignore case. This means a case-only\n+\t * rename, in a case-insensitive filesystem, is treated here as\n+\t * a \"self-swap\" or mode change.\n \t */\n-\tif ((tpatch = in_fn_table(state, new_name)) &&\n+\tif ((tpatch = in_fs_fn_table(state, new_name)) &&\n+\t (was_deleted(tpatch) || to_be_deleted(tpatch)))\n+\t\tok_if_exists_in_fs = 1;\n+\telse\n+\t\tok_if_exists_in_fs = 0;\n+\n+\tif ((tpatch = in_index_fn_table(state, new_name)) &&\n \t (was_deleted(tpatch) || to_be_deleted(tpatch)))\n-\t\tok_if_exists = 1;\n+\t\tok_if_exists_in_index = 1;\n \telse\n-\t\tok_if_exists = 0;\n+\t\tok_if_exists_in_index = 0;\n \n \tif (new_name &&\n \t ((0 < patch->is_new) || patch->is_rename || patch->is_copy)) {\n-\t\tint err = check_to_create(state, new_name, ok_if_exists);\n+\t\tint err = check_to_create(state, new_name, ok_if_exists_in_fs,\n+\t\t\t\t\t ok_if_exists_in_index);\n \n \t\tif (err && state->threeway) {\n \t\t\tpatch->direct_to_threeway = 1;\n@@ -4870,7 +4910,8 @@ static int apply_patch(struct apply_state *state,\n end:\n \tfree_patch_list(list);\n \tstrbuf_release(&buf);\n-\tstring_list_clear(&state->fn_table, 0);\n+\tstring_list_clear(&state->fs_fn_table, 0);\n+\tstring_list_clear(&state->index_fn_table, 0);\n \treturn res;\n }\n \ndiff --git a/apply.h b/apply.h\nindex 7cd38b1443c..a1419672507 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -95,8 +95,11 @@ struct apply_state {\n \t/*\n \t * Records filenames that have been touched, in order to handle\n \t * the case where more than one patches touch the same file.\n+\t * Two separate structures because with ignorecase, one of them\n+\t * needs to be case-insensitive and the other not.\n \t */\n-\tstruct string_list fn_table;\n+\tstruct string_list fs_fn_table;\n+\tstruct string_list index_fn_table;\n \n \t/*\n \t * This is to save reporting routines before using\ndiff --git a/t/t4142-apply-icase.sh b/t/t4142-apply-icase.sh\nindex 17eb023a437..1c785133d16 100755\n--- a/t/t4142-apply-icase.sh\n+++ b/t/t4142-apply-icase.sh\n@@ -30,7 +30,16 @@ test_expect_success setup '\n git diff HEAD HEAD^ -- file1 >deletion-patch &&\n git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&\n git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&\n- git diff --cached HEAD -- file0 >modify-file0-patch\n+ git diff --cached HEAD -- file0 >modify-file0-patch &&\n+\n+ # then set up for swap\n+ git reset --hard current &&\n+ test_commit \"swappable\" file3 \"different content for file3\" swappable &&\n+ file3blob=$(git rev-parse :file3) &&\n+ git rm --cached file1 file3 &&\n+ git update-index --add --cacheinfo 100644,$file1blob,File3 &&\n+ git update-index --add --cacheinfo 100644,$file3blob,File1 &&\n+ git diff --cached HEAD -- file1 file3 File1 File3 >swap-file1-and-file3-to-File3-and-File1-patch\n '\n \n # Basic creation, deletion, modification and renaming.\n@@ -53,7 +62,7 @@ test_expect_success 'creation and deletion' '\n test_must_fail git rev-parse --verify :file1\n '\n \n-test_expect_success 'modificaiton' '\n+test_expect_success 'modification (index-only)' '\n # start at \"initial\" with file0 only\n git reset --hard initial &&\n \n@@ -70,7 +79,7 @@ test_expect_success 'modificaiton' '\n test_cmp_rev :file0 \"$file0blob\"\n '\n \n-test_expect_success 'rename file1 to file2' '\n+test_expect_success 'rename file1 to file2 (index-only)' '\n # start from file0 and file1\n git reset --hard current &&\n \n@@ -91,7 +100,7 @@ test_expect_success 'rename file1 to file2' '\n test_cmp_rev :file1 \"$file1blob\"\n '\n \n-test_expect_success 'rename file1 to file2' '\n+test_expect_success 'rename file1 to File1 (index-only)' '\n # start from file0 and file1\n git reset --hard current &&\n \n@@ -112,17 +121,138 @@ test_expect_success 'rename file1 to file2' '\n test_cmp_rev :file1 \"$file1blob\"\n '\n \n-# We may want to add tests with working tree here, without \"--cached\" and\n-# with and without \"--index\" here. For example, should modify-file0-patch\n-# apply cleanly if we have File0 with $file0blob in the index and the working\n-# tree if core.icase is set?\n+# involve filesystem on renames\n+test_expect_success 'rename file1 to File1 (with ignorecase, working tree)' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --index rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :File1 \"$file1blob\" &&\n+ git -c core.ignorecase=true apply --index -R rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success CASE_INSENSITIVE_FS 'rename file1 to File1 (without ignorecase, case-insensitive FS)' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # rename file1 to File1 without ignorecase (fails as expected)\n+ test_must_fail git -c core.ignorecase=false apply --index rename-file1-to-File1-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success !CASE_INSENSITIVE_FS 'rename file1 to File1 (without ignorecase, case-sensitive FS)' '\n+ # start from file0 and file1\n+ git reset --hard current &&\n+\n+ # rename file1 to File1 without ignorecase\n+ git -c core.ignorecase=false apply --index rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :File1 \"$file1blob\" &&\n+ git -c core.ignorecase=false apply --index -R rename-file1-to-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n+'\n+\n+test_expect_success 'rename file1 to file2 with working tree conflict' '\n+ # start from file0 and file1, and file2 untracked\n+ git reset --hard current &&\n+ test_when_finished \"rm file2\" &&\n+ touch file2 &&\n+\n+ # rename file1 to file2 with conflict\n+ test_must_fail git -c core.ignorecase=false apply --index rename-file1-to-file2-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n \n-test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '\n- : sample\n+ # do the same with ignorecase\n+ test_must_fail git -c core.ignorecase=true apply --index rename-file1-to-file2-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\"\n '\n \n-test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '\n- : sample\n+test_expect_success 'rename file1 to file2 with case-insensitive conflict (index-only - ignorecase disabled)' '\n+ # start from file0 and file1, and File2 in index\n+ git reset --hard current &&\n+ git update-index --add --cacheinfo 100644,$file3blob,File2 &&\n+\n+ # rename file1 to file2 without ignorecase\n+ git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_cmp_rev :file2 \"$file1blob\" &&\n+ git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&\n+ test_must_fail git rev-parse --verify :file2 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :File2 \"$file3blob\"\n+'\n+\n+test_expect_failure 'rename file1 to file2 with case-insensitive conflict (index-only - ignorecase enabled)' '\n+ # start from file0 and file1, and File2 in index\n+ git reset --hard current &&\n+ git update-index --add --cacheinfo 100644,$file3blob,File2 &&\n+\n+ # rename file1 to file2 with ignorecase, with a \"File2\" conflicting file in place - expect failure.\n+ # instead of failure, we get success with \"File1\" and \"file1\" both existing in the index, despite\n+ # the ignorecase configuration.\n+ test_must_fail git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :File2 \"$file3blob\"\n+'\n+\n+test_expect_success 'rename file1 to File1 with case-sensitive conflict (index-only)' '\n+ # start from file0 and file1, and File1 in index\n+ git reset --hard current &&\n+ git update-index --add --cacheinfo 100644,$file3blob,File1 &&\n+\n+ # On a case-insensitive filesystem with core.ignorecase on, a single git\n+ # \"reset --hard\" will actually leave things wrong because of the\n+ # index-to-working-tree discrepancy - see \"reset --hard handles\n+ # index-only case-insensitive duplicate\" under t7104-reset-hard.sh.\n+ # We are creating this unexpected state, so we should explicitly queue\n+ # an extra reset. If reset ever starts to handle this case, this will\n+ # become unnecessary but also not harmful.\n+ test_when_finished \"git reset --hard\" &&\n+\n+ # rename file1 to File1 when File1 is already in index (fails with conflict)\n+ test_must_fail git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :File1 \"$file3blob\" &&\n+\n+ # do the same with ignorecase\n+ test_must_fail git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&\n+ git rev-parse --verify :file1 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :File1 \"$file3blob\"\n+'\n+\n+test_expect_success CASE_INSENSITIVE_FS 'case-insensitive swap - file1 to File2 and file2 to File1 (working tree)' '\n+ # start from file0, file1, and file3\n+ git reset --hard swappable &&\n+\n+ # \"swap\" file1 and file3 to case-insensitive versions without ignorecase on case-insensitive FS (fails as expected)\n+ test_must_fail git -c core.ignorecase=false apply --index swap-file1-and-file3-to-File3-and-File1-patch &&\n+ git rev-parse --verify :file1 &&\n+ git rev-parse --verify :file3 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :file3 \"$file3blob\" &&\n+\n+ # do the same with ignorecase\n+ git -c core.ignorecase=true apply --index swap-file1-and-file3-to-File3-and-File1-patch &&\n+ test_must_fail git rev-parse --verify :file1 &&\n+ test_must_fail git rev-parse --verify :file3 &&\n+ test_cmp_rev :File3 \"$file1blob\" &&\n+ test_cmp_rev :File1 \"$file3blob\" &&\n+ git -c core.ignorecase=true apply --index -R swap-file1-and-file3-to-File3-and-File1-patch &&\n+ test_must_fail git rev-parse --verify :File1 &&\n+ test_must_fail git rev-parse --verify :File3 &&\n+ test_cmp_rev :file1 \"$file1blob\" &&\n+ test_cmp_rev :file3 \"$file3blob\"\n '\n \n test_done\n-- \ngitgitgadget\n"}]}