{"thread":{"id":"56490","subject":"[PATCH 0/3] windows: allow building without NO_UNIX_SOCKETS","startedAt":"2021-09-12T20:29:18Z","lastAt":"2021-11-02T00:45:03Z","messageCount":27,"participants":["Carlo Marcelo Arenas Belón","Junio C Hamano","Bagas Sanjaya","Carlo Arenas","Johannes Schindelin","Jeff King","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"435651","messageId":"20210912202830.25720-1-carenas@gmail.com","threadId":"56490","inReplyTo":null,"subject":"[PATCH 0/3] windows: allow building without NO_UNIX_SOCKETS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-12T20:28:27Z","receivedAt":"2021-09-12T20:29:18Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,\nit might not be necessary in the future, because Windows added support\nfor Unix Sockets in late 2017.\n\nThe first patch is messy, specially considering how little was needed\nto build and run `git credential-cache` and its daemon, nothing broke\non the trace2 code or tests, but additional testing to make sure will\nbe recommended.\n\nCarlo Marcelo Arenas Belón (3):\n  t0301: fixes for windows compatibility\n  credential-cache: check for windows specific errors\n  git-compat-util: include declaration for unix sockets\n\n builtin/credential-cache.c  |  5 +++--\n git-compat-util.h           |  3 +++\n t/t0301-credential-cache.sh | 28 ++++++++++++++++++++--------\n 3 files changed, 26 insertions(+), 10 deletions(-)\n\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435652","messageId":"20210912202830.25720-2-carenas@gmail.com","threadId":"56490","inReplyTo":"20210912202830.25720-1-carenas@gmail.com","subject":"[PATCH 1/3] t0301: fixes for windows compatibility","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-12T20:28:28Z","receivedAt":"2021-09-12T20:29:18Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"In preparation for a future patch that will allow building with\nUnix Sockets in Windows, workaround a couple of issues from the\nMingw-W64 compatibility layer.\n\ntest -S is not able to detect that a file is a socket, so use\ntest -f instead.\n\n`mkdir -m` will always fail with permission problems, so instead\ncall mkdir followed by chmod.\n\nThe last invocation of mkdir would likely need the same treatment\nbut SYMLINK is unlikely to be enabled on Windows so it has been\npunted for now.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n t/t0301-credential-cache.sh | 28 ++++++++++++++++++++--------\n 1 file changed, 20 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh\nindex ebd5fa5249..f5cce6c6a6 100755\n--- a/t/t0301-credential-cache.sh\n+++ b/t/t0301-credential-cache.sh\n@@ -9,6 +9,17 @@ test -z \"$NO_UNIX_SOCKETS\" || {\n \ttest_done\n }\n \n+# in Windows, Unix Sockets look just like regular files\n+uname_s=$(uname -s)\n+case $uname_s in\n+*MINGW*)\n+\tFLAG=-f\n+\t;;\n+*)\n+\tFLAG=-S\n+\t;;\n+esac\n+\n # don't leave a stale daemon running\n test_atexit 'git credential-cache exit'\n \n@@ -21,7 +32,7 @@ test_expect_success 'socket defaults to ~/.cache/git/credential/socket' '\n \t\trmdir -p .cache/git/credential/\n \t\" &&\n \ttest_path_is_missing \"$HOME/.git-credential-cache\" &&\n-\ttest -S \"$HOME/.cache/git/credential/socket\"\n+\ttest $FLAG \"$HOME/.cache/git/credential/socket\"\n '\n \n XDG_CACHE_HOME=\"$HOME/xdg\"\n@@ -31,7 +42,7 @@ helper_test cache\n \n test_expect_success \"use custom XDG_CACHE_HOME if set and default sockets are not created\" '\n \ttest_when_finished \"git credential-cache exit\" &&\n-\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\" &&\n+\ttest $FLAG \"$XDG_CACHE_HOME/git/credential/socket\" &&\n \ttest_path_is_missing \"$HOME/.git-credential-cache/socket\" &&\n \ttest_path_is_missing \"$HOME/.cache/git/credential/socket\"\n '\n@@ -48,7 +59,7 @@ test_expect_success 'credential-cache --socket option overrides default location\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/dir/socket\"\n+\ttest $FLAG \"$HOME/dir/socket\"\n '\n \n test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n@@ -62,7 +73,7 @@ test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.cache/git/credential/socket\" &&\n+\ttest $FLAG \"$HOME/.cache/git/credential/socket\" &&\n \tXDG_CACHE_HOME=\"$HOME/xdg\" &&\n \texport XDG_CACHE_HOME &&\n \tcheck approve cache <<-\\EOF &&\n@@ -71,7 +82,7 @@ test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\"\n+\ttest $FLAG \"$XDG_CACHE_HOME/git/credential/socket\"\n '\n \n test_expect_success 'use user socket if user directory exists' '\n@@ -79,14 +90,15 @@ test_expect_success 'use user socket if user directory exists' '\n \t\tgit credential-cache exit &&\n \t\trmdir \\\"\\$HOME/.git-credential-cache/\\\"\n \t\" &&\n-\tmkdir -p -m 700 \"$HOME/.git-credential-cache/\" &&\n+\tmkdir -p \"$HOME/.git-credential-cache/\" &&\n+\tchmod 700 \"$HOME/.git-credential-cache/\" &&\n \tcheck approve cache <<-\\EOF &&\n \tprotocol=https\n \thost=example.com\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.git-credential-cache/socket\"\n+\ttest $FLAG \"$HOME/.git-credential-cache/socket\"\n '\n \n test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' '\n@@ -103,7 +115,7 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.git-credential-cache/socket\"\n+\ttest $FLAG \"$HOME/.git-credential-cache/socket\"\n '\n \n helper_test_timeout cache --timeout=1\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435653","messageId":"20210912202830.25720-4-carenas@gmail.com","threadId":"56490","inReplyTo":"20210912202830.25720-1-carenas@gmail.com","subject":"[PATCH 3/3] git-compat-util: include declaration for unix sockets","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-12T20:28:30Z","receivedAt":"2021-09-12T20:29:23Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Available since Windows 10 release 1803, therefore only added if\nnot using NO_UNIX_SOCKETS (which is not the current default).\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n git-compat-util.h | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b46605300a..6a420d104c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -160,6 +160,9 @@\n # endif\n #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */\n #include <winsock2.h>\n+#ifndef NO_UNIX_SOCKETS\n+#include <afunix.h>\n+#endif\n #include <windows.h>\n #define GIT_WINDOWS_NATIVE\n #endif\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435654","messageId":"20210912202830.25720-3-carenas@gmail.com","threadId":"56490","inReplyTo":"20210912202830.25720-1-carenas@gmail.com","subject":"[PATCH 2/3] credential-cache: check for windows specific errors","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-12T20:28:29Z","receivedAt":"2021-09-12T20:29:27Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Connect and reset errors aren't what will be expected by POSIX but\nare compatible with the ones used by WinSock.\n\nAlternatively a translation layer for read could be added (similar\nto the one provided by mingw-write()) to translate the odd EINVAL,\ninto a more reasonable EPIPE, but this is more localized and still\nunlikely to cause confusion in other systems.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n builtin/credential-cache.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/credential-cache.c b/builtin/credential-cache.c\nindex 76a6ba3722..b12a79ae01 100644\n--- a/builtin/credential-cache.c\n+++ b/builtin/credential-cache.c\n@@ -28,7 +28,8 @@ static int send_request(const char *socket, const struct strbuf *out)\n \t\tint r;\n \n \t\tr = read_in_full(fd, in, sizeof(in));\n-\t\tif (r == 0 || (r < 0 && errno == ECONNRESET))\n+\t\tif (r == 0 ||\n+\t\t\t(r < 0 && (errno == ECONNRESET || errno == EINVAL)))\n \t\t\tbreak;\n \t\tif (r < 0)\n \t\t\tdie_errno(\"read error from cache daemon\");\n@@ -75,7 +76,7 @@ static void do_cache(const char *socket, const char *action, int timeout,\n \t}\n \n \tif (send_request(socket, &buf) < 0) {\n-\t\tif (errno != ENOENT && errno != ECONNREFUSED)\n+\t\tif (errno != ENOENT && errno != ECONNREFUSED && errno != ENETDOWN)\n \t\t\tdie_errno(\"unable to connect to cache daemon\");\n \t\tif (flags & FLAG_SPAWN) {\n \t\t\tspawn_daemon(socket);\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435680","messageId":"xmqq1r5ti8vn.fsf@gitster.g","threadId":"56490","inReplyTo":"20210912202830.25720-2-carenas@gmail.com","subject":"Re: [PATCH 1/3] t0301: fixes for windows compatibility","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-13T01:04:12Z","receivedAt":"2021-09-13T01:10:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:\n\n> In preparation for a future patch that will allow building with\n> Unix Sockets in Windows, workaround a couple of issues from the\n> Mingw-W64 compatibility layer.\n>\n> test -S is not able to detect that a file is a socket, so use\n> test -f instead.\n\nThe cause deserves to be sympathized, but ...\n\n> +# in Windows, Unix Sockets look just like regular files\n> +uname_s=$(uname -s)\n> +case $uname_s in\n> +*MINGW*)\n> +\tFLAG=-f\n> +\t;;\n> +*)\n> +\tFLAG=-S\n> +\t;;\n> +esac\n> +\n>  # don't leave a stale daemon running\n>  test_atexit 'git credential-cache exit'\n>  \n> @@ -21,7 +32,7 @@ test_expect_success 'socket defaults to ~/.cache/git/credential/socket' '\n>  \t\trmdir -p .cache/git/credential/\n>  \t\" &&\n>  \ttest_path_is_missing \"$HOME/.git-credential-cache\" &&\n> -\ttest -S \"$HOME/.cache/git/credential/socket\"\n> +\ttest $FLAG \"$HOME/.cache/git/credential/socket\"\n\nThis is horrible.  Unlike the original, it is now impossible for a\ncasual reader to tell what this thing is testing, because $FLAG is\nso generic a name that does not convey anything to readers.\n\nIntroduce a helper, say, \"test_socket_exists\", and replace \"test -S\"\nwith it.  In the implementation of that test_socket_exists() helper,\nyou can hide the difference between -f and -S.  In such a small scope,\nyou can even call the variable $FLAG if you want, as it is so isolated.\n\n> -\tmkdir -p -m 700 \"$HOME/.git-credential-cache/\" &&\n> +\tmkdir -p \"$HOME/.git-credential-cache/\" &&\n> +\tchmod 700 \"$HOME/.git-credential-cache/\" &&\n\nOK.  This is the only use of \"mkdir -m MODE\" in the test suite.  It\nis strange that you are OK with \"mkdir && chmod 700\" but not OK with\n\"mkdir -m 700\" (it just is illogical, but we have to live with it,\nas the real world may not be logical after all).\n\nIt is somewhat strange that we insist on mode 700 but the test does\nnot have SANITY as its prerequisite.  Does it really matter if we\nset the permission bits to close the directory from others?  If not,\nour \"portability clean-up\" could just be to lose \"-m 700\" here.\n\nThanks.\n\n>  \tcheck approve cache <<-\\EOF &&\n>  \tprotocol=https\n>  \thost=example.com\n>  \tusername=store-user\n>  \tpassword=store-pass\n>  \tEOF\n> -\ttest -S \"$HOME/.git-credential-cache/socket\"\n> +\ttest $FLAG \"$HOME/.git-credential-cache/socket\"\n>  '\n>  \n>  test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' '\n> @@ -103,7 +115,7 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to\n>  \tusername=store-user\n>  \tpassword=store-pass\n>  \tEOF\n> -\ttest -S \"$HOME/.git-credential-cache/socket\"\n> +\ttest $FLAG \"$HOME/.git-credential-cache/socket\"\n>  '\n>  \n>  helper_test_timeout cache --timeout=1\n"},{"id":"435681","messageId":"xmqqwnnlgu0b.fsf@gitster.g","threadId":"56490","inReplyTo":"20210912202830.25720-3-carenas@gmail.com","subject":"Re: [PATCH 2/3] credential-cache: check for windows specific errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-13T01:10:44Z","receivedAt":"2021-09-13T01:10:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:\n\n> Connect and reset errors aren't what will be expected by POSIX but\n> are compatible with the ones used by WinSock.\n>\n> Alternatively a translation layer for read could be added (similar\n> to the one provided by mingw-write()) to translate the odd EINVAL,\n> into a more reasonable EPIPE, but this is more localized and still\n> unlikely to cause confusion in other systems.\n\nAre we sure EINVAL or ENETDOWN from outside the Windows world is\ntolerable after read_in_full() failure in this context?\n\nI'd rather see something like\n\n\tif (!r || (r < 0 && connection_closed(errno)))\n\t\tbreak;\n\nwith a local helper function connection_closed() allowing EINVAL\nonly with \"#ifdef WINDOWS\" to be safe.  The same for \"the socket is\nnot yet open\" codepath with another local helper function.\n\nThanks.\n\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>  builtin/credential-cache.c | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c\n> index 76a6ba3722..b12a79ae01 100644\n> --- a/builtin/credential-cache.c\n> +++ b/builtin/credential-cache.c\n> @@ -28,7 +28,8 @@ static int send_request(const char *socket, const struct strbuf *out)\n>  \t\tint r;\n>  \n>  \t\tr = read_in_full(fd, in, sizeof(in));\n> -\t\tif (r == 0 || (r < 0 && errno == ECONNRESET))\n> +\t\tif (r == 0 ||\n> +\t\t\t(r < 0 && (errno == ECONNRESET || errno == EINVAL)))\n>  \t\t\tbreak;\n>  \t\tif (r < 0)\n>  \t\t\tdie_errno(\"read error from cache daemon\");\n> @@ -75,7 +76,7 @@ static void do_cache(const char *socket, const char *action, int timeout,\n>  \t}\n>  \n>  \tif (send_request(socket, &buf) < 0) {\n> -\t\tif (errno != ENOENT && errno != ECONNREFUSED)\n> +\t\tif (errno != ENOENT && errno != ECONNREFUSED && errno != ENETDOWN)\n>  \t\t\tdie_errno(\"unable to connect to cache daemon\");\n>  \t\tif (flags & FLAG_SPAWN) {\n>  \t\t\tspawn_daemon(socket);\n"},{"id":"435691","messageId":"0606160d-1192-2074-b1ad-9c5d86c953d5@gmail.com","threadId":"56490","inReplyTo":"20210912202830.25720-2-carenas@gmail.com","subject":"Re: [PATCH 1/3] t0301: fixes for windows compatibility","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-09-13T05:34:41Z","receivedAt":"2021-09-13T05:34:46Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 13/09/21 03.28, Carlo Marcelo Arenas Belón wrote:\n> test -S is not able to detect that a file is a socket, so use\n> test -f instead.\n> \n\nIsn't test -f just check for socket as regular file?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"435695","messageId":"CAPUEspi1NzM34whXOT8T7NgJ1SCBGTPETcF0MY-UHz+kRqCTZQ@mail.gmail.com","threadId":"56490","inReplyTo":"0606160d-1192-2074-b1ad-9c5d86c953d5@gmail.com","subject":"Re: [PATCH 1/3] t0301: fixes for windows compatibility","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-09-13T07:13:50Z","receivedAt":"2021-09-13T07:14:06Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sun, Sep 12, 2021 at 10:34 PM Bagas Sanjaya <bagasdotme@gmail.com> wrote:\n> On 13/09/21 03.28, Carlo Marcelo Arenas Belón wrote:\n> > test -S is not able to detect that a file is a socket, so use\n> > test -f instead.\n>\n> Isn't test -f just check for socket as regular file?\n\nand that is exactly how they look; ironically a -f check in Linux\nfails for sockets so maybe better to do -e?\n\nan empty file with nothing that indicates in Windows Explorer or a\nstat call (from WSL or git bash), that they are anything else.\n\nCarlo\n"},{"id":"435696","messageId":"20210913085600.35506-1-carenas@gmail.com","threadId":"56490","inReplyTo":"20210912202830.25720-1-carenas@gmail.com","subject":"[PATCH v2 0/3] windows: allow building without NO_UNIX_SOCKETS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-13T08:55:57Z","receivedAt":"2021-09-13T08:56:30Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,\nit might not be necessary in the future, because Windows added support\nfor Unix Sockets in late 2017.\n\nThe implementation of Unix Sockets, uses an internal NTFS mechanism\nand is therefore not visible at the filesystem layer, like it is in\nUNIX, but seems to be good enough to allow to build and run the\n`git credential-cache` and its daemon; additional testing to confirm\ntrace2 (builds and doesn't fail any tests) is functional will be\nneeded.\n\nV2 reuses the same third patch from V1, and applies all suggested\nfeedback on patches 1 and 2 and should apply cleanly as a reroll of\ncb/unix-sockets-with-windows.\n\nCarlo Marcelo Arenas Belón (3):\n  t0301: fixes for windows compatibility\n  credential-cache: check for windows specific errors\n  git-compat-util: include declaration for unix sockets\n\n builtin/credential-cache.c  | 30 ++++++++++++++++++++++++++++--\n git-compat-util.h           |  3 +++\n t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------\n 3 files changed, 55 insertions(+), 10 deletions(-)\n\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435697","messageId":"20210913085600.35506-2-carenas@gmail.com","threadId":"56490","inReplyTo":"20210913085600.35506-1-carenas@gmail.com","subject":"[PATCH v2 1/3] t0301: fixes for windows compatibility","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-13T08:55:58Z","receivedAt":"2021-09-13T08:56:32Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"In preparation for a future patch that will allow building with\nUnix Sockets in Windows, workaround a couple of issues from the\nMingw-W64 compatibility layer.\n\ntest -S is not able to detect that a file is a socket, so use\ntest -e instead (through a library function).\n\n`mkdir -m` will always fail with permission problems, so instead\ncall mkdir followed by chmod.\n\nThe last invocation of mkdir would likely need the same treatment\nbut SYMLINK is unlikely to be enabled on Windows so it has been\npunted for now.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\nv2:\n* avoid the confusing -f test as suggested by Bagas\n* try to help casual readers as suggested by Junio\n\n t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------\n 1 file changed, 24 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh\nindex ebd5fa5249..1f7b1e29e6 100755\n--- a/t/t0301-credential-cache.sh\n+++ b/t/t0301-credential-cache.sh\n@@ -9,6 +9,21 @@ test -z \"$NO_UNIX_SOCKETS\" || {\n \ttest_done\n }\n \n+test_path_is_socket () {\n+\ttest -S \"$1\"\n+}\n+\n+# in Windows, Unix Sockets look just like regular files\n+uname_s=$(uname -s)\n+case $uname_s in\n+*MINGW*)\n+\ttest_socket_exist=test_path_exists\n+\t;;\n+*)\n+\ttest_socket_exist=test_path_is_socket\n+\t;;\n+esac\n+\n # don't leave a stale daemon running\n test_atexit 'git credential-cache exit'\n \n@@ -21,7 +36,7 @@ test_expect_success 'socket defaults to ~/.cache/git/credential/socket' '\n \t\trmdir -p .cache/git/credential/\n \t\" &&\n \ttest_path_is_missing \"$HOME/.git-credential-cache\" &&\n-\ttest -S \"$HOME/.cache/git/credential/socket\"\n+\t$test_socket_exist \"$HOME/.cache/git/credential/socket\"\n '\n \n XDG_CACHE_HOME=\"$HOME/xdg\"\n@@ -31,7 +46,7 @@ helper_test cache\n \n test_expect_success \"use custom XDG_CACHE_HOME if set and default sockets are not created\" '\n \ttest_when_finished \"git credential-cache exit\" &&\n-\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\" &&\n+\t$test_socket_exist \"$XDG_CACHE_HOME/git/credential/socket\" &&\n \ttest_path_is_missing \"$HOME/.git-credential-cache/socket\" &&\n \ttest_path_is_missing \"$HOME/.cache/git/credential/socket\"\n '\n@@ -48,7 +63,7 @@ test_expect_success 'credential-cache --socket option overrides default location\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/dir/socket\"\n+\t$test_socket_exist \"$HOME/dir/socket\"\n '\n \n test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n@@ -62,7 +77,7 @@ test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.cache/git/credential/socket\" &&\n+\t$test_socket_exist \"$HOME/.cache/git/credential/socket\" &&\n \tXDG_CACHE_HOME=\"$HOME/xdg\" &&\n \texport XDG_CACHE_HOME &&\n \tcheck approve cache <<-\\EOF &&\n@@ -71,7 +86,7 @@ test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\"\n+\t$test_socket_exist \"$XDG_CACHE_HOME/git/credential/socket\"\n '\n \n test_expect_success 'use user socket if user directory exists' '\n@@ -79,14 +94,15 @@ test_expect_success 'use user socket if user directory exists' '\n \t\tgit credential-cache exit &&\n \t\trmdir \\\"\\$HOME/.git-credential-cache/\\\"\n \t\" &&\n-\tmkdir -p -m 700 \"$HOME/.git-credential-cache/\" &&\n+\tmkdir -p \"$HOME/.git-credential-cache/\" &&\n+\tchmod 700 \"$HOME/.git-credential-cache/\" &&\n \tcheck approve cache <<-\\EOF &&\n \tprotocol=https\n \thost=example.com\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.git-credential-cache/socket\"\n+\t$test_socket_exist \"$HOME/.git-credential-cache/socket\"\n '\n \n test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' '\n@@ -103,7 +119,7 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.git-credential-cache/socket\"\n+\t$test_socket_exist \"$HOME/.git-credential-cache/socket\"\n '\n \n helper_test_timeout cache --timeout=1\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435698","messageId":"20210913085600.35506-3-carenas@gmail.com","threadId":"56490","inReplyTo":"20210913085600.35506-1-carenas@gmail.com","subject":"[PATCH v2 2/3] credential-cache: check for windows specific errors","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-13T08:55:59Z","receivedAt":"2021-09-13T08:56:33Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Connect and reset errors aren't what will be expected by POSIX but\nare compatible with the ones used by WinSock.\n\nTo avoid any possibility of confusion with other systems checks\nfor disconnection and availability had been abstracted into helper\nfunctions that are platform specific.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\nV2:\n* Use helper functions to separate error handling as suggested by Junio\n\n builtin/credential-cache.c | 30 ++++++++++++++++++++++++++++--\n 1 file changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/credential-cache.c b/builtin/credential-cache.c\nindex e8a7415747..fd9f33d993 100644\n--- a/builtin/credential-cache.c\n+++ b/builtin/credential-cache.c\n@@ -11,6 +11,32 @@\n #define FLAG_SPAWN 0x1\n #define FLAG_RELAY 0x2\n \n+#ifdef _WIN32\n+\n+static int connection_closed(int error)\n+{\n+\treturn (error == EINVAL);\n+}\n+\n+static int connection_fatally_broken(int error)\n+{\n+\treturn (error != ENOENT) && (error != ENETDOWN);\n+}\n+\n+#else\n+\n+static int connection_closed(int error)\n+{\n+\treturn (error == ECONNRESET);\n+}\n+\n+static int connection_fatally_broken(int error)\n+{\n+\treturn (error != ENOENT) && (error != ECONNREFUSED);\n+}\n+\n+#endif\n+\n static int send_request(const char *socket, const struct strbuf *out)\n {\n \tint got_data = 0;\n@@ -28,7 +54,7 @@ static int send_request(const char *socket, const struct strbuf *out)\n \t\tint r;\n \n \t\tr = read_in_full(fd, in, sizeof(in));\n-\t\tif (r == 0 || (r < 0 && errno == ECONNRESET))\n+\t\tif (r == 0 || (r < 0 && connection_closed(errno)))\n \t\t\tbreak;\n \t\tif (r < 0)\n \t\t\tdie_errno(\"read error from cache daemon\");\n@@ -75,7 +101,7 @@ static void do_cache(const char *socket, const char *action, int timeout,\n \t}\n \n \tif (send_request(socket, &buf) < 0) {\n-\t\tif (errno != ENOENT && errno != ECONNREFUSED)\n+\t\tif (connection_fatally_broken(errno))\n \t\t\tdie_errno(\"unable to connect to cache daemon\");\n \t\tif (flags & FLAG_SPAWN) {\n \t\t\tspawn_daemon(socket);\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435699","messageId":"20210913085600.35506-4-carenas@gmail.com","threadId":"56490","inReplyTo":"20210913085600.35506-1-carenas@gmail.com","subject":"[PATCH v2 3/3] git-compat-util: include declaration for unix sockets","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-13T08:56:00Z","receivedAt":"2021-09-13T08:56:35Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Available since Windows 10 release 1803, therefore only added if\nnot using NO_UNIX_SOCKETS (which is not the current default).\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n git-compat-util.h | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b46605300a..6a420d104c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -160,6 +160,9 @@\n # endif\n #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */\n #include <winsock2.h>\n+#ifndef NO_UNIX_SOCKETS\n+#include <afunix.h>\n+#endif\n #include <windows.h>\n #define GIT_WINDOWS_NATIVE\n #endif\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435710","messageId":"nycvar.QRO.7.76.6.2109131339540.55@tvgsbejvaqbjf.bet","threadId":"56490","inReplyTo":"20210913085600.35506-1-carenas@gmail.com","subject":"Re: [PATCH v2 0/3] windows: allow building without NO_UNIX_SOCKETS","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-09-13T11:42:57Z","receivedAt":"2021-09-13T11:43:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Mon, 13 Sep 2021, Carlo Marcelo Arenas Belón wrote:\n\n> Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,\n> it might not be necessary in the future, because Windows added support\n> for Unix Sockets in late 2017.\n\nThe fact that some recent Windows 10 versions support Unix sockets is\nnice, no doubt. As far as Git for Windows is concerned (which does not\nrecompile itself during an installation based on the Windows version it\nfound), we cannot use it, of course. At least not until all of the\nprevious Windows versions are _long_ out of service. For example, we still\nsupport Windows 7 even if it reached it's \"End-Of-Life\".\n\nHaving said that: thank you for working on this. For users who build their\nGit themselves (which is definitely something Git for Windows makes\neasy), the option to use Unix sockets is really nice.\n\nCiao,\nDscho\n\n> The implementation of Unix Sockets, uses an internal NTFS mechanism\n> and is therefore not visible at the filesystem layer, like it is in\n> UNIX, but seems to be good enough to allow to build and run the\n> `git credential-cache` and its daemon; additional testing to confirm\n> trace2 (builds and doesn't fail any tests) is functional will be\n> needed.\n>\n> V2 reuses the same third patch from V1, and applies all suggested\n> feedback on patches 1 and 2 and should apply cleanly as a reroll of\n> cb/unix-sockets-with-windows.\n>\n> Carlo Marcelo Arenas Belón (3):\n>   t0301: fixes for windows compatibility\n>   credential-cache: check for windows specific errors\n>   git-compat-util: include declaration for unix sockets\n>\n>  builtin/credential-cache.c  | 30 ++++++++++++++++++++++++++++--\n>  git-compat-util.h           |  3 +++\n>  t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------\n>  3 files changed, 55 insertions(+), 10 deletions(-)\n>\n> --\n> 2.33.0.481.g26d3bed244\n>\n>\n"},{"id":"435712","messageId":"nycvar.QRO.7.76.6.2109131343440.55@tvgsbejvaqbjf.bet","threadId":"56490","inReplyTo":"20210913085600.35506-2-carenas@gmail.com","subject":"Re: [PATCH v2 1/3] t0301: fixes for windows compatibility","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-09-13T11:50:59Z","receivedAt":"2021-09-13T11:51:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Mon, 13 Sep 2021, Carlo Marcelo Arenas Belón wrote:\n\n> In preparation for a future patch that will allow building with\n> Unix Sockets in Windows, workaround a couple of issues from the\n> Mingw-W64 compatibility layer.\n>\n> test -S is not able to detect that a file is a socket, so use\n\nIs that really true, even with recent MSYS2 versions? I thought I saw some\npatch flying around on the Cygwin mailing list that added support for Unix\nsockets...\n\n> test -e instead (through a library function).\n>\n> `mkdir -m` will always fail with permission problems, so instead\n> call mkdir followed by chmod.\n\nMaybe explain that Windows' permission model is a lot more sophisticated\n(using Access Control Lists) and is therefore unable to interpret\npermissions like `0700`.\n\nAnd we probably need to mention then, too, that it is funny that `mkdir\n-m` complains while `chmod` does _not_... Ah, historical reasons...\n\nThanks,\nDscho\n\n>\n> The last invocation of mkdir would likely need the same treatment\n> but SYMLINK is unlikely to be enabled on Windows so it has been\n> punted for now.\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n> v2:\n> * avoid the confusing -f test as suggested by Bagas\n> * try to help casual readers as suggested by Junio\n>\n>  t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------\n>  1 file changed, 24 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh\n> index ebd5fa5249..1f7b1e29e6 100755\n> --- a/t/t0301-credential-cache.sh\n> +++ b/t/t0301-credential-cache.sh\n> @@ -9,6 +9,21 @@ test -z \"$NO_UNIX_SOCKETS\" || {\n>  \ttest_done\n>  }\n>\n> +test_path_is_socket () {\n> +\ttest -S \"$1\"\n> +}\n> +\n> +# in Windows, Unix Sockets look just like regular files\n> +uname_s=$(uname -s)\n> +case $uname_s in\n> +*MINGW*)\n> +\ttest_socket_exist=test_path_exists\n> +\t;;\n> +*)\n> +\ttest_socket_exist=test_path_is_socket\n> +\t;;\n> +esac\n\nA more canonical way would probably be to imitate what we do with `pwd` in\n`t/test-lib.sh`:\n\n\ttest_path_is_socket () {\n\t\ttest -S \"$1\"\n\t}\n\n\tcase $uname_s in\n\t*MINGW*)\n\t\ttest_path_is_socket () {\n\t\t\t# `test -S` cannot detect Win10's Unix sockets\n\t\t\ttest -e \"$1\"\n\t\t}\n\t\t;;\n\tesac\n\n> +\n>  # don't leave a stale daemon running\n>  test_atexit 'git credential-cache exit'\n>\n> @@ -21,7 +36,7 @@ test_expect_success 'socket defaults to ~/.cache/git/credential/socket' '\n>  \t\trmdir -p .cache/git/credential/\n>  \t\" &&\n>  \ttest_path_is_missing \"$HOME/.git-credential-cache\" &&\n> -\ttest -S \"$HOME/.cache/git/credential/socket\"\n> +\t$test_socket_exist \"$HOME/.cache/git/credential/socket\"\n>  '\n>\n>  XDG_CACHE_HOME=\"$HOME/xdg\"\n> @@ -31,7 +46,7 @@ helper_test cache\n>\n>  test_expect_success \"use custom XDG_CACHE_HOME if set and default sockets are not created\" '\n>  \ttest_when_finished \"git credential-cache exit\" &&\n> -\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\" &&\n> +\t$test_socket_exist \"$XDG_CACHE_HOME/git/credential/socket\" &&\n>  \ttest_path_is_missing \"$HOME/.git-credential-cache/socket\" &&\n>  \ttest_path_is_missing \"$HOME/.cache/git/credential/socket\"\n>  '\n> @@ -48,7 +63,7 @@ test_expect_success 'credential-cache --socket option overrides default location\n>  \tusername=store-user\n>  \tpassword=store-pass\n>  \tEOF\n> -\ttest -S \"$HOME/dir/socket\"\n> +\t$test_socket_exist \"$HOME/dir/socket\"\n>  '\n>\n>  test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n> @@ -62,7 +77,7 @@ test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n>  \tusername=store-user\n>  \tpassword=store-pass\n>  \tEOF\n> -\ttest -S \"$HOME/.cache/git/credential/socket\" &&\n> +\t$test_socket_exist \"$HOME/.cache/git/credential/socket\" &&\n>  \tXDG_CACHE_HOME=\"$HOME/xdg\" &&\n>  \texport XDG_CACHE_HOME &&\n>  \tcheck approve cache <<-\\EOF &&\n> @@ -71,7 +86,7 @@ test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n>  \tusername=store-user\n>  \tpassword=store-pass\n>  \tEOF\n> -\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\"\n> +\t$test_socket_exist \"$XDG_CACHE_HOME/git/credential/socket\"\n>  '\n>\n>  test_expect_success 'use user socket if user directory exists' '\n> @@ -79,14 +94,15 @@ test_expect_success 'use user socket if user directory exists' '\n>  \t\tgit credential-cache exit &&\n>  \t\trmdir \\\"\\$HOME/.git-credential-cache/\\\"\n>  \t\" &&\n> -\tmkdir -p -m 700 \"$HOME/.git-credential-cache/\" &&\n> +\tmkdir -p \"$HOME/.git-credential-cache/\" &&\n> +\tchmod 700 \"$HOME/.git-credential-cache/\" &&\n>  \tcheck approve cache <<-\\EOF &&\n>  \tprotocol=https\n>  \thost=example.com\n>  \tusername=store-user\n>  \tpassword=store-pass\n>  \tEOF\n> -\ttest -S \"$HOME/.git-credential-cache/socket\"\n> +\t$test_socket_exist \"$HOME/.git-credential-cache/socket\"\n>  '\n>\n>  test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' '\n> @@ -103,7 +119,7 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to\n>  \tusername=store-user\n>  \tpassword=store-pass\n>  \tEOF\n> -\ttest -S \"$HOME/.git-credential-cache/socket\"\n> +\t$test_socket_exist \"$HOME/.git-credential-cache/socket\"\n>  '\n>\n>  helper_test_timeout cache --timeout=1\n> --\n> 2.33.0.481.g26d3bed244\n>\n>\n"},{"id":"435713","messageId":"nycvar.QRO.7.76.6.2109131357280.55@tvgsbejvaqbjf.bet","threadId":"56490","inReplyTo":"20210913085600.35506-3-carenas@gmail.com","subject":"Re: [PATCH v2 2/3] credential-cache: check for windows specific errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-09-13T11:58:55Z","receivedAt":"2021-09-13T11:59:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Mon, 13 Sep 2021, Carlo Marcelo Arenas Belón wrote:\n\n> Connect and reset errors aren't what will be expected by POSIX but\n> are compatible with the ones used by WinSock.\n>\n> To avoid any possibility of confusion with other systems checks\n> for disconnection and availability had been abstracted into helper\n> functions that are platform specific.\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n> V2:\n> * Use helper functions to separate error handling as suggested by Junio\n>\n>  builtin/credential-cache.c | 30 ++++++++++++++++++++++++++++--\n>  1 file changed, 28 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c\n> index e8a7415747..fd9f33d993 100644\n> --- a/builtin/credential-cache.c\n> +++ b/builtin/credential-cache.c\n> @@ -11,6 +11,32 @@\n>  #define FLAG_SPAWN 0x1\n>  #define FLAG_RELAY 0x2\n>\n> +#ifdef _WIN32\n\nWhile that works, I think we prefer `WIN32` (`_WIN32` is only used in\n`compat/` and `contrib/`).\n\nOther than that, looks good!\n\nCiao,\nDscho\n\n> +\n> +static int connection_closed(int error)\n> +{\n> +\treturn (error == EINVAL);\n> +}\n> +\n> +static int connection_fatally_broken(int error)\n> +{\n> +\treturn (error != ENOENT) && (error != ENETDOWN);\n> +}\n> +\n> +#else\n> +\n> +static int connection_closed(int error)\n> +{\n> +\treturn (error == ECONNRESET);\n> +}\n> +\n> +static int connection_fatally_broken(int error)\n> +{\n> +\treturn (error != ENOENT) && (error != ECONNREFUSED);\n> +}\n> +\n> +#endif\n> +\n>  static int send_request(const char *socket, const struct strbuf *out)\n>  {\n>  \tint got_data = 0;\n> @@ -28,7 +54,7 @@ static int send_request(const char *socket, const struct strbuf *out)\n>  \t\tint r;\n>\n>  \t\tr = read_in_full(fd, in, sizeof(in));\n> -\t\tif (r == 0 || (r < 0 && errno == ECONNRESET))\n> +\t\tif (r == 0 || (r < 0 && connection_closed(errno)))\n>  \t\t\tbreak;\n>  \t\tif (r < 0)\n>  \t\t\tdie_errno(\"read error from cache daemon\");\n> @@ -75,7 +101,7 @@ static void do_cache(const char *socket, const char *action, int timeout,\n>  \t}\n>\n>  \tif (send_request(socket, &buf) < 0) {\n> -\t\tif (errno != ENOENT && errno != ECONNREFUSED)\n> +\t\tif (connection_fatally_broken(errno))\n>  \t\t\tdie_errno(\"unable to connect to cache daemon\");\n>  \t\tif (flags & FLAG_SPAWN) {\n>  \t\t\tspawn_daemon(socket);\n> --\n> 2.33.0.481.g26d3bed244\n>\n>\n"},{"id":"435714","messageId":"nycvar.QRO.7.76.6.2109131359130.55@tvgsbejvaqbjf.bet","threadId":"56490","inReplyTo":"20210913085600.35506-4-carenas@gmail.com","subject":"Re: [PATCH v2 3/3] git-compat-util: include declaration for unix sockets","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-09-13T11:59:48Z","receivedAt":"2021-09-13T11:59:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Mon, 13 Sep 2021, Carlo Marcelo Arenas Belón wrote:\n\n> Available since Windows 10 release 1803, therefore only added if\n> not using NO_UNIX_SOCKETS (which is not the current default).\n\nI wouldn't mind one bit if we did not have a double negation ;-)\n\nOther than that, looks good!\n\nCiao,\nDscho\n\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>  git-compat-util.h | 3 +++\n>  1 file changed, 3 insertions(+)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index b46605300a..6a420d104c 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -160,6 +160,9 @@\n>  # endif\n>  #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */\n>  #include <winsock2.h>\n> +#ifndef NO_UNIX_SOCKETS\n> +#include <afunix.h>\n> +#endif\n>  #include <windows.h>\n>  #define GIT_WINDOWS_NATIVE\n>  #endif\n> --\n> 2.33.0.481.g26d3bed244\n>\n>\n"},{"id":"435757","messageId":"xmqqtuiofj71.fsf@gitster.g","threadId":"56490","inReplyTo":"CAPUEspi1NzM34whXOT8T7NgJ1SCBGTPETcF0MY-UHz+kRqCTZQ@mail.gmail.com","subject":"Re: [PATCH 1/3] t0301: fixes for windows compatibility","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-13T18:01:54Z","receivedAt":"2021-09-13T18:01:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Arenas <carenas@gmail.com> writes:\n\n> On Sun, Sep 12, 2021 at 10:34 PM Bagas Sanjaya <bagasdotme@gmail.com> wrote:\n>> On 13/09/21 03.28, Carlo Marcelo Arenas Belón wrote:\n>> > test -S is not able to detect that a file is a socket, so use\n>> > test -f instead.\n>>\n>> Isn't test -f just check for socket as regular file?\n>\n> and that is exactly how they look; ironically a -f check in Linux\n> fails for sockets so maybe better to do -e?\n>\n> an empty file with nothing that indicates in Windows Explorer or a\n> stat call (from WSL or git bash), that they are anything else.\n\nIt actually is a quite attractive idea to use \"-e\", or even more\npreferrably, test_path_exists.  For example:\n\n@@ -31,7 +42,7 @@ helper_test cache\n \n test_expect_success \"use custom XDG_CACHE_HOME if set and default sockets are not created\" '\n \ttest_when_finished \"git credential-cache exit\" &&\n-\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\" &&\n+\ttest $FLAG \"$XDG_CACHE_HOME/git/credential/socket\" &&\n \ttest_path_is_missing \"$HOME/.git-credential-cache/socket\" &&\n \ttest_path_is_missing \"$HOME/.cache/git/credential/socket\"\n '\n\ntest_path_exists contrasts better with the two test_path_is_missing\nand explains what is being tested better.  Before this part, we have\nrun some \"git credential\" test, and there are three possible places\nthat the socket may appear (XDG, HOME/.git-credential-cache/ and\nHOME/.cache/git/credential/), and we want to make sure only one of\nthem gets it.\n\nOne possible downside is that it makes us rely more on our knowledge\nthat we communicate via unix-domain socket (i.e. what the \"socket\"\nthe test is checking is).  By assuming that a mere presence of some\nfilesystem entity at the inspected path is OK, we may not notice a\nbreakage that creates a regular file or a directory there by mistake,\nyet successfully carry out the credential tests.  It may even be a\ngood thing, if future ourselves have somehow found out how to use a\nregular file or a directory for IPC instead of using a socket ;-).\n\nThanks.\n\n\n\n"},{"id":"435760","messageId":"xmqqpmtcfiuq.fsf@gitster.g","threadId":"56490","inReplyTo":"nycvar.QRO.7.76.6.2109131343440.55@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 1/3] t0301: fixes for windows compatibility","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-13T18:09:17Z","receivedAt":"2021-09-13T18:09:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> +test_path_is_socket () {\n>> +\ttest -S \"$1\"\n>> +}\n>> +\n>> +# in Windows, Unix Sockets look just like regular files\n>> +uname_s=$(uname -s)\n>> +case $uname_s in\n>> +*MINGW*)\n>> +\ttest_socket_exist=test_path_exists\n>> +\t;;\n>> +*)\n>> +\ttest_socket_exist=test_path_is_socket\n>> +\t;;\n>> +esac\n>\n> A more canonical way would probably be to imitate what we do with `pwd` in\n> `t/test-lib.sh`:\n\nThanks for bringing up a better practice.\n\nReferring to a variable when calling a function gives a \"we are\ndoing something unusual\" signal and it loses half its abstraction\nvalue at the callsites.  E.g.\n\n>>  \ttest_when_finished \"git credential-cache exit\" &&\n>> -\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\" &&\n>> +\t$test_socket_exist \"$XDG_CACHE_HOME/git/credential/socket\" &&\n>>  \ttest_path_is_missing \"$HOME/.git-credential-cache/socket\" &&\n>>  \ttest_path_is_missing \"$HOME/.cache/git/credential/socket\"\n\nI actually do not think it is so bad to just use test_path_exists\nwithout per-platform conditional in this case, but if we want to be\nmore conservative, I agree with you that\n\n\tcase ... in\n\t*MINGW*)\n                test_path_is_socket () {\n                        test_path_exists \"$@\"\n                }\n        \t;;\n\t*)\n                test_path_is_socket () {\n                        test -S \"$1\"\n                }\n\t\t;;\n\tesac\n\nis the way to go.\n"},{"id":"435829","messageId":"20210914072600.11552-1-carenas@gmail.com","threadId":"56490","inReplyTo":"20210913085600.35506-1-carenas@gmail.com","subject":"[PATCH v3 0/3] windows: allow building without NO_UNIX_SOCKETS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-14T07:25:57Z","receivedAt":"2021-09-14T07:26:20Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,\nit might not be necessary in the future, because Windows added support\nfor Unix Sockets in late 2017.\n\nThe implementation of Unix Sockets, uses an internal NTFS mechanism\nand is therefore not visible at the filesystem layer, like it is in\nUNIX, but seems to be good enough to allow to build and run the\n`git credential-cache` and its daemon, with not many changes.\n\nAdditional testing to confirm trace2 (which builds and doesn't fail any\ntests) is functional will be also needed.\n\nInterestingly, Cygwin has its own implementation of Unix Sockets that\nare visible as such even in the Git Bash environment, but that are also\nincompatible with the native ones.\n\nV3 Changes the commit messages as suggested and fixes the original ugly\nimplementation of the workarounds for the first patch and shoule apply\ncleanly as a reroll of cb/unix-sockets-with-windows.\n\nCarlo Marcelo Arenas Belón (3):\n  t0301: fixes for windows compatibility\n  credential-cache: check for windows specific errors\n  git-compat-util: include declaration for unix sockets in windows\n\n builtin/credential-cache.c  | 30 ++++++++++++++++++++++++++++--\n git-compat-util.h           |  3 +++\n t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------\n 3 files changed, 55 insertions(+), 10 deletions(-)\n\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435830","messageId":"20210914072600.11552-2-carenas@gmail.com","threadId":"56490","inReplyTo":"20210914072600.11552-1-carenas@gmail.com","subject":"[PATCH v3 1/3] t0301: fixes for windows compatibility","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-14T07:25:58Z","receivedAt":"2021-09-14T07:26:20Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"In preparation for a future patch that will allow building with\nUnix Sockets in Windows, workaround a couple of issues from the\nMingw-W64 compatibility layer.\n\ntest -S is not able to detect that a file is a socket, so use\ntest -e instead (through a library function).\n\n`mkdir -m` can't represent a valid ACL directly and fails with\npermission problems, so instead call mkdir followed by chmod, which\nhas been enhanced to do so.\n\nThe last invocation of mkdir would likely need the same treatment\nbut SYMLINK is unlikely to be enabled on Windows so it has been\npunted for now.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\nv3:\n* avoid using a variable to hold the function for testing and instead\n  use the cleaner syntax suggested by Dscho\nv2:\n* avoid the confusing -f test as suggested by Bagas\n* no more FLAG variable as suggested by Junio\n\n t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++--------\n 1 file changed, 24 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh\nindex ebd5fa5249..698b7159f0 100755\n--- a/t/t0301-credential-cache.sh\n+++ b/t/t0301-credential-cache.sh\n@@ -9,6 +9,21 @@ test -z \"$NO_UNIX_SOCKETS\" || {\n \ttest_done\n }\n \n+uname_s=$(uname -s)\n+case $uname_s in\n+*MINGW*)\n+\ttest_path_is_socket () {\n+\t\t# `test -S` cannot detect Win10's Unix sockets\n+\t\ttest_path_exists \"$1\"\n+\t}\n+\t;;\n+*)\n+\ttest_path_is_socket () {\n+\t\ttest -S \"$1\"\n+\t}\n+\t;;\n+esac\n+\n # don't leave a stale daemon running\n test_atexit 'git credential-cache exit'\n \n@@ -21,7 +36,7 @@ test_expect_success 'socket defaults to ~/.cache/git/credential/socket' '\n \t\trmdir -p .cache/git/credential/\n \t\" &&\n \ttest_path_is_missing \"$HOME/.git-credential-cache\" &&\n-\ttest -S \"$HOME/.cache/git/credential/socket\"\n+\ttest_path_is_socket \"$HOME/.cache/git/credential/socket\"\n '\n \n XDG_CACHE_HOME=\"$HOME/xdg\"\n@@ -31,7 +46,7 @@ helper_test cache\n \n test_expect_success \"use custom XDG_CACHE_HOME if set and default sockets are not created\" '\n \ttest_when_finished \"git credential-cache exit\" &&\n-\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\" &&\n+\ttest_path_is_socket \"$XDG_CACHE_HOME/git/credential/socket\" &&\n \ttest_path_is_missing \"$HOME/.git-credential-cache/socket\" &&\n \ttest_path_is_missing \"$HOME/.cache/git/credential/socket\"\n '\n@@ -48,7 +63,7 @@ test_expect_success 'credential-cache --socket option overrides default location\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/dir/socket\"\n+\ttest_path_is_socket \"$HOME/dir/socket\"\n '\n \n test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n@@ -62,7 +77,7 @@ test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.cache/git/credential/socket\" &&\n+\ttest_path_is_socket \"$HOME/.cache/git/credential/socket\" &&\n \tXDG_CACHE_HOME=\"$HOME/xdg\" &&\n \texport XDG_CACHE_HOME &&\n \tcheck approve cache <<-\\EOF &&\n@@ -71,7 +86,7 @@ test_expect_success \"use custom XDG_CACHE_HOME even if xdg socket exists\" '\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$XDG_CACHE_HOME/git/credential/socket\"\n+\ttest_path_is_socket \"$XDG_CACHE_HOME/git/credential/socket\"\n '\n \n test_expect_success 'use user socket if user directory exists' '\n@@ -79,14 +94,15 @@ test_expect_success 'use user socket if user directory exists' '\n \t\tgit credential-cache exit &&\n \t\trmdir \\\"\\$HOME/.git-credential-cache/\\\"\n \t\" &&\n-\tmkdir -p -m 700 \"$HOME/.git-credential-cache/\" &&\n+\tmkdir -p \"$HOME/.git-credential-cache/\" &&\n+\tchmod 700 \"$HOME/.git-credential-cache/\" &&\n \tcheck approve cache <<-\\EOF &&\n \tprotocol=https\n \thost=example.com\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.git-credential-cache/socket\"\n+\ttest_path_is_socket \"$HOME/.git-credential-cache/socket\"\n '\n \n test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' '\n@@ -103,7 +119,7 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to\n \tusername=store-user\n \tpassword=store-pass\n \tEOF\n-\ttest -S \"$HOME/.git-credential-cache/socket\"\n+\ttest_path_is_socket \"$HOME/.git-credential-cache/socket\"\n '\n \n helper_test_timeout cache --timeout=1\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435831","messageId":"20210914072600.11552-3-carenas@gmail.com","threadId":"56490","inReplyTo":"20210914072600.11552-1-carenas@gmail.com","subject":"[PATCH v3 2/3] credential-cache: check for windows specific errors","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-14T07:25:59Z","receivedAt":"2021-09-14T07:26:24Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Connect and reset errors aren't what will be expected by POSIX but\nare instead compatible with the ones used by WinSock.\n\nTo avoid any possibility of confusion with other systems, checks\nfor disconnection and availability had been abstracted into helper\nfunctions that are platform specific.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\nv3:\n* use a better define as suggested by Dscho\nv2:\n* Use helper functions to separate error handling as suggested by Junio\n\n builtin/credential-cache.c | 30 ++++++++++++++++++++++++++++--\n 1 file changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/credential-cache.c b/builtin/credential-cache.c\nindex e8a7415747..78c02ad531 100644\n--- a/builtin/credential-cache.c\n+++ b/builtin/credential-cache.c\n@@ -11,6 +11,32 @@\n #define FLAG_SPAWN 0x1\n #define FLAG_RELAY 0x2\n \n+#ifdef GIT_WINDOWS_NATIVE\n+\n+static int connection_closed(int error)\n+{\n+\treturn (error == EINVAL);\n+}\n+\n+static int connection_fatally_broken(int error)\n+{\n+\treturn (error != ENOENT) && (error != ENETDOWN);\n+}\n+\n+#else\n+\n+static int connection_closed(int error)\n+{\n+\treturn (error == ECONNRESET);\n+}\n+\n+static int connection_fatally_broken(int error)\n+{\n+\treturn (error != ENOENT) && (error != ECONNREFUSED);\n+}\n+\n+#endif\n+\n static int send_request(const char *socket, const struct strbuf *out)\n {\n \tint got_data = 0;\n@@ -28,7 +54,7 @@ static int send_request(const char *socket, const struct strbuf *out)\n \t\tint r;\n \n \t\tr = read_in_full(fd, in, sizeof(in));\n-\t\tif (r == 0 || (r < 0 && errno == ECONNRESET))\n+\t\tif (r == 0 || (r < 0 && connection_closed(errno)))\n \t\t\tbreak;\n \t\tif (r < 0)\n \t\t\tdie_errno(\"read error from cache daemon\");\n@@ -75,7 +101,7 @@ static void do_cache(const char *socket, const char *action, int timeout,\n \t}\n \n \tif (send_request(socket, &buf) < 0) {\n-\t\tif (errno != ENOENT && errno != ECONNREFUSED)\n+\t\tif (connection_fatally_broken(errno))\n \t\t\tdie_errno(\"unable to connect to cache daemon\");\n \t\tif (flags & FLAG_SPAWN) {\n \t\t\tspawn_daemon(socket);\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435832","messageId":"20210914072600.11552-4-carenas@gmail.com","threadId":"56490","inReplyTo":"20210914072600.11552-1-carenas@gmail.com","subject":"[PATCH v3 3/3] git-compat-util: include declaration for unix sockets in windows","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-14T07:26:00Z","receivedAt":"2021-09-14T07:26:25Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Available since Windows 10 release 1803 and Windows Server 2019.\n\nNO_UNIX_SOCKETS is still the default for Windows builds, as they need\nto keep backward compatibility with releases up to Windows 7, but allow\nincluding the header otherwise.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\nv3:\n* better commit message as suggested by Dscho\n\n git-compat-util.h | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b46605300a..6a420d104c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -160,6 +160,9 @@\n # endif\n #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */\n #include <winsock2.h>\n+#ifndef NO_UNIX_SOCKETS\n+#include <afunix.h>\n+#endif\n #include <windows.h>\n #define GIT_WINDOWS_NATIVE\n #endif\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"435880","messageId":"xmqq7dfj3ygj.fsf@gitster.g","threadId":"56490","inReplyTo":"20210914072600.11552-1-carenas@gmail.com","subject":"Re: [PATCH v3 0/3] windows: allow building without NO_UNIX_SOCKETS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-14T16:37:32Z","receivedAt":"2021-09-14T16:37:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:\n\n> Eventhough NO_UNIX_SOCKETS was specifically added to support Windows,\n> it might not be necessary in the future, because Windows added support\n> for Unix Sockets in late 2017.\n\nThanks.  I found that this round was a pleasant read.\n\nWill queue, and let's merge it down to 'next' soonish.\n"},{"id":"435881","messageId":"xmqqzgsf2jmo.fsf@gitster.g","threadId":"56490","inReplyTo":"20210914072600.11552-3-carenas@gmail.com","subject":"Re: [PATCH v3 2/3] credential-cache: check for windows specific errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-14T16:43:11Z","receivedAt":"2021-09-14T16:43:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:\n\n> @@ -75,7 +101,7 @@ static void do_cache(const char *socket, const char *action, int timeout,\n>  \t}\n>  \n>  \tif (send_request(socket, &buf) < 0) {\n> -\t\tif (errno != ENOENT && errno != ECONNREFUSED)\n> +\t\tif (connection_fatally_broken(errno))\n>  \t\t\tdie_errno(\"unable to connect to cache daemon\");\n>  \t\tif (flags & FLAG_SPAWN) {\n>  \t\t\tspawn_daemon(socket);\n\nIn my earlier review I suggested a helper that checks\nrecoverability, hence leading to a code structure like this:\n\n\t\tif (!connection_not_yet_open(errno))\n\t\t\tdie_errno(...);\n\t\t/* otherwise, (re)establish connection and retry */\n\t\t...\n\nbut the phrasing and semantics you chose to check for unrecoverable\nstate reads much better.\n\nThanks.\n"},{"id":"435909","messageId":"xmqqilz30yap.fsf@gitster.g","threadId":"56490","inReplyTo":"20210914072600.11552-3-carenas@gmail.com","subject":"What should happen in credential-cache on recoverable error without SPAWN option?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-14T19:09:18Z","receivedAt":"2021-09-14T19:09:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"While reviewing Carlo's \"credential-cache: check for windows\nspecific errors\", I noticed this piece of code, that came from\n8ec6c8d7 (credential-cache: report more daemon connection errors,\n2012-01-09):\n\n\tif (send_request(socket, &buf) < 0) {\n\t\tif (errno != ENOENT && errno != ECONNREFUSED)\n\t\t\tdie_errno(\"unable to connect to cache daemon\");\n\t\tif (flags & FLAG_SPAWN) {\n\t\t\tspawn_daemon(socket);\n\t\t\tif (send_request(socket, &buf) < 0)\n\t\t\t\tdie_errno(\"unable to connect to cache daemon\");\n\t\t}\n\t}\n\nWhat would happen when we get a resumable error and then weren't\ngiven the SPAWN flag?  It seems that do_cache() simply returns\nwithout dying.  Shouldn't we get \"unable to connect\" in such a case?\n\nThis in turn came from 98c2924c (credentials: unable to connect to\ncache daemon, 2012-01-07), before it, the code read like this:\n\n\tif (!send_request(socket, &buf))\n\t\treturn;\n\tif (flags & FLAG_SPAWN) {\n \t\tspawn_daemon(socket);\n\t\tsend_request(socket, &buf);\n \t}\n\nso it looks to me that I am puzzled by an incomplete error handling\nintroduced by 98c2924c, and if we wanted to complete it, it may\nperhaps look like this patch on top of Carlo's patch?\n\nOr am I barking up a wrong tree and error checking in this command\ndoes not really make a real-life difference?\n\nThanks.\n\n\n builtin/credential-cache.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git c/builtin/credential-cache.c w/builtin/credential-cache.c\nindex 78c02ad531..a41a17e58f 100644\n--- c/builtin/credential-cache.c\n+++ w/builtin/credential-cache.c\n@@ -101,13 +101,11 @@ static void do_cache(const char *socket, const char *action, int timeout,\n \t}\n \n \tif (send_request(socket, &buf) < 0) {\n-\t\tif (connection_fatally_broken(errno))\n+\t\tif (connection_fatally_broken(errno) && !(flag & FLAG_SPAWN))\n+\t\t\tdie_errno(\"unable to connect to cache daemon\");\n+\t\tspawn_daemon(socket);\n+\t\tif (send_request(socket, &buf) < 0)\n \t\t\tdie_errno(\"unable to connect to cache daemon\");\n-\t\tif (flags & FLAG_SPAWN) {\n-\t\t\tspawn_daemon(socket);\n-\t\t\tif (send_request(socket, &buf) < 0)\n-\t\t\t\tdie_errno(\"unable to connect to cache daemon\");\n-\t\t}\n \t}\n \tstrbuf_release(&buf);\n }\n"},{"id":"435913","messageId":"YUD5I/hwxpcyocEU@coredump.intra.peff.net","threadId":"56490","inReplyTo":"xmqqilz30yap.fsf@gitster.g","subject":"Re: What should happen in credential-cache on recoverable error without SPAWN option?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-14T19:33:55Z","receivedAt":"2021-09-14T19:34:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 14, 2021 at 12:09:18PM -0700, Junio C Hamano wrote:\n\n> While reviewing Carlo's \"credential-cache: check for windows\n> specific errors\", I noticed this piece of code, that came from\n> 8ec6c8d7 (credential-cache: report more daemon connection errors,\n> 2012-01-09):\n> \n> \tif (send_request(socket, &buf) < 0) {\n> \t\tif (errno != ENOENT && errno != ECONNREFUSED)\n> \t\t\tdie_errno(\"unable to connect to cache daemon\");\n> \t\tif (flags & FLAG_SPAWN) {\n> \t\t\tspawn_daemon(socket);\n> \t\t\tif (send_request(socket, &buf) < 0)\n> \t\t\t\tdie_errno(\"unable to connect to cache daemon\");\n> \t\t}\n> \t}\n> \n> What would happen when we get a resumable error and then weren't\n> given the SPAWN flag?  It seems that do_cache() simply returns\n> without dying.  Shouldn't we get \"unable to connect\" in such a case?\n\nIt's subtle, but I think we end up doing the right thing. Those errors\naren't just \"resumable\"; they are \"we do not have a daemon to talk to at\nall\".\n\nThe \"exit\", \"get\", and \"erase\" operations do not pass the SPAWN flag.\nBut if there is no daemon, they are all noops.\n\nThe \"store\" operation is the only one which uses SPAWN, and of course\nthere we want to spin up a daemon so that we can put something in it.\n\nIt may be that SPAWN could have a better name to make this more clear.\n\n> diff --git c/builtin/credential-cache.c w/builtin/credential-cache.c\n> index 78c02ad531..a41a17e58f 100644\n> --- c/builtin/credential-cache.c\n> +++ w/builtin/credential-cache.c\n> @@ -101,13 +101,11 @@ static void do_cache(const char *socket, const char *action, int timeout,\n>  \t}\n>  \n>  \tif (send_request(socket, &buf) < 0) {\n> -\t\tif (connection_fatally_broken(errno))\n> +\t\tif (connection_fatally_broken(errno) && !(flag & FLAG_SPAWN))\n> +\t\t\tdie_errno(\"unable to connect to cache daemon\");\n> +\t\tspawn_daemon(socket);\n> +\t\tif (send_request(socket, &buf) < 0)\n>  \t\t\tdie_errno(\"unable to connect to cache daemon\");\n> -\t\tif (flags & FLAG_SPAWN) {\n> -\t\t\tspawn_daemon(socket);\n> -\t\t\tif (send_request(socket, &buf) < 0)\n> -\t\t\t\tdie_errno(\"unable to connect to cache daemon\");\n> -\t\t}\n>  \t}\n>  \tstrbuf_release(&buf);\n>  }\n\nIf you do this, then I think we'll start producing spurious errors\nduring normal use of the helper. Most interaction will generally start\nwith a \"get\" request to the helpers. So if you don't have anything\ncached and the daemon stopped running, right now we just don't return a\ncredential. With this we'd complain \"unable to connect to daemon\".\n\nAnd then of course we'd follow that up by asking for the credential and\nspinning up a daemon to store it. But that first request after the\ndaemon times out will always say \"unable to connect\".\n\n-Peff\n"},{"id":"440283","messageId":"211102.86y2671jbe.gmgdl@evledraar.gmail.com","threadId":"56490","inReplyTo":"20210914072600.11552-2-carenas@gmail.com","subject":"Re: [PATCH v3 1/3] t0301: fixes for windows compatibility","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-02T00:37:01Z","receivedAt":"2021-11-02T00:45:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Sep 14 2021, Carlo Marcelo Arenas Belón wrote:\n\n> [...]\n> `mkdir -m` can't represent a valid ACL directly and fails with\n> permission problems, so instead call mkdir followed by chmod, which\n> has been enhanced to do so.\n\nThis looks like a good change:\n\n>  test_expect_success 'use user socket if user directory exists' '\n> @@ -79,14 +94,15 @@ test_expect_success 'use user socket if user directory exists' '\n>  \t\tgit credential-cache exit &&\n>  \t\trmdir \\\"\\$HOME/.git-credential-cache/\\\"\n>  \t\" &&\n> -\tmkdir -p -m 700 \"$HOME/.git-credential-cache/\" &&\n> +\tmkdir -p \"$HOME/.git-credential-cache/\" &&\n> +\tchmod 700 \"$HOME/.git-credential-cache/\" &&\n>  \tcheck approve cache <<-\\EOF &&\n>  \tprotocol=https\n>  \thost=example.com\n>  \tusername=store-user\n>  \tpassword=store-pass\n>  \tEOF\n> -\ttest -S \"$HOME/.git-credential-cache/socket\"\n> +\ttest_path_is_socket \"$HOME/.git-credential-cache/socket\"\n>  '\n\nBut this adjacent changes is also needed to make this pass on\nAIX. I.e. for the adjacent test, which I assume \"works\" on your Windows\nsetup because it doesn't have SYMLINKS:\n\ndiff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh\nindex 698b7159f03..120b50e8c14 100755\n--- a/t/t0301-credential-cache.sh\n+++ b/t/t0301-credential-cache.sh\n@@ -111,7 +111,8 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to\n                rmdir \\\"\\$HOME/dir/\\\" &&\n                rm \\\"\\$HOME/.git-credential-cache\\\"\n        \" &&\n-       mkdir -p -m 700 \"$HOME/dir/\" &&\n+       mkdir -p \"$HOME/dir/\" &&\n+       chmod 700 \"$HOME/dir/\" &&\n        ln -s \"$HOME/dir\" \"$HOME/.git-credential-cache\" &&\n        check approve cache <<-\\EOF &&\n        protocol=https\n\nFWIW on AIX this fails because apparently they got quoting wrong and end\nup trying to shell out to \"chmod 700 [trash dir name up to the first\nspace]\", i.e. the \"trash directory\" part.\n\nNot an rc0 issue, this has been in \"master\" for a while...\n"}]}