Fix a test on NTFS (and probably HFS+)
cover Fix a test on NTFS (and probably HFS+)
My colleague Jameson Miller once presented me with a nice puzzle why the test suite failed on their system.
Turns out that it is possible in PowerShell to spell the directory with a different case than on disk in cd <directory>, and subsequent calls to get the current working directory will use that case, rather than what is recorded on disk. And since case-insensitive filesystems are frowned upon in the Linux world, Git's test suite was not prepared for such a scenario.
Johannes Schindelin (1):
t0001: fix on case-insensitive filesystems
t/t0001-init.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-151%2Fdscho%2Ffunny-cased-cwd-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-151/dscho/funny-cased-cwd-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/151
Signature · 1 line
gitgitgadget
PATCH 1/1 t0001: fix on case-insensitive filesystems
From: Johannes Schindelin <[email protected]>
On a case-insensitive filesystem, such as HFS+ or NTFS, it is possible that the idea Bash has of the current directory differs in case from what Git thinks it is. That's totally okay, though, and we should not expect otherwise.
Reported by Jameson Miller.
--- t/t0001-init.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)
t/t0001-init.sh +12 -0
@@ -307,10 +307,20 @@ test_expect_success 'init prefers command line to GIT_DIR' ' test_path_is_missing otherdir/refs ' +downcase_on_case_insensitive_fs () { + test false = "$(git config --get core.filemode)" || return 0 + for f + do + tr A-Z a-z <"$f" >"$f".downcased && + mv -f "$f".downcased "$f" || return 1 + done +} + test_expect_success 'init with separate gitdir' ' rm -rf newdir && git init --separate-git-dir realgitdir newdir && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir realgitdir/refs '
@@ -365,6 +375,7 @@ test_expect_success 're-init to update git link' ' git init --separate-git-dir ../surrealgitdir ) && echo "gitdir: $(pwd)/surrealgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir surrealgitdir/refs && test_path_is_missing realgitdir/refs
@@ -378,6 +389,7 @@ test_expect_success 're-init to move gitdir' ' git init --separate-git-dir ../realgitdir ) && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir realgitdir/refs '
Signature · 1 line
gitgitgadget
Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems
Hi Dscho,
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 311
Quoted 10 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
On a case-insensitive filesystem, such as HFS+ or NTFS, it is possible that the idea Bash has of the current directory differs in case from what Git thinks it is. That's totally okay, though, and we should not expect otherwise.
t/t0001-init.sh +2
-0+downcase_on_case_insensitive_fs () { + test false = "$(git config --get core.filemode)" || return 0
I think it would be worthwhile to add --type=bool to this git-config call. See, e.g., the FILEMODE prereq in t/test-lib.sh. From my understanding, this check would regress if someone did s/false/no/ in builtin/init-db.c, so this check is perhaps not as robust as it could be. (Now, as for *why* someone would do such a change...)
I do wonder if this is the right way to check for a case-insensitive filesystem. According to git-config(1), this variable tells whether "the executable bit of files in the working tree is to be honored". I can see how this property could correlate with the filesystem being case-insensitive, but from git-config(1), I would have expected core.ignoreCase to be queried instead.
You're no doubt a lot more familiar with filesystem case-insensitivity and how it interacts with Git than I am. Any light you could shed on this would be much appreciated.
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 315
Quoted 4 lines
t/t0001-init.sh +4
-0+ for f + do + tr A-Z a-z <"$f" >"$f".downcased && + mv -f "$f".downcased "$f" || return 1
Makes sense. Good error-handling.
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 317
Quoted 2 lines
t/t0001-init.sh +2
-0+ done +}
Cheers
Martin
Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems
In reply to Johannes Schindelin via GitGitGadgett/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 312
Quoted 11 lines · Johannes Schindelin via GitGitGadget
t/t0001-init.sh +3
-0@@ -307,10 +307,20 @@ test_expect_success 'init prefers command line to GIT_DIR' ' test_path_is_missing otherdir/refs ' +downcase_on_case_insensitive_fs () { + test false = "$(git config --get core.filemode)" || return 0 + for f
TIL that “for f” is equivalent to “for f in "$@"”. Thanks for teaching me something new.
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 323
Quoted 11 lines
t/t0001-init.sh +7
-0+ do + tr A-Z a-z <"$f" >"$f".downcased && + mv -f "$f".downcased "$f" || return 1 + done +} + test_expect_success 'init with separate gitdir' ' rm -rf newdir && git init --separate-git-dir realgitdir newdir && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git &&
I wonder if there's maybe a simpler way. If we canonicalize paths when writing them to the gitdir file, then writing "$(pwd -P)" on the line above should produce the right result.
Now, technically, POSIX doesn't require case canonicalization of the path with "pwd -P", but then again, POSIX doesn't permit case-insensitive file systems, and we know the behavior on macOS uses bash, which does the right thing in this case because it calls realpath(3). I've tested that it also does the right thing on Linux when the worktree containing the Git checkout is in a path with symlinks.
I don't know how that works on Windows, but if it does, it might be simpler.
Signature · 2 lines
brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204
Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems
Quoted 17 lines · brian m. carlson
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 323
Quoted 5 lines
test_expect_success'init with separate gitdir' 't/t0001-init.sh +1
-0rm -rf newdir && git init --separate-git-dir realgitdir newdir && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git &&I wonder if there's maybe a simpler way. If we canonicalize paths when writing them to the gitdir file, then writing "$(pwd -P)" on the line above should produce the right result.
Now, technically, POSIX doesn't require case canonicalization of the path with "pwd -P", but then again, POSIX doesn't permit case-insensitive file systems, and we know the behavior on macOS uses bash, which does the right thing in this case because it calls realpath(3). I've tested that it also does the right thing on Linux when the worktree containing the Git checkout is in a path with symlinks.
Quoted 2 lines
I don't know how that works on Windows, but if it does, it might be simpler.
Yup, it would be a worthwhile avenue to pursue; on the negative side, a single-liner "no, unfortunately that would not work on Windows" would also be useful.
Thanks.
Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems
Hi,
Quoted 11 lines · Junio C Hamano
Quoted 9 lines · brian m. carlson
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 323
Quoted 5 lines
test_expect_success'init with separate gitdir' 't/t0001-init.sh +1
-0rm -rf newdir && git init --separate-git-dir realgitdir newdir && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git &&I wonder if there's maybe a simpler way. If we canonicalize paths when writing them to the gitdir file, then writing "$(pwd -P)" on the line above should produce the right result.
When you execute pwd -P in Git for Windows' SDK's Bash (which is an MSYS2 Bash), you get the emulated Unix path. For example, in my main Git worktree, this results in
$ pwd -P /usr/src/git
However, the actual (i.e. Windows) path seen by git.exe is C:/git-sdk-64/usr/src/git.
Quoted 3 lines
Quoted 3 lines
Now, technically, POSIX doesn't require case canonicalization of the path with "pwd -P", but then again, POSIX doesn't permit case-insensitive file systems,
I was not sure about this statement (that POSIX does not permit case-insensitive file systems), so I whipped up this gem in a quick web search (https://unix.stackexchange.com/a/358407):
According to the POSIX specification:
The system may provide non-standard extensions. These are features not required by POSIX.1-2008 and may include, but are not limited to:
--snip--
Non-conforming file systems (for example, legacy file systems for which _POSIX_NO_TRUNC is false, case-insensitive file systems, or network file systems)
Which means that I now know once and for all that POSIX does not dictate whether a file system should, or should not, be case-insensitive.
Quoted 4 lines
Quoted 4 lines
and we know the behavior on macOS uses bash, which does the right thing in this case because it calls realpath(3). I've tested that it also does the right thing on Linux when the worktree containing the Git checkout is in a path with symlinks.
I am honestly not a big fan of relying on testing things on the major three platforms and then assuming that everything will work also on the long tail of operating systems/setups.
That is exactly the kind of thinking that led me to believe that relying on REG_STARTEND was a sane thing, and it simply wasn't. It caused quite a bit of pain, and my original approach would have prevented that, and after testing on the major three platforms I let myself be talked into dropping the original approach.
Quoted 6 lines
Quoted 2 lines
I don't know how that works on Windows, but if it does, it might be simpler.
Yup, it would be a worthwhile avenue to pursue; on the negative side, a single-liner "no, unfortunately that would not work on Windows" would also be useful.
1) no, unfortunately that would not work on Windows. In a PowerShell, when I call c: followed by cd \GIT-SDK-64, it reports the current directory as C:\GIT-SDK-64 (i.e. with the wrong case, the real name, on disk, is C:\git-sdk-64). When I then call Git's Bash to execute git -PW (the W stands for: gimme a Windows path), it reports C:/GIT-SDK-64. Incorrect case.
2) It might look more elegant from the design perspective, but oh my goodness do I not want to be a developer who has no knowledge of this design decision, being tasked to debug any related issue.
It would be very "magic" that Git relies on its having written a normalized path, not dealing well with Git worktrees initialized by alternative Git implementations that did not normalize that path, or previous Git versions that also did not, for example.
And normalizing the path for the sake of having this test case pass? Nah. I like it explicit. And that's what my patch does. No magic.
Ciao,
Dscho
Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems
In reply to Martin ÅgrenHi Martin,
Quoted 19 lines · Martin Ågren
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 311
Quoted 10 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
On a case-insensitive filesystem, such as HFS+ or NTFS, it is possible that the idea Bash has of the current directory differs in case from what Git thinks it is. That's totally okay, though, and we should not expect otherwise.
t/t0001-init.sh +2
-0+downcase_on_case_insensitive_fs () { + test false = "$(git config --get core.filemode)" || return 0I think it would be worthwhile to add
--type=boolto this git-config call. See, e.g., the FILEMODE prereq int/test-lib.sh. From my understanding, this check would regress if someone did s/false/no/ inbuiltin/init-db.c, so this check is perhaps not as robust as it could be. (Now, as for *why* someone would do such a change...)
Quoted 6 lines
I do wonder if this is the right way to check for a case-insensitive filesystem. According to git-config(1), this variable tells whether "the executable bit of files in the working tree is to be honored". I can see how this property could correlate with the filesystem being case-insensitive, but from git-config(1), I would have expected core.ignoreCase to be queried instead.
Oh my, you're right. I do not know how the filemode slipped in...
Thanks!
Dscho
Quoted 17 lines
You're no doubt a lot more familiar with filesystem case-insensitivity and how it interacts with Git than I am. Any light you could shed on this would be much appreciated.
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 315
Quoted 4 lines
t/t0001-init.sh +4
-0+ for f + do + tr A-Z a-z <"$f" >"$f".downcased && + mv -f "$f".downcased "$f" || return 1Makes sense. Good error-handling.
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 317
Quoted 2 lines
t/t0001-init.sh +2
-0+ done +}Cheers
Martin
Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems
In reply to a message not in the archive[Re-sending, as the Git mailing list seems to have decided to silently drop this patch, I cannot see it on public-inbox.org, at least]
From: Johannes Schindelin <[email protected]>
On a case-insensitive filesystem, such as HFS+ or NTFS, it is possible that the idea Bash has of the current directory differs in case from what Git thinks it is. That's totally okay, though, and we should not expect otherwise.
On Windows, for example, when you call
cd C:\GIT-SDK-64
in a PowerShell and there exists a directory called C:\git-sdk-64, the current directory will be reported in all upper-case. Even in a Bash that you might call from that PowerShell. Git, however, will have normalized this via GetFinalPathByHandle(), and the expectation in t0001 that the recorded gitdir will match what pwd says will be violated.
Let's address this by forcing the comparison to be case-insensitive when core.ignoreCase is true.
Reported by Jameson Miller.
--- t/t0001-init.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
t/t0001-init.sh +14 -0
@@ -307,10 +307,22 @@ test_expect_success 'init prefers command line to GIT_DIR' ' test_path_is_missing otherdir/refs ' +downcase_on_case_insensitive_fs () { + test true = "$(git config --get --type=bool core.ignorecase)" || + return 0 + + for f + do + tr A-Z a-z <"$f" >"$f".downcased && + mv -f "$f".downcased "$f" || return 1 + done +} + test_expect_success 'init with separate gitdir' ' rm -rf newdir && git init --separate-git-dir realgitdir newdir && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir realgitdir/refs '
@@ -365,6 +377,7 @@ test_expect_success 're-init to update git link' ' git init --separate-git-dir ../surrealgitdir ) && echo "gitdir: $(pwd)/surrealgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir surrealgitdir/refs && test_path_is_missing realgitdir/refs
@@ -378,6 +391,7 @@ test_expect_success 're-init to move gitdir' ' git init --separate-git-dir ../realgitdir ) && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir realgitdir/refs '
--
gitgitgadget
Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems
Quoted 2 lines · Johannes Schindelin
[Re-sending, as the Git mailing list seems to have decided to silently drop this patch, I cannot see it on public-inbox.org, at least]
Move that below the three-dash lines; otherwise the above two lines will make your in-body From: ineffective.
Quoted 23 lines
From: Johannes Schindelin <[email protected]>
On a case-insensitive filesystem, such as HFS+ or NTFS, it is possible that the idea Bash has of the current directory differs in case from what Git thinks it is. That's totally okay, though, and we should not
...
Let's address this by forcing the comparison to be case-insensitive whencore.ignoreCaseistrue.Reported by Jameson Miller.
Signed-off-by Johannes Schindelin <[email protected]>--- t/t0001-init.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+)t/t0001-init.sh +0
-0@@ -307,10 +307,22 @@ test_expect_success 'init prefers command line to GIT_DIR' ' test_path_is_missing otherdir/refs
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 317
Quoted 10 lines
t/t0001-init.sh +8
-0' +downcase_on_case_insensitive_fs () { + test true = "$(git config --get --type=bool core.ignorecase)" || + return 0 + + for f + do + tr A-Z a-z <"$f" >"$f".downcased && + mv -f "$f".downcased "$f" || return 1
It is easier to read to enclose the whole filename inside dq pair, i.e.
tr A-Z a-z <"$f" >"$f.downcased" && mv -f "$f.downcased" "$f" || return 1
Not grave enough to be worthy of a reroll on this one alone, though.
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 325
Quoted 5 lines
t/t0001-init.sh +1
-0test_expect_success 'init with separate gitdir' ' rm -rf newdir && git init --separate-git-dir realgitdir newdir && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git &&
It is somewhat brittle to munge what is in "newdir/.git", which will affect later tests that use newdir/ as a repository (this one will be removed soon, but the last one in the sequence this patch touches will persist and is used by other tests). It's not like we are testing that Git will function OK even after futzing with the ".git" pointer file (if so, we'd want a separate test that specifically is designed to test that behaviour).
I wonder if we want (possibly local) test_cmp_fspath that can be used more like
test_cmp_fspath "$(pwd)/realgitdir" "$(sed -e "s/^gitdir: //" newdir/.git)"
to compare two paths, honoring the case sensitivity of the filesystem.
Quoted 21 lines
t/t0001-init.sh +2
-1test_cmp expected newdir/.git && test_path_is_dir realgitdir/refs '@@ -365,6 +377,7 @@ test_expect_success 're-init to update git link' ' git init --separate-git-dir ../surrealgitdir ) && echo "gitdir: $(pwd)/surrealgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir surrealgitdir/refs && test_path_is_missing realgitdir/refs@@ -378,6 +391,7 @@ test_expect_success 're-init to move gitdir' ' git init --separate-git-dir ../realgitdir ) && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir realgitdir/refs ' --gitgitgadget
Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems
In reply to brian m. carlsonQuoted 9 lines · brian m. carlson
Quoted 8 lines · Johannes Schindelin via GitGitGadget
t/t0001-init.sh +0
-0@@ -307,10 +307,20 @@ test_expect_success 'init prefers command line to GIT_DIR' ' test_path_is_missing otherdir/refs '
Quoted 6 lines
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 312
Quoted 3 lines
t/t0001-init.sh +3
-0+downcase_on_case_insensitive_fs () { + test false = "$(git config --get core.filemode)" || return 0 + for fTIL that “for f” is equivalent to “for f in "$@"”. Thanks for teaching me something new.
See also test_have_prereq in test-lib-functions.sh where this trick is combined with IFS to loop over a "param,like,this" split by ",".
Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems
In reply to Junio C HamanoHi Junio,
Quoted 7 lines · Junio C Hamano
Quoted 2 lines · Johannes Schindelin
[Re-sending, as the Git mailing list seems to have decided to silently drop this patch, I cannot see it on public-inbox.org, at least]
Move that below the three-dash lines; otherwise the above two lines will make your in-body From: ineffective.
Indeed, sorry, I forgot.
Quoted 20 lines
Quoted 6 lines
t/t0001-init.sh +0
-0@@ -307,10 +307,22 @@ test_expect_success 'init prefers command line to GIT_DIR' ' test_path_is_missing otherdir/refst/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 317
Quoted 10 lines
t/t0001-init.sh +8
-0' +downcase_on_case_insensitive_fs () { + test true = "$(git config --get --type=bool core.ignorecase)" || + return 0 + + for f + do + tr A-Z a-z <"$f" >"$f".downcased && + mv -f "$f".downcased "$f" || return 1It is easier to read to enclose the whole filename inside dq pair, i.e.
Quoted 2 lines
tr A-Z a-z <"$f" >"$f.downcased" && mv -f "$f.downcased" "$f" || return 1
Okay.
Quoted 16 lines
Not grave enough to be worthy of a reroll on this one alone, though.
t/t0001-init.sh test_expect_success 'init prefers command line to GIT_DIR' ' · line 325
Quoted 5 lines
t/t0001-init.sh +1
-0test_expect_success 'init with separate gitdir' ' rm -rf newdir && git init --separate-git-dir realgitdir newdir && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git &&It is somewhat brittle to munge what is in "newdir/.git", which will affect later tests that use newdir/ as a repository (this one will be removed soon, but the last one in the sequence this patch touches will persist and is used by other tests). It's not like we are testing that Git will function OK even after futzing with the ".git" pointer file (if so, we'd want a separate test that specifically is designed to test that behaviour).
Quoted 7 lines
I wonder if we want (possibly local)
test_cmp_fspaththat can be used more liketest_cmp_fspath "$(pwd)/realgitdir" "$(sed -e "s/^gitdir: //" newdir/.git)"to compare two paths, honoring the case sensitivity of the filesystem.
I agree that that's a much better approach to fix the issue.
Thanks,
Dscho
Quoted 23 lines
Quoted 22 lines
t/t0001-init.sh +2
-1test_cmp expected newdir/.git && test_path_is_dir realgitdir/refs '@@ -365,6 +377,7 @@ test_expect_success 're-init to update git link' ' git init --separate-git-dir ../surrealgitdir ) && echo "gitdir: $(pwd)/surrealgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir surrealgitdir/refs && test_path_is_missing realgitdir/refs@@ -378,6 +391,7 @@ test_expect_success 're-init to move gitdir' ' git init --separate-git-dir ../realgitdir ) && echo "gitdir: $(pwd)/realgitdir" >expected && + downcase_on_case_insensitive_fs expected newdir/.git && test_cmp expected newdir/.git && test_path_is_dir realgitdir/refs ' --gitgitgadget
Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems
Quoted 9 lines · Johannes Schindelin
Quoted 7 lines
I wonder if we want (possibly local)
test_cmp_fspaththat can be used more liketest_cmp_fspath "$(pwd)/realgitdir" "$(sed -e "s/^gitdir: //" newdir/.git)"to compare two paths, honoring the case sensitivity of the filesystem.
I agree that that's a much better approach to fix the issue.
I find that response somewhat surprising :-|. In any case, I am not sure what kind of 'path' other than the filesystem one we would deal with in the context of Git and its test suite, so perhaps we should drop 'fs' from the name of the helper function if we were to go that route.
Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems
Hi Junio,
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Johannes Schindelin
Quoted 7 lines
I wonder if we want (possibly local)
test_cmp_fspaththat can be used more liketest_cmp_fspath "$(pwd)/realgitdir" "$(sed -e "s/^gitdir: //" newdir/.git)"to compare two paths, honoring the case sensitivity of the filesystem.
Quoted 7 lines
Quoted 1 line
I agree that that's a much better approach to fix the issue.
I find that response somewhat surprising :-|. In any case, I am not sure what kind of 'path' other than the filesystem one we would deal with in the context of Git and its test suite, so perhaps we should drop 'fs' from the name of the helper function if we were to go that route.
The other path category is the paths in the index, which _are_ case-sensitive, no matter what core.ignoreCase says.
So I'd rather keep the fs.
Ciao,
Dscho
Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems
In reply to a message not in the archiveFrom: Johannes Schindelin <[email protected]>
On a case-insensitive filesystem, such as HFS+ or NTFS, it is possible that the idea Bash has of the current directory differs in case from what Git thinks it is. That's totally okay, though, and we should not expect otherwise.
On Windows, for example, when you call
cd C:\GIT-SDK-64
in a PowerShell and there exists a directory called C:\git-sdk-64, the current directory will be reported in all upper-case. Even in a Bash that you might call from that PowerShell. Git, however, will have normalized this via GetFinalPathByHandle(), and the expectation in t0001 that the recorded gitdir will match what pwd says will be violated.
Let's address this by comparing these paths in a case-insensitive manner when core.ignoreCase is true.
Reported by Jameson Miller.
---
Again, re-sending, as something in the mail (my guess is the non-ASCII character in Martin's surname) seems to upset vger so much that it drops the mail unceremoniously.
t/t0001-init.sh | 22 ++++++++-------------- t/test-lib-functions.sh | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 14 deletions(-)
t/t0001-init.sh +8 -14
@@ -310,8 +310,8 @@ test_expect_success 'init prefers command line to GIT_DIR' ' test_expect_success 'init with separate gitdir' ' rm -rf newdir && git init --separate-git-dir realgitdir newdir && - echo "gitdir: $(pwd)/realgitdir" >expected && - test_cmp expected newdir/.git && + newdir_git="$(cat newdir/.git)" && + test_cmp_fspath "$(pwd)/realgitdir" "${newdir_git#gitdir: }" && test_path_is_dir realgitdir/refs '
@@ -360,12 +360,9 @@ test_expect_success 're-init on .git file' ' ' test_expect_success 're-init to update git link' ' - ( - cd newdir && - git init --separate-git-dir ../surrealgitdir - ) && - echo "gitdir: $(pwd)/surrealgitdir" >expected && - test_cmp expected newdir/.git && + git -C newdir init --separate-git-dir ../surrealgitdir && + newdir_git="$(cat newdir/.git)" && + test_cmp_fspath "$(pwd)/surrealgitdir" "${newdir_git#gitdir: }" && test_path_is_dir surrealgitdir/refs && test_path_is_missing realgitdir/refs '
@@ -373,12 +370,9 @@ test_expect_success 're-init to update git link' ' test_expect_success 're-init to move gitdir' ' rm -rf newdir realgitdir surrealgitdir && git init newdir && - ( - cd newdir && - git init --separate-git-dir ../realgitdir - ) && - echo "gitdir: $(pwd)/realgitdir" >expected && - test_cmp expected newdir/.git && + git -C newdir init --separate-git-dir ../realgitdir && + newdir_git="$(cat newdir/.git)" && + test_cmp_fspath "$(pwd)/realgitdir" "${newdir_git#gitdir: }" && test_path_is_dir realgitdir/refs '
t/test-lib-functions.sh +15 -0
@@ -879,6 +879,21 @@ test_cmp_rev () { fi } +# Compare paths respecting core.ignoreCase +test_cmp_fspath () { + if test "x$1" = "x$2" + then + return 0 + fi + + if test true != "$(git config --get --type=bool core.ignorecase)" + then + return 1 + fi + + test "x$(echo "$1" | tr A-Z a-z)" = "x$(echo "$2" | tr A-Z a-z)" +} + # Print a sequence of integers in increasing order, either with # two arguments (start and end): #
--
gitgitgadget
Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems
Quoted 21 lines · Johannes Schindelin
From: Johannes Schindelin <[email protected]>
On a case-insensitive filesystem, such as HFS+ or NTFS, it is possible that the idea Bash has of the current directory differs in case from what Git thinks it is. That's totally okay, though, and we should not expect otherwise.
On Windows, for example, when you call
cd C:\GIT-SDK-64in a PowerShell and there exists a directory called
C:\git-sdk-64, the current directory will be reported in all upper-case. Even in a Bash that you might call from that PowerShell. Git, however, will have normalized this viaGetFinalPathByHandle(), and the expectation in t0001 that the recorded gitdir will match whatpwdsays will be violated.Let's address this by comparing these paths in a case-insensitive manner when
core.ignoreCaseistrue.
Quoted 8 lines
Reported by Jameson Miller.
Signed-off-by Johannes Schindelin <[email protected]>---
Again, re-sending, as something in the mail (my guess is the non-ASCII character in Martin's surname) seems to upset vger so much that it drops the mail unceremoniously.
Hmph, but in the copy I am responding to, I can see non-ASCII Martin's surname in the CC: header just fine, so vger may not be at fault, perhaps?
In any case, cmp-fspath looks nicely implemented. Will queue.
Thanks.
Quoted 80 lines
t/t0001-init.sh | 22 ++++++++-------------- t/test-lib-functions.sh | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 14 deletions(-)t/t0001-init.sh +8
-14@@ -310,8 +310,8 @@ test_expect_success 'init prefers command line to GIT_DIR' ' test_expect_success 'init with separate gitdir' ' rm -rf newdir && git init --separate-git-dir realgitdir newdir && - echo "gitdir: $(pwd)/realgitdir" >expected && - test_cmp expected newdir/.git && + newdir_git="$(cat newdir/.git)" && + test_cmp_fspath "$(pwd)/realgitdir" "${newdir_git#gitdir: }" && test_path_is_dir realgitdir/refs '@@ -360,12 +360,9 @@ test_expect_success 're-init on .git file' ' ' test_expect_success 're-init to update git link' ' - ( - cd newdir && - git init --separate-git-dir ../surrealgitdir - ) && - echo "gitdir: $(pwd)/surrealgitdir" >expected && - test_cmp expected newdir/.git && + git -C newdir init --separate-git-dir ../surrealgitdir && + newdir_git="$(cat newdir/.git)" && + test_cmp_fspath "$(pwd)/surrealgitdir" "${newdir_git#gitdir: }" && test_path_is_dir surrealgitdir/refs && test_path_is_missing realgitdir/refs '@@ -373,12 +370,9 @@ test_expect_success 're-init to update git link' ' test_expect_success 're-init to move gitdir' ' rm -rf newdir realgitdir surrealgitdir && git init newdir && - ( - cd newdir && - git init --separate-git-dir ../realgitdir - ) && - echo "gitdir: $(pwd)/realgitdir" >expected && - test_cmp expected newdir/.git && + git -C newdir init --separate-git-dir ../realgitdir && + newdir_git="$(cat newdir/.git)" && + test_cmp_fspath "$(pwd)/realgitdir" "${newdir_git#gitdir: }" && test_path_is_dir realgitdir/refs 't/test-lib-functions.sh +15
-1@@ -879,6 +879,21 @@ test_cmp_rev () { fi } +# Compare paths respecting core.ignoreCase +test_cmp_fspath () { + if test "x$1" = "x$2" + then + return 0 + fi + + if test true != "$(git config --get --type=bool core.ignorecase)" + then + return 1 + fi + + test "x$(echo "$1" | tr A-Z a-z)" = "x$(echo "$2" | tr A-Z a-z)" +} + # Print a sequence of integers in increasing order, either with # two arguments (start and end): # --gitgitgadget
Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems
In reply to Johannes SchindelinQuoted 4 lines · Johannes Schindelin
The other path category is the paths in the index, which _are_ case-sensitive, no matter what core.ignoreCase says.
So I'd rather keep the
fs.
Sensible. Thanks.
vger vs GitGitGadget, was Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems
In reply to Junio C HamanoHi Junio,
Quoted 11 lines · Junio C Hamano
Quoted 5 lines · Johannes Schindelin
---
Again, re-sending, as something in the mail (my guess is the non-ASCII character in Martin's surname) seems to upset vger so much that it drops the mail unceremoniously.
Hmph, but in the copy I am responding to, I can see non-ASCII Martin's surname in the CC: header just fine, so vger may not be at fault, perhaps?
You can see the non-ASCII surname fine because you are responding to my reply, which I sent via Alpine.
The original mail was sent by GitGitGadget via nodemailer, using GMail's SMTP end point, and the fact that _I_ received it means that it was partially successful in delivering that mail.
I can only assume that GMX or Alpine did something to the mail that made it look better to vger.
*goes-to-unleash-the-power-of-investigation*
So I have a theory now why vger regurgitated on that mail: it had the unquoted name brian m. carlson in the Cc: line.
Expecting that this is the culprit, I opened
https://github.com/gitgitgadget/gitgitgadget/pull/98 to fix that.
Thanks,
Dscho
Re: vger vs GitGitGadget, was Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems
Quoted 10 lines · Johannes Schindelin
Quoted 7 lines
Quoted 3 lines
Again, re-sending, as something in the mail (my guess is the non-ASCII character in Martin's surname) seems to upset vger so much that it drops the mail unceremoniously.
Hmph, but in the copy I am responding to, I can see non-ASCII Martin's surname in the CC: header just fine, so vger may not be at fault, perhaps?
You can see the non-ASCII surname fine because you are responding to my reply, which I sent via Alpine.
Ah, OK. So the difference is on the sending end and vger is merely being a coalmine canary for detectig broken sender.
Quoted 2 lines
So I have a theory now why vger regurgitated on that mail: it had the unquoted name brian m. carlson in the Cc: line.
Ah, yes, I used to get that when sending out my messages, as Gnus/message mode lets me write any garbage on To: or Cc: line ;-)