Re: [PATCH v2 1/3] t0301: fixes for windows compatibility

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 13, 2021, 11:50 UTC
Message-ID: <nycvar.QRO.7.76.6.2109131343440.55@tvgsbejvaqbjf.bet>
In-Reply-To: <20210913085600.35506-2-carenas@gmail.com>

Hi Carlo,

On Mon, 13 Sep 2021, Carlo Marcelo Arenas Belón wrote:

Show 5 quoted lines

> In preparation for a future patch that will allow building with
> Unix Sockets in Windows, workaround a couple of issues from the
> Mingw-W64 compatibility layer.
>
> test -S is not able to detect that a file is a socket, so use

Is that really true, even with recent MSYS2 versions? I thought I saw some patch flying around on the Cygwin mailing list that added support for Unix sockets...

> test -e instead (through a library function).
>
> `mkdir -m` will always fail with permission problems, so instead
> call mkdir followed by chmod.

Maybe explain that Windows' permission model is a lot more sophisticated (using Access Control Lists) and is therefore unable to interpret permissions like `0700`.

And we probably need to mention then, too, that it is funny that `mkdir -m` complains while `chmod` does _not_... Ah, historical reasons...

Thanks, Dscho

Show 36 quoted lines

>
> The last invocation of mkdir would likely need the same treatment
> but SYMLINK is unlikely to be enabled on Windows so it has been
> punted for now.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> v2:
> * avoid the confusing -f test as suggested by Bagas
> * try to help casual readers as suggested by Junio
>
>  t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh
> index ebd5fa5249..1f7b1e29e6 100755
> --- a/t/t0301-credential-cache.sh
> +++ b/t/t0301-credential-cache.sh
> @@ -9,6 +9,21 @@ test -z "$NO_UNIX_SOCKETS" || {
>  	test_done
>  }
>
> +test_path_is_socket () {
> +	test -S "$1"
> +}
> +
> +# in Windows, Unix Sockets look just like regular files
> +uname_s=$(uname -s)
> +case $uname_s in
> +*MINGW*)
> +	test_socket_exist=test_path_exists
> +	;;
> +*)
> +	test_socket_exist=test_path_is_socket
> +	;;
> +esac

A more canonical way would probably be to imitate what we do with `pwd` in `t/test-lib.sh`:

	test_path_is_socket () {
		test -S "$1"
	}

	case $uname_s in
	*MINGW*)
		test_path_is_socket () {
			# `test -S` cannot detect Win10's Unix sockets
			test -e "$1"
		}
		;;
	esac

Show 80 quoted lines

> +
>  # don't leave a stale daemon running
>  test_atexit 'git credential-cache exit'
>
> @@ -21,7 +36,7 @@ test_expect_success 'socket defaults to ~/.cache/git/credential/socket' '
>  		rmdir -p .cache/git/credential/
>  	" &&
>  	test_path_is_missing "$HOME/.git-credential-cache" &&
> -	test -S "$HOME/.cache/git/credential/socket"
> +	$test_socket_exist "$HOME/.cache/git/credential/socket"
>  '
>
>  XDG_CACHE_HOME="$HOME/xdg"
> @@ -31,7 +46,7 @@ helper_test cache
>
>  test_expect_success "use custom XDG_CACHE_HOME if set and default sockets are not created" '
>  	test_when_finished "git credential-cache exit" &&
> -	test -S "$XDG_CACHE_HOME/git/credential/socket" &&
> +	$test_socket_exist "$XDG_CACHE_HOME/git/credential/socket" &&
>  	test_path_is_missing "$HOME/.git-credential-cache/socket" &&
>  	test_path_is_missing "$HOME/.cache/git/credential/socket"
>  '
> @@ -48,7 +63,7 @@ test_expect_success 'credential-cache --socket option overrides default location
>  	username=store-user
>  	password=store-pass
>  	EOF
> -	test -S "$HOME/dir/socket"
> +	$test_socket_exist "$HOME/dir/socket"
>  '
>
>  test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" '
> @@ -62,7 +77,7 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" '
>  	username=store-user
>  	password=store-pass
>  	EOF
> -	test -S "$HOME/.cache/git/credential/socket" &&
> +	$test_socket_exist "$HOME/.cache/git/credential/socket" &&
>  	XDG_CACHE_HOME="$HOME/xdg" &&
>  	export XDG_CACHE_HOME &&
>  	check approve cache <<-\EOF &&
> @@ -71,7 +86,7 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" '
>  	username=store-user
>  	password=store-pass
>  	EOF
> -	test -S "$XDG_CACHE_HOME/git/credential/socket"
> +	$test_socket_exist "$XDG_CACHE_HOME/git/credential/socket"
>  '
>
>  test_expect_success 'use user socket if user directory exists' '
> @@ -79,14 +94,15 @@ test_expect_success 'use user socket if user directory exists' '
>  		git credential-cache exit &&
>  		rmdir \"\$HOME/.git-credential-cache/\"
>  	" &&
> -	mkdir -p -m 700 "$HOME/.git-credential-cache/" &&
> +	mkdir -p "$HOME/.git-credential-cache/" &&
> +	chmod 700 "$HOME/.git-credential-cache/" &&
>  	check approve cache <<-\EOF &&
>  	protocol=https
>  	host=example.com
>  	username=store-user
>  	password=store-pass
>  	EOF
> -	test -S "$HOME/.git-credential-cache/socket"
> +	$test_socket_exist "$HOME/.git-credential-cache/socket"
>  '
>
>  test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' '
> @@ -103,7 +119,7 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to
>  	username=store-user
>  	password=store-pass
>  	EOF
> -	test -S "$HOME/.git-credential-cache/socket"
> +	$test_socket_exist "$HOME/.git-credential-cache/socket"
>  '
>
>  helper_test_timeout cache --timeout=1
> --
> 2.33.0.481.g26d3bed244
>
>