{"thread":{"id":"31345","subject":"RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact","startedAt":"2012-08-24T13:05:26Z","lastAt":"2012-09-04T11:49:16Z","messageCount":5,"participants":["Joachim Schmitz","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"197771","messageId":"004001cd81f9$21e68e10$65b3aa30$@schmitz-digital.de","threadId":"31345","inReplyTo":null,"subject":"RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-24T13:05:26Z","receivedAt":"2012-08-24T13:05:26Z","isPatch":true,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Joachim Schmitz [mailto:jojo@schmitz-digital.de]\n> Sent: Friday, August 24, 2012 11:45 AM\n> To: Junio C Hamano (gitster@pobox.com)\n> Cc: git@vger.kernel.org; Erik Faye-Lund (kusmabite@gmail.com)\n> Subject: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact\n> \n> \n> Signed-off-by: Joachim Schmitz <jojo@schmitz-digital.de>\n> ---\n> This time I hopefully didn't screw up whitespace and line breaks.\n> \n>  Makefile            | 18 ++++++++++++++----\n>  compat/win32/poll.c |  8 ++++++--\n>  2 files changed, 20 insertions(+), 6 deletions(-)\n> \n> diff --git a/Makefile b/Makefile\n> index 66e8216..e150816 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -152,6 +152,11 @@ all::\n>  #\n>  # Define NO_MMAP if you want to avoid mmap.\n>  #\n> +# Define NO_SYS_POLL_H if you don't have sys/poll.h.\n> +#\n> +# Define NO_POLL if you do not have or do not want to use poll.\n> +# This also implies NO_SYS_POLL_H.\n> +#\n>  # Define NO_PTHREADS if you do not have or do not want to use Pthreads.\n>  #\n>  # Define NO_PREAD if you have a problem with pread() system call (e.g.\n> @@ -1216,7 +1221,7 @@ ifeq ($(uname_S),Windows)\n>  \tNO_PREAD = YesPlease\n>  \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n>  \tNO_LIBGEN_H = YesPlease\n> -\tNO_SYS_POLL_H = YesPlease\n> +\tNO_POLL = YesPlease\n>  \tNO_SYMLINK_HEAD = YesPlease\n>  \tNO_IPV6 = YesPlease\n>  \tNO_UNIX_SOCKETS = YesPlease\n> @@ -1257,7 +1262,7 @@ ifeq ($(uname_S),Windows)\n>  \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -\n> D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n>  \tCOMPAT_OBJS = compat/msvc.o compat/winansi.o \\\n>  \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n> -\t\tcompat/win32/poll.o compat/win32/dirent.o\n> +\t\tcompat/win32/dirent.o\n>  \tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -\n> Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n>  \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n>  \tEXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib\n> @@ -1312,7 +1317,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n>  \tNO_PREAD = YesPlease\n>  \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n>  \tNO_LIBGEN_H = YesPlease\n> -\tNO_SYS_POLL_H = YesPlease\n> +\tNO_POLL = YesPlease\n>  \tNO_SYMLINK_HEAD = YesPlease\n>  \tNO_UNIX_SOCKETS = YesPlease\n>  \tNO_SETENV = YesPlease\n> @@ -1347,7 +1352,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n>  \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n>  \tCOMPAT_OBJS += compat/mingw.o compat/winansi.o \\\n>  \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n> -\t\tcompat/win32/poll.o compat/win32/dirent.o\n> +\t\tcompat/win32/dirent.o\n>  \tEXTLIBS += -lws2_32\n>  \tPTHREAD_LIBS =\n>  \tX = .exe\n> @@ -1601,6 +1606,11 @@ ifdef NO_GETTEXT\n>  \tBASIC_CFLAGS += -DNO_GETTEXT\n>  \tUSE_GETTEXT_SCHEME ?= fallthrough\n>  endif\n> +ifdef NO_POLL\n> +\tNO_SYS_POLL_H = YesPlease\n> +\tCOMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it finds poll.h\n> +\tCOMPAT_OBJS += compat/win32/poll.o\n> +endif\n>  ifdef NO_STRCASESTR\n>  \tCOMPAT_CFLAGS += -DNO_STRCASESTR\n>  \tCOMPAT_OBJS += compat/strcasestr.o\n> diff --git a/compat/win32/poll.c b/compat/win32/poll.c\n> index 403eaa7..49541f1 100644\n> --- a/compat/win32/poll.c\n> +++ b/compat/win32/poll.c\n> @@ -24,7 +24,9 @@\n>  # pragma GCC diagnostic ignored \"-Wtype-limits\"\n>  #endif\n> \n> -#include <malloc.h>\n> +#if defined(WIN32)\n> +# include <malloc.h>\n> +#endif\n> \n>  #include <sys/types.h>\n> \n> @@ -48,7 +50,9 @@\n>  #else\n>  # include <sys/time.h>\n>  # include <sys/socket.h>\n> -# include <sys/select.h>\n> +# ifndef NO_SYS_SELECT_H\n> +#  include <sys/select.h>\n> +# endif\n>  # include <unistd.h>\n>  #endif\n> \n> --\n> 1.7.12\n\n\nThere is a downside with this: In order to make use of it, in Makefile it \nadds \"-Icompat/win32\" to COMPAR_CFLAGS. This results in \ncompat/win32/dirent.h to be found, rather than /usr/include/dirent.h. \nThis should be fine for WIN32, but for everybody else may not. \nFor HP NonStop in particular it results in a warning:\n\n        };\n         ^\n\"... /compat/win32/dirent.h\", line 17: warning(133): expected an identifier\n\nAnd this is because there it uses an unnamed union, which is a GCC extension \n(just like unnamed struct), but not part of C89/C99/POSIX.\n\nOne possible solution might be to move compat/win32/poll.[ch] to compat/.\nAnother is to just ignore the warning, at least here it seems to work just fine?\nOr to avoid using an unnamed union. But the later 2 cases would still mean that\nwe include the wrong dirent.h, so the 1st solution seems the cleanest.\n\nAny other idea?\nLet me know your thoughts...\n\nBye, Jojo\n"},{"id":"197780","messageId":"7vsjbc1dg1.fsf@alter.siamese.dyndns.org","threadId":"31345","inReplyTo":"004001cd81f9$21e68e10$65b3aa30$@schmitz-digital.de","subject":"Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-24T16:07:10Z","receivedAt":"2012-08-24T16:07:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n\n> There is a downside with this: In order to make use of it, in Makefile it \n> adds \"-Icompat/win32\" to COMPAR_CFLAGS. This results in \n> compat/win32/dirent.h to be found, rather than /usr/include/dirent.h. \n> This should be fine for WIN32, but for everybody else may not. \n> For HP NonStop in particular it results in a warning:\n>\n>         };\n>          ^\n> \"... /compat/win32/dirent.h\", line 17: warning(133): expected an identifier\n>\n> And this is because there it uses an unnamed union, which is a GCC extension \n> (just like unnamed struct), but not part of C89/C99/POSIX.\n>\n> One possible solution might be to move compat/win32/poll.[ch] to compat/.\n\nI think that is the most sensible way to go, because poll.[ch]\n\n (1) has proven itself to be useful outside the context of win32, and\n (2) the code is coming from gnulib anyway.\n\nI thought I already suggested going that route in my previous\nreview, but perhaps I forgot to do so.\n"},{"id":"197789","messageId":"005401cd822a$67687a30$36396e90$@schmitz-digital.de","threadId":"31345","inReplyTo":"7vsjbc1dg1.fsf@alter.siamese.dyndns.org","subject":"RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-24T18:58:09Z","receivedAt":"2012-08-24T18:58:09Z","isPatch":true,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Junio C Hamano [mailto:gitster@pobox.com]\n> Sent: Friday, August 24, 2012 6:07 PM\n> To: Joachim Schmitz\n> Cc: git@vger.kernel.org; Erik Faye-Lund\n> Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact\n> \n> \"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n> \n> > There is a downside with this: In order to make use of it, in Makefile it\n> > adds \"-Icompat/win32\" to COMPAR_CFLAGS. This results in\n> > compat/win32/dirent.h to be found, rather than /usr/include/dirent.h.\n> > This should be fine for WIN32, but for everybody else may not.\n> > For HP NonStop in particular it results in a warning:\n> >\n> >         };\n> >          ^\n> > \"... /compat/win32/dirent.h\", line 17: warning(133): expected an identifier\n> >\n> > And this is because there it uses an unnamed union, which is a GCC extension\n> > (just like unnamed struct), but not part of C89/C99/POSIX.\n> >\n> > One possible solution might be to move compat/win32/poll.[ch] to compat/.\n> \n> I think that is the most sensible way to go, because poll.[ch]\n> \n>  (1) has proven itself to be useful outside the context of win32, and\n>  (2) the code is coming from gnulib anyway.\n> \n> I thought I already suggested going that route in my previous\n> review, but perhaps I forgot to do so.\n\nNo, I believe you did, but I had forgotten about it. Could/should that be a 2nd patch?\nOr a v3 of this one?\n\nDifferent, but related question: would poll.[ch] be allowed to #include \"git-compat-util.h\"?\n\nBye, Jojo\n"},{"id":"197791","messageId":"7vboi0yswz.fsf@alter.siamese.dyndns.org","threadId":"31345","inReplyTo":"005401cd822a$67687a30$36396e90$@schmitz-digital.de","subject":"Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-24T19:46:36Z","receivedAt":"2012-08-24T19:46:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n\n> Different, but related question: would poll.[ch] be allowed to #include \"git-compat-util.h\"?\n\nSeeing other existing generic wrappers directly under compat/,\ne.g. fopen.c, mkdtemp.c, doing so, I would say why not.\n\nWindows folks (I see Erik is already CC'ed, which is good ;-),\nplease work with Joachim to make sure such a move won't break your\nbuilds.  I believe that it should just be the matter of updating a\ncouple of paths in the top-level Makefile.\n\nThanks.\n"},{"id":"198296","messageId":"002701cd8a93$50773e30$f165ba90$@schmitz-digital.de","threadId":"31345","inReplyTo":"7vboi0yswz.fsf@alter.siamese.dyndns.org","subject":"RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-09-04T11:49:16Z","receivedAt":"2012-09-04T11:49:16Z","isPatch":true,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Junio C Hamano [mailto:gitster@pobox.com]\n> Sent: Friday, August 24, 2012 9:47 PM\n> To: Joachim Schmitz\n> Cc: git@vger.kernel.org; 'Erik Faye-Lund'\n> Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact\n> \n> \"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n> \n> > Different, but related question: would poll.[ch] be allowed to #include \"git-compat-util.h\"?\n> \n> Seeing other existing generic wrappers directly under compat/,\n> e.g. fopen.c, mkdtemp.c, doing so, I would say why not.\n> \n> Windows folks (I see Erik is already CC'ed, which is good ;-),\n> please work with Joachim to make sure such a move won't break your\n> builds.  I believe that it should just be the matter of updating a\n> couple of paths in the top-level Makefile.\n\nHaven't heard anything from the Windows folks yet.\n\nI'd prefer to move compat/win32/poll.[ch] into compat/poll.\nThen adjust a few paths in Makefile and that would be the 1st patch\n\nA 2nd patch would be my already proposed ones that make this usable for others (me in this case ;-)), namely wrapping 2 #inludes.\n\ndiff --git a/compat/poll/poll.c b/compat/poll/poll.c\nindex 403eaa7..49541f1 100644\n--- a/compat/poll/poll.c\n+++ b/compat/poll/poll.c\n@@ -24,7 +24,9 @@\n # pragma GCC diagnostic ignored \"-Wtype-limits\"\n #endif\n\n-#include <malloc.h>\n+#if defined(WIN32)\n+# include <malloc.h>\n+#endif\n\n #include <sys/types.h>\n\n@@ -48,7 +50,9 @@\n #else\n # include <sys/time.h>\n # include <sys/socket.h>\n-# include <sys/select.h>\n+# ifndef NO_SYS_SELECT_H\n+#  include <sys/select.h>\n+# endif\n # include <unistd.h>\n #endif\n\n--\n1.7.12\n"}]}