[PATCH 0/3] windows: allow building without NO_UNIX_SOCKETS

Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,
it might not be necessary in the future, because Windows added support
for Unix Sockets in late 2017.

The first patch is messy, specially considering how little was needed
to build and run `git credential-cache` and its daemon, nothing broke
on the trace2 code or tests, but additional testing to make sure will
be recommended.

Carlo Marcelo Arenas Belón (3):
  t0301: fixes for windows compatibility
  credential-cache: check for windows specific errors
  git-compat-util: include declaration for unix sockets

 builtin/credential-cache.c  |  5 +++--
 git-compat-util.h           |  3 +++
 t/t0301-credential-cache.sh | 28 ++++++++++++++++++++--------
 3 files changed, 26 insertions(+), 10 deletions(-)

-- 
2.33.0.481.g26d3bed244

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
test -f instead.

`mkdir -m` will always fail with permission problems, so instead
call mkdir followed by chmod.

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 <redacted>
---
 t/t0301-credential-cache.sh | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh
index ebd5fa5249..f5cce6c6a6 100755
--- a/t/t0301-credential-cache.sh
+++ b/t/t0301-credential-cache.sh

@@ -9,6 +9,17 @@ test -z "$NO_UNIX_SOCKETS" || {
 	test_done
 }
 
+# in Windows, Unix Sockets look just like regular files
+uname_s=$(uname -s)
+case $uname_s in
+*MINGW*)
+	FLAG=-f
+	;;
+*)
+	FLAG=-S
+	;;
+esac
+
 # don't leave a stale daemon running
 test_atexit 'git credential-cache exit'
 

@@ -21,7 +32,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 $FLAG "$HOME/.cache/git/credential/socket"
 '
 
 XDG_CACHE_HOME="$HOME/xdg"

@@ -31,7 +42,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 $FLAG "$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 +59,7 @@ test_expect_success 'credential-cache --socket option overrides default location
 	username=store-user
 	password=store-pass
 	EOF
-	test -S "$HOME/dir/socket"
+	test $FLAG "$HOME/dir/socket"
 '
 
 test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" '

@@ -62,7 +73,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 $FLAG "$HOME/.cache/git/credential/socket" &&
 	XDG_CACHE_HOME="$HOME/xdg" &&
 	export XDG_CACHE_HOME &&
 	check approve cache <<-\EOF &&

@@ -71,7 +82,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 $FLAG "$XDG_CACHE_HOME/git/credential/socket"
 '
 
 test_expect_success 'use user socket if user directory exists' '

@@ -79,14 +90,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 $FLAG "$HOME/.git-credential-cache/socket"
 '
 
 test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' '

@@ -103,7 +115,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 $FLAG "$HOME/.git-credential-cache/socket"
 '
 
 helper_test_timeout cache --timeout=1

-- 
2.33.0.481.g26d3bed244

Available since Windows 10 release 1803, therefore only added if
not using NO_UNIX_SOCKETS (which is not the current default).

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
 git-compat-util.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index b46605300a..6a420d104c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -160,6 +160,9 @@
 # endif
 #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
 #include <winsock2.h>
+#ifndef NO_UNIX_SOCKETS
+#include <afunix.h>
+#endif
 #include <windows.h>
 #define GIT_WINDOWS_NATIVE
 #endif

-- 
2.33.0.481.g26d3bed244

Connect and reset errors aren't what will be expected by POSIX but
are compatible with the ones used by WinSock.

Alternatively a translation layer for read could be added (similar
to the one provided by mingw-write()) to translate the odd EINVAL,
into a more reasonable EPIPE, but this is more localized and still
unlikely to cause confusion in other systems.

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
 builtin/credential-cache.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c
index 76a6ba3722..b12a79ae01 100644
--- a/builtin/credential-cache.c
+++ b/builtin/credential-cache.c

@@ -28,7 +28,8 @@ static int send_request(const char *socket, const struct strbuf *out)
 		int r;
 
 		r = read_in_full(fd, in, sizeof(in));
-		if (r == 0 || (r < 0 && errno == ECONNRESET))
+		if (r == 0 ||
+			(r < 0 && (errno == ECONNRESET || errno == EINVAL)))
 			break;
 		if (r < 0)
 			die_errno("read error from cache daemon");

@@ -75,7 +76,7 @@ static void do_cache(const char *socket, const char *action, int timeout,
 	}
 
 	if (send_request(socket, &buf) < 0) {
-		if (errno != ENOENT && errno != ECONNREFUSED)
+		if (errno != ENOENT && errno != ECONNREFUSED && errno != ENETDOWN)
 			die_errno("unable to connect to cache daemon");
 		if (flags & FLAG_SPAWN) {
 			spawn_daemon(socket);

-- 
2.33.0.481.g26d3bed244

On 13/09/21 03.28, Carlo Marcelo Arenas Belón wrote:

test -S is not able to detect that a file is a socket, so use
test -f instead.

Isn't test -f just check for socket as regular file?

-- 
An old man doll... just what I always wanted! - Clara

On Sun, Sep 12, 2021 at 10:34 PM Bagas Sanjaya [off-list ref] wrote:

On 13/09/21 03.28, Carlo Marcelo Arenas Belón wrote:

quoted

test -S is not able to detect that a file is a socket, so use
test -f instead.

Isn't test -f just check for socket as regular file?

and that is exactly how they look; ironically a -f check in Linux
fails for sockets so maybe better to do -e?

an empty file with nothing that indicates in Windows Explorer or a
stat call (from WSL or git bash), that they are anything else.

Carlo

Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,
it might not be necessary in the future, because Windows added support
for Unix Sockets in late 2017.

The implementation of Unix Sockets, uses an internal NTFS mechanism
and is therefore not visible at the filesystem layer, like it is in
UNIX, but seems to be good enough to allow to build and run the
`git credential-cache` and its daemon; additional testing to confirm
trace2 (builds and doesn't fail any tests) is functional will be
needed.

V2 reuses the same third patch from V1, and applies all suggested
feedback on patches 1 and 2 and should apply cleanly as a reroll of
cb/unix-sockets-with-windows.

Carlo Marcelo Arenas Belón (3):
  t0301: fixes for windows compatibility
  credential-cache: check for windows specific errors
  git-compat-util: include declaration for unix sockets

 builtin/credential-cache.c  | 30 ++++++++++++++++++++++++++++--
 git-compat-util.h           |  3 +++
 t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------
 3 files changed, 55 insertions(+), 10 deletions(-)

-- 
2.33.0.481.g26d3bed244

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
test -e instead (through a library function).

`mkdir -m` will always fail with permission problems, so instead
call mkdir followed by chmod.

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 <redacted>
---
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
+
 # 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

Connect and reset errors aren't what will be expected by POSIX but
are compatible with the ones used by WinSock.

To avoid any possibility of confusion with other systems checks
for disconnection and availability had been abstracted into helper
functions that are platform specific.

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
V2:
* Use helper functions to separate error handling as suggested by Junio

 builtin/credential-cache.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c
index e8a7415747..fd9f33d993 100644
--- a/builtin/credential-cache.c
+++ b/builtin/credential-cache.c

@@ -11,6 +11,32 @@
 #define FLAG_SPAWN 0x1
 #define FLAG_RELAY 0x2
 
+#ifdef _WIN32
+
+static int connection_closed(int error)
+{
+	return (error == EINVAL);
+}
+
+static int connection_fatally_broken(int error)
+{
+	return (error != ENOENT) && (error != ENETDOWN);
+}
+
+#else
+
+static int connection_closed(int error)
+{
+	return (error == ECONNRESET);
+}
+
+static int connection_fatally_broken(int error)
+{
+	return (error != ENOENT) && (error != ECONNREFUSED);
+}
+
+#endif
+
 static int send_request(const char *socket, const struct strbuf *out)
 {
 	int got_data = 0;

@@ -28,7 +54,7 @@ static int send_request(const char *socket, const struct strbuf *out)
 		int r;
 
 		r = read_in_full(fd, in, sizeof(in));
-		if (r == 0 || (r < 0 && errno == ECONNRESET))
+		if (r == 0 || (r < 0 && connection_closed(errno)))
 			break;
 		if (r < 0)
 			die_errno("read error from cache daemon");

@@ -75,7 +101,7 @@ static void do_cache(const char *socket, const char *action, int timeout,
 	}
 
 	if (send_request(socket, &buf) < 0) {
-		if (errno != ENOENT && errno != ECONNREFUSED)
+		if (connection_fatally_broken(errno))
 			die_errno("unable to connect to cache daemon");
 		if (flags & FLAG_SPAWN) {
 			spawn_daemon(socket);

-- 
2.33.0.481.g26d3bed244

Available since Windows 10 release 1803, therefore only added if
not using NO_UNIX_SOCKETS (which is not the current default).

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
 git-compat-util.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index b46605300a..6a420d104c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -160,6 +160,9 @@
 # endif
 #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
 #include <winsock2.h>
+#ifndef NO_UNIX_SOCKETS
+#include <afunix.h>
+#endif
 #include <windows.h>
 #define GIT_WINDOWS_NATIVE
 #endif

-- 
2.33.0.481.g26d3bed244

Hi Carlo,

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

Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,
it might not be necessary in the future, because Windows added support
for Unix Sockets in late 2017.

The fact that some recent Windows 10 versions support Unix sockets is
nice, no doubt. As far as Git for Windows is concerned (which does not
recompile itself during an installation based on the Windows version it
found), we cannot use it, of course. At least not until all of the
previous Windows versions are _long_ out of service. For example, we still
support Windows 7 even if it reached it's "End-Of-Life".

Having said that: thank you for working on this. For users who build their
Git themselves (which is definitely something Git for Windows makes
easy), the option to use Unix sockets is really nice.

Ciao,
Dscho

The implementation of Unix Sockets, uses an internal NTFS mechanism
and is therefore not visible at the filesystem layer, like it is in
UNIX, but seems to be good enough to allow to build and run the
`git credential-cache` and its daemon; additional testing to confirm
trace2 (builds and doesn't fail any tests) is functional will be
needed.

V2 reuses the same third patch from V1, and applies all suggested
feedback on patches 1 and 2 and should apply cleanly as a reroll of
cb/unix-sockets-with-windows.

Carlo Marcelo Arenas Belón (3):
  t0301: fixes for windows compatibility
  credential-cache: check for windows specific errors
  git-compat-util: include declaration for unix sockets

 builtin/credential-cache.c  | 30 ++++++++++++++++++++++++++++--
 git-compat-util.h           |  3 +++
 t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------
 3 files changed, 55 insertions(+), 10 deletions(-)

--
2.33.0.481.g26d3bed244

Hi Carlo,

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

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

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

Hi Carlo,

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

While that works, I think we prefer `WIN32` (`_WIN32` is only used in
`compat/` and `contrib/`).

Other than that, looks good!

Ciao,
Dscho

Hi Carlo,

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

Available since Windows 10 release 1803, therefore only added if
not using NO_UNIX_SOCKETS (which is not the current default).

I wouldn't mind one bit if we did not have a double negation ;-)

Other than that, looks good!

Ciao,
Dscho

Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,
it might not be necessary in the future, because Windows added support
for Unix Sockets in late 2017.

The implementation of Unix Sockets, uses an internal NTFS mechanism
and is therefore not visible at the filesystem layer, like it is in
UNIX, but seems to be good enough to allow to build and run the
`git credential-cache` and its daemon, with not many changes.

Additional testing to confirm trace2 (which builds and doesn't fail any
tests) is functional will be also needed.

Interestingly, Cygwin has its own implementation of Unix Sockets that
are visible as such even in the Git Bash environment, but that are also
incompatible with the native ones.

V3 Changes the commit messages as suggested and fixes the original ugly
implementation of the workarounds for the first patch and shoule apply
cleanly as a reroll of cb/unix-sockets-with-windows.

Carlo Marcelo Arenas Belón (3):
  t0301: fixes for windows compatibility
  credential-cache: check for windows specific errors
  git-compat-util: include declaration for unix sockets in windows

 builtin/credential-cache.c  | 30 ++++++++++++++++++++++++++++--
 git-compat-util.h           |  3 +++
 t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------
 3 files changed, 55 insertions(+), 10 deletions(-)

-- 
2.33.0.481.g26d3bed244

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
test -e instead (through a library function).

`mkdir -m` can't represent a valid ACL directly and fails with
permission problems, so instead call mkdir followed by chmod, which
has been enhanced to do so.

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 <redacted>
---
v3:
* avoid using a variable to hold the function for testing and instead
  use the cleaner syntax suggested by Dscho
v2:
* avoid the confusing -f test as suggested by Bagas
* no more FLAG variable 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..698b7159f0 100755
--- a/t/t0301-credential-cache.sh
+++ b/t/t0301-credential-cache.sh

@@ -9,6 +9,21 @@ test -z "$NO_UNIX_SOCKETS" || {
 	test_done
 }
 
+uname_s=$(uname -s)
+case $uname_s in
+*MINGW*)
+	test_path_is_socket () {
+		# `test -S` cannot detect Win10's Unix sockets
+		test_path_exists "$1"
+	}
+	;;
+*)
+	test_path_is_socket () {
+		test -S "$1"
+	}
+	;;
+esac
+
 # 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_path_is_socket "$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_path_is_socket "$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_path_is_socket "$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_path_is_socket "$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_path_is_socket "$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_path_is_socket "$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_path_is_socket "$HOME/.git-credential-cache/socket"
 '
 
 helper_test_timeout cache --timeout=1

-- 
2.33.0.481.g26d3bed244

Connect and reset errors aren't what will be expected by POSIX but
are instead compatible with the ones used by WinSock.

To avoid any possibility of confusion with other systems, checks
for disconnection and availability had been abstracted into helper
functions that are platform specific.

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
v3:
* use a better define as suggested by Dscho
v2:
* Use helper functions to separate error handling as suggested by Junio

 builtin/credential-cache.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c
index e8a7415747..78c02ad531 100644
--- a/builtin/credential-cache.c
+++ b/builtin/credential-cache.c

@@ -11,6 +11,32 @@
 #define FLAG_SPAWN 0x1
 #define FLAG_RELAY 0x2
 
+#ifdef GIT_WINDOWS_NATIVE
+
+static int connection_closed(int error)
+{
+	return (error == EINVAL);
+}
+
+static int connection_fatally_broken(int error)
+{
+	return (error != ENOENT) && (error != ENETDOWN);
+}
+
+#else
+
+static int connection_closed(int error)
+{
+	return (error == ECONNRESET);
+}
+
+static int connection_fatally_broken(int error)
+{
+	return (error != ENOENT) && (error != ECONNREFUSED);
+}
+
+#endif
+
 static int send_request(const char *socket, const struct strbuf *out)
 {
 	int got_data = 0;

@@ -28,7 +54,7 @@ static int send_request(const char *socket, const struct strbuf *out)
 		int r;
 
 		r = read_in_full(fd, in, sizeof(in));
-		if (r == 0 || (r < 0 && errno == ECONNRESET))
+		if (r == 0 || (r < 0 && connection_closed(errno)))
 			break;
 		if (r < 0)
 			die_errno("read error from cache daemon");

@@ -75,7 +101,7 @@ static void do_cache(const char *socket, const char *action, int timeout,
 	}
 
 	if (send_request(socket, &buf) < 0) {
-		if (errno != ENOENT && errno != ECONNREFUSED)
+		if (connection_fatally_broken(errno))
 			die_errno("unable to connect to cache daemon");
 		if (flags & FLAG_SPAWN) {
 			spawn_daemon(socket);

-- 
2.33.0.481.g26d3bed244

Available since Windows 10 release 1803 and Windows Server 2019.

NO_UNIX_SOCKETS is still the default for Windows builds, as they need
to keep backward compatibility with releases up to Windows 7, but allow
including the header otherwise.

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
v3:
* better commit message as suggested by Dscho

 git-compat-util.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index b46605300a..6a420d104c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -160,6 +160,9 @@
 # endif
 #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
 #include <winsock2.h>
+#ifndef NO_UNIX_SOCKETS
+#include <afunix.h>
+#endif
 #include <windows.h>
 #define GIT_WINDOWS_NATIVE
 #endif

-- 
2.33.0.481.g26d3bed244

While reviewing Carlo's "credential-cache: check for windows
specific errors", I noticed this piece of code, that came from
8ec6c8d7 (credential-cache: report more daemon connection errors,
2012-01-09):

	if (send_request(socket, &buf) < 0) {
		if (errno != ENOENT && errno != ECONNREFUSED)
			die_errno("unable to connect to cache daemon");
		if (flags & FLAG_SPAWN) {
			spawn_daemon(socket);
			if (send_request(socket, &buf) < 0)
				die_errno("unable to connect to cache daemon");
		}
	}

What would happen when we get a resumable error and then weren't
given the SPAWN flag?  It seems that do_cache() simply returns
without dying.  Shouldn't we get "unable to connect" in such a case?

This in turn came from 98c2924c (credentials: unable to connect to
cache daemon, 2012-01-07), before it, the code read like this:

	if (!send_request(socket, &buf))
		return;
	if (flags & FLAG_SPAWN) {
 		spawn_daemon(socket);
		send_request(socket, &buf);
 	}

so it looks to me that I am puzzled by an incomplete error handling
introduced by 98c2924c, and if we wanted to complete it, it may
perhaps look like this patch on top of Carlo's patch?

Or am I barking up a wrong tree and error checking in this command
does not really make a real-life difference?

Thanks.


 builtin/credential-cache.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git c/builtin/credential-cache.c w/builtin/credential-cache.c
index 78c02ad531..a41a17e58f 100644
--- c/builtin/credential-cache.c
+++ w/builtin/credential-cache.c

@@ -101,13 +101,11 @@ static void do_cache(const char *socket, const char *action, int timeout,
 	}
 
 	if (send_request(socket, &buf) < 0) {
-		if (connection_fatally_broken(errno))
+		if (connection_fatally_broken(errno) && !(flag & FLAG_SPAWN))
+			die_errno("unable to connect to cache daemon");
+		spawn_daemon(socket);
+		if (send_request(socket, &buf) < 0)
 			die_errno("unable to connect to cache daemon");
-		if (flags & FLAG_SPAWN) {
-			spawn_daemon(socket);
-			if (send_request(socket, &buf) < 0)
-				die_errno("unable to connect to cache daemon");
-		}
 	}
 	strbuf_release(&buf);
 }

On Tue, Sep 14, 2021 at 12:09:18PM -0700, Junio C Hamano wrote:

While reviewing Carlo's "credential-cache: check for windows
specific errors", I noticed this piece of code, that came from
8ec6c8d7 (credential-cache: report more daemon connection errors,
2012-01-09):

	if (send_request(socket, &buf) < 0) {
		if (errno != ENOENT && errno != ECONNREFUSED)
			die_errno("unable to connect to cache daemon");
		if (flags & FLAG_SPAWN) {
			spawn_daemon(socket);
			if (send_request(socket, &buf) < 0)
				die_errno("unable to connect to cache daemon");
		}
	}

What would happen when we get a resumable error and then weren't
given the SPAWN flag?  It seems that do_cache() simply returns
without dying.  Shouldn't we get "unable to connect" in such a case?

It's subtle, but I think we end up doing the right thing. Those errors
aren't just "resumable"; they are "we do not have a daemon to talk to at
all".

The "exit", "get", and "erase" operations do not pass the SPAWN flag.
But if there is no daemon, they are all noops.

The "store" operation is the only one which uses SPAWN, and of course
there we want to spin up a daemon so that we can put something in it.

It may be that SPAWN could have a better name to make this more clear.

If you do this, then I think we'll start producing spurious errors
during normal use of the helper. Most interaction will generally start
with a "get" request to the helpers. So if you don't have anything
cached and the daemon stopped running, right now we just don't return a
credential. With this we'd complain "unable to connect to daemon".

And then of course we'd follow that up by asking for the credential and
spinning up a daemon to store it. But that first request after the
daemon times out will always say "unable to connect".

-Peff

On Tue, Sep 14 2021, Carlo Marcelo Arenas Belón wrote:

[...]
`mkdir -m` can't represent a valid ACL directly and fails with
permission problems, so instead call mkdir followed by chmod, which
has been enhanced to do so.

This looks like a good change:

But this adjacent changes is also needed to make this pass on
AIX. I.e. for the adjacent test, which I assume "works" on your Windows
setup because it doesn't have SYMLINKS:

diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh
index 698b7159f03..120b50e8c14 100755
--- a/t/t0301-credential-cache.sh
+++ b/t/t0301-credential-cache.sh

@@ -111,7 +111,8 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to
                rmdir \"\$HOME/dir/\" &&
                rm \"\$HOME/.git-credential-cache\"
        " &&
-       mkdir -p -m 700 "$HOME/dir/" &&
+       mkdir -p "$HOME/dir/" &&
+       chmod 700 "$HOME/dir/" &&
        ln -s "$HOME/dir" "$HOME/.git-credential-cache" &&
        check approve cache <<-\EOF &&
        protocol=https

FWIW on AIX this fails because apparently they got quoting wrong and end
up trying to shell out to "chmod 700 [trash dir name up to the first
space]", i.e. the "trash directory" part.

Not an rc0 issue, this has been in "master" for a while...