* [PATCH 0/1] Fix a test on NTFS (and probably HFS+) @ 2019-06-08 14:43 Johannes Schindelin via GitGitGadget 2019-06-08 14:43 ` [PATCH 1/1] t0001: fix on case-insensitive filesystems Johannes Schindelin via GitGitGadget [not found] ` <pull.151.v2.git.gitgitgadget@gmail.com> 0 siblings, 2 replies; 18+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-06-08 14:43 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 -- gitgitgadget ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/1] t0001: fix on case-insensitive filesystems 2019-06-08 14:43 [PATCH 0/1] Fix a test on NTFS (and probably HFS+) Johannes Schindelin via GitGitGadget @ 2019-06-08 14:43 ` Johannes Schindelin via GitGitGadget 2019-06-09 19:23 ` Martin Ågren 2019-06-09 20:13 ` brian m. carlson [not found] ` <pull.151.v2.git.gitgitgadget@gmail.com> 1 sibling, 2 replies; 18+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-06-08 14:43 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> 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. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t0001-init.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 42a263cada..f54a69e2d9 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -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 ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems 2019-06-08 14:43 ` [PATCH 1/1] t0001: fix on case-insensitive filesystems Johannes Schindelin via GitGitGadget @ 2019-06-09 19:23 ` Martin Ågren 2019-06-19 20:24 ` Johannes Schindelin 2019-06-09 20:13 ` brian m. carlson 1 sibling, 1 reply; 18+ messages in thread From: Martin Ågren @ 2019-06-09 19:23 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget Cc: Git Mailing List, Junio C Hamano, Johannes Schindelin Hi Dscho, On Sat, 8 Jun 2019 at 16:45, Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > 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. > +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. > + for f > + do > + tr A-Z a-z <"$f" >"$f".downcased && > + mv -f "$f".downcased "$f" || return 1 Makes sense. Good error-handling. > + done > +} Cheers Martin ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems 2019-06-09 19:23 ` Martin Ågren @ 2019-06-19 20:24 ` Johannes Schindelin 0 siblings, 0 replies; 18+ messages in thread From: Johannes Schindelin @ 2019-06-19 20:24 UTC (permalink / raw) To: Martin Ågren Cc: Johannes Schindelin via GitGitGadget, Git Mailing List, Junio C Hamano [-- Attachment #1: Type: text/plain, Size: 1868 bytes --] Hi Martin, On Sun, 9 Jun 2019, Martin Ågren wrote: > On Sat, 8 Jun 2019 at 16:45, Johannes Schindelin via GitGitGadget > <gitgitgadget@gmail.com> wrote: > > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > 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. > > > +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. Oh my, you're right. I do not know how the filemode slipped in... Thanks! Dscho > 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. > > > + for f > > + do > > + tr A-Z a-z <"$f" >"$f".downcased && > > + mv -f "$f".downcased "$f" || return 1 > > Makes sense. Good error-handling. > > > + done > > +} > > Cheers > Martin > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems 2019-06-08 14:43 ` [PATCH 1/1] t0001: fix on case-insensitive filesystems Johannes Schindelin via GitGitGadget 2019-06-09 19:23 ` Martin Ågren @ 2019-06-09 20:13 ` brian m. carlson 2019-06-10 16:32 ` Junio C Hamano 2019-06-21 19:33 ` Ævar Arnfjörð Bjarmason 1 sibling, 2 replies; 18+ messages in thread From: brian m. carlson @ 2019-06-09 20:13 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget Cc: git, Junio C Hamano, Johannes Schindelin [-- Attachment #1: Type: text/plain, Size: 1680 bytes --] On 2019-06-08 at 14:43:43, Johannes Schindelin via GitGitGadget wrote: > diff --git a/t/t0001-init.sh b/t/t0001-init.sh > index 42a263cada..f54a69e2d9 100755 > --- a/t/t0001-init.sh > +++ b/t/t0001-init.sh > @@ -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. > + 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. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems 2019-06-09 20:13 ` brian m. carlson @ 2019-06-10 16:32 ` Junio C Hamano 2019-06-19 20:17 ` Johannes Schindelin 2019-06-21 19:33 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2019-06-10 16:32 UTC (permalink / raw) To: brian m. carlson Cc: Johannes Schindelin via GitGitGadget, git, Johannes Schindelin "brian m. carlson" <sandals@crustytoothpaste.net> writes: >> 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. 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems 2019-06-10 16:32 ` Junio C Hamano @ 2019-06-19 20:17 ` Johannes Schindelin 0 siblings, 0 replies; 18+ messages in thread From: Johannes Schindelin @ 2019-06-19 20:17 UTC (permalink / raw) To: Junio C Hamano Cc: brian m. carlson, Johannes Schindelin via GitGitGadget, git Hi, On Mon, 10 Jun 2019, Junio C Hamano wrote: > "brian m. carlson" <sandals@crustytoothpaste.net> writes: > > >> 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. 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`. > > 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. > > 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. > > 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems 2019-06-09 20:13 ` brian m. carlson 2019-06-10 16:32 ` Junio C Hamano @ 2019-06-21 19:33 ` Ævar Arnfjörð Bjarmason 1 sibling, 0 replies; 18+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-06-21 19:33 UTC (permalink / raw) To: brian m. carlson Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano, Johannes Schindelin On Sun, Jun 09 2019, brian m. carlson wrote: > On 2019-06-08 at 14:43:43, Johannes Schindelin via GitGitGadget wrote: >> diff --git a/t/t0001-init.sh b/t/t0001-init.sh >> index 42a263cada..f54a69e2d9 100755 >> --- a/t/t0001-init.sh >> +++ b/t/t0001-init.sh >> @@ -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. 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 ",". ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <pull.151.v2.git.gitgitgadget@gmail.com>]
[parent not found: <c2fdcf28e725c91a1a48c34226223866ad14bc0a.1560978437.git.gitgitgadget@gmail.com>]
* Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems [not found] ` <c2fdcf28e725c91a1a48c34226223866ad14bc0a.1560978437.git.gitgitgadget@gmail.com> @ 2019-06-21 14:34 ` Johannes Schindelin 2019-06-21 17:31 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Johannes Schindelin @ 2019-06-21 14:34 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget Cc: git, Martin Ågren, brian m. carlson, Junio C Hamano [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 <johannes.schindelin@gmx.de> 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. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t0001-init.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 42a263cada..68f713884f 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems 2019-06-21 14:34 ` [PATCH v2 " Johannes Schindelin @ 2019-06-21 17:31 ` Junio C Hamano 2019-06-24 10:13 ` Johannes Schindelin 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2019-06-21 17:31 UTC (permalink / raw) To: Johannes Schindelin Cc: Johannes Schindelin via GitGitGadget, git, Martin Ågren, brian m. carlson Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > [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. > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > 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 when > `core.ignoreCase` is `true`. > > Reported by Jameson Miller. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > t/t0001-init.sh | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/t/t0001-init.sh b/t/t0001-init.sh > index 42a263cada..68f713884f 100755 > --- a/t/t0001-init.sh > +++ b/t/t0001-init.sh > @@ -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 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. > 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 && 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. > 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems 2019-06-21 17:31 ` Junio C Hamano @ 2019-06-24 10:13 ` Johannes Schindelin 2019-06-24 17:09 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Johannes Schindelin @ 2019-06-24 10:13 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Schindelin via GitGitGadget, git, Martin Ågren, brian m. carlson Hi Junio, On Fri, 21 Jun 2019, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > [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. > > diff --git a/t/t0001-init.sh b/t/t0001-init.sh > > index 42a263cada..68f713884f 100755 > > --- a/t/t0001-init.sh > > +++ b/t/t0001-init.sh > > @@ -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 > > 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 Okay. > Not grave enough to be worthy of a reroll on this one alone, though. > > > 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 && > > 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. I agree that that's a much better approach to fix the issue. Thanks, Dscho > > > 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 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems 2019-06-24 10:13 ` Johannes Schindelin @ 2019-06-24 17:09 ` Junio C Hamano 2019-06-24 17:38 ` Johannes Schindelin 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2019-06-24 17:09 UTC (permalink / raw) To: Johannes Schindelin Cc: Johannes Schindelin via GitGitGadget, git, Martin Ågren, brian m. carlson Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> 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. > > 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems 2019-06-24 17:09 ` Junio C Hamano @ 2019-06-24 17:38 ` Johannes Schindelin 2019-06-24 19:22 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Johannes Schindelin @ 2019-06-24 17:38 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Schindelin via GitGitGadget, git, Martin Ågren, brian m. carlson Hi Junio, On Mon, 24 Jun 2019, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > >> 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. > > > > 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems 2019-06-24 17:38 ` Johannes Schindelin @ 2019-06-24 19:22 ` Junio C Hamano 0 siblings, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2019-06-24 19:22 UTC (permalink / raw) To: Johannes Schindelin Cc: Johannes Schindelin via GitGitGadget, git, Martin Ågren, brian m. carlson Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <pull.151.v3.git.gitgitgadget@gmail.com>]
[parent not found: <1f0ceee8687e9032a7777f764b34b1c9ccc68f38.1561379363.git.gitgitgadget@gmail.com>]
* Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems [not found] ` <1f0ceee8687e9032a7777f764b34b1c9ccc68f38.1561379363.git.gitgitgadget@gmail.com> @ 2019-06-24 17:40 ` Johannes Schindelin 2019-06-24 18:55 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Johannes Schindelin @ 2019-06-24 17:40 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget Cc: git, Martin Ågren, brian m. carlson, Junio C Hamano From: Johannes Schindelin <johannes.schindelin@gmx.de> 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. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- 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(-) diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 42a263cada..b796fa25ac 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -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 ' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 80402a428f..26218a6c53 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems 2019-06-24 17:40 ` [PATCH v3 " Johannes Schindelin @ 2019-06-24 18:55 ` Junio C Hamano 2019-06-25 10:50 ` vger vs GitGitGadget, was " Johannes Schindelin 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2019-06-24 18:55 UTC (permalink / raw) To: Johannes Schindelin Cc: Johannes Schindelin via GitGitGadget, git, Martin Ågren, brian m. carlson Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > 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. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > > 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. > > t/t0001-init.sh | 22 ++++++++-------------- > t/test-lib-functions.sh | 15 +++++++++++++++ > 2 files changed, 23 insertions(+), 14 deletions(-) > > diff --git a/t/t0001-init.sh b/t/t0001-init.sh > index 42a263cada..b796fa25ac 100755 > --- a/t/t0001-init.sh > +++ b/t/t0001-init.sh > @@ -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 > ' > > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > index 80402a428f..26218a6c53 100644 > --- a/t/test-lib-functions.sh > +++ b/t/test-lib-functions.sh > @@ -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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* vger vs GitGitGadget, was Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems 2019-06-24 18:55 ` Junio C Hamano @ 2019-06-25 10:50 ` Johannes Schindelin 2019-06-25 19:42 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Johannes Schindelin @ 2019-06-25 10:50 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Schindelin via GitGitGadget, git, Martin Ågren, brian m. carlson Hi Junio, On Mon, 24 Jun 2019, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > --- > > > > 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: vger vs GitGitGadget, was Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems 2019-06-25 10:50 ` vger vs GitGitGadget, was " Johannes Schindelin @ 2019-06-25 19:42 ` Junio C Hamano 0 siblings, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2019-06-25 19:42 UTC (permalink / raw) To: Johannes Schindelin Cc: Johannes Schindelin via GitGitGadget, git, Martin Ågren, brian m. carlson Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> > 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. > 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 ;-) ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-06-25 19:42 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-08 14:43 [PATCH 0/1] Fix a test on NTFS (and probably HFS+) Johannes Schindelin via GitGitGadget 2019-06-08 14:43 ` [PATCH 1/1] t0001: fix on case-insensitive filesystems Johannes Schindelin via GitGitGadget 2019-06-09 19:23 ` Martin Ågren 2019-06-19 20:24 ` Johannes Schindelin 2019-06-09 20:13 ` brian m. carlson 2019-06-10 16:32 ` Junio C Hamano 2019-06-19 20:17 ` Johannes Schindelin 2019-06-21 19:33 ` Ævar Arnfjörð Bjarmason [not found] ` <pull.151.v2.git.gitgitgadget@gmail.com> [not found] ` <c2fdcf28e725c91a1a48c34226223866ad14bc0a.1560978437.git.gitgitgadget@gmail.com> 2019-06-21 14:34 ` [PATCH v2 " Johannes Schindelin 2019-06-21 17:31 ` Junio C Hamano 2019-06-24 10:13 ` Johannes Schindelin 2019-06-24 17:09 ` Junio C Hamano 2019-06-24 17:38 ` Johannes Schindelin 2019-06-24 19:22 ` Junio C Hamano [not found] ` <pull.151.v3.git.gitgitgadget@gmail.com> [not found] ` <1f0ceee8687e9032a7777f764b34b1c9ccc68f38.1561379363.git.gitgitgadget@gmail.com> 2019-06-24 17:40 ` [PATCH v3 " Johannes Schindelin 2019-06-24 18:55 ` Junio C Hamano 2019-06-25 10:50 ` vger vs GitGitGadget, was " Johannes Schindelin 2019-06-25 19:42 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).