{"thread":{"id":"55727","subject":"[PATCH] simple-ipc: correct ifdefs when NO_PTHREADS is defined","startedAt":"2021-05-18T15:36:40Z","lastAt":"2021-05-20T23:02:09Z","messageCount":8,"participants":["Jeff Hostetler via GitGitGadget","Junio C Hamano","Jeff Hostetler"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"424887","messageId":"pull.955.git.1621352192238.gitgitgadget@gmail.com","threadId":"55727","inReplyTo":null,"subject":"[PATCH] simple-ipc: correct ifdefs when NO_PTHREADS is defined","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-18T15:36:31Z","receivedAt":"2021-05-18T15:36:40Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler <jeffhost@microsoft.com>\n\nSimple IPC always requires threads (in addition to various\nplatform-specific IPC support).  Fix the ifdefs in the Makefile\nto define SUPPORTS_SIMPLE_IPC when appropriate.\n\nPreviously, the Unix version of the code would only verify that\nUnix domain sockets were available.\n\nThis problem was reported here:\nhttps://lore.kernel.org/git/YKN5lXs4AoK%2FJFTO@coredump.intra.peff.net/T/#m08be8f1942ea8a2c36cfee0e51cdf06489fdeafc\n\nReported-by: Randall S. Becker <rsbecker@nexbridge.com>\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Jeff Hostetler <jeffhost@microsoft.com>\n---\n    simple-ipc: correct ifdefs when NO_PTHREADS is defined\n    \n    Simple IPC always requires threads (in addition to various\n    platform-specific IPC support). Fix the ifdefs in the Makefile to define\n    SUPPORTS_SIMPLE_IPC when appropriate.\n    \n    Previously, the Unix version of the code would only verify that Unix\n    domain sockets were available.\n    \n    This problem was reported here:\n    https://lore.kernel.org/git/YKN5lXs4AoK%2FJFTO@coredump.intra.peff.net/T/#m08be8f1942ea8a2c36cfee0e51cdf06489fdeafc\n    \n    Reported-by: Randall S. Becker rsbecker@nexbridge.com Helped-by: Jeff\n    King peff@peff.net Signed-off-by: Jeff Hostetler jeffhost@microsoft.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-955%2Fjeffhostetler%2Ffixup-simple-ipc-no-pthreads-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-955/jeffhostetler/fixup-simple-ipc-no-pthreads-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/955\n\n Makefile                            | 14 ++++++++++++--\n compat/simple-ipc/ipc-unix-socket.c |  8 ++++++++\n compat/simple-ipc/ipc-win32.c       |  4 ++++\n simple-ipc.h                        |  4 ----\n 4 files changed, 24 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 3a2d3c80a81a..30df67fd62eb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1687,13 +1687,23 @@ ifdef NO_UNIX_SOCKETS\n else\n \tLIB_OBJS += unix-socket.o\n \tLIB_OBJS += unix-stream-server.o\n-\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n-\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n endif\n \n+# Simple-ipc requires threads and platform-specific IPC support.\n+# (We group all Unix variants in the top-level else because Windows\n+# also defines NO_UNIX_SOCKETS.)\n ifdef USE_WIN32_IPC\n+\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n \tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n \tLIB_OBJS += compat/simple-ipc/ipc-win32.o\n+else\n+ifndef NO_PTHREADS\n+ifndef NO_UNIX_SOCKETS\n+\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n+\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n+\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n+endif\n+endif\n endif\n \n ifdef NO_ICONV\ndiff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c\nindex 38689b278df3..c23b17973983 100644\n--- a/compat/simple-ipc/ipc-unix-socket.c\n+++ b/compat/simple-ipc/ipc-unix-socket.c\n@@ -6,10 +6,16 @@\n #include \"unix-socket.h\"\n #include \"unix-stream-server.h\"\n \n+#ifdef SUPPORTS_SIMPLE_IPC\n+\n #ifdef NO_UNIX_SOCKETS\n #error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets\n #endif\n \n+#ifdef NO_PTHREADS\n+#error compat/simple-ipc/ipc-unix-socket.c requires pthreads\n+#endif\n+\n enum ipc_active_state ipc_get_active_state(const char *path)\n {\n \tenum ipc_active_state state = IPC_STATE__OTHER_ERROR;\n@@ -997,3 +1003,5 @@ void ipc_server_free(struct ipc_server_data *server_data)\n \tfree(server_data->fifo_fds);\n \tfree(server_data);\n }\n+\n+#endif /* SUPPORTS_SIMPLE_IPC */\ndiff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c\nindex 8f89c02037e3..958bb562ebb6 100644\n--- a/compat/simple-ipc/ipc-win32.c\n+++ b/compat/simple-ipc/ipc-win32.c\n@@ -4,6 +4,8 @@\n #include \"pkt-line.h\"\n #include \"thread-utils.h\"\n \n+#ifdef SUPPORTS_SIMPLE_IPC\n+\n #ifndef GIT_WINDOWS_NATIVE\n #error This file can only be compiled on Windows\n #endif\n@@ -749,3 +751,5 @@ void ipc_server_free(struct ipc_server_data *server_data)\n \n \tfree(server_data);\n }\n+\n+#endif /* SUPPORTS_SIMPLE_IPC */\ndiff --git a/simple-ipc.h b/simple-ipc.h\nindex dc3606e30bd6..2c48a5ee0047 100644\n--- a/simple-ipc.h\n+++ b/simple-ipc.h\n@@ -5,10 +5,6 @@\n  * See Documentation/technical/api-simple-ipc.txt\n  */\n \n-#if defined(GIT_WINDOWS_NATIVE) || !defined(NO_UNIX_SOCKETS)\n-#define SUPPORTS_SIMPLE_IPC\n-#endif\n-\n #ifdef SUPPORTS_SIMPLE_IPC\n #include \"pkt-line.h\"\n \n\nbase-commit: bf949ade81106fbda068c1fdb2c6fd1cb1babe7e\n-- \ngitgitgadget\n"},{"id":"424907","messageId":"xmqqk0nv1rc4.fsf@gitster.g","threadId":"55727","inReplyTo":"pull.955.git.1621352192238.gitgitgadget@gmail.com","subject":"Re: [PATCH] simple-ipc: correct ifdefs when NO_PTHREADS is defined","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-19T00:48:27Z","receivedAt":"2021-05-19T00:48:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jeff Hostetler via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> +# Simple-ipc requires threads and platform-specific IPC support.\n> +# (We group all Unix variants in the top-level else because Windows\n> +# also defines NO_UNIX_SOCKETS.)\n>  ifdef USE_WIN32_IPC\n> +\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n>  \tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n>  \tLIB_OBJS += compat/simple-ipc/ipc-win32.o\n> +else\n> +ifndef NO_PTHREADS\n> +ifndef NO_UNIX_SOCKETS\n> +\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n> +\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n> +\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n> +endif\n> +endif\n\nOK, so \"!defined(NO_PTHREADS) && !defined(NO_UNIX_SOCKETS)\" is the\nrequirement for SIMPLE_IPC unless you are on Windows.\n\n> diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c\n> index 38689b278df3..c23b17973983 100644\n> --- a/compat/simple-ipc/ipc-unix-socket.c\n> +++ b/compat/simple-ipc/ipc-unix-socket.c\n> @@ -6,10 +6,16 @@\n>  #include \"unix-socket.h\"\n>  #include \"unix-stream-server.h\"\n>  \n> +#ifdef SUPPORTS_SIMPLE_IPC\n> +\n>  #ifdef NO_UNIX_SOCKETS\n>  #error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets\n>  #endif\n>  \n> +#ifdef NO_PTHREADS\n> +#error compat/simple-ipc/ipc-unix-socket.c requires pthreads\n> +#endif\n> +\n\nDo we want to duplicate the requirement here and risk them drifting\napart?\n\n> @@ -997,3 +1003,5 @@ void ipc_server_free(struct ipc_server_data *server_data)\n>  \tfree(server_data->fifo_fds);\n>  \tfree(server_data);\n>  }\n> +\n> +#endif /* SUPPORTS_SIMPLE_IPC */\n\nIf anything, I do not think we want a huge #ifdef/#endif around the\nwhole file.  Feeding this source to your compiler when these three C\nproprocessor macros do not agree with its use is an error, so perhaps\nlose all of these #ifdef/#endif around the three macros and refer human\nreaders to the top-level Makefile with a comment, e.g.\n\n/*\n * Non Windows platforms need !NO_UNIX_SOCKETS and !NO_PTHREADS\n * to compile and use this file.  See the top-level Makefile.\n */\n\nif we really wanted to have a way to help builders identify the\nreason why their build is failing.\n\n"},{"id":"424963","messageId":"79bf42e7-3923-a901-53eb-1aac13c53e6b@jeffhostetler.com","threadId":"55727","inReplyTo":"xmqqk0nv1rc4.fsf@gitster.g","subject":"Re: [PATCH] simple-ipc: correct ifdefs when NO_PTHREADS is defined","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2021-05-19T14:29:39Z","receivedAt":"2021-05-19T14:29:43Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 5/18/21 8:48 PM, Junio C Hamano wrote:\n> \"Jeff Hostetler via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n>> +# Simple-ipc requires threads and platform-specific IPC support.\n>> +# (We group all Unix variants in the top-level else because Windows\n>> +# also defines NO_UNIX_SOCKETS.)\n>>   ifdef USE_WIN32_IPC\n>> +\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n>>   \tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n>>   \tLIB_OBJS += compat/simple-ipc/ipc-win32.o\n>> +else\n>> +ifndef NO_PTHREADS\n>> +ifndef NO_UNIX_SOCKETS\n>> +\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n>> +\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n>> +\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n>> +endif\n>> +endif\n> \n> OK, so \"!defined(NO_PTHREADS) && !defined(NO_UNIX_SOCKETS)\" is the\n> requirement for SIMPLE_IPC unless you are on Windows.\n> \n>> diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c\n>> index 38689b278df3..c23b17973983 100644\n>> --- a/compat/simple-ipc/ipc-unix-socket.c\n>> +++ b/compat/simple-ipc/ipc-unix-socket.c\n>> @@ -6,10 +6,16 @@\n>>   #include \"unix-socket.h\"\n>>   #include \"unix-stream-server.h\"\n>>   \n>> +#ifdef SUPPORTS_SIMPLE_IPC\n>> +\n>>   #ifdef NO_UNIX_SOCKETS\n>>   #error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets\n>>   #endif\n>>   \n>> +#ifdef NO_PTHREADS\n>> +#error compat/simple-ipc/ipc-unix-socket.c requires pthreads\n>> +#endif\n>> +\n> \n> Do we want to duplicate the requirement here and risk them drifting\n> apart?\n> \n>> @@ -997,3 +1003,5 @@ void ipc_server_free(struct ipc_server_data *server_data)\n>>   \tfree(server_data->fifo_fds);\n>>   \tfree(server_data);\n>>   }\n>> +\n>> +#endif /* SUPPORTS_SIMPLE_IPC */\n> \n> If anything, I do not think we want a huge #ifdef/#endif around the\n> whole file.  Feeding this source to your compiler when these three C\n> proprocessor macros do not agree with its use is an error, so perhaps\n> lose all of these #ifdef/#endif around the three macros and refer human\n> readers to the top-level Makefile with a comment, e.g.\n> \n> /*\n>   * Non Windows platforms need !NO_UNIX_SOCKETS and !NO_PTHREADS\n>   * to compile and use this file.  See the top-level Makefile.\n>   */\n> \n> if we really wanted to have a way to help builders identify the\n> reason why their build is failing.\n> \n\nWould it be better to just have something like the following at the\ntop of the source files and leave the details to the Makefile:\n\n\n#ifndef SUPPORTS_SIMPLE_IPC\n/*\n  * This source file should only be included when Simple IPC\n  * is supported.  See the top-level Makefile.\n  */\n#error SUPPORTS_SIMPLE_IPC not defined\n#endif\n\n"},{"id":"424993","messageId":"xmqqh7iyxtxc.fsf@gitster.g","threadId":"55727","inReplyTo":"79bf42e7-3923-a901-53eb-1aac13c53e6b@jeffhostetler.com","subject":"Re: [PATCH] simple-ipc: correct ifdefs when NO_PTHREADS is defined","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-19T22:03:43Z","receivedAt":"2021-05-19T22:03:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler <git@jeffhostetler.com> writes:\n\n>>>   #ifdef NO_UNIX_SOCKETS\n>>>   #error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets\n>>>   #endif\n>>>   +#ifdef NO_PTHREADS\n>>> +#error compat/simple-ipc/ipc-unix-socket.c requires pthreads\n>>> +#endif\n>>> +\n>> Do we want to duplicate the requirement here and risk them drifting\n>> apart?\n>>  ...\n> Would it be better to just have something like the following at the\n> top of the source files and leave the details to the Makefile:\n>\n>\n> #ifndef SUPPORTS_SIMPLE_IPC\n> /*\n>  * This source file should only be included when Simple IPC\n>  * is supported.  See the top-level Makefile.\n>  */\n> #error SUPPORTS_SIMPLE_IPC not defined\n> #endif\n\nYeah, that is a much better message, with even less duplication,\nthan what I sent.\n\nI do not think #ifndef/#error/#endif adds much value, though.  After\nall, the Makefile does not even tell us to feed this file to the\ncompiler when the C preprocessor macro is not defined, so presumably\nwhoever hits the #error knows s/he is doing something not supported,\nand the point of the new message is to help those who we failed by\nleaving the rest of the source file unbuildable even when we defined\nthe C preprocessor macro in the Makefile (like the mistaken\ndependency on pthreads that we missed).\n\nThanks.\n"},{"id":"425104","messageId":"pull.955.v2.git.1621520547726.gitgitgadget@gmail.com","threadId":"55727","inReplyTo":"pull.955.git.1621352192238.gitgitgadget@gmail.com","subject":"[PATCH v2] simple-ipc: correct ifdefs when NO_PTHREADS is defined","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-20T14:22:27Z","receivedAt":"2021-05-20T14:22:32Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler <jeffhost@microsoft.com>\n\nSimple IPC always requires threads (in addition to various\nplatform-specific IPC support).  Fix the ifdefs in the Makefile\nto define SUPPORTS_SIMPLE_IPC when appropriate.\n\nPreviously, the Unix version of the code would only verify that\nUnix domain sockets were available.\n\nThis problem was reported here:\nhttps://lore.kernel.org/git/YKN5lXs4AoK%2FJFTO@coredump.intra.peff.net/T/#m08be8f1942ea8a2c36cfee0e51cdf06489fdeafc\n\nReported-by: Randall S. Becker <rsbecker@nexbridge.com>\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Jeff Hostetler <jeffhost@microsoft.com>\n---\n    simple-ipc: correct ifdefs when NO_PTHREADS is defined\n    \n    Here is V2 of this fixup. I've removed the whole file ifdefs and\n    replaced them with a simple #ifndef/#error/#endif as a warning.\n    \n    Jeff\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-955%2Fjeffhostetler%2Ffixup-simple-ipc-no-pthreads-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-955/jeffhostetler/fixup-simple-ipc-no-pthreads-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/955\n\nRange-diff vs v1:\n\n 1:  4adcf35ea6e4 ! 1:  119412f52ff5 simple-ipc: correct ifdefs when NO_PTHREADS is defined\n     @@ Makefile: ifdef NO_UNIX_SOCKETS\n      -\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n       endif\n       \n     -+# Simple-ipc requires threads and platform-specific IPC support.\n     -+# (We group all Unix variants in the top-level else because Windows\n     -+# also defines NO_UNIX_SOCKETS.)\n     ++# Simple IPC requires threads and platform-specific IPC support.\n     ++# Only platforms that have both should include these source files\n     ++# in the build.\n     ++#\n     ++# On Windows-based systems, Simple IPC requires threads and Windows\n     ++# Named Pipes.  These are always available, so Simple IPC support\n     ++# is optional.\n     ++#\n     ++# On Unix-based systems, Simple IPC requires pthreads and Unix\n     ++# domain sockets.  So support is only enabled when both are present.\n     ++#\n       ifdef USE_WIN32_IPC\n      +\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n       \tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n     @@ Makefile: ifdef NO_UNIX_SOCKETS\n       \n       ifdef NO_ICONV\n      \n     + ## compat/simple-ipc/ipc-shared.c ##\n     +@@\n     + #include \"pkt-line.h\"\n     + #include \"thread-utils.h\"\n     + \n     +-#ifdef SUPPORTS_SIMPLE_IPC\n     ++#ifndef SUPPORTS_SIMPLE_IPC\n     ++/*\n     ++ * This source file should only be compiled when Simple IPC is supported.\n     ++ * See the top-level Makefile.\n     ++ */\n     ++#error SUPPORTS_SIMPLE_IPC not defined\n     ++#endif\n     + \n     + int ipc_server_run(const char *path, const struct ipc_server_opts *opts,\n     + \t\t   ipc_server_application_cb *application_cb,\n     +@@ compat/simple-ipc/ipc-shared.c: int ipc_server_run(const char *path, const struct ipc_server_opts *opts,\n     + \n     + \treturn ret;\n     + }\n     +-\n     +-#endif /* SUPPORTS_SIMPLE_IPC */\n     +\n       ## compat/simple-ipc/ipc-unix-socket.c ##\n      @@\n       #include \"unix-socket.h\"\n       #include \"unix-stream-server.h\"\n       \n     -+#ifdef SUPPORTS_SIMPLE_IPC\n     -+\n     - #ifdef NO_UNIX_SOCKETS\n     - #error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets\n     +-#ifdef NO_UNIX_SOCKETS\n     +-#error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets\n     ++#ifndef SUPPORTS_SIMPLE_IPC\n     ++/*\n     ++ * This source file should only be compiled when Simple IPC is supported.\n     ++ * See the top-level Makefile.\n     ++ */\n     ++#error SUPPORTS_SIMPLE_IPC not defined\n       #endif\n       \n     -+#ifdef NO_PTHREADS\n     -+#error compat/simple-ipc/ipc-unix-socket.c requires pthreads\n     -+#endif\n     -+\n       enum ipc_active_state ipc_get_active_state(const char *path)\n     - {\n     - \tenum ipc_active_state state = IPC_STATE__OTHER_ERROR;\n     -@@ compat/simple-ipc/ipc-unix-socket.c: void ipc_server_free(struct ipc_server_data *server_data)\n     - \tfree(server_data->fifo_fds);\n     - \tfree(server_data);\n     - }\n     -+\n     -+#endif /* SUPPORTS_SIMPLE_IPC */\n      \n       ## compat/simple-ipc/ipc-win32.c ##\n      @@\n       #include \"pkt-line.h\"\n       #include \"thread-utils.h\"\n       \n     -+#ifdef SUPPORTS_SIMPLE_IPC\n     -+\n     - #ifndef GIT_WINDOWS_NATIVE\n     - #error This file can only be compiled on Windows\n     +-#ifndef GIT_WINDOWS_NATIVE\n     +-#error This file can only be compiled on Windows\n     ++#ifndef SUPPORTS_SIMPLE_IPC\n     ++/*\n     ++ * This source file should only be compiled when Simple IPC is supported.\n     ++ * See the top-level Makefile.\n     ++ */\n     ++#error SUPPORTS_SIMPLE_IPC not defined\n       #endif\n     -@@ compat/simple-ipc/ipc-win32.c: void ipc_server_free(struct ipc_server_data *server_data)\n       \n     - \tfree(server_data);\n     - }\n     -+\n     -+#endif /* SUPPORTS_SIMPLE_IPC */\n     + static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)\n      \n       ## simple-ipc.h ##\n      @@\n\n\n Makefile                            | 22 ++++++++++++++++++++--\n compat/simple-ipc/ipc-shared.c      | 10 +++++++---\n compat/simple-ipc/ipc-unix-socket.c |  8 ++++++--\n compat/simple-ipc/ipc-win32.c       |  8 ++++++--\n simple-ipc.h                        |  4 ----\n 5 files changed, 39 insertions(+), 13 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 3a2d3c80a81a..ea4c0a77604d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1687,13 +1687,31 @@ ifdef NO_UNIX_SOCKETS\n else\n \tLIB_OBJS += unix-socket.o\n \tLIB_OBJS += unix-stream-server.o\n-\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n-\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n endif\n \n+# Simple IPC requires threads and platform-specific IPC support.\n+# Only platforms that have both should include these source files\n+# in the build.\n+#\n+# On Windows-based systems, Simple IPC requires threads and Windows\n+# Named Pipes.  These are always available, so Simple IPC support\n+# is optional.\n+#\n+# On Unix-based systems, Simple IPC requires pthreads and Unix\n+# domain sockets.  So support is only enabled when both are present.\n+#\n ifdef USE_WIN32_IPC\n+\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n \tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n \tLIB_OBJS += compat/simple-ipc/ipc-win32.o\n+else\n+ifndef NO_PTHREADS\n+ifndef NO_UNIX_SOCKETS\n+\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n+\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n+\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n+endif\n+endif\n endif\n \n ifdef NO_ICONV\ndiff --git a/compat/simple-ipc/ipc-shared.c b/compat/simple-ipc/ipc-shared.c\nindex 1edec8159532..1b9d359ab681 100644\n--- a/compat/simple-ipc/ipc-shared.c\n+++ b/compat/simple-ipc/ipc-shared.c\n@@ -4,7 +4,13 @@\n #include \"pkt-line.h\"\n #include \"thread-utils.h\"\n \n-#ifdef SUPPORTS_SIMPLE_IPC\n+#ifndef SUPPORTS_SIMPLE_IPC\n+/*\n+ * This source file should only be compiled when Simple IPC is supported.\n+ * See the top-level Makefile.\n+ */\n+#error SUPPORTS_SIMPLE_IPC not defined\n+#endif\n \n int ipc_server_run(const char *path, const struct ipc_server_opts *opts,\n \t\t   ipc_server_application_cb *application_cb,\n@@ -24,5 +30,3 @@ int ipc_server_run(const char *path, const struct ipc_server_opts *opts,\n \n \treturn ret;\n }\n-\n-#endif /* SUPPORTS_SIMPLE_IPC */\ndiff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c\nindex 38689b278df3..1927e6ef4bca 100644\n--- a/compat/simple-ipc/ipc-unix-socket.c\n+++ b/compat/simple-ipc/ipc-unix-socket.c\n@@ -6,8 +6,12 @@\n #include \"unix-socket.h\"\n #include \"unix-stream-server.h\"\n \n-#ifdef NO_UNIX_SOCKETS\n-#error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets\n+#ifndef SUPPORTS_SIMPLE_IPC\n+/*\n+ * This source file should only be compiled when Simple IPC is supported.\n+ * See the top-level Makefile.\n+ */\n+#error SUPPORTS_SIMPLE_IPC not defined\n #endif\n \n enum ipc_active_state ipc_get_active_state(const char *path)\ndiff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c\nindex 8f89c02037e3..8dc7bda087da 100644\n--- a/compat/simple-ipc/ipc-win32.c\n+++ b/compat/simple-ipc/ipc-win32.c\n@@ -4,8 +4,12 @@\n #include \"pkt-line.h\"\n #include \"thread-utils.h\"\n \n-#ifndef GIT_WINDOWS_NATIVE\n-#error This file can only be compiled on Windows\n+#ifndef SUPPORTS_SIMPLE_IPC\n+/*\n+ * This source file should only be compiled when Simple IPC is supported.\n+ * See the top-level Makefile.\n+ */\n+#error SUPPORTS_SIMPLE_IPC not defined\n #endif\n \n static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)\ndiff --git a/simple-ipc.h b/simple-ipc.h\nindex dc3606e30bd6..2c48a5ee0047 100644\n--- a/simple-ipc.h\n+++ b/simple-ipc.h\n@@ -5,10 +5,6 @@\n  * See Documentation/technical/api-simple-ipc.txt\n  */\n \n-#if defined(GIT_WINDOWS_NATIVE) || !defined(NO_UNIX_SOCKETS)\n-#define SUPPORTS_SIMPLE_IPC\n-#endif\n-\n #ifdef SUPPORTS_SIMPLE_IPC\n #include \"pkt-line.h\"\n \n\nbase-commit: bf949ade81106fbda068c1fdb2c6fd1cb1babe7e\n-- \ngitgitgadget\n"},{"id":"425112","messageId":"a4eceb72-6fbf-3fd1-5ba8-ef03900bf7e7@jeffhostetler.com","threadId":"55727","inReplyTo":"pull.955.v2.git.1621520547726.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] simple-ipc: correct ifdefs when NO_PTHREADS is defined","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2021-05-20T15:11:50Z","receivedAt":"2021-05-20T15:11:57Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 5/20/21 10:22 AM, Jeff Hostetler via GitGitGadget wrote:\n> From: Jeff Hostetler <jeffhost@microsoft.com>\n> \n> Simple IPC always requires threads (in addition to various\n> platform-specific IPC support).  Fix the ifdefs in the Makefile\n> to define SUPPORTS_SIMPLE_IPC when appropriate.\n> \n\nI got in a hurry this morning and forgot to update the CMake script.\nI'll look at that now.\n\nJeff\n\n"},{"id":"425139","messageId":"pull.955.v3.git.1621535291406.gitgitgadget@gmail.com","threadId":"55727","inReplyTo":"pull.955.v2.git.1621520547726.gitgitgadget@gmail.com","subject":"[PATCH v3] simple-ipc: correct ifdefs when NO_PTHREADS is defined","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-20T18:28:10Z","receivedAt":"2021-05-20T18:28:16Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler <jeffhost@microsoft.com>\n\nSimple IPC always requires threads (in addition to various\nplatform-specific IPC support).  Fix the ifdefs in the Makefile\nto define SUPPORTS_SIMPLE_IPC when appropriate.\n\nPreviously, the Unix version of the code would only verify that\nUnix domain sockets were available.\n\nThis problem was reported here:\nhttps://lore.kernel.org/git/YKN5lXs4AoK%2FJFTO@coredump.intra.peff.net/T/#m08be8f1942ea8a2c36cfee0e51cdf06489fdeafc\n\nReported-by: Randall S. Becker <rsbecker@nexbridge.com>\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Jeff Hostetler <jeffhost@microsoft.com>\n---\n    simple-ipc: correct ifdefs when NO_PTHREADS is defined\n    \n    Here is V3 of this fixup. I've added fixups for the CMake builds.\n    \n    Jeff\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-955%2Fjeffhostetler%2Ffixup-simple-ipc-no-pthreads-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-955/jeffhostetler/fixup-simple-ipc-no-pthreads-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/955\n\nRange-diff vs v2:\n\n 1:  119412f52ff5 ! 1:  d4f4170414e3 simple-ipc: correct ifdefs when NO_PTHREADS is defined\n     @@ compat/simple-ipc/ipc-win32.c\n       \n       static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)\n      \n     + ## contrib/buildsystems/CMakeLists.txt ##\n     +@@ contrib/buildsystems/CMakeLists.txt: endif()\n     + \n     + if(CMAKE_SYSTEM_NAME STREQUAL \"Windows\")\n     + \tlist(APPEND compat_SOURCES compat/simple-ipc/ipc-shared.c compat/simple-ipc/ipc-win32.c)\n     ++\tadd_compile_definitions(SUPPORTS_SIMPLE_IPC)\n     ++\tset(SUPPORTS_SIMPLE_IPC 1)\n     + else()\n     +-\tlist(APPEND compat_SOURCES compat/simple-ipc/ipc-shared.c compat/simple-ipc/ipc-unix-socket.c)\n     ++\t# Simple IPC requires both Unix sockets and pthreads on Unix-based systems.\n     ++\tif(NOT NO_UNIX_SOCKETS AND NOT NO_PTHREADS)\n     ++\t\tlist(APPEND compat_SOURCES compat/simple-ipc/ipc-shared.c compat/simple-ipc/ipc-unix-socket.c)\n     ++\t\tadd_compile_definitions(SUPPORTS_SIMPLE_IPC)\n     ++\t\tset(SUPPORTS_SIMPLE_IPC 1)\n     ++\tendif()\n     + endif()\n     + \n     + set(EXE_EXTENSION ${CMAKE_EXECUTABLE_SUFFIX})\n     +@@ contrib/buildsystems/CMakeLists.txt: file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"X='${EXE_EXTENSION}'\\n\")\n     + file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"NO_GETTEXT='${NO_GETTEXT}'\\n\")\n     + file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"RUNTIME_PREFIX='${RUNTIME_PREFIX}'\\n\")\n     + file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"NO_PYTHON='${NO_PYTHON}'\\n\")\n     ++file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"SUPPORTS_SIMPLE_IPC='${SUPPORTS_SIMPLE_IPC}'\\n\")\n     + if(WIN32)\n     + \tfile(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"PATH=\\\"$PATH:$TEST_DIRECTORY/../compat/vcbuild/vcpkg/installed/x64-windows/bin\\\"\\n\")\n     + endif()\n     +\n       ## simple-ipc.h ##\n      @@\n        * See Documentation/technical/api-simple-ipc.txt\n\n\n Makefile                            | 22 ++++++++++++++++++++--\n compat/simple-ipc/ipc-shared.c      | 10 +++++++---\n compat/simple-ipc/ipc-unix-socket.c |  8 ++++++--\n compat/simple-ipc/ipc-win32.c       |  8 ++++++--\n contrib/buildsystems/CMakeLists.txt | 10 +++++++++-\n simple-ipc.h                        |  4 ----\n 6 files changed, 48 insertions(+), 14 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 3a2d3c80a81a..ea4c0a77604d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1687,13 +1687,31 @@ ifdef NO_UNIX_SOCKETS\n else\n \tLIB_OBJS += unix-socket.o\n \tLIB_OBJS += unix-stream-server.o\n-\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n-\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n endif\n \n+# Simple IPC requires threads and platform-specific IPC support.\n+# Only platforms that have both should include these source files\n+# in the build.\n+#\n+# On Windows-based systems, Simple IPC requires threads and Windows\n+# Named Pipes.  These are always available, so Simple IPC support\n+# is optional.\n+#\n+# On Unix-based systems, Simple IPC requires pthreads and Unix\n+# domain sockets.  So support is only enabled when both are present.\n+#\n ifdef USE_WIN32_IPC\n+\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n \tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n \tLIB_OBJS += compat/simple-ipc/ipc-win32.o\n+else\n+ifndef NO_PTHREADS\n+ifndef NO_UNIX_SOCKETS\n+\tBASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC\n+\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n+\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n+endif\n+endif\n endif\n \n ifdef NO_ICONV\ndiff --git a/compat/simple-ipc/ipc-shared.c b/compat/simple-ipc/ipc-shared.c\nindex 1edec8159532..1b9d359ab681 100644\n--- a/compat/simple-ipc/ipc-shared.c\n+++ b/compat/simple-ipc/ipc-shared.c\n@@ -4,7 +4,13 @@\n #include \"pkt-line.h\"\n #include \"thread-utils.h\"\n \n-#ifdef SUPPORTS_SIMPLE_IPC\n+#ifndef SUPPORTS_SIMPLE_IPC\n+/*\n+ * This source file should only be compiled when Simple IPC is supported.\n+ * See the top-level Makefile.\n+ */\n+#error SUPPORTS_SIMPLE_IPC not defined\n+#endif\n \n int ipc_server_run(const char *path, const struct ipc_server_opts *opts,\n \t\t   ipc_server_application_cb *application_cb,\n@@ -24,5 +30,3 @@ int ipc_server_run(const char *path, const struct ipc_server_opts *opts,\n \n \treturn ret;\n }\n-\n-#endif /* SUPPORTS_SIMPLE_IPC */\ndiff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c\nindex 38689b278df3..1927e6ef4bca 100644\n--- a/compat/simple-ipc/ipc-unix-socket.c\n+++ b/compat/simple-ipc/ipc-unix-socket.c\n@@ -6,8 +6,12 @@\n #include \"unix-socket.h\"\n #include \"unix-stream-server.h\"\n \n-#ifdef NO_UNIX_SOCKETS\n-#error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets\n+#ifndef SUPPORTS_SIMPLE_IPC\n+/*\n+ * This source file should only be compiled when Simple IPC is supported.\n+ * See the top-level Makefile.\n+ */\n+#error SUPPORTS_SIMPLE_IPC not defined\n #endif\n \n enum ipc_active_state ipc_get_active_state(const char *path)\ndiff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c\nindex 8f89c02037e3..8dc7bda087da 100644\n--- a/compat/simple-ipc/ipc-win32.c\n+++ b/compat/simple-ipc/ipc-win32.c\n@@ -4,8 +4,12 @@\n #include \"pkt-line.h\"\n #include \"thread-utils.h\"\n \n-#ifndef GIT_WINDOWS_NATIVE\n-#error This file can only be compiled on Windows\n+#ifndef SUPPORTS_SIMPLE_IPC\n+/*\n+ * This source file should only be compiled when Simple IPC is supported.\n+ * See the top-level Makefile.\n+ */\n+#error SUPPORTS_SIMPLE_IPC not defined\n #endif\n \n static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 75ed198a6a36..a87841340e6a 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -252,8 +252,15 @@ endif()\n \n if(CMAKE_SYSTEM_NAME STREQUAL \"Windows\")\n \tlist(APPEND compat_SOURCES compat/simple-ipc/ipc-shared.c compat/simple-ipc/ipc-win32.c)\n+\tadd_compile_definitions(SUPPORTS_SIMPLE_IPC)\n+\tset(SUPPORTS_SIMPLE_IPC 1)\n else()\n-\tlist(APPEND compat_SOURCES compat/simple-ipc/ipc-shared.c compat/simple-ipc/ipc-unix-socket.c)\n+\t# Simple IPC requires both Unix sockets and pthreads on Unix-based systems.\n+\tif(NOT NO_UNIX_SOCKETS AND NOT NO_PTHREADS)\n+\t\tlist(APPEND compat_SOURCES compat/simple-ipc/ipc-shared.c compat/simple-ipc/ipc-unix-socket.c)\n+\t\tadd_compile_definitions(SUPPORTS_SIMPLE_IPC)\n+\t\tset(SUPPORTS_SIMPLE_IPC 1)\n+\tendif()\n endif()\n \n set(EXE_EXTENSION ${CMAKE_EXECUTABLE_SUFFIX})\n@@ -974,6 +981,7 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"X='${EXE_EXTENSION}'\\n\")\n file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"NO_GETTEXT='${NO_GETTEXT}'\\n\")\n file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"RUNTIME_PREFIX='${RUNTIME_PREFIX}'\\n\")\n file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"NO_PYTHON='${NO_PYTHON}'\\n\")\n+file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"SUPPORTS_SIMPLE_IPC='${SUPPORTS_SIMPLE_IPC}'\\n\")\n if(WIN32)\n \tfile(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS \"PATH=\\\"$PATH:$TEST_DIRECTORY/../compat/vcbuild/vcpkg/installed/x64-windows/bin\\\"\\n\")\n endif()\ndiff --git a/simple-ipc.h b/simple-ipc.h\nindex dc3606e30bd6..2c48a5ee0047 100644\n--- a/simple-ipc.h\n+++ b/simple-ipc.h\n@@ -5,10 +5,6 @@\n  * See Documentation/technical/api-simple-ipc.txt\n  */\n \n-#if defined(GIT_WINDOWS_NATIVE) || !defined(NO_UNIX_SOCKETS)\n-#define SUPPORTS_SIMPLE_IPC\n-#endif\n-\n #ifdef SUPPORTS_SIMPLE_IPC\n #include \"pkt-line.h\"\n \n\nbase-commit: bf949ade81106fbda068c1fdb2c6fd1cb1babe7e\n-- \ngitgitgadget\n"},{"id":"425168","messageId":"xmqq5yzdt3fc.fsf@gitster.g","threadId":"55727","inReplyTo":"pull.955.v3.git.1621535291406.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] simple-ipc: correct ifdefs when NO_PTHREADS is defined","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-20T23:01:59Z","receivedAt":"2021-05-20T23:02:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jeff Hostetler via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>  Makefile                            | 22 ++++++++++++++++++++--\n>  compat/simple-ipc/ipc-shared.c      | 10 +++++++---\n>  compat/simple-ipc/ipc-unix-socket.c |  8 ++++++--\n>  compat/simple-ipc/ipc-win32.c       |  8 ++++++--\n>  contrib/buildsystems/CMakeLists.txt | 10 +++++++++-\n>  simple-ipc.h                        |  4 ----\n>  6 files changed, 48 insertions(+), 14 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 3a2d3c80a81a..ea4c0a77604d 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1687,13 +1687,31 @@ ifdef NO_UNIX_SOCKETS\n>  else\n>  \tLIB_OBJS += unix-socket.o\n>  \tLIB_OBJS += unix-stream-server.o\n> -\tLIB_OBJS += compat/simple-ipc/ipc-shared.o\n> -\tLIB_OBJS += compat/simple-ipc/ipc-unix-socket.o\n>  endif\n>  \n> +# Simple IPC requires threads and platform-specific IPC support.\n> +# Only platforms that have both should include these source files\n> +# in the build.\n> +#\n> +# On Windows-based systems, Simple IPC requires threads and Windows\n> +# Named Pipes.  These are always available, so Simple IPC support\n> +# is optional.\n\nThe last part for windows feels funny in that even if they were not\nalways available, the builder can still choose to compile Simple IPC\nsupport out, hence it is optional (this is true for both Windows and\nothers).  In other words, \"prereqs are always satisified\" does not\nlead to \"hence it is optional\".\n\nBut let's leave it as-is.  We could rewrite it to \"..., so Simple\nIPC is always enabled\", though.\n\n> +#\n> +# On Unix-based systems, Simple IPC requires pthreads and Unix\n> +# domain sockets.  So support is only enabled when both are present.\n\nOther than that, looks good to me.\n\nWill queue.\n"}]}