Re: [PATCH] mingw: fix regression in t1308-config-set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <johannes.schindelin@xxxxxx> writes:

> When we tried to fix in 58461bd (t1308: do not get fooled by symbolic
> links to the source tree, 2016-06-02) an obscure case where the user
> cd's into Git's source code via a symbolic link, a regression was
> introduced that affects all test runs on Windows.

Thanks for producing a fix quickly after the topic hit 'master'.

The original came from

  http://thread.gmane.org/gmane.comp.version-control.git/296021/focus=296199

which was merged at e5e5bb67 (Merge branch 'jk/upload-pack-hook'
into next, 2016-06-28) to 'next'.  

I see J6t raised C:/foo vs /c/foo issue in the thread later, but
unfortunately I didn't notice it.

I added a few missing Cc: and quoted the whole patch here to those
who were involved; I think this update is correct, but just trying
to make sure people know.

Not limited to this particular topic, there probably are some things
we can and should add to the procedure to prevent further episodes
like this, but I am not seeing anything immediately obvious offhand.
There already is a way to prominently mark a topic to be not-ready
with an outstanding issue called "What's cooking" report, but it is
maintained manually and it can be leaky without extra set of eyes
constantly monitoring.

> The original patch introducing the test case in question was careful to
> use `$(pwd)` instead of `$PWD`.
>
> This was done to account for the fact that Git's test suite uses shell
> scripting even on Windows, where the shell's Unix-y paths are
> incompatible with the main Git executable's idea of paths: it only
> accepts Windows paths.
>
> It is an awkward but necessary thing, then, to use `$(pwd)` (which gives
> us a Windows path) when interacting with the Git executable and `$PWD`
> (which gives the shell's idea of the current working directory in Unix-y
> form) for shell scripts, including the test suite itself.
>
> Obviously this broke the use case of the Git maintainer when changing
> the working directory into Git's source code directory via a symlink,
> i.e. when `$(pwd)` does not agree with `$PWD`.
>
> However, we must not fix that use case at the expense of regressing
> another use case.
>
> Let's special-case Windows here, even if it is ugly, for lack of a more
> elegant solution.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> Published-As: https://github.com/dscho/git/releases/tag/t1308-on-windows-v1
>
> 	Side note: it was not at all clear to me how 58461bd fixed the
> 	problem by replacing $(pwd) with $HOME, given that HOME is set to
> 	$TRASH_DIRECTORY which is set to $TEST_OUTPUT_DIRECTORY/... after
> 	TEST_OUTPUT_DIRECTORY was set to TEST_DIRECTORY which in turn was
> 	set to $(pwd).
>
> 	I guess the reason is that -P in `cd -P "$TRASH DIRECTORY"`, but
> 	then I *really* do not understand how $(pwd) and $PWD could
> 	disagree.
> 	Oh well. I have to move on to the next fire.
>
>  t/t1308-config-set.sh | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
> index a06e71c..7655c94 100755
> --- a/t/t1308-config-set.sh
> +++ b/t/t1308-config-set.sh
> @@ -233,11 +233,19 @@ cmdline_config="'foo.bar=from-cmdline'"
>  test_expect_success 'iteration shows correct origins' '
>  	echo "[foo]bar = from-repo" >.git/config &&
>  	echo "[foo]bar = from-home" >.gitconfig &&
> +	if test_have_prereq MINGW
> +	then
> +		# Use Windows path (i.e. *not* $HOME)
> +		HOME_GITCONFIG=$(pwd)/.gitconfig
> +	else
> +		# Do not get fooled by symbolic links, i.e. $HOME != $(pwd)
> +		HOME_GITCONFIG=$HOME/.gitconfig
> +	fi &&
>  	cat >expect <<-EOF &&
>  	key=foo.bar
>  	value=from-home
>  	origin=file
> -	name=$HOME/.gitconfig
> +	name=$HOME_GITCONFIG
>  	scope=global
>  
>  	key=foo.bar
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]