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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 21, 2019, 17:31 UTC
Message-ID: <xmqqh88ikfpb.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.1906211632570.44@tvgsbejvaqbjf.bet>

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.

Show 33 quoted lines

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

Show 5 quoted lines

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

Show 21 quoted lines

>  	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