[PATCH] apply: support case-only renames in case-insensitive filesystems

[PATCH] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks via GitGitGadget]

From: Tao Klerks <tao@klerks.biz>

"git apply" checks, when validating a patch, to ensure that any files
being added aren't already in the worktree.

When this check runs on a case-only rename, in a case-insensitive
filesystem, this leads to a false positive - the command fails with an
error like:
error: File1: already exists in working directory

Fix this existence check to allow the file to exist, for a case-only
rename when config core.ignorecase is set.

Also add a test for this case, while verifying that conflicting file
conditions are still caught correctly, including case-only conflicts on
case-sensitive filesystems.

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
    apply: support case-only renames in case-insensitive filesystems
    
    As suggested recently in thread
    CAPMMpojwV+f=z9sgc_GaUOTFBCUVdbrGW8WjatWWmC3WTcsoXw@mail.gmail.com,
    proposing a fix to git-apply for case-only renames on case-insensitive
    filesystems.
    
    Also including tests to check both the corrected behavior and the
    corresponding legitimate errors.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1257%2FTaoK%2Ftao-apply-case-insensitive-renames-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1257/TaoK/tao-apply-case-insensitive-renames-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1257

 apply.c                                  |  2 +
 t/t4141-apply-case-insensitive-rename.sh | 50 ++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100755 t/t4141-apply-case-insensitive-rename.sh

diff --git a/apply.c b/apply.c
index 2b7cd930efa..ccba7f90393 100644
--- a/apply.c
+++ b/apply.c
@@ -3942,6 +3942,8 @@ static int check_patch(struct apply_state *state, struct patch *patch)
 	if ((tpatch = in_fn_table(state, new_name)) &&
 	    (was_deleted(tpatch) || to_be_deleted(tpatch)))
 		ok_if_exists = 1;
+	else if (ignore_case && !strcasecmp(old_name, new_name))
+		ok_if_exists = 1;
 	else
 		ok_if_exists = 0;
 
diff --git a/t/t4141-apply-case-insensitive-rename.sh b/t/t4141-apply-case-insensitive-rename.sh
new file mode 100755
index 00000000000..6b394252ff8
--- /dev/null
+++ b/t/t4141-apply-case-insensitive-rename.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='git apply should handle case-only renames on case-insensitive filesystems'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# Please note, this test assumes that core.ignorecase is set appropriately for the filesystem,
+# as tested in t0050. Case-only rename conflicts are only tested in case-sensitive filesystems.
+
+if ! test_have_prereq CASE_INSENSITIVE_FS
+then
+	test_set_prereq CASE_SENSITIVE_FS
+	echo nuts
+fi
+
+test_expect_success setup '
+	echo "This is some content in the file." > file1 &&
+	echo "A completely different file." > file2 &&
+	git update-index --add file1 &&
+	git update-index --add file2 &&
+	cat >case_only_rename_patch <<-\EOF
+	diff --git a/file1 b/File1
+	similarity index 100%
+	rename from file1
+	rename to File1
+	EOF
+'
+
+test_expect_success 'refuse to apply rename patch with conflict' '
+	cat >conflict_patch <<-\EOF &&
+	diff --git a/file1 b/file2
+	similarity index 100%
+	rename from file1
+	rename to file2
+	EOF
+	test_must_fail git apply --index conflict_patch
+'
+
+test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '
+	test_when_finished "git mv File1 file2" &&
+	git mv file2 File1 &&
+	test_must_fail git apply --index case_only_rename_patch
+'
+
+test_expect_success 'apply case-only rename patch without conflict' '
+	git apply --index case_only_rename_patch
+'
+
+test_done

base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64
-- 
gitgitgadget

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Junio C Hamano]

"Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Tao Klerks <tao@klerks.biz>
>
> "git apply" checks, when validating a patch, to ensure that any files
> being added aren't already in the worktree.
>
> When this check runs on a case-only rename, in a case-insensitive
> filesystem, this leads to a false positive - the command fails with an
> error like:
> error: File1: already exists in working directory
>
> Fix this existence check to allow the file to exist, for a case-only
> rename when config core.ignorecase is set.

Hmph, close, but the patch as-posted may be fatally buggy, I think.

At the beginning of the function there is this block:

	const char *old_name = patch->old_name;
	const char *new_name = patch->new_name;
	const char *name = old_name ? old_name : new_name;

which makes us realize that old_name CAN legitimately be NULL.  That
is true for a creation patch.  new_name can also be NULL for a
deletion patch.

>  	if ((tpatch = in_fn_table(state, new_name)) &&
>  	    (was_deleted(tpatch) || to_be_deleted(tpatch)))
>  		ok_if_exists = 1;
> +	else if (ignore_case && !strcasecmp(old_name, new_name))
> +		ok_if_exists = 1;

You'd get a segfault when the patch is creating a file at new_name,
or deleting a file at old_name, wouldn't you?

We need a new test or two to see if a straight creation or deletion
patch does work correctly with icase set, before we even dream of
handling rename patches.  Not having tests for such basic cases is
quite surprising, but apparently the above line passed the CI.

>  	else
>  		ok_if_exists = 0;

Having said that, I wonder what the existing check before the new
condition is doing?  Especially, what is in_fn_table() for and how
does it try to do its work?

Reading the big comment before it, it seems that it tries to deal
with tricky delete/create case already.  With a typechange patch
that first removes a regular file "hello.txt" and then creates a
symbolic link "hello.txt" is exempted from the "what you are
creating should not exist already" rule by using in_fn_table()
check.  If it tries to create a symlink "Hello.txt" instead,
shouldn't we allow it the same way on case-insensitive systems?  I
do not think in_fn_table() pays attention to "ignore_case" option,
so there may be an existing bug there already, regardless of the
problem you are trying to address with your patch.

And I wonder if doing case-insensitive match in in_fn_table() lets
us cover this new case as well as "fixing" the existing issue.

In any case, here are such two tests to make sure creation and
deletion patches on icase systems are not broken by careless
mistakes like the one in this patch.

 t/t4114-apply-typechange.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git c/t/t4114-apply-typechange.sh w/t/t4114-apply-typechange.sh
index da3e64f811..e565ad8da1 100755
--- c/t/t4114-apply-typechange.sh
+++ w/t/t4114-apply-typechange.sh
@@ -126,4 +126,44 @@ test_expect_success 'directory becomes symlink' '
 test_debug 'cat patch'
 
 
+test_expect_success 'file becomes nothing' '
+	git checkout -f initial &&
+	test_when_finished "git reset --hard HEAD" &&
+
+	# prepare a patch to remove path "foo"
+	git rm --cached foo &&
+	git diff-index -p --cached HEAD >patch &&
+
+	# such a patch should apply cleanly to the index
+	git reset HEAD &&
+	git apply --cached patch &&
+
+	# and even with icase set.
+	git reset HEAD &&
+	git -c core.ignorecase=true apply --cached patch
+'
+
+test_debug 'cat patch'
+
+test_expect_success 'nothing becomes a file' '
+	git checkout -f initial &&
+	test_when_finished "git reset --hard HEAD" &&
+
+	# prepare a patch to add path "foo"
+	git rm --cached foo &&
+	git diff-index -p --cached -R HEAD >patch &&
+
+	# such a patch should apply cleanly to the index without "foo"
+	git reset HEAD &&
+	git rm --cached foo &&
+	git apply --cached patch &&
+
+	# and even with icase set.
+	git reset HEAD &&
+	git rm --cached foo &&
+	git -c core.ignorecase=true apply --cached patch
+'
+
+test_debug 'cat patch'
+
 test_done

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Junio C Hamano]

"Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +if ! test_have_prereq CASE_INSENSITIVE_FS
> +then
> +	test_set_prereq CASE_SENSITIVE_FS
> +	echo nuts
> +fi

You can easily say !CASE_INSENSITIVE_FS as the prerequiste, so I do
not see the point of this.  I do not see the point of "nuts", either.

But it probably is a moot point as I do not think you should do the
prerequisite at all.

Instead, you can explicitly set the core.ignorecase configuration,
i.e. "git -c core.ignorecase=yes/no", and possibly "apply --cached"
so that you do not have to worry about the case sensitivity of the
filesystem at all.

> +test_expect_success setup '
> +	echo "This is some content in the file." > file1 &&

Style.  Redirection operator ">" sticks to its operand, i.e.

	echo "This is some content in the file." >file1 &&

> +	echo "A completely different file." > file2 &&
> +	git update-index --add file1 &&
> +	git update-index --add file2 &&
> +	cat >case_only_rename_patch <<-\EOF
> +	diff --git a/file1 b/File1
> +	similarity index 100%
> +	rename from file1
> +	rename to File1
> +	EOF

You are better off not writing the diff output manually.  Instead,
you can let the test write it for you, e.g.

	echo "This is some content in the file." >file1 &&
	git update-index --add file1 &&
        file1blob=$(git rev-parse :file1) &&
	git commit -m "Initial - file1" &&
	git update-index --add --cacheinfo 100644,$file1blob,File1 &&
	git rm --cached file1 &&
	git diff --cached -M HEAD >case-only-rename-patch

If you want to be extra careful not to rely on your filesystem
corrupting the pathnames you feed (e.g. the redireciton to "file1"
might create file FILE1 on MS-DOS ;-), you could even do:

	file1blob=$(echo "This is some content in the file." |
		    git hash-object -w --stdin) &&
	file2blob=$(echo "A completeloy different contents." |
		    git hash-object -w --stdin) &&
	git update-index --add --cacheinfo 100644,$file1blob,file1 &&

	git commit -m "Initial - file1" &&
	git update-index --add --cacheinfo 100644,$file1blob,File1 &&
	git rm --cached file1 &&
	git diff --cached -M HEAD >rename-file1-to-File2 &&

	git reset --hard HEAD &&
        git update-index --add --cacheinfo 100644,$file1blob,file2 &&
	git rm --cached file1 &&
	git diff --cached -M HEAD >rename-file1-to-file2 &&

	# from here on, HEAD has file1 and file2
	git reset --hard HEAD &&
	git update-index --add --cacheinfo 100644,$file2blob,file2 &&
	git commit -m 'file1 and file2'

> +'
> +
> +test_expect_success 'refuse to apply rename patch with conflict' '
> +	cat >conflict_patch <<-\EOF &&
> +	diff --git a/file1 b/file2
> +	similarity index 100%
> +	rename from file1
> +	rename to file2
> +	EOF
> +	test_must_fail git apply --index conflict_patch

And then, you could use --cached (not --index) to bypass the working
tree altogether, which is a good way to test the feature without
getting affected by the underlying filesystem.  Check both case
sensitive and case insensitive cases:

	# Start from a known state
	git reset --hard HEAD &&
	test_must_fail git -c core.ignorecase=no apply --cached rename-file1-to-file2 &&

	# Start from a known state
	git reset --hard HEAD &&
	test_must_fail git -c core.ignorecase=yes apply --cached rename-file1-to-file2 &&

> +'
> +
> +test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '

Lose the prerequisite, replace --index with --cached, and force core.ignorecase
to both case insensitive and sensitive to check the behaviour.

> +	test_when_finished "git mv File1 file2" &&
> +	git mv file2 File1 &&
> +	test_must_fail git apply --index case_only_rename_patch
> +'
> +
> +test_expect_success 'apply case-only rename patch without conflict' '

Likewise, try both sensitive and insensitive one.

> +	git apply --index case_only_rename_patch
> +'
> +
> +test_done
>
> base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64

Thanks.

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> ...  I
> do not think in_fn_table() pays attention to "ignore_case" option,
> so there may be an existing bug there already, regardless of the
> problem you are trying to address with your patch.
>
> And I wonder if doing case-insensitive match in in_fn_table() lets
> us cover this new case as well as "fixing" the existing issue.

While I still think in_fn_table() should be looked into for an
existing case sensitivity bug, I think this one is different enough
that in_fn_table() logic wouild not trigger for it, and a patch to
add an extra piece of logic for renames is probably needed.

It might be sufficient to tighten the condition so that it triggers
only to the case you wanted to handle, i.e. a rename between the
same name.

	else if (ignore_case && old_name && new_name &&
		 !strcasecmp(old_name, new_name))

(the "both names must be non-NULL" check is new).

Thanks.

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> And then, you could use --cached (not --index) to bypass the working
> tree altogether, which is a good way to test the feature without
> getting affected by the underlying filesystem.  Check both case
> sensitive and case insensitive cases:
> ...
> Likewise, try both sensitive and insensitive one.

As I already wrote tests for basic cases, I'm sending them out,

so that you may extend them with your new cases so that new code you
write can be checked.

Thanks.

----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 13 Jun 2022 11:05:54 -0700
Subject: [PATCH] t4141: test "git apply" with core.ignorecase

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4141-apply-icase.sh | 128 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100755 t/t4141-apply-icase.sh

diff --git a/t/t4141-apply-icase.sh b/t/t4141-apply-icase.sh
new file mode 100755
index 0000000000..9b70ff82c3
--- /dev/null
+++ b/t/t4141-apply-icase.sh
@@ -0,0 +1,128 @@
+#!/bin/sh
+
+test_description='git apply with core.ignorecase'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	# initial commit has file0 only
+	test_commit "initial" file0 "initial commit with file0" initial &&
+
+	# current commit has file1 as well
+	test_commit "current" file1 "initial content of file1" current &&
+	file0blob=$(git rev-parse :file0) &&
+	file1blob=$(git rev-parse :file1) &&
+
+	# prepare sample patches
+	# file0 is modified
+	echo modification to file0 >file0 &&
+	git add file0 &&
+	modifiedfile0blob=$(git rev-parse :file0) &&
+
+	# file1 is removed and then ...
+	git rm --cached file1 &&
+	# ... identical copies are placed at File1 and file2
+	git update-index --add --cacheinfo 100644,$file1blob,file2 &&
+	git update-index --add --cacheinfo 100644,$file1blob,File1 &&
+
+	# then various patches to do basic things
+	git diff HEAD^ HEAD -- file1 >creation-patch &&
+	git diff HEAD HEAD^ -- file1 >deletion-patch &&
+	git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&
+	git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&
+	git diff --cached HEAD -- file0 >modify-file0-patch
+'
+
+# Basic creation, deletion, modification and renaming.
+test_expect_success 'creation and deletion' '
+	# start at "initial" with file0 only
+	git reset --hard initial &&
+
+	# add file1
+	git -c core.ignorecase=false apply --cached creation-patch &&
+	test_cmp_rev :file1 "$file1blob" &&
+
+	# remove file1
+	git -c core.ignorecase=false apply --cached deletion-patch &&
+	test_must_fail git rev-parse --verify :file1 &&
+
+	# do the same with ignorecase
+	git -c core.ignorecase=true apply --cached creation-patch &&
+	test_cmp_rev :file1 "$file1blob" &&
+	git -c core.ignorecase=true apply --cached deletion-patch &&
+	test_must_fail git rev-parse --verify :file1
+'
+
+test_expect_success 'modificaiton' '
+	# start at "initial" with file0 only
+	git reset --hard initial &&
+
+	# modify file0
+	git -c core.ignorecase=false apply --cached modify-file0-patch &&
+	test_cmp_rev :file0 "$modifiedfile0blob" &&
+	git -c core.ignorecase=false apply --cached -R modify-file0-patch &&
+	test_cmp_rev :file0 "$file0blob" &&
+
+	# do the same with ignorecase
+	git -c core.ignorecase=true apply --cached modify-file0-patch &&
+	test_cmp_rev :file0 "$modifiedfile0blob" &&
+	git -c core.ignorecase=true apply --cached -R modify-file0-patch &&
+	test_cmp_rev :file0 "$file0blob"
+'
+
+test_expect_success 'rename file1 to file2' '
+	# start from file0 and file1
+	git reset --hard current &&
+
+	# rename file1 to file2
+	git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&
+	test_must_fail git rev-parse --verify :file1 &&
+	test_cmp_rev :file2 "$file1blob" &&
+	git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&
+	test_must_fail git rev-parse --verify :file2 &&
+	test_cmp_rev :file1 "$file1blob" &&
+
+	# do the same with ignorecase
+	git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&
+	test_must_fail git rev-parse --verify :file1 &&
+	test_cmp_rev :file2 "$file1blob" &&
+	git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&
+	test_must_fail git rev-parse --verify :file2 &&
+	test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success 'rename file1 to file2' '
+	# start from file0 and file1
+	git reset --hard current &&
+
+	# rename file1 to File1
+	git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&
+	test_must_fail git rev-parse --verify :file1 &&
+	test_cmp_rev :File1 "$file1blob" &&
+	git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&
+	test_must_fail git rev-parse --verify :File1 &&
+	test_cmp_rev :file1 "$file1blob" &&
+
+	# do the same with ignorecase
+	git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&
+	test_must_fail git rev-parse --verify :file1 &&
+	test_cmp_rev :File1 "$file1blob" &&
+	git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&
+	test_must_fail git rev-parse --verify :File1 &&
+	test_cmp_rev :file1 "$file1blob"
+'
+
+# We may want to add tests with working tree here, without "--cached" and
+# with and without "--index" here.  For example, should modify-file0-patch
+# apply cleanly if we have File0 with $file0blob in the index and the working
+# tree if core.icase is set?
+
+test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '
+	: sample
+'
+
+test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '
+	: sample
+'
+
+test_done
-- 
2.36.1-513-gd2306e2395

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks]

On Sat, Jun 11, 2022 at 9:17 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Tao Klerks <tao@klerks.biz>
> >
> > "git apply" checks, when validating a patch, to ensure that any files
> > being added aren't already in the worktree.
> >
> > When this check runs on a case-only rename, in a case-insensitive
> > filesystem, this leads to a false positive - the command fails with an
> > error like:
> > error: File1: already exists in working directory
> >
> > Fix this existence check to allow the file to exist, for a case-only
> > rename when config core.ignorecase is set.
>
> Hmph, close, but the patch as-posted may be fatally buggy, I think.
>

Yes indeed, very much so.

> At the beginning of the function there is this block:
>
>         const char *old_name = patch->old_name;
>         const char *new_name = patch->new_name;
>         const char *name = old_name ? old_name : new_name;
>
> which makes us realize that old_name CAN legitimately be NULL.  That
> is true for a creation patch.  new_name can also be NULL for a
> deletion patch.
>

Yep, I was aware of the nulls, but I was unaware that passing nulls
into "strcasecmp()" was a bad thing to do. I just assumed a non-zero
comparison result would ensue.

> >       if ((tpatch = in_fn_table(state, new_name)) &&
> >           (was_deleted(tpatch) || to_be_deleted(tpatch)))
> >               ok_if_exists = 1;
> > +     else if (ignore_case && !strcasecmp(old_name, new_name))
> > +             ok_if_exists = 1;
>
> You'd get a segfault when the patch is creating a file at new_name,
> or deleting a file at old_name, wouldn't you?
>

Indeed you do (when ignorecase is true of course).

> We need a new test or two to see if a straight creation or deletion
> patch does work correctly with icase set, before we even dream of
> handling rename patches.  Not having tests for such basic cases is
> quite surprising, but apparently the above line passed the CI.
>

This is where I made some very bad assumptions: I only manually ran
the new "t4141-apply-case-insensitive-rename.sh" test, and assumed
that the test suite ran against linux, windows, and OSX, with the
latter two running on case-insensitive filesystems. I assumed that
both case-sensitive and case-insensitive code paths would be tested by
the complete CI suite.

The OSX tests were not running for me at all in GitGitGadget (seems to
be an ongoing struggle), but I assumed that everything was still
tested in case-insensitive mode because of the windows suite. It looks
like that was wrong, although I still don´t know how/why.

Had I run "t4114-apply-typechange.sh" (or probably some others in the
41XX range) on the OSX environment where I happen to have developed
this weekend, I would have seen the failures immediately.

> >       else
> >               ok_if_exists = 0;
>
> Having said that, I wonder what the existing check before the new
> condition is doing?  Especially, what is in_fn_table() for and how
> does it try to do its work?
>
> Reading the big comment before it, it seems that it tries to deal
> with tricky delete/create case already.  With a typechange patch
> that first removes a regular file "hello.txt" and then creates a
> symbolic link "hello.txt" is exempted from the "what you are
> creating should not exist already" rule by using in_fn_table()
> check.  If it tries to create a symlink "Hello.txt" instead,
> shouldn't we allow it the same way on case-insensitive systems?  I
> do not think in_fn_table() pays attention to "ignore_case" option,
> so there may be an existing bug there already, regardless of the
> problem you are trying to address with your patch.
>
> And I wonder if doing case-insensitive match in in_fn_table() lets
> us cover this new case as well as "fixing" the existing issue.
>

Yep, I confirmed that as you expect, it does fix the issue I set out
to fix, and as you noted also fixes other (slightly more obscure?)
existing issues with "git apply" on case-insensitive filesystems.

This time I tested all of t41XX on a case-insensitive system, and the
CI process ran in GitLab, presumably on case-sensitive filesystems
only.

I'm not sure what more to look out for, and will note as much in the
patch v2 comments.

> In any case, here are such two tests to make sure creation and
> deletion patches on icase systems are not broken by careless
> mistakes like the one in this patch.

I have a question related to this:

*Do* we expect to run the full test suite on case-insensitive systems
in gitlab, or do we expect to need to add explicit "-C
core.ignorecase" tests as you have done here? The latter seems risky
both because the behavior is not representatively tested (because it's
still actually running in a case-sensitive filesystem), and because
it's hard to predict all the things that should be explicitly retested
in this way.

I don't think these specific tests were necessary, and I guess they
are replaced by later ones in this thread, so I will ignore this bit
specifically.

Thanks for the careful review, my apologies for the careless patch.

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks]

On Mon, Jun 13, 2022 at 1:30 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +if ! test_have_prereq CASE_INSENSITIVE_FS
> > +then
> > +     test_set_prereq CASE_SENSITIVE_FS
> > +     echo nuts
> > +fi
>
> You can easily say !CASE_INSENSITIVE_FS as the prerequiste, so I do
> not see the point of this.  I do not see the point of "nuts", either.

I was not aware of negated prerequisite support (I did not see it in
the README nor in any examples I scanned), but I agree this is much
cleaner of course!

"nuts" was a debugging leak, my apologies.

>
> But it probably is a moot point as I do not think you should do the
> prerequisite at all.
>
> Instead, you can explicitly set the core.ignorecase configuration,
> i.e. "git -c core.ignorecase=yes/no", and possibly "apply --cached"
> so that you do not have to worry about the case sensitivity of the
> filesystem at all.

Sure, I can see how we can test most of the case-sensitive logic, even
on a case-insensitive filesystem, with "--cached" and "-c
core.ignorecase=no". I'm not sure whether there's a need to test the
same things against the actual file system or not (certainly in the
case-insensitive path there is, as this is where the errors/conflicts
actually occur).

>
> > +test_expect_success setup '
> > +     echo "This is some content in the file." > file1 &&
>
> Style.  Redirection operator ">" sticks to its operand, i.e.
>
>         echo "This is some content in the file." >file1 &&
>

Thx.

> > +     echo "A completely different file." > file2 &&
> > +     git update-index --add file1 &&
> > +     git update-index --add file2 &&
> > +     cat >case_only_rename_patch <<-\EOF
> > +     diff --git a/file1 b/File1
> > +     similarity index 100%
> > +     rename from file1
> > +     rename to File1
> > +     EOF
>
> You are better off not writing the diff output manually.  Instead,
> you can let the test write it for you, e.g.
>
>         echo "This is some content in the file." >file1 &&
>         git update-index --add file1 &&
>         file1blob=$(git rev-parse :file1) &&
>         git commit -m "Initial - file1" &&
>         git update-index --add --cacheinfo 100644,$file1blob,File1 &&
>         git rm --cached file1 &&
>         git diff --cached -M HEAD >case-only-rename-patch
>

Makes sense, thx.

> If you want to be extra careful not to rely on your filesystem
> corrupting the pathnames you feed (e.g. the redireciton to "file1"
> might create file FILE1 on MS-DOS ;-), you could even do:
>
>         file1blob=$(echo "This is some content in the file." |
>                     git hash-object -w --stdin) &&
>         file2blob=$(echo "A completeloy different contents." |
>                     git hash-object -w --stdin) &&
>         git update-index --add --cacheinfo 100644,$file1blob,file1 &&
>
>         git commit -m "Initial - file1" &&
>         git update-index --add --cacheinfo 100644,$file1blob,File1 &&
>         git rm --cached file1 &&
>         git diff --cached -M HEAD >rename-file1-to-File2 &&
>
>         git reset --hard HEAD &&
>         git update-index --add --cacheinfo 100644,$file1blob,file2 &&
>         git rm --cached file1 &&
>         git diff --cached -M HEAD >rename-file1-to-file2 &&
>
>         # from here on, HEAD has file1 and file2
>         git reset --hard HEAD &&
>         git update-index --add --cacheinfo 100644,$file2blob,file2 &&
>         git commit -m 'file1 and file2'
>

Cool, but probably excessive? (do we support MS-DOS??)

> > +'
> > +
> > +test_expect_success 'refuse to apply rename patch with conflict' '
> > +     cat >conflict_patch <<-\EOF &&
> > +     diff --git a/file1 b/file2
> > +     similarity index 100%
> > +     rename from file1
> > +     rename to file2
> > +     EOF
> > +     test_must_fail git apply --index conflict_patch
>
> And then, you could use --cached (not --index) to bypass the working
> tree altogether, which is a good way to test the feature without
> getting affected by the underlying filesystem.  Check both case
> sensitive and case insensitive cases:
>
>         # Start from a known state
>         git reset --hard HEAD &&
>         test_must_fail git -c core.ignorecase=no apply --cached rename-file1-to-file2 &&
>
>         # Start from a known state
>         git reset --hard HEAD &&
>         test_must_fail git -c core.ignorecase=yes apply --cached rename-file1-to-file2 &&
>

Makes sense, understanding that this tests "happy paths" - it doesn't
fail even if talking to the (case-insensitive) filesystem actually
would (which here it wouldn't of course).

> > +'
> > +
> > +test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '
>
> Lose the prerequisite, replace --index with --cached, and force core.ignorecase
> to both case insensitive and sensitive to check the behaviour.
>

Sure, makes sense - you can test case-sensitive behaviors in git
without needing a case-sensitive FS.

> > +     test_when_finished "git mv File1 file2" &&
> > +     git mv file2 File1 &&
> > +     test_must_fail git apply --index case_only_rename_patch
> > +'
> > +
> > +test_expect_success 'apply case-only rename patch without conflict' '
>
> Likewise, try both sensitive and insensitive one.
>

This one will fail on a case-insensitive filesystem if you disable
core.ignorecase, so explicitly trying with both settings in a single
test, without prerequisites, presumably isn't the right thing. I
assume the right thing is to have 2 versions of the same test, one
which expects success in all cases on a case-sensitive filesystem, and
one which expects failure when case-insensitivity is disabled on a
case-insensitive filesystem?

> > +     git apply --index case_only_rename_patch
> > +'
> > +
> > +test_done
> >
> > base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64
>
> Thanks.
>

Thank you!

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks]

On Mon, Jun 13, 2022 at 1:35 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > ...  I
> > do not think in_fn_table() pays attention to "ignore_case" option,
> > so there may be an existing bug there already, regardless of the
> > problem you are trying to address with your patch.
> >
> > And I wonder if doing case-insensitive match in in_fn_table() lets
> > us cover this new case as well as "fixing" the existing issue.
>
> While I still think in_fn_table() should be looked into for an
> existing case sensitivity bug, I think this one is different enough
> that in_fn_table() logic wouild not trigger for it, and a patch to
> add an extra piece of logic for renames is probably needed.
>

Having spent some time with this yesterday and today, I'm quite
confident not only that you were right about the general
case-sensitivity fix required here, but also that it fixes the
case-only rename issue. It turns it (on case-insensitive filesystems)
into a similar issue to the mode change, which is treated as a remove
and add of the same file.

As you suggested, it is possible to construct "case-differing file
swaps" which are not file swaps on a case-sensitive FS but are on a
case-insensitive one, and without a fix these fail. The same (very
small) change fixes both issues.

> It might be sufficient to tighten the condition so that it triggers
> only to the case you wanted to handle, i.e. a rename between the
> same name.
>
>         else if (ignore_case && old_name && new_name &&
>                  !strcasecmp(old_name, new_name))
>
> (the "both names must be non-NULL" check is new).
>

This was my original tack, but I think it makes more sense to make the
general fix and explain how it also handles this case.

New patch coming.

Thanks!

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks]

On Mon, Jun 13, 2022 at 8:12 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > And then, you could use --cached (not --index) to bypass the working
> > tree altogether, which is a good way to test the feature without
> > getting affected by the underlying filesystem.  Check both case
> > sensitive and case insensitive cases:
> > ...
> > Likewise, try both sensitive and insensitive one.
>
> As I already wrote tests for basic cases, I'm sending them out,
>
> so that you may extend them with your new cases so that new code you
> write can be checked.
>

Thanks! I'm not sure how to handle this procedurally, especially as
I'm using GitGitGadget, so my patches must always be anchored on a
commit found in the public mirror.

For now I'll add this as a new first commit in my patch, which becomes
my patch series I guess (but keeping your commit metadata
as-specified). Please let me know if there's a better way.

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks]

On Tue, Jun 14, 2022 at 8:22 AM Tao Klerks <tao@klerks.biz> wrote:
>
>
> New patch coming.
>

Quick update on this - while exploring test scenarios I found an
edge-case where these changes *seem* to introduce a regression. The
probably-problematic behavior is that if there is a
differing-in-case-only (case-insensitive duplicate) file (with
different content) in the index, but obviously not on the filesystem
as the case-insensitive filesystem can't allow both files to exist,
the "git apply --index" of a case-only rename from the original
filename to the "duplicate" filename will replace that duplicate file
in the index, replacing its original content. With these changes, the
rename of a file can, under very specific circumstances, cause a
"rename" patch to replace/delete unrelated data in a staged-only (and
maybe also committed) file.

I need to do more testing to understand the relationship between this
behavior and similar scenarios in, for example, git checkout.

The current patch can be found at
https://github.com/gitgitgadget/git/pull/1257, but I'm not sure when
I'll have time to investigate further, settle whether this is a
(meaningful) regression or not, think about how to address it if so,
and submit a v2 :(

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Junio C Hamano]

Tao Klerks <tao@klerks.biz> writes:

>> We need a new test or two to see if a straight creation or deletion
>> patch does work correctly with icase set, before we even dream of
>> handling rename patches.  Not having tests for such basic cases is
>> quite surprising, but apparently the above line passed the CI.
>
> This is where I made some very bad assumptions: I only manually ran
> the new "t4141-apply-case-insensitive-rename.sh" test, and assumed
> that the test suite ran against linux, windows, and OSX, with the
> latter two running on case-insensitive filesystems. I assumed that
> both case-sensitive and case-insensitive code paths would be tested by
> the complete CI suite.

Apparently we were surprised the same way ;-)

> *Do* we expect to run the full test suite on case-insensitive systems
> in gitlab, or do we expect to need to add explicit "-C
> core.ignorecase" tests as you have done here?

Running all tests on case-insensitive systems and expect them to
pass is reasonable; we need to sprinkle !CASE_INSENSITIVE_FS
prerequiste to skip certain tests that exercise functionalities that
case insensitive filesystem will never be able to support (e.g. you
cannot by their design have file1.txt and File1.txt at the same time
on the filesystem, so any test with "test_cmp file1.txt FIle1.txt"
must be marked with !CASE_INSENSITIVE_FS prerequisite).

When the system I am primary owrking on is case sensitive, it is
always nice to be able to discover that I broke something on case
INsensitive system before I conclude my WIP into a commit and throw
it at CI.  We may have to case-insensitively treat the paths in the
index in order to match what the working tree would do to make "git
checkout -- <path>" work case-insentively, and doing in-index-only
mode of operation with core.ignorecase=yes on case-sensitive system
may be a way to "emulate" some of the requirement case-insentive
systems have with these "-c core.ignorecase" trick, but of course
not all scenarios can be tested without being on case-insensitive
systems.

So we need both, I think.

Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks]

On Sat, Jun 18, 2022 at 2:45 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tao Klerks <tao@klerks.biz> writes:
>
> >  I assumed that
> > both case-sensitive and case-insensitive code paths would be tested by
> > the complete CI suite.
>
>
> When the system I am primary owrking on is case sensitive, it is
> always nice to be able to discover that I broke something on case
> INsensitive system before I conclude my WIP into a commit and throw
> it at CI.  We may have to case-insensitively treat the paths in the
> index in order to match what the working tree would do to make "git
> checkout -- <path>" work case-insentively, and doing in-index-only
> mode of operation with core.ignorecase=yes on case-sensitive system
> may be a way to "emulate" some of the requirement case-insentive
> systems have with these "-c core.ignorecase" trick, but of course
> not all scenarios can be tested without being on case-insensitive
> systems.
>
> So we need both, I think.
>

Understood, makes sense, thank you.

I made some changes that seem to resolve the regression that I had
previously noted, but I'm not sure the approach makes sense, it feels
like there must be a better way. I will submit an RFC series at this
point I think.

[PATCH v2 0/3] RFC: apply: support case-only renames in case-insensitive filesystems

[Tao Klerks via GitGitGadget]

As suggested recently in thread
CAPMMpojwV+f=z9sgc_GaUOTFBCUVdbrGW8WjatWWmC3WTcsoXw@mail.gmail.com,
proposing a fix to git-apply for case-only renames on case-insensitive
filesystems.

Changes in V2:

 * Prepended a commit from Junio, with new apply tests to build on
 * Added a largely-unrelated new known failing test, concerning reset --hard
   in the presence of index case conflicts on a case-insensitive filesystem,
   which we later need to work around in a corner-case test
 * Moved test cases to build on Junio's new test file
 * Switched fix approach from "allow same-name commit explicitly" to "track
   files marked for deletion case-insensitively for filesystem checks",
   which addresses the issue noted and other more obscure ones like "rename
   swap with case change"
 * Added a test case for "rename swap with case change"
 * Added test cases setting "core.ignorecase" on and off explicitly
 * Added a test case exposing one remaining surprising behavior

POSSIBLE CONCERN:

This fix was originally much simpler - it just made the "fn_table" string
list use a case-insensitive string comparison - using case-insensitive
comparisons when dealing with all replacement checks, both on the index and
on the filesystem.

However, with that simple implementation, there was at least one edge-case
where data loss could result: If the index contained two files differing
only by case, with different content, and we were doing a case-only rename,
a swap, or some other operation involving the deletion and creation of a
file with that name (ignoring case), then both of the files with that name
in the index would be overwritten - even though only one of them had the
expected content, and even though the one deleted might never have been
committed.

It seems as though the core.ignorecase option should typically only apply to
filesystem checks - that the index is always case-sensitive.

The current fix proposal therefore splits the string list used for "can I
create a file that already exists?" checks into two such structures - one
string list used for filesystem checks, which is case-insensitive when
specified by core.ignorecase, and one used for index checks, which is always
case-sensitive.

The resulting duplication is not appealing, but I'm not sure how to address
it / how to do this more elegantly. I'm also still not completely certain
that my rule of thumb about the index always being case-sensitive is the
right way of thinking of things.

Junio C Hamano (1):
  t4141: test "git apply" with core.ignorecase

Tao Klerks (2):
  reset: new failing test for reset of case-insensitive duplicate in
    index
  apply: support case-only renames in case-insensitive filesystems

 apply.c                |  81 +++++++++----
 apply.h                |   5 +-
 t/t4141-apply-icase.sh | 258 +++++++++++++++++++++++++++++++++++++++++
 t/t7104-reset-hard.sh  |  11 ++
 4 files changed, 334 insertions(+), 21 deletions(-)
 create mode 100755 t/t4141-apply-icase.sh


base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1257%2FTaoK%2Ftao-apply-case-insensitive-renames-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1257/TaoK/tao-apply-case-insensitive-renames-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1257

Range-diff vs v1:

 1:  b8bd612aa1e ! 1:  efd3bd4cdda apply: support case-only renames in case-insensitive filesystems
     @@
       ## Metadata ##
     -Author: Tao Klerks <tao@klerks.biz>
     +Author: Junio C Hamano <gitster@pobox.com>
      
       ## Commit message ##
     -    apply: support case-only renames in case-insensitive filesystems
     +    t4141: test "git apply" with core.ignorecase
      
     -    "git apply" checks, when validating a patch, to ensure that any files
     -    being added aren't already in the worktree.
     +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
      
     -    When this check runs on a case-only rename, in a case-insensitive
     -    filesystem, this leads to a false positive - the command fails with an
     -    error like:
     -    error: File1: already exists in working directory
     -
     -    Fix this existence check to allow the file to exist, for a case-only
     -    rename when config core.ignorecase is set.
     -
     -    Also add a test for this case, while verifying that conflicting file
     -    conditions are still caught correctly, including case-only conflicts on
     -    case-sensitive filesystems.
     -
     -    Signed-off-by: Tao Klerks <tao@klerks.biz>
     -
     - ## apply.c ##
     -@@ apply.c: static int check_patch(struct apply_state *state, struct patch *patch)
     - 	if ((tpatch = in_fn_table(state, new_name)) &&
     - 	    (was_deleted(tpatch) || to_be_deleted(tpatch)))
     - 		ok_if_exists = 1;
     -+	else if (ignore_case && !strcasecmp(old_name, new_name))
     -+		ok_if_exists = 1;
     - 	else
     - 		ok_if_exists = 0;
     - 
     -
     - ## t/t4141-apply-case-insensitive-rename.sh (new) ##
     + ## t/t4141-apply-icase.sh (new) ##
      @@
      +#!/bin/sh
      +
     -+test_description='git apply should handle case-only renames on case-insensitive filesystems'
     ++test_description='git apply with core.ignorecase'
      +
     -+TEST_PASSES_SANITIZE_LEAK=true
      +. ./test-lib.sh
      +
     -+# Please note, this test assumes that core.ignorecase is set appropriately for the filesystem,
     -+# as tested in t0050. Case-only rename conflicts are only tested in case-sensitive filesystems.
     ++test_expect_success setup '
     ++       # initial commit has file0 only
     ++       test_commit "initial" file0 "initial commit with file0" initial &&
      +
     -+if ! test_have_prereq CASE_INSENSITIVE_FS
     -+then
     -+	test_set_prereq CASE_SENSITIVE_FS
     -+	echo nuts
     -+fi
     ++       # current commit has file1 as well
     ++       test_commit "current" file1 "initial content of file1" current &&
     ++       file0blob=$(git rev-parse :file0) &&
     ++       file1blob=$(git rev-parse :file1) &&
      +
     -+test_expect_success setup '
     -+	echo "This is some content in the file." > file1 &&
     -+	echo "A completely different file." > file2 &&
     -+	git update-index --add file1 &&
     -+	git update-index --add file2 &&
     -+	cat >case_only_rename_patch <<-\EOF
     -+	diff --git a/file1 b/File1
     -+	similarity index 100%
     -+	rename from file1
     -+	rename to File1
     -+	EOF
     ++       # prepare sample patches
     ++       # file0 is modified
     ++       echo modification to file0 >file0 &&
     ++       git add file0 &&
     ++       modifiedfile0blob=$(git rev-parse :file0) &&
     ++
     ++       # file1 is removed and then ...
     ++       git rm --cached file1 &&
     ++       # ... identical copies are placed at File1 and file2
     ++       git update-index --add --cacheinfo 100644,$file1blob,file2 &&
     ++       git update-index --add --cacheinfo 100644,$file1blob,File1 &&
     ++
     ++       # then various patches to do basic things
     ++       git diff HEAD^ HEAD -- file1 >creation-patch &&
     ++       git diff HEAD HEAD^ -- file1 >deletion-patch &&
     ++       git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&
     ++       git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&
     ++       git diff --cached HEAD -- file0 >modify-file0-patch
      +'
      +
     -+test_expect_success 'refuse to apply rename patch with conflict' '
     -+	cat >conflict_patch <<-\EOF &&
     -+	diff --git a/file1 b/file2
     -+	similarity index 100%
     -+	rename from file1
     -+	rename to file2
     -+	EOF
     -+	test_must_fail git apply --index conflict_patch
     ++# Basic creation, deletion, modification and renaming.
     ++test_expect_success 'creation and deletion' '
     ++       # start at "initial" with file0 only
     ++       git reset --hard initial &&
     ++
     ++       # add file1
     ++       git -c core.ignorecase=false apply --cached creation-patch &&
     ++       test_cmp_rev :file1 "$file1blob" &&
     ++
     ++       # remove file1
     ++       git -c core.ignorecase=false apply --cached deletion-patch &&
     ++       test_must_fail git rev-parse --verify :file1 &&
     ++
     ++       # do the same with ignorecase
     ++       git -c core.ignorecase=true apply --cached creation-patch &&
     ++       test_cmp_rev :file1 "$file1blob" &&
     ++       git -c core.ignorecase=true apply --cached deletion-patch &&
     ++       test_must_fail git rev-parse --verify :file1
      +'
      +
     -+test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '
     -+	test_when_finished "git mv File1 file2" &&
     -+	git mv file2 File1 &&
     -+	test_must_fail git apply --index case_only_rename_patch
     ++test_expect_success 'modificaiton' '
     ++       # start at "initial" with file0 only
     ++       git reset --hard initial &&
     ++
     ++       # modify file0
     ++       git -c core.ignorecase=false apply --cached modify-file0-patch &&
     ++       test_cmp_rev :file0 "$modifiedfile0blob" &&
     ++       git -c core.ignorecase=false apply --cached -R modify-file0-patch &&
     ++       test_cmp_rev :file0 "$file0blob" &&
     ++
     ++       # do the same with ignorecase
     ++       git -c core.ignorecase=true apply --cached modify-file0-patch &&
     ++       test_cmp_rev :file0 "$modifiedfile0blob" &&
     ++       git -c core.ignorecase=true apply --cached -R modify-file0-patch &&
     ++       test_cmp_rev :file0 "$file0blob"
     ++'
     ++
     ++test_expect_success 'rename file1 to file2' '
     ++       # start from file0 and file1
     ++       git reset --hard current &&
     ++
     ++       # rename file1 to file2
     ++       git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&
     ++       test_must_fail git rev-parse --verify :file1 &&
     ++       test_cmp_rev :file2 "$file1blob" &&
     ++       git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&
     ++       test_must_fail git rev-parse --verify :file2 &&
     ++       test_cmp_rev :file1 "$file1blob" &&
     ++
     ++       # do the same with ignorecase
     ++       git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&
     ++       test_must_fail git rev-parse --verify :file1 &&
     ++       test_cmp_rev :file2 "$file1blob" &&
     ++       git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&
     ++       test_must_fail git rev-parse --verify :file2 &&
     ++       test_cmp_rev :file1 "$file1blob"
     ++'
     ++
     ++test_expect_success 'rename file1 to file2' '
     ++       # start from file0 and file1
     ++       git reset --hard current &&
     ++
     ++       # rename file1 to File1
     ++       git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&
     ++       test_must_fail git rev-parse --verify :file1 &&
     ++       test_cmp_rev :File1 "$file1blob" &&
     ++       git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&
     ++       test_must_fail git rev-parse --verify :File1 &&
     ++       test_cmp_rev :file1 "$file1blob" &&
     ++
     ++       # do the same with ignorecase
     ++       git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&
     ++       test_must_fail git rev-parse --verify :file1 &&
     ++       test_cmp_rev :File1 "$file1blob" &&
     ++       git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&
     ++       test_must_fail git rev-parse --verify :File1 &&
     ++       test_cmp_rev :file1 "$file1blob"
     ++'
     ++
     ++# We may want to add tests with working tree here, without "--cached" and
     ++# with and without "--index" here.  For example, should modify-file0-patch
     ++# apply cleanly if we have File0 with $file0blob in the index and the working
     ++# tree if core.icase is set?
     ++
     ++test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '
     ++       : sample
      +'
      +
     -+test_expect_success 'apply case-only rename patch without conflict' '
     -+	git apply --index case_only_rename_patch
     ++test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '
     ++       : sample
      +'
      +
      +test_done
 -:  ----------- > 2:  1226fbd3caf reset: new failing test for reset of case-insensitive duplicate in index
 -:  ----------- > 3:  04d83283716 apply: support case-only renames in case-insensitive filesystems

-- 
gitgitgadget

[PATCH v2 1/3] t4141: test "git apply" with core.ignorecase

[Junio C Hamano via GitGitGadget]

From: Junio C Hamano <gitster@pobox.com>

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4141-apply-icase.sh | 128 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100755 t/t4141-apply-icase.sh

diff --git a/t/t4141-apply-icase.sh b/t/t4141-apply-icase.sh
new file mode 100755
index 00000000000..17eb023a437
--- /dev/null
+++ b/t/t4141-apply-icase.sh
@@ -0,0 +1,128 @@
+#!/bin/sh
+
+test_description='git apply with core.ignorecase'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+       # initial commit has file0 only
+       test_commit "initial" file0 "initial commit with file0" initial &&
+
+       # current commit has file1 as well
+       test_commit "current" file1 "initial content of file1" current &&
+       file0blob=$(git rev-parse :file0) &&
+       file1blob=$(git rev-parse :file1) &&
+
+       # prepare sample patches
+       # file0 is modified
+       echo modification to file0 >file0 &&
+       git add file0 &&
+       modifiedfile0blob=$(git rev-parse :file0) &&
+
+       # file1 is removed and then ...
+       git rm --cached file1 &&
+       # ... identical copies are placed at File1 and file2
+       git update-index --add --cacheinfo 100644,$file1blob,file2 &&
+       git update-index --add --cacheinfo 100644,$file1blob,File1 &&
+
+       # then various patches to do basic things
+       git diff HEAD^ HEAD -- file1 >creation-patch &&
+       git diff HEAD HEAD^ -- file1 >deletion-patch &&
+       git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&
+       git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&
+       git diff --cached HEAD -- file0 >modify-file0-patch
+'
+
+# Basic creation, deletion, modification and renaming.
+test_expect_success 'creation and deletion' '
+       # start at "initial" with file0 only
+       git reset --hard initial &&
+
+       # add file1
+       git -c core.ignorecase=false apply --cached creation-patch &&
+       test_cmp_rev :file1 "$file1blob" &&
+
+       # remove file1
+       git -c core.ignorecase=false apply --cached deletion-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --cached creation-patch &&
+       test_cmp_rev :file1 "$file1blob" &&
+       git -c core.ignorecase=true apply --cached deletion-patch &&
+       test_must_fail git rev-parse --verify :file1
+'
+
+test_expect_success 'modificaiton' '
+       # start at "initial" with file0 only
+       git reset --hard initial &&
+
+       # modify file0
+       git -c core.ignorecase=false apply --cached modify-file0-patch &&
+       test_cmp_rev :file0 "$modifiedfile0blob" &&
+       git -c core.ignorecase=false apply --cached -R modify-file0-patch &&
+       test_cmp_rev :file0 "$file0blob" &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --cached modify-file0-patch &&
+       test_cmp_rev :file0 "$modifiedfile0blob" &&
+       git -c core.ignorecase=true apply --cached -R modify-file0-patch &&
+       test_cmp_rev :file0 "$file0blob"
+'
+
+test_expect_success 'rename file1 to file2' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # rename file1 to file2
+       git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :file2 "$file1blob" &&
+       git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file2 &&
+       test_cmp_rev :file1 "$file1blob" &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :file2 "$file1blob" &&
+       git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file2 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success 'rename file1 to file2' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # rename file1 to File1
+       git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :File1 "$file1blob" &&
+       git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_cmp_rev :file1 "$file1blob" &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :File1 "$file1blob" &&
+       git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+# We may want to add tests with working tree here, without "--cached" and
+# with and without "--index" here.  For example, should modify-file0-patch
+# apply cleanly if we have File0 with $file0blob in the index and the working
+# tree if core.icase is set?
+
+test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '
+       : sample
+'
+
+test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '
+       : sample
+'
+
+test_done
-- 
gitgitgadget

[PATCH v2 2/3] reset: new failing test for reset of case-insensitive duplicate in index

[Tao Klerks via GitGitGadget]

From: Tao Klerks <tao@klerks.biz>

On case-insensitive filesystems, where core.ignorecase is normally set,
the index is still case-sensitive, and surprising outcomes are possible
when the index contains states that cannot be represented on the file
system.

Add an "expect_failure" test to illustrate one such situation, where two
files differing only in case are in the index, and a "reset --hard" ends
up creating an unexpected worktree change.

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
 t/t7104-reset-hard.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh
index cf9697eba9a..55050ac831e 100755
--- a/t/t7104-reset-hard.sh
+++ b/t/t7104-reset-hard.sh
@@ -44,4 +44,15 @@ test_expect_success 'reset --hard did not corrupt index or cache-tree' '
 
 '
 
+test_expect_failure CASE_INSENSITIVE_FS 'reset --hard handles index-only case-insensitive duplicate' '
+	test_commit "initial" file1 "initial commit with file1" initial &&
+	file1blob=$(git rev-parse :file1) &&
+	git update-index --add --cacheinfo 100644,$file1blob,File1 &&
+
+	# reset --hard accidentally leaves the working tree with a deleted file.
+	git reset --hard &&
+	git status --porcelain -uno >wt_changes_remaining &&
+	test_must_be_empty wt_changes_remaining
+'
+
 test_done
-- 
gitgitgadget

[PATCH v2 3/3] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks via GitGitGadget]

From: Tao Klerks <tao@klerks.biz>

"git apply" checks, when validating a patch, to ensure that any files
being added aren't already in the worktree.

When this check runs on a case-only rename, in a case-insensitive
filesystem, this leads to a false positive - the command fails with an
error like:
error: File1: already exists in working directory

There is a mechanism to ensure that "seemingly conflicting" files are
handled correctly - for example overlapping rename pairs or swaps -
this mechanism treats renames as add/remove pairs, and would end up
treating a case-only rename as a "self-swap"... Except it does not
account for case-insensitive filesystems yet.

Because the index is inherently case-sensitive even on a
case-insensitive filesystem, we actually need this mechanism to be
handle both requirements, lest we fail to account for conflicting
files only in the index.

Fix the "rename chain" existence exemption mechanism to account for
case-insensitive config, fixing case-only-rename-handling as a
"self-swap" and also fixing less-common "case-insensitive rename
pairs" when config core.ignorecase is set, but keep the index checks
file-sensitive.

Also add test cases around these behaviors - verifying that conflicting
file conditions are still caught correctly, including case-only
conflicts on case-sensitive filesystems, and edge cases around
case-sensitive index behaviors on a case-insensitive filesystem.

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
 apply.c                |  81 ++++++++++++++++------
 apply.h                |   5 +-
 t/t4141-apply-icase.sh | 154 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 207 insertions(+), 33 deletions(-)

diff --git a/apply.c b/apply.c
index 2b7cd930efa..2bd59b63edd 100644
--- a/apply.c
+++ b/apply.c
@@ -101,7 +101,9 @@ int init_apply_state(struct apply_state *state,
 	state->ws_error_action = warn_on_ws_error;
 	state->ws_ignore_action = ignore_ws_none;
 	state->linenr = 1;
-	string_list_init_nodup(&state->fn_table);
+	string_list_init_nodup(&state->fs_fn_table);
+	state->fs_fn_table.cmp = fspathcmp;
+	string_list_init_nodup(&state->index_fn_table);
 	string_list_init_nodup(&state->limit_by_name);
 	strset_init(&state->removed_symlinks);
 	strset_init(&state->kept_symlinks);
@@ -122,7 +124,10 @@ void clear_apply_state(struct apply_state *state)
 	strset_clear(&state->kept_symlinks);
 	strbuf_release(&state->root);
 
-	/* &state->fn_table is cleared at the end of apply_patch() */
+	/*
+	 * &state->fs_fn_table and &state->index_fn_table are cleared at the
+	 * end of apply_patch()
+	 */
 }
 
 static void mute_routine(const char *msg, va_list params)
@@ -3270,14 +3275,28 @@ static int read_file_or_gitlink(const struct cache_entry *ce, struct strbuf *buf
 	return read_blob_object(buf, &ce->oid, ce->ce_mode);
 }
 
-static struct patch *in_fn_table(struct apply_state *state, const char *name)
+static struct patch *in_fs_fn_table(struct apply_state *state, const char *name)
 {
 	struct string_list_item *item;
 
 	if (!name)
 		return NULL;
 
-	item = string_list_lookup(&state->fn_table, name);
+	item = string_list_lookup(&state->fs_fn_table, name);
+	if (item)
+		return (struct patch *)item->util;
+
+	return NULL;
+}
+
+static struct patch *in_index_fn_table(struct apply_state *state, const char *name)
+{
+	struct string_list_item *item;
+
+	if (!name)
+		return NULL;
+
+	item = string_list_lookup(&state->index_fn_table, name);
 	if (item)
 		return (struct patch *)item->util;
 
@@ -3309,7 +3328,7 @@ static int was_deleted(struct patch *patch)
 	return patch == PATH_WAS_DELETED;
 }
 
-static void add_to_fn_table(struct apply_state *state, struct patch *patch)
+static void add_to_fn_tables(struct apply_state *state, struct patch *patch)
 {
 	struct string_list_item *item;
 
@@ -3319,7 +3338,9 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)
 	 * file creations and copies
 	 */
 	if (patch->new_name) {
-		item = string_list_insert(&state->fn_table, patch->new_name);
+		item = string_list_insert(&state->fs_fn_table, patch->new_name);
+		item->util = patch;
+		item = string_list_insert(&state->index_fn_table, patch->new_name);
 		item->util = patch;
 	}
 
@@ -3328,7 +3349,9 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)
 	 * later chunks shouldn't patch old names
 	 */
 	if ((patch->new_name == NULL) || (patch->is_rename)) {
-		item = string_list_insert(&state->fn_table, patch->old_name);
+		item = string_list_insert(&state->fs_fn_table, patch->old_name);
+		item->util = PATH_WAS_DELETED;
+		item = string_list_insert(&state->index_fn_table, patch->old_name);
 		item->util = PATH_WAS_DELETED;
 	}
 }
@@ -3341,7 +3364,9 @@ static void prepare_fn_table(struct apply_state *state, struct patch *patch)
 	while (patch) {
 		if ((patch->new_name == NULL) || (patch->is_rename)) {
 			struct string_list_item *item;
-			item = string_list_insert(&state->fn_table, patch->old_name);
+			item = string_list_insert(&state->fs_fn_table, patch->old_name);
+			item->util = PATH_TO_BE_DELETED;
+			item = string_list_insert(&state->index_fn_table, patch->old_name);
 			item->util = PATH_TO_BE_DELETED;
 		}
 		patch = patch->next;
@@ -3371,7 +3396,7 @@ static struct patch *previous_patch(struct apply_state *state,
 	if (patch->is_copy || patch->is_rename)
 		return NULL; /* "git" patches do not depend on the order */
 
-	previous = in_fn_table(state, patch->old_name);
+	previous = in_index_fn_table(state, patch->old_name);
 	if (!previous)
 		return NULL;
 
@@ -3681,7 +3706,7 @@ static int apply_data(struct apply_state *state, struct patch *patch,
 	}
 	patch->result = image.buf;
 	patch->resultsize = image.len;
-	add_to_fn_table(state, patch);
+	add_to_fn_tables(state, patch);
 	free(image.line_allocated);
 
 	if (0 < patch->is_delete && patch->resultsize)
@@ -3780,11 +3805,12 @@ static int check_preimage(struct apply_state *state,
 
 static int check_to_create(struct apply_state *state,
 			   const char *new_name,
-			   int ok_if_exists)
+			   int ok_if_exists_in_fs,
+			   int ok_if_exists_in_index)
 {
 	struct stat nst;
 
-	if (state->check_index && (!ok_if_exists || !state->cached)) {
+	if (state->check_index && (!ok_if_exists_in_index || !state->cached)) {
 		int pos;
 
 		pos = index_name_pos(state->repo->index, new_name, strlen(new_name));
@@ -3792,7 +3818,7 @@ static int check_to_create(struct apply_state *state,
 			struct cache_entry *ce = state->repo->index->cache[pos];
 
 			/* allow ITA, as they do not yet exist in the index */
-			if (!ok_if_exists && !(ce->ce_flags & CE_INTENT_TO_ADD))
+			if (!ok_if_exists_in_index && !(ce->ce_flags & CE_INTENT_TO_ADD))
 				return EXISTS_IN_INDEX;
 
 			/* ITA entries can never match working tree files */
@@ -3805,7 +3831,7 @@ static int check_to_create(struct apply_state *state,
 		return 0;
 
 	if (!lstat(new_name, &nst)) {
-		if (S_ISDIR(nst.st_mode) || ok_if_exists)
+		if (S_ISDIR(nst.st_mode) || ok_if_exists_in_fs)
 			return 0;
 		/*
 		 * A leading component of new_name might be a symlink
@@ -3915,7 +3941,8 @@ static int check_patch(struct apply_state *state, struct patch *patch)
 	const char *name = old_name ? old_name : new_name;
 	struct cache_entry *ce = NULL;
 	struct patch *tpatch;
-	int ok_if_exists;
+	int ok_if_exists_in_fs;
+	int ok_if_exists_in_index;
 	int status;
 
 	patch->rejected = 1; /* we will drop this after we succeed */
@@ -3938,16 +3965,29 @@ static int check_patch(struct apply_state *state, struct patch *patch)
 	 * B; ask to_be_deleted() about the later rename.  Removal of
 	 * B and rename from A to B is handled the same way by asking
 	 * was_deleted().
+	 *
+	 * These exemptions account for the core.ignorecase config -
+	 * a file that differs only by case is also considered "deleted"
+	 * if git is configured to ignore case. This means a case-only
+	 * rename, in a case-insensitive filesystem, is treated here as
+	 * a "self-swap" or mode change.
 	 */
-	if ((tpatch = in_fn_table(state, new_name)) &&
+	if ((tpatch = in_fs_fn_table(state, new_name)) &&
+	    (was_deleted(tpatch) || to_be_deleted(tpatch)))
+		ok_if_exists_in_fs = 1;
+	else
+		ok_if_exists_in_fs = 0;
+
+	if ((tpatch = in_index_fn_table(state, new_name)) &&
 	    (was_deleted(tpatch) || to_be_deleted(tpatch)))
-		ok_if_exists = 1;
+		ok_if_exists_in_index = 1;
 	else
-		ok_if_exists = 0;
+		ok_if_exists_in_index = 0;
 
 	if (new_name &&
 	    ((0 < patch->is_new) || patch->is_rename || patch->is_copy)) {
-		int err = check_to_create(state, new_name, ok_if_exists);
+		int err = check_to_create(state, new_name, ok_if_exists_in_fs,
+					  ok_if_exists_in_index);
 
 		if (err && state->threeway) {
 			patch->direct_to_threeway = 1;
@@ -4808,7 +4848,8 @@ static int apply_patch(struct apply_state *state,
 end:
 	free_patch_list(list);
 	strbuf_release(&buf);
-	string_list_clear(&state->fn_table, 0);
+	string_list_clear(&state->fs_fn_table, 0);
+	string_list_clear(&state->index_fn_table, 0);
 	return res;
 }
 
diff --git a/apply.h b/apply.h
index b9f18ce87d1..b520ce8c40a 100644
--- a/apply.h
+++ b/apply.h
@@ -95,8 +95,11 @@ struct apply_state {
 	/*
 	 * Records filenames that have been touched, in order to handle
 	 * the case where more than one patches touch the same file.
+	 * Two separate structures because with ignorecase, one of them
+	 * needs to be case-insensitive and the other not.
 	 */
-	struct string_list fn_table;
+	struct string_list fs_fn_table;
+	struct string_list index_fn_table;
 
 	/*
 	 * This is to save reporting routines before using
diff --git a/t/t4141-apply-icase.sh b/t/t4141-apply-icase.sh
index 17eb023a437..1c785133d16 100755
--- a/t/t4141-apply-icase.sh
+++ b/t/t4141-apply-icase.sh
@@ -30,7 +30,16 @@ test_expect_success setup '
        git diff HEAD HEAD^ -- file1 >deletion-patch &&
        git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&
        git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&
-       git diff --cached HEAD -- file0 >modify-file0-patch
+       git diff --cached HEAD -- file0 >modify-file0-patch &&
+
+       # then set up for swap
+       git reset --hard current &&
+       test_commit "swappable" file3 "different content for file3" swappable &&
+       file3blob=$(git rev-parse :file3) &&
+       git rm --cached file1 file3 &&
+       git update-index --add --cacheinfo 100644,$file1blob,File3 &&
+       git update-index --add --cacheinfo 100644,$file3blob,File1 &&
+       git diff --cached HEAD -- file1 file3 File1 File3 >swap-file1-and-file3-to-File3-and-File1-patch
 '
 
 # Basic creation, deletion, modification and renaming.
@@ -53,7 +62,7 @@ test_expect_success 'creation and deletion' '
        test_must_fail git rev-parse --verify :file1
 '
 
-test_expect_success 'modificaiton' '
+test_expect_success 'modification (index-only)' '
        # start at "initial" with file0 only
        git reset --hard initial &&
 
@@ -70,7 +79,7 @@ test_expect_success 'modificaiton' '
        test_cmp_rev :file0 "$file0blob"
 '
 
-test_expect_success 'rename file1 to file2' '
+test_expect_success 'rename file1 to file2 (index-only)' '
        # start from file0 and file1
        git reset --hard current &&
 
@@ -91,7 +100,7 @@ test_expect_success 'rename file1 to file2' '
        test_cmp_rev :file1 "$file1blob"
 '
 
-test_expect_success 'rename file1 to file2' '
+test_expect_success 'rename file1 to File1 (index-only)' '
        # start from file0 and file1
        git reset --hard current &&
 
@@ -112,17 +121,138 @@ test_expect_success 'rename file1 to file2' '
        test_cmp_rev :file1 "$file1blob"
 '
 
-# We may want to add tests with working tree here, without "--cached" and
-# with and without "--index" here.  For example, should modify-file0-patch
-# apply cleanly if we have File0 with $file0blob in the index and the working
-# tree if core.icase is set?
+# involve filesystem on renames
+test_expect_success 'rename file1 to File1 (with ignorecase, working tree)' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --index rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :File1 "$file1blob" &&
+       git -c core.ignorecase=true apply --index -R rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success CASE_INSENSITIVE_FS 'rename file1 to File1 (without ignorecase, case-insensitive FS)' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # rename file1 to File1 without ignorecase (fails as expected)
+       test_must_fail git -c core.ignorecase=false apply --index rename-file1-to-File1-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success !CASE_INSENSITIVE_FS 'rename file1 to File1 (without ignorecase, case-sensitive FS)' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # rename file1 to File1 without ignorecase
+       git -c core.ignorecase=false apply --index rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :File1 "$file1blob" &&
+       git -c core.ignorecase=false apply --index -R rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success 'rename file1 to file2 with working tree conflict' '
+       # start from file0 and file1, and file2 untracked
+       git reset --hard current &&
+       test_when_finished "rm file2" &&
+       touch file2 &&
+
+       # rename file1 to file2 with conflict
+       test_must_fail git -c core.ignorecase=false apply --index rename-file1-to-file2-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob" &&
 
-test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '
-       : sample
+       # do the same with ignorecase
+       test_must_fail git -c core.ignorecase=true apply --index rename-file1-to-file2-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob"
 '
 
-test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '
-       : sample
+test_expect_success 'rename file1 to file2 with case-insensitive conflict (index-only - ignorecase disabled)' '
+       # start from file0 and file1, and File2 in index
+       git reset --hard current &&
+       git update-index --add --cacheinfo 100644,$file3blob,File2 &&
+
+       # rename file1 to file2 without ignorecase
+       git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :file2 "$file1blob" &&
+       git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file2 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :File2 "$file3blob"
+'
+
+test_expect_failure 'rename file1 to file2 with case-insensitive conflict (index-only - ignorecase enabled)' '
+       # start from file0 and file1, and File2 in index
+       git reset --hard current &&
+       git update-index --add --cacheinfo 100644,$file3blob,File2 &&
+
+       # rename file1 to file2 with ignorecase, with a "File2" conflicting file in place - expect failure.
+       # instead of failure, we get success with "File1" and "file1" both existing in the index, despite
+       # the ignorecase configuration.
+       test_must_fail git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :File2 "$file3blob"
+'
+
+test_expect_success 'rename file1 to File1 with case-sensitive conflict (index-only)' '
+       # start from file0 and file1, and File1 in index
+       git reset --hard current &&
+       git update-index --add --cacheinfo 100644,$file3blob,File1 &&
+
+       # On a case-insensitive filesystem with core.ignorecase on, a single git
+       # "reset --hard" will actually leave things wrong because of the
+       # index-to-working-tree discrepancy - see "reset --hard handles
+       # index-only case-insensitive duplicate" under t7104-reset-hard.sh.
+       # We are creating this unexpected state, so we should explicitly queue
+       # an extra reset. If reset ever starts to handle this case, this will
+       # become unnecessary but also not harmful.
+       test_when_finished "git reset --hard" &&
+
+       # rename file1 to File1 when File1 is already in index (fails with conflict)
+       test_must_fail git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :File1 "$file3blob" &&
+
+       # do the same with ignorecase
+       test_must_fail git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :File1 "$file3blob"
+'
+
+test_expect_success CASE_INSENSITIVE_FS 'case-insensitive swap - file1 to File2 and file2 to File1 (working tree)' '
+       # start from file0, file1, and file3
+       git reset --hard swappable &&
+
+       # "swap" file1 and file3 to case-insensitive versions without ignorecase on case-insensitive FS (fails as expected)
+       test_must_fail git -c core.ignorecase=false apply --index swap-file1-and-file3-to-File3-and-File1-patch &&
+       git rev-parse --verify :file1 &&
+       git rev-parse --verify :file3 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :file3 "$file3blob" &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --index swap-file1-and-file3-to-File3-and-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_must_fail git rev-parse --verify :file3 &&
+       test_cmp_rev :File3 "$file1blob" &&
+       test_cmp_rev :File1 "$file3blob" &&
+       git -c core.ignorecase=true apply --index -R swap-file1-and-file3-to-File3-and-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_must_fail git rev-parse --verify :File3 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :file3 "$file3blob"
 '
 
 test_done
-- 
gitgitgadget

Re: [PATCH v2 0/3] RFC: apply: support case-only renames in case-insensitive filesystems

[Tao Klerks]

Hi folks,

These patches are a few months old now, but they still apply cleanly
and I'm not sure how to improve them.

I'd appreciate any feedback on both the approach, and the detailed
code changes themselves, that could help me make this a viable
patchset to fix case-only renames on file-insensitive filesystems
using "git apply".

Thanks,
Tao

On Sun, Jun 19, 2022 at 6:10 PM Tao Klerks via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> As suggested recently in thread
> CAPMMpojwV+f=z9sgc_GaUOTFBCUVdbrGW8WjatWWmC3WTcsoXw@mail.gmail.com,
> proposing a fix to git-apply for case-only renames on case-insensitive
> filesystems.
>
> Changes in V2:
>
>  * Prepended a commit from Junio, with new apply tests to build on
>  * Added a largely-unrelated new known failing test, concerning reset --hard
>    in the presence of index case conflicts on a case-insensitive filesystem,
>    which we later need to work around in a corner-case test
>  * Moved test cases to build on Junio's new test file
>  * Switched fix approach from "allow same-name commit explicitly" to "track
>    files marked for deletion case-insensitively for filesystem checks",
>    which addresses the issue noted and other more obscure ones like "rename
>    swap with case change"
>  * Added a test case for "rename swap with case change"
>  * Added test cases setting "core.ignorecase" on and off explicitly
>  * Added a test case exposing one remaining surprising behavior
>
> POSSIBLE CONCERN:
>
> This fix was originally much simpler - it just made the "fn_table" string
> list use a case-insensitive string comparison - using case-insensitive
> comparisons when dealing with all replacement checks, both on the index and
> on the filesystem.
>
> However, with that simple implementation, there was at least one edge-case
> where data loss could result: If the index contained two files differing
> only by case, with different content, and we were doing a case-only rename,
> a swap, or some other operation involving the deletion and creation of a
> file with that name (ignoring case), then both of the files with that name
> in the index would be overwritten - even though only one of them had the
> expected content, and even though the one deleted might never have been
> committed.
>
> It seems as though the core.ignorecase option should typically only apply to
> filesystem checks - that the index is always case-sensitive.
>
> The current fix proposal therefore splits the string list used for "can I
> create a file that already exists?" checks into two such structures - one
> string list used for filesystem checks, which is case-insensitive when
> specified by core.ignorecase, and one used for index checks, which is always
> case-sensitive.
>
> The resulting duplication is not appealing, but I'm not sure how to address
> it / how to do this more elegantly. I'm also still not completely certain
> that my rule of thumb about the index always being case-sensitive is the
> right way of thinking of things.
>
> Junio C Hamano (1):
>   t4141: test "git apply" with core.ignorecase
>
> Tao Klerks (2):
>   reset: new failing test for reset of case-insensitive duplicate in
>     index
>   apply: support case-only renames in case-insensitive filesystems
>
>  apply.c                |  81 +++++++++----
>  apply.h                |   5 +-
>  t/t4141-apply-icase.sh | 258 +++++++++++++++++++++++++++++++++++++++++
>  t/t7104-reset-hard.sh  |  11 ++
>  4 files changed, 334 insertions(+), 21 deletions(-)
>  create mode 100755 t/t4141-apply-icase.sh
>
>
> base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1257%2FTaoK%2Ftao-apply-case-insensitive-renames-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1257/TaoK/tao-apply-case-insensitive-renames-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1257
>
> Range-diff vs v1:
>
>  1:  b8bd612aa1e ! 1:  efd3bd4cdda apply: support case-only renames in case-insensitive filesystems
>      @@
>        ## Metadata ##
>      -Author: Tao Klerks <tao@klerks.biz>
>      +Author: Junio C Hamano <gitster@pobox.com>
>
>        ## Commit message ##
>      -    apply: support case-only renames in case-insensitive filesystems
>      +    t4141: test "git apply" with core.ignorecase
>
>      -    "git apply" checks, when validating a patch, to ensure that any files
>      -    being added aren't already in the worktree.
>      +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
>      -    When this check runs on a case-only rename, in a case-insensitive
>      -    filesystem, this leads to a false positive - the command fails with an
>      -    error like:
>      -    error: File1: already exists in working directory
>      -
>      -    Fix this existence check to allow the file to exist, for a case-only
>      -    rename when config core.ignorecase is set.
>      -
>      -    Also add a test for this case, while verifying that conflicting file
>      -    conditions are still caught correctly, including case-only conflicts on
>      -    case-sensitive filesystems.
>      -
>      -    Signed-off-by: Tao Klerks <tao@klerks.biz>
>      -
>      - ## apply.c ##
>      -@@ apply.c: static int check_patch(struct apply_state *state, struct patch *patch)
>      -  if ((tpatch = in_fn_table(state, new_name)) &&
>      -      (was_deleted(tpatch) || to_be_deleted(tpatch)))
>      -          ok_if_exists = 1;
>      -+ else if (ignore_case && !strcasecmp(old_name, new_name))
>      -+         ok_if_exists = 1;
>      -  else
>      -          ok_if_exists = 0;
>      -
>      -
>      - ## t/t4141-apply-case-insensitive-rename.sh (new) ##
>      + ## t/t4141-apply-icase.sh (new) ##
>       @@
>       +#!/bin/sh
>       +
>      -+test_description='git apply should handle case-only renames on case-insensitive filesystems'
>      ++test_description='git apply with core.ignorecase'
>       +
>      -+TEST_PASSES_SANITIZE_LEAK=true
>       +. ./test-lib.sh
>       +
>      -+# Please note, this test assumes that core.ignorecase is set appropriately for the filesystem,
>      -+# as tested in t0050. Case-only rename conflicts are only tested in case-sensitive filesystems.
>      ++test_expect_success setup '
>      ++       # initial commit has file0 only
>      ++       test_commit "initial" file0 "initial commit with file0" initial &&
>       +
>      -+if ! test_have_prereq CASE_INSENSITIVE_FS
>      -+then
>      -+ test_set_prereq CASE_SENSITIVE_FS
>      -+ echo nuts
>      -+fi
>      ++       # current commit has file1 as well
>      ++       test_commit "current" file1 "initial content of file1" current &&
>      ++       file0blob=$(git rev-parse :file0) &&
>      ++       file1blob=$(git rev-parse :file1) &&
>       +
>      -+test_expect_success setup '
>      -+ echo "This is some content in the file." > file1 &&
>      -+ echo "A completely different file." > file2 &&
>      -+ git update-index --add file1 &&
>      -+ git update-index --add file2 &&
>      -+ cat >case_only_rename_patch <<-\EOF
>      -+ diff --git a/file1 b/File1
>      -+ similarity index 100%
>      -+ rename from file1
>      -+ rename to File1
>      -+ EOF
>      ++       # prepare sample patches
>      ++       # file0 is modified
>      ++       echo modification to file0 >file0 &&
>      ++       git add file0 &&
>      ++       modifiedfile0blob=$(git rev-parse :file0) &&
>      ++
>      ++       # file1 is removed and then ...
>      ++       git rm --cached file1 &&
>      ++       # ... identical copies are placed at File1 and file2
>      ++       git update-index --add --cacheinfo 100644,$file1blob,file2 &&
>      ++       git update-index --add --cacheinfo 100644,$file1blob,File1 &&
>      ++
>      ++       # then various patches to do basic things
>      ++       git diff HEAD^ HEAD -- file1 >creation-patch &&
>      ++       git diff HEAD HEAD^ -- file1 >deletion-patch &&
>      ++       git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&
>      ++       git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&
>      ++       git diff --cached HEAD -- file0 >modify-file0-patch
>       +'
>       +
>      -+test_expect_success 'refuse to apply rename patch with conflict' '
>      -+ cat >conflict_patch <<-\EOF &&
>      -+ diff --git a/file1 b/file2
>      -+ similarity index 100%
>      -+ rename from file1
>      -+ rename to file2
>      -+ EOF
>      -+ test_must_fail git apply --index conflict_patch
>      ++# Basic creation, deletion, modification and renaming.
>      ++test_expect_success 'creation and deletion' '
>      ++       # start at "initial" with file0 only
>      ++       git reset --hard initial &&
>      ++
>      ++       # add file1
>      ++       git -c core.ignorecase=false apply --cached creation-patch &&
>      ++       test_cmp_rev :file1 "$file1blob" &&
>      ++
>      ++       # remove file1
>      ++       git -c core.ignorecase=false apply --cached deletion-patch &&
>      ++       test_must_fail git rev-parse --verify :file1 &&
>      ++
>      ++       # do the same with ignorecase
>      ++       git -c core.ignorecase=true apply --cached creation-patch &&
>      ++       test_cmp_rev :file1 "$file1blob" &&
>      ++       git -c core.ignorecase=true apply --cached deletion-patch &&
>      ++       test_must_fail git rev-parse --verify :file1
>       +'
>       +
>      -+test_expect_success CASE_SENSITIVE_FS 'refuse to apply case-only rename patch with conflict, in case-sensitive FS' '
>      -+ test_when_finished "git mv File1 file2" &&
>      -+ git mv file2 File1 &&
>      -+ test_must_fail git apply --index case_only_rename_patch
>      ++test_expect_success 'modificaiton' '
>      ++       # start at "initial" with file0 only
>      ++       git reset --hard initial &&
>      ++
>      ++       # modify file0
>      ++       git -c core.ignorecase=false apply --cached modify-file0-patch &&
>      ++       test_cmp_rev :file0 "$modifiedfile0blob" &&
>      ++       git -c core.ignorecase=false apply --cached -R modify-file0-patch &&
>      ++       test_cmp_rev :file0 "$file0blob" &&
>      ++
>      ++       # do the same with ignorecase
>      ++       git -c core.ignorecase=true apply --cached modify-file0-patch &&
>      ++       test_cmp_rev :file0 "$modifiedfile0blob" &&
>      ++       git -c core.ignorecase=true apply --cached -R modify-file0-patch &&
>      ++       test_cmp_rev :file0 "$file0blob"
>      ++'
>      ++
>      ++test_expect_success 'rename file1 to file2' '
>      ++       # start from file0 and file1
>      ++       git reset --hard current &&
>      ++
>      ++       # rename file1 to file2
>      ++       git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&
>      ++       test_must_fail git rev-parse --verify :file1 &&
>      ++       test_cmp_rev :file2 "$file1blob" &&
>      ++       git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&
>      ++       test_must_fail git rev-parse --verify :file2 &&
>      ++       test_cmp_rev :file1 "$file1blob" &&
>      ++
>      ++       # do the same with ignorecase
>      ++       git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&
>      ++       test_must_fail git rev-parse --verify :file1 &&
>      ++       test_cmp_rev :file2 "$file1blob" &&
>      ++       git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&
>      ++       test_must_fail git rev-parse --verify :file2 &&
>      ++       test_cmp_rev :file1 "$file1blob"
>      ++'
>      ++
>      ++test_expect_success 'rename file1 to file2' '
>      ++       # start from file0 and file1
>      ++       git reset --hard current &&
>      ++
>      ++       # rename file1 to File1
>      ++       git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&
>      ++       test_must_fail git rev-parse --verify :file1 &&
>      ++       test_cmp_rev :File1 "$file1blob" &&
>      ++       git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&
>      ++       test_must_fail git rev-parse --verify :File1 &&
>      ++       test_cmp_rev :file1 "$file1blob" &&
>      ++
>      ++       # do the same with ignorecase
>      ++       git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&
>      ++       test_must_fail git rev-parse --verify :file1 &&
>      ++       test_cmp_rev :File1 "$file1blob" &&
>      ++       git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&
>      ++       test_must_fail git rev-parse --verify :File1 &&
>      ++       test_cmp_rev :file1 "$file1blob"
>      ++'
>      ++
>      ++# We may want to add tests with working tree here, without "--cached" and
>      ++# with and without "--index" here.  For example, should modify-file0-patch
>      ++# apply cleanly if we have File0 with $file0blob in the index and the working
>      ++# tree if core.icase is set?
>      ++
>      ++test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '
>      ++       : sample
>       +'
>       +
>      -+test_expect_success 'apply case-only rename patch without conflict' '
>      -+ git apply --index case_only_rename_patch
>      ++test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '
>      ++       : sample
>       +'
>       +
>       +test_done
>  -:  ----------- > 2:  1226fbd3caf reset: new failing test for reset of case-insensitive duplicate in index
>  -:  ----------- > 3:  04d83283716 apply: support case-only renames in case-insensitive filesystems
>
> --
> gitgitgadget

[PATCH v3 0/3] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks via GitGitGadget]

As suggested almost a year ago in thread
CAPMMpojwV+f=z9sgc_GaUOTFBCUVdbrGW8WjatWWmC3WTcsoXw@mail.gmail.com,
proposing a fix to git-apply for case-only renames on case-insensitive
filesystems.

Changes in V3:

 * Rebased onto recent main
 * Renumbered now-duplicate-number test t4141 to t4142
 * Removed "RFC" prefix to officially submit; I don't see a better
   direction, and haven't received any corresponding feedback

As mentioned in V2, I'm not super-happy with the duplication of filename
tracking tables, but I do think this bug needs to be fixed, and I don't see
any other way to do so. The fundamental rule this change implements is that
filesystem filename duplication checks should respect the core.ignorecase
option, but index filename duplication checks should not.

Junio C Hamano (1):
  t4142: test "git apply" with core.ignorecase

Tao Klerks (2):
  reset: new failing test for reset of case-insensitive duplicate in
    index
  apply: support case-only renames in case-insensitive filesystems

 apply.c                |  81 +++++++++----
 apply.h                |   5 +-
 t/t4142-apply-icase.sh | 258 +++++++++++++++++++++++++++++++++++++++++
 t/t7104-reset-hard.sh  |  11 ++
 4 files changed, 334 insertions(+), 21 deletions(-)
 create mode 100755 t/t4142-apply-icase.sh


base-commit: 4a714b37029a4b63dbd22f7d7ed81f7a0d693680
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1257%2FTaoK%2Ftao-apply-case-insensitive-renames-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1257/TaoK/tao-apply-case-insensitive-renames-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1257

Range-diff vs v2:

 1:  efd3bd4cdda ! 1:  8ad60943c66 t4141: test "git apply" with core.ignorecase
     @@ Metadata
      Author: Junio C Hamano <gitster@pobox.com>
      
       ## Commit message ##
     -    t4141: test "git apply" with core.ignorecase
     +    t4142: test "git apply" with core.ignorecase
      
          Signed-off-by: Junio C Hamano <gitster@pobox.com>
      
     - ## t/t4141-apply-icase.sh (new) ##
     + ## t/t4142-apply-icase.sh (new) ##
      @@
      +#!/bin/sh
      +
 2:  1226fbd3caf = 2:  ab1cdd95e03 reset: new failing test for reset of case-insensitive duplicate in index
 3:  04d83283716 ! 3:  52359738532 apply: support case-only renames in case-insensitive filesystems
     @@ Commit message
          account for case-insensitive filesystems yet.
      
          Because the index is inherently case-sensitive even on a
     -    case-insensitive filesystem, we actually need this mechanism to be
     +    case-insensitive filesystem, we actually need this mechanism to
          handle both requirements, lest we fail to account for conflicting
          files only in the index.
      
     @@ apply.c: void clear_apply_state(struct apply_state *state)
      +	 */
       }
       
     - static void mute_routine(const char *msg, va_list params)
     + static void mute_routine(const char *msg UNUSED, va_list params UNUSED)
      @@ apply.c: static int read_file_or_gitlink(const struct cache_entry *ce, struct strbuf *buf
       	return read_blob_object(buf, &ce->oid, ce->ce_mode);
       }
     @@ apply.h: struct apply_state {
       	/*
       	 * This is to save reporting routines before using
      
     - ## t/t4141-apply-icase.sh ##
     -@@ t/t4141-apply-icase.sh: test_expect_success setup '
     + ## t/t4142-apply-icase.sh ##
     +@@ t/t4142-apply-icase.sh: test_expect_success setup '
              git diff HEAD HEAD^ -- file1 >deletion-patch &&
              git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&
              git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&
     @@ t/t4141-apply-icase.sh: test_expect_success setup '
       '
       
       # Basic creation, deletion, modification and renaming.
     -@@ t/t4141-apply-icase.sh: test_expect_success 'creation and deletion' '
     +@@ t/t4142-apply-icase.sh: test_expect_success 'creation and deletion' '
              test_must_fail git rev-parse --verify :file1
       '
       
     @@ t/t4141-apply-icase.sh: test_expect_success 'creation and deletion' '
              # start at "initial" with file0 only
              git reset --hard initial &&
       
     -@@ t/t4141-apply-icase.sh: test_expect_success 'modificaiton' '
     +@@ t/t4142-apply-icase.sh: test_expect_success 'modificaiton' '
              test_cmp_rev :file0 "$file0blob"
       '
       
     @@ t/t4141-apply-icase.sh: test_expect_success 'modificaiton' '
              # start from file0 and file1
              git reset --hard current &&
       
     -@@ t/t4141-apply-icase.sh: test_expect_success 'rename file1 to file2' '
     +@@ t/t4142-apply-icase.sh: test_expect_success 'rename file1 to file2' '
              test_cmp_rev :file1 "$file1blob"
       '
       
     @@ t/t4141-apply-icase.sh: test_expect_success 'rename file1 to file2' '
              # start from file0 and file1
              git reset --hard current &&
       
     -@@ t/t4141-apply-icase.sh: test_expect_success 'rename file1 to file2' '
     +@@ t/t4142-apply-icase.sh: test_expect_success 'rename file1 to file2' '
              test_cmp_rev :file1 "$file1blob"
       '
       

-- 
gitgitgadget

[PATCH v3 1/3] t4142: test "git apply" with core.ignorecase

[Junio C Hamano via GitGitGadget]

From: Junio C Hamano <gitster@pobox.com>

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4142-apply-icase.sh | 128 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100755 t/t4142-apply-icase.sh

diff --git a/t/t4142-apply-icase.sh b/t/t4142-apply-icase.sh
new file mode 100755
index 00000000000..17eb023a437
--- /dev/null
+++ b/t/t4142-apply-icase.sh
@@ -0,0 +1,128 @@
+#!/bin/sh
+
+test_description='git apply with core.ignorecase'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+       # initial commit has file0 only
+       test_commit "initial" file0 "initial commit with file0" initial &&
+
+       # current commit has file1 as well
+       test_commit "current" file1 "initial content of file1" current &&
+       file0blob=$(git rev-parse :file0) &&
+       file1blob=$(git rev-parse :file1) &&
+
+       # prepare sample patches
+       # file0 is modified
+       echo modification to file0 >file0 &&
+       git add file0 &&
+       modifiedfile0blob=$(git rev-parse :file0) &&
+
+       # file1 is removed and then ...
+       git rm --cached file1 &&
+       # ... identical copies are placed at File1 and file2
+       git update-index --add --cacheinfo 100644,$file1blob,file2 &&
+       git update-index --add --cacheinfo 100644,$file1blob,File1 &&
+
+       # then various patches to do basic things
+       git diff HEAD^ HEAD -- file1 >creation-patch &&
+       git diff HEAD HEAD^ -- file1 >deletion-patch &&
+       git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&
+       git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&
+       git diff --cached HEAD -- file0 >modify-file0-patch
+'
+
+# Basic creation, deletion, modification and renaming.
+test_expect_success 'creation and deletion' '
+       # start at "initial" with file0 only
+       git reset --hard initial &&
+
+       # add file1
+       git -c core.ignorecase=false apply --cached creation-patch &&
+       test_cmp_rev :file1 "$file1blob" &&
+
+       # remove file1
+       git -c core.ignorecase=false apply --cached deletion-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --cached creation-patch &&
+       test_cmp_rev :file1 "$file1blob" &&
+       git -c core.ignorecase=true apply --cached deletion-patch &&
+       test_must_fail git rev-parse --verify :file1
+'
+
+test_expect_success 'modificaiton' '
+       # start at "initial" with file0 only
+       git reset --hard initial &&
+
+       # modify file0
+       git -c core.ignorecase=false apply --cached modify-file0-patch &&
+       test_cmp_rev :file0 "$modifiedfile0blob" &&
+       git -c core.ignorecase=false apply --cached -R modify-file0-patch &&
+       test_cmp_rev :file0 "$file0blob" &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --cached modify-file0-patch &&
+       test_cmp_rev :file0 "$modifiedfile0blob" &&
+       git -c core.ignorecase=true apply --cached -R modify-file0-patch &&
+       test_cmp_rev :file0 "$file0blob"
+'
+
+test_expect_success 'rename file1 to file2' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # rename file1 to file2
+       git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :file2 "$file1blob" &&
+       git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file2 &&
+       test_cmp_rev :file1 "$file1blob" &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :file2 "$file1blob" &&
+       git -c core.ignorecase=true apply --cached -R rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file2 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success 'rename file1 to file2' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # rename file1 to File1
+       git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :File1 "$file1blob" &&
+       git -c core.ignorecase=false apply --cached -R rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_cmp_rev :file1 "$file1blob" &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :File1 "$file1blob" &&
+       git -c core.ignorecase=true apply --cached -R rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+# We may want to add tests with working tree here, without "--cached" and
+# with and without "--index" here.  For example, should modify-file0-patch
+# apply cleanly if we have File0 with $file0blob in the index and the working
+# tree if core.icase is set?
+
+test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '
+       : sample
+'
+
+test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '
+       : sample
+'
+
+test_done
-- 
gitgitgadget

[PATCH v3 2/3] reset: new failing test for reset of case-insensitive duplicate in index

[Tao Klerks via GitGitGadget]

From: Tao Klerks <tao@klerks.biz>

On case-insensitive filesystems, where core.ignorecase is normally set,
the index is still case-sensitive, and surprising outcomes are possible
when the index contains states that cannot be represented on the file
system.

Add an "expect_failure" test to illustrate one such situation, where two
files differing only in case are in the index, and a "reset --hard" ends
up creating an unexpected worktree change.

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
 t/t7104-reset-hard.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh
index cf9697eba9a..55050ac831e 100755
--- a/t/t7104-reset-hard.sh
+++ b/t/t7104-reset-hard.sh
@@ -44,4 +44,15 @@ test_expect_success 'reset --hard did not corrupt index or cache-tree' '
 
 '
 
+test_expect_failure CASE_INSENSITIVE_FS 'reset --hard handles index-only case-insensitive duplicate' '
+	test_commit "initial" file1 "initial commit with file1" initial &&
+	file1blob=$(git rev-parse :file1) &&
+	git update-index --add --cacheinfo 100644,$file1blob,File1 &&
+
+	# reset --hard accidentally leaves the working tree with a deleted file.
+	git reset --hard &&
+	git status --porcelain -uno >wt_changes_remaining &&
+	test_must_be_empty wt_changes_remaining
+'
+
 test_done
-- 
gitgitgadget

[PATCH v3 3/3] apply: support case-only renames in case-insensitive filesystems

[Tao Klerks via GitGitGadget]

From: Tao Klerks <tao@klerks.biz>

"git apply" checks, when validating a patch, to ensure that any files
being added aren't already in the worktree.

When this check runs on a case-only rename, in a case-insensitive
filesystem, this leads to a false positive - the command fails with an
error like:
error: File1: already exists in working directory

There is a mechanism to ensure that "seemingly conflicting" files are
handled correctly - for example overlapping rename pairs or swaps -
this mechanism treats renames as add/remove pairs, and would end up
treating a case-only rename as a "self-swap"... Except it does not
account for case-insensitive filesystems yet.

Because the index is inherently case-sensitive even on a
case-insensitive filesystem, we actually need this mechanism to
handle both requirements, lest we fail to account for conflicting
files only in the index.

Fix the "rename chain" existence exemption mechanism to account for
case-insensitive config, fixing case-only-rename-handling as a
"self-swap" and also fixing less-common "case-insensitive rename
pairs" when config core.ignorecase is set, but keep the index checks
file-sensitive.

Also add test cases around these behaviors - verifying that conflicting
file conditions are still caught correctly, including case-only
conflicts on case-sensitive filesystems, and edge cases around
case-sensitive index behaviors on a case-insensitive filesystem.

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
 apply.c                |  81 ++++++++++++++++------
 apply.h                |   5 +-
 t/t4142-apply-icase.sh | 154 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 207 insertions(+), 33 deletions(-)

diff --git a/apply.c b/apply.c
index 6212ab3a1b3..a2e2f6b531d 100644
--- a/apply.c
+++ b/apply.c
@@ -113,7 +113,9 @@ int init_apply_state(struct apply_state *state,
 	state->ws_error_action = warn_on_ws_error;
 	state->ws_ignore_action = ignore_ws_none;
 	state->linenr = 1;
-	string_list_init_nodup(&state->fn_table);
+	string_list_init_nodup(&state->fs_fn_table);
+	state->fs_fn_table.cmp = fspathcmp;
+	string_list_init_nodup(&state->index_fn_table);
 	string_list_init_nodup(&state->limit_by_name);
 	strset_init(&state->removed_symlinks);
 	strset_init(&state->kept_symlinks);
@@ -134,7 +136,10 @@ void clear_apply_state(struct apply_state *state)
 	strset_clear(&state->kept_symlinks);
 	strbuf_release(&state->root);
 
-	/* &state->fn_table is cleared at the end of apply_patch() */
+	/*
+	 * &state->fs_fn_table and &state->index_fn_table are cleared at the
+	 * end of apply_patch()
+	 */
 }
 
 static void mute_routine(const char *msg UNUSED, va_list params UNUSED)
@@ -3294,14 +3299,28 @@ static int read_file_or_gitlink(const struct cache_entry *ce, struct strbuf *buf
 	return read_blob_object(buf, &ce->oid, ce->ce_mode);
 }
 
-static struct patch *in_fn_table(struct apply_state *state, const char *name)
+static struct patch *in_fs_fn_table(struct apply_state *state, const char *name)
 {
 	struct string_list_item *item;
 
 	if (!name)
 		return NULL;
 
-	item = string_list_lookup(&state->fn_table, name);
+	item = string_list_lookup(&state->fs_fn_table, name);
+	if (item)
+		return (struct patch *)item->util;
+
+	return NULL;
+}
+
+static struct patch *in_index_fn_table(struct apply_state *state, const char *name)
+{
+	struct string_list_item *item;
+
+	if (!name)
+		return NULL;
+
+	item = string_list_lookup(&state->index_fn_table, name);
 	if (item)
 		return (struct patch *)item->util;
 
@@ -3333,7 +3352,7 @@ static int was_deleted(struct patch *patch)
 	return patch == PATH_WAS_DELETED;
 }
 
-static void add_to_fn_table(struct apply_state *state, struct patch *patch)
+static void add_to_fn_tables(struct apply_state *state, struct patch *patch)
 {
 	struct string_list_item *item;
 
@@ -3343,7 +3362,9 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)
 	 * file creations and copies
 	 */
 	if (patch->new_name) {
-		item = string_list_insert(&state->fn_table, patch->new_name);
+		item = string_list_insert(&state->fs_fn_table, patch->new_name);
+		item->util = patch;
+		item = string_list_insert(&state->index_fn_table, patch->new_name);
 		item->util = patch;
 	}
 
@@ -3352,7 +3373,9 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)
 	 * later chunks shouldn't patch old names
 	 */
 	if ((patch->new_name == NULL) || (patch->is_rename)) {
-		item = string_list_insert(&state->fn_table, patch->old_name);
+		item = string_list_insert(&state->fs_fn_table, patch->old_name);
+		item->util = PATH_WAS_DELETED;
+		item = string_list_insert(&state->index_fn_table, patch->old_name);
 		item->util = PATH_WAS_DELETED;
 	}
 }
@@ -3365,7 +3388,9 @@ static void prepare_fn_table(struct apply_state *state, struct patch *patch)
 	while (patch) {
 		if ((patch->new_name == NULL) || (patch->is_rename)) {
 			struct string_list_item *item;
-			item = string_list_insert(&state->fn_table, patch->old_name);
+			item = string_list_insert(&state->fs_fn_table, patch->old_name);
+			item->util = PATH_TO_BE_DELETED;
+			item = string_list_insert(&state->index_fn_table, patch->old_name);
 			item->util = PATH_TO_BE_DELETED;
 		}
 		patch = patch->next;
@@ -3395,7 +3420,7 @@ static struct patch *previous_patch(struct apply_state *state,
 	if (patch->is_copy || patch->is_rename)
 		return NULL; /* "git" patches do not depend on the order */
 
-	previous = in_fn_table(state, patch->old_name);
+	previous = in_index_fn_table(state, patch->old_name);
 	if (!previous)
 		return NULL;
 
@@ -3706,7 +3731,7 @@ static int apply_data(struct apply_state *state, struct patch *patch,
 	}
 	patch->result = image.buf;
 	patch->resultsize = image.len;
-	add_to_fn_table(state, patch);
+	add_to_fn_tables(state, patch);
 	free(image.line_allocated);
 
 	if (0 < patch->is_delete && patch->resultsize)
@@ -3805,11 +3830,12 @@ static int check_preimage(struct apply_state *state,
 
 static int check_to_create(struct apply_state *state,
 			   const char *new_name,
-			   int ok_if_exists)
+			   int ok_if_exists_in_fs,
+			   int ok_if_exists_in_index)
 {
 	struct stat nst;
 
-	if (state->check_index && (!ok_if_exists || !state->cached)) {
+	if (state->check_index && (!ok_if_exists_in_index || !state->cached)) {
 		int pos;
 
 		pos = index_name_pos(state->repo->index, new_name, strlen(new_name));
@@ -3817,7 +3843,7 @@ static int check_to_create(struct apply_state *state,
 			struct cache_entry *ce = state->repo->index->cache[pos];
 
 			/* allow ITA, as they do not yet exist in the index */
-			if (!ok_if_exists && !(ce->ce_flags & CE_INTENT_TO_ADD))
+			if (!ok_if_exists_in_index && !(ce->ce_flags & CE_INTENT_TO_ADD))
 				return EXISTS_IN_INDEX;
 
 			/* ITA entries can never match working tree files */
@@ -3830,7 +3856,7 @@ static int check_to_create(struct apply_state *state,
 		return 0;
 
 	if (!lstat(new_name, &nst)) {
-		if (S_ISDIR(nst.st_mode) || ok_if_exists)
+		if (S_ISDIR(nst.st_mode) || ok_if_exists_in_fs)
 			return 0;
 		/*
 		 * A leading component of new_name might be a symlink
@@ -3940,7 +3966,8 @@ static int check_patch(struct apply_state *state, struct patch *patch)
 	const char *name = old_name ? old_name : new_name;
 	struct cache_entry *ce = NULL;
 	struct patch *tpatch;
-	int ok_if_exists;
+	int ok_if_exists_in_fs;
+	int ok_if_exists_in_index;
 	int status;
 
 	patch->rejected = 1; /* we will drop this after we succeed */
@@ -3963,16 +3990,29 @@ static int check_patch(struct apply_state *state, struct patch *patch)
 	 * B; ask to_be_deleted() about the later rename.  Removal of
 	 * B and rename from A to B is handled the same way by asking
 	 * was_deleted().
+	 *
+	 * These exemptions account for the core.ignorecase config -
+	 * a file that differs only by case is also considered "deleted"
+	 * if git is configured to ignore case. This means a case-only
+	 * rename, in a case-insensitive filesystem, is treated here as
+	 * a "self-swap" or mode change.
 	 */
-	if ((tpatch = in_fn_table(state, new_name)) &&
+	if ((tpatch = in_fs_fn_table(state, new_name)) &&
+	    (was_deleted(tpatch) || to_be_deleted(tpatch)))
+		ok_if_exists_in_fs = 1;
+	else
+		ok_if_exists_in_fs = 0;
+
+	if ((tpatch = in_index_fn_table(state, new_name)) &&
 	    (was_deleted(tpatch) || to_be_deleted(tpatch)))
-		ok_if_exists = 1;
+		ok_if_exists_in_index = 1;
 	else
-		ok_if_exists = 0;
+		ok_if_exists_in_index = 0;
 
 	if (new_name &&
 	    ((0 < patch->is_new) || patch->is_rename || patch->is_copy)) {
-		int err = check_to_create(state, new_name, ok_if_exists);
+		int err = check_to_create(state, new_name, ok_if_exists_in_fs,
+					  ok_if_exists_in_index);
 
 		if (err && state->threeway) {
 			patch->direct_to_threeway = 1;
@@ -4870,7 +4910,8 @@ static int apply_patch(struct apply_state *state,
 end:
 	free_patch_list(list);
 	strbuf_release(&buf);
-	string_list_clear(&state->fn_table, 0);
+	string_list_clear(&state->fs_fn_table, 0);
+	string_list_clear(&state->index_fn_table, 0);
 	return res;
 }
 
diff --git a/apply.h b/apply.h
index 7cd38b1443c..a1419672507 100644
--- a/apply.h
+++ b/apply.h
@@ -95,8 +95,11 @@ struct apply_state {
 	/*
 	 * Records filenames that have been touched, in order to handle
 	 * the case where more than one patches touch the same file.
+	 * Two separate structures because with ignorecase, one of them
+	 * needs to be case-insensitive and the other not.
 	 */
-	struct string_list fn_table;
+	struct string_list fs_fn_table;
+	struct string_list index_fn_table;
 
 	/*
 	 * This is to save reporting routines before using
diff --git a/t/t4142-apply-icase.sh b/t/t4142-apply-icase.sh
index 17eb023a437..1c785133d16 100755
--- a/t/t4142-apply-icase.sh
+++ b/t/t4142-apply-icase.sh
@@ -30,7 +30,16 @@ test_expect_success setup '
        git diff HEAD HEAD^ -- file1 >deletion-patch &&
        git diff --cached HEAD -- file1 file2 >rename-file1-to-file2-patch &&
        git diff --cached HEAD -- file1 File1 >rename-file1-to-File1-patch &&
-       git diff --cached HEAD -- file0 >modify-file0-patch
+       git diff --cached HEAD -- file0 >modify-file0-patch &&
+
+       # then set up for swap
+       git reset --hard current &&
+       test_commit "swappable" file3 "different content for file3" swappable &&
+       file3blob=$(git rev-parse :file3) &&
+       git rm --cached file1 file3 &&
+       git update-index --add --cacheinfo 100644,$file1blob,File3 &&
+       git update-index --add --cacheinfo 100644,$file3blob,File1 &&
+       git diff --cached HEAD -- file1 file3 File1 File3 >swap-file1-and-file3-to-File3-and-File1-patch
 '
 
 # Basic creation, deletion, modification and renaming.
@@ -53,7 +62,7 @@ test_expect_success 'creation and deletion' '
        test_must_fail git rev-parse --verify :file1
 '
 
-test_expect_success 'modificaiton' '
+test_expect_success 'modification (index-only)' '
        # start at "initial" with file0 only
        git reset --hard initial &&
 
@@ -70,7 +79,7 @@ test_expect_success 'modificaiton' '
        test_cmp_rev :file0 "$file0blob"
 '
 
-test_expect_success 'rename file1 to file2' '
+test_expect_success 'rename file1 to file2 (index-only)' '
        # start from file0 and file1
        git reset --hard current &&
 
@@ -91,7 +100,7 @@ test_expect_success 'rename file1 to file2' '
        test_cmp_rev :file1 "$file1blob"
 '
 
-test_expect_success 'rename file1 to file2' '
+test_expect_success 'rename file1 to File1 (index-only)' '
        # start from file0 and file1
        git reset --hard current &&
 
@@ -112,17 +121,138 @@ test_expect_success 'rename file1 to file2' '
        test_cmp_rev :file1 "$file1blob"
 '
 
-# We may want to add tests with working tree here, without "--cached" and
-# with and without "--index" here.  For example, should modify-file0-patch
-# apply cleanly if we have File0 with $file0blob in the index and the working
-# tree if core.icase is set?
+# involve filesystem on renames
+test_expect_success 'rename file1 to File1 (with ignorecase, working tree)' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --index rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :File1 "$file1blob" &&
+       git -c core.ignorecase=true apply --index -R rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success CASE_INSENSITIVE_FS 'rename file1 to File1 (without ignorecase, case-insensitive FS)' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # rename file1 to File1 without ignorecase (fails as expected)
+       test_must_fail git -c core.ignorecase=false apply --index rename-file1-to-File1-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success !CASE_INSENSITIVE_FS 'rename file1 to File1 (without ignorecase, case-sensitive FS)' '
+       # start from file0 and file1
+       git reset --hard current &&
+
+       # rename file1 to File1 without ignorecase
+       git -c core.ignorecase=false apply --index rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :File1 "$file1blob" &&
+       git -c core.ignorecase=false apply --index -R rename-file1-to-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_cmp_rev :file1 "$file1blob"
+'
+
+test_expect_success 'rename file1 to file2 with working tree conflict' '
+       # start from file0 and file1, and file2 untracked
+       git reset --hard current &&
+       test_when_finished "rm file2" &&
+       touch file2 &&
+
+       # rename file1 to file2 with conflict
+       test_must_fail git -c core.ignorecase=false apply --index rename-file1-to-file2-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob" &&
 
-test_expect_success CASE_INSENSITIVE_FS 'a test only for icase fs' '
-       : sample
+       # do the same with ignorecase
+       test_must_fail git -c core.ignorecase=true apply --index rename-file1-to-file2-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob"
 '
 
-test_expect_success !CASE_INSENSITIVE_FS 'a test only for !icase fs' '
-       : sample
+test_expect_success 'rename file1 to file2 with case-insensitive conflict (index-only - ignorecase disabled)' '
+       # start from file0 and file1, and File2 in index
+       git reset --hard current &&
+       git update-index --add --cacheinfo 100644,$file3blob,File2 &&
+
+       # rename file1 to file2 without ignorecase
+       git -c core.ignorecase=false apply --cached rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_cmp_rev :file2 "$file1blob" &&
+       git -c core.ignorecase=false apply --cached -R rename-file1-to-file2-patch &&
+       test_must_fail git rev-parse --verify :file2 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :File2 "$file3blob"
+'
+
+test_expect_failure 'rename file1 to file2 with case-insensitive conflict (index-only - ignorecase enabled)' '
+       # start from file0 and file1, and File2 in index
+       git reset --hard current &&
+       git update-index --add --cacheinfo 100644,$file3blob,File2 &&
+
+       # rename file1 to file2 with ignorecase, with a "File2" conflicting file in place - expect failure.
+       # instead of failure, we get success with "File1" and "file1" both existing in the index, despite
+       # the ignorecase configuration.
+       test_must_fail git -c core.ignorecase=true apply --cached rename-file1-to-file2-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :File2 "$file3blob"
+'
+
+test_expect_success 'rename file1 to File1 with case-sensitive conflict (index-only)' '
+       # start from file0 and file1, and File1 in index
+       git reset --hard current &&
+       git update-index --add --cacheinfo 100644,$file3blob,File1 &&
+
+       # On a case-insensitive filesystem with core.ignorecase on, a single git
+       # "reset --hard" will actually leave things wrong because of the
+       # index-to-working-tree discrepancy - see "reset --hard handles
+       # index-only case-insensitive duplicate" under t7104-reset-hard.sh.
+       # We are creating this unexpected state, so we should explicitly queue
+       # an extra reset. If reset ever starts to handle this case, this will
+       # become unnecessary but also not harmful.
+       test_when_finished "git reset --hard" &&
+
+       # rename file1 to File1 when File1 is already in index (fails with conflict)
+       test_must_fail git -c core.ignorecase=false apply --cached rename-file1-to-File1-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :File1 "$file3blob" &&
+
+       # do the same with ignorecase
+       test_must_fail git -c core.ignorecase=true apply --cached rename-file1-to-File1-patch &&
+       git rev-parse --verify :file1 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :File1 "$file3blob"
+'
+
+test_expect_success CASE_INSENSITIVE_FS 'case-insensitive swap - file1 to File2 and file2 to File1 (working tree)' '
+       # start from file0, file1, and file3
+       git reset --hard swappable &&
+
+       # "swap" file1 and file3 to case-insensitive versions without ignorecase on case-insensitive FS (fails as expected)
+       test_must_fail git -c core.ignorecase=false apply --index swap-file1-and-file3-to-File3-and-File1-patch &&
+       git rev-parse --verify :file1 &&
+       git rev-parse --verify :file3 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :file3 "$file3blob" &&
+
+       # do the same with ignorecase
+       git -c core.ignorecase=true apply --index swap-file1-and-file3-to-File3-and-File1-patch &&
+       test_must_fail git rev-parse --verify :file1 &&
+       test_must_fail git rev-parse --verify :file3 &&
+       test_cmp_rev :File3 "$file1blob" &&
+       test_cmp_rev :File1 "$file3blob" &&
+       git -c core.ignorecase=true apply --index -R swap-file1-and-file3-to-File3-and-File1-patch &&
+       test_must_fail git rev-parse --verify :File1 &&
+       test_must_fail git rev-parse --verify :File3 &&
+       test_cmp_rev :file1 "$file1blob" &&
+       test_cmp_rev :file3 "$file3blob"
 '
 
 test_done
-- 
gitgitgadget