[PATCH 0/1] Fix a test on NTFS (and probably HFS+)

[PATCH 0/1] Fix a test on NTFS (and probably HFS+)

[Johannes Schindelin via GitGitGadget]

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

[PATCH 1/1] t0001: fix on case-insensitive filesystems

[Johannes Schindelin via GitGitGadget]

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

Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems

[Martin Ågren]

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

Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems

[Johannes Schindelin]

[-- 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
>

Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems

[brian m. carlson]

[-- 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 --]

Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems

[Junio C Hamano]

"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.

Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems

[Johannes Schindelin]

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

Re: [PATCH 1/1] t0001: fix on case-insensitive filesystems

[Ævar Arnfjörð Bjarmason]

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 ",".

Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems

[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]

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

Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems

[Junio C Hamano]

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

Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems

[Johannes Schindelin]

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
>

Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems

[Junio C Hamano]

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.

Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems

[Johannes Schindelin]

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

Re: [PATCH v2 1/1] t0001: fix on case-insensitive filesystems

[Junio C Hamano]

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.

Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems

[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.

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

Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems

[Junio C Hamano]

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

vger vs GitGitGadget, was Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems

[Johannes Schindelin]

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

Re: vger vs GitGitGadget, was Re: [PATCH v3 1/1] t0001: fix on case-insensitive filesystems

[Junio C Hamano]

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 ;-)