Re: [RFC/PATCH 0/5] Fix fetch regression with transport helpers

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 5, 2019, 08:12 UTC
Message-ID: <nycvar.QRO.7.76.6.1906051005060.1775@tvgsbejvaqbjf.bet>
In-Reply-To: <20190604021330.16130-1-felipe.contreras@gmail.com>

On Mon, 3 Jun 2019, Felipe Contreras wrote:

Show 11 quoted lines

> Felipe Contreras (5):
>   t5801 (remote-helpers): cleanup refspec stuff
>   t5801 (remote-helpers): add test to fetch tags
>   fetch: trivial cleanup
>   fetch: make the code more understandable
>   fetch: fix regression with transport helpers
>
>  builtin/fetch.c            | 28 ++++++++++++++++++----------
>  t/t5801-remote-helpers.sh  | 18 ++++++++++++++----
>  t/t5801/git-remote-testgit | 22 +++++++++++++---------
>  3 files changed, 45 insertions(+), 23 deletions(-)

This fails on macOS, in t5601, both in our osx-clang and osx-gcc jobs, as well as in the StaticAnalysis job. For details, see https://dev.azure.com/gitgitgadget/git/_build/results?buildId=10206

The t5601 failure looks like this:

-- snip -- checking prerequisite: CASE_INSENSITIVE_FS

mkdir -p "$TRASH_DIRECTORY/prereq-test-dir" &&
(
	cd "$TRASH_DIRECTORY/prereq-test-dir" &&
	echo good >CamelCase &&
	echo bad >camelcase &&
	test "$(cat CamelCase)" != good

)
++ mkdir -p '/Users/vsts/agent/2.150.3/work/1/s/t/trash
directory.t5601-clone/prereq-test-dir'
++ cd '/Users/vsts/agent/2.150.3/work/1/s/t/trash
directory.t5601-clone/prereq-test-dir'
++ echo good
++ echo bad
+++ cat CamelCase
++ test bad '!=' good
prerequisite CASE_INSENSITIVE_FS ok
expecting success:
	grep X icasefs/warning &&
	grep x icasefs/warning &&
	test_i18ngrep "the following paths have collided" icasefs/warning

++ grep X icasefs/warning error: last command exited with $?=1 not ok 99 - colliding file detection -- snap --

My guess is that your changes remove something that was expected before, and is still expected, and that this was only tested on Linux, and only on a file system with case-sensitive file names.

The StaticAnalysis job (which is admittedly a misnomer) points out another few valid issues, but that is probably because Junio applied this patch series on top of a very old commit. I, at least, could not spot any file in the Coccinelle report that was touched by this here patch series.

Ciao, Johannes