Re: [PATCH] apply: support case-only renames in case-insensitive filesystems
- From
Tao Klerks <tao@klerks.biz>
- Date
- Jun 14, 2022, 06:16 UTC
- Message-ID
- <CAPMMpogTcKqw6SHon9soj_CqPf-E8SmHpJ1FRRBKaCcOVnyHRg@mail.gmail.com>
- In-Reply-To
- <xmqqr13t8np7.fsf@gitster.g>
On Mon, Jun 13, 2022 at 1:30 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 11 quoted lines
> > "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.
Show 8 quoted lines
> > 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).
Show 8 quoted lines
> > > +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.
Show 21 quoted lines
> > + 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.
Show 25 quoted lines
> 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??)
Show 24 quoted lines
> > +' > > + > > +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).
Show 7 quoted lines
> > +' > > + > > +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.
Show 9 quoted lines
> > + 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?
Show 9 quoted lines
> > + git apply --index case_only_rename_patch > > +' > > + > > +test_done > > > > base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64 > > Thanks. >
Thank you!