RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Friday, August 24, 2012 9:47 PM To: Joachim Schmitz Cc: git@vger.kernel.org; 'Erik Faye-Lund' Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact Joachim Schmitz j...@schmitz-digital.de writes: Different, but related question: would poll.[ch] be allowed to #include git-compat-util.h? Seeing other existing generic wrappers directly under compat/, e.g. fopen.c, mkdtemp.c, doing so, I would say why not. Windows folks (I see Erik is already CC'ed, which is good ;-), please work with Joachim to make sure such a move won't break your builds. I believe that it should just be the matter of updating a couple of paths in the top-level Makefile. Haven't heard anything from the Windows folks yet. I'd prefer to move compat/win32/poll.[ch] into compat/poll. Then adjust a few paths in Makefile and that would be the 1st patch A 2nd patch would be my already proposed ones that make this usable for others (me in this case ;-)), namely wrapping 2 #inludes. diff --git a/compat/poll/poll.c b/compat/poll/poll.c index 403eaa7..49541f1 100644 --- a/compat/poll/poll.c +++ b/compat/poll/poll.c @@ -24,7 +24,9 @@ # pragma GCC diagnostic ignored -Wtype-limits #endif -#include malloc.h +#if defined(WIN32) +# include malloc.h +#endif #include sys/types.h @@ -48,7 +50,9 @@ #else # include sys/time.h # include sys/socket.h -# include sys/select.h +# ifndef NO_SYS_SELECT_H +# include sys/select.h +# endif # include unistd.h #endif -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- This time I hopefully didn't screw up whitespace and line breaks. Makefile| 18 ++ compat/win32/poll.c | 8 ++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 66e8216..e150816 100644 --- a/Makefile +++ b/Makefile @@ -152,6 +152,11 @@ all:: # # Define NO_MMAP if you want to avoid mmap. # +# Define NO_SYS_POLL_H if you don't have sys/poll.h. +# +# Define NO_POLL if you do not have or do not want to use poll. +# This also implies NO_SYS_POLL_H. +# # Define NO_PTHREADS if you do not have or do not want to use Pthreads. # # Define NO_PREAD if you have a problem with pread() system call (e.g. @@ -1216,7 +1221,7 @@ ifeq ($(uname_S),Windows) NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease - NO_SYS_POLL_H = YesPlease + NO_POLL = YesPlease NO_SYMLINK_HEAD = YesPlease NO_IPV6 = YesPlease NO_UNIX_SOCKETS = YesPlease @@ -1257,7 +1262,7 @@ ifeq ($(uname_S),Windows) BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE COMPAT_OBJS = compat/msvc.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ - compat/win32/poll.o compat/win32/dirent.o + compat/win32/dirent.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\.exe\ BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib @@ -1312,7 +1317,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease - NO_SYS_POLL_H = YesPlease + NO_POLL = YesPlease NO_SYMLINK_HEAD = YesPlease NO_UNIX_SOCKETS = YesPlease NO_SETENV = YesPlease @@ -1347,7 +1352,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\ COMPAT_OBJS += compat/mingw.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ - compat/win32/poll.o compat/win32/dirent.o + compat/win32/dirent.o EXTLIBS += -lws2_32 PTHREAD_LIBS = X = .exe @@ -1601,6 +1606,11 @@ ifdef NO_GETTEXT BASIC_CFLAGS += -DNO_GETTEXT USE_GETTEXT_SCHEME ?= fallthrough endif +ifdef NO_POLL + NO_SYS_POLL_H = YesPlease + COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it finds poll.h + COMPAT_OBJS += compat/win32/poll.o +endif ifdef NO_STRCASESTR COMPAT_CFLAGS += -DNO_STRCASESTR COMPAT_OBJS += compat/strcasestr.o diff --git a/compat/win32/poll.c b/compat/win32/poll.c index 403eaa7..49541f1 100644 --- a/compat/win32/poll.c +++ b/compat/win32/poll.c @@ -24,7 +24,9 @@ # pragma GCC diagnostic ignored -Wtype-limits #endif -#include malloc.h +#if defined(WIN32) +# include malloc.h +#endif #include sys/types.h @@ -48,7 +50,9 @@ #else # include sys/time.h # include sys/socket.h -# include sys/select.h +# ifndef NO_SYS_SELECT_H +# include sys/select.h +# endif # include unistd.h #endif -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
From: Joachim Schmitz [mailto:j...@schmitz-digital.de] Sent: Friday, August 24, 2012 11:45 AM To: Junio C Hamano (gits...@pobox.com) Cc: git@vger.kernel.org; Erik Faye-Lund (kusmab...@gmail.com) Subject: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- This time I hopefully didn't screw up whitespace and line breaks. Makefile| 18 ++ compat/win32/poll.c | 8 ++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 66e8216..e150816 100644 --- a/Makefile +++ b/Makefile @@ -152,6 +152,11 @@ all:: # # Define NO_MMAP if you want to avoid mmap. # +# Define NO_SYS_POLL_H if you don't have sys/poll.h. +# +# Define NO_POLL if you do not have or do not want to use poll. +# This also implies NO_SYS_POLL_H. +# # Define NO_PTHREADS if you do not have or do not want to use Pthreads. # # Define NO_PREAD if you have a problem with pread() system call (e.g. @@ -1216,7 +1221,7 @@ ifeq ($(uname_S),Windows) NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease - NO_SYS_POLL_H = YesPlease + NO_POLL = YesPlease NO_SYMLINK_HEAD = YesPlease NO_IPV6 = YesPlease NO_UNIX_SOCKETS = YesPlease @@ -1257,7 +1262,7 @@ ifeq ($(uname_S),Windows) BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H - D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE COMPAT_OBJS = compat/msvc.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ - compat/win32/poll.o compat/win32/dirent.o + compat/win32/dirent.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex - Icompat/win32 -DSTRIP_EXTENSION=\.exe\ BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib @@ -1312,7 +1317,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease - NO_SYS_POLL_H = YesPlease + NO_POLL = YesPlease NO_SYMLINK_HEAD = YesPlease NO_UNIX_SOCKETS = YesPlease NO_SETENV = YesPlease @@ -1347,7 +1352,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\ COMPAT_OBJS += compat/mingw.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ - compat/win32/poll.o compat/win32/dirent.o + compat/win32/dirent.o EXTLIBS += -lws2_32 PTHREAD_LIBS = X = .exe @@ -1601,6 +1606,11 @@ ifdef NO_GETTEXT BASIC_CFLAGS += -DNO_GETTEXT USE_GETTEXT_SCHEME ?= fallthrough endif +ifdef NO_POLL + NO_SYS_POLL_H = YesPlease + COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it finds poll.h + COMPAT_OBJS += compat/win32/poll.o +endif ifdef NO_STRCASESTR COMPAT_CFLAGS += -DNO_STRCASESTR COMPAT_OBJS += compat/strcasestr.o diff --git a/compat/win32/poll.c b/compat/win32/poll.c index 403eaa7..49541f1 100644 --- a/compat/win32/poll.c +++ b/compat/win32/poll.c @@ -24,7 +24,9 @@ # pragma GCC diagnostic ignored -Wtype-limits #endif -#include malloc.h +#if defined(WIN32) +# include malloc.h +#endif #include sys/types.h @@ -48,7 +50,9 @@ #else # include sys/time.h # include sys/socket.h -# include sys/select.h +# ifndef NO_SYS_SELECT_H +# include sys/select.h +# endif # include unistd.h #endif -- 1.7.12 There is a downside with this: In order to make use of it, in Makefile it adds -Icompat/win32 to COMPAR_CFLAGS. This results in compat/win32/dirent.h to be found, rather than /usr/include/dirent.h. This should be fine for WIN32, but for everybody else may not. For HP NonStop in particular it results in a warning: }; ^ ... /compat/win32/dirent.h, line 17: warning(133): expected an identifier And this is because there it uses an unnamed union, which is a GCC extension (just like unnamed struct), but not part of C89/C99/POSIX. One possible solution might be to move compat/win32/poll.[ch] to compat/. Another is to just ignore the warning, at least here it seems to work just fine? Or to avoid using an unnamed union. But the later 2 cases would still mean that we include the wrong dirent.h, so the 1st solution seems the cleanest. Any other idea? Let me know your thoughts... Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
Joachim Schmitz j...@schmitz-digital.de writes: There is a downside with this: In order to make use of it, in Makefile it adds -Icompat/win32 to COMPAR_CFLAGS. This results in compat/win32/dirent.h to be found, rather than /usr/include/dirent.h. This should be fine for WIN32, but for everybody else may not. For HP NonStop in particular it results in a warning: }; ^ ... /compat/win32/dirent.h, line 17: warning(133): expected an identifier And this is because there it uses an unnamed union, which is a GCC extension (just like unnamed struct), but not part of C89/C99/POSIX. One possible solution might be to move compat/win32/poll.[ch] to compat/. I think that is the most sensible way to go, because poll.[ch] (1) has proven itself to be useful outside the context of win32, and (2) the code is coming from gnulib anyway. I thought I already suggested going that route in my previous review, but perhaps I forgot to do so. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Friday, August 24, 2012 6:07 PM To: Joachim Schmitz Cc: git@vger.kernel.org; Erik Faye-Lund Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact Joachim Schmitz j...@schmitz-digital.de writes: There is a downside with this: In order to make use of it, in Makefile it adds -Icompat/win32 to COMPAR_CFLAGS. This results in compat/win32/dirent.h to be found, rather than /usr/include/dirent.h. This should be fine for WIN32, but for everybody else may not. For HP NonStop in particular it results in a warning: }; ^ ... /compat/win32/dirent.h, line 17: warning(133): expected an identifier And this is because there it uses an unnamed union, which is a GCC extension (just like unnamed struct), but not part of C89/C99/POSIX. One possible solution might be to move compat/win32/poll.[ch] to compat/. I think that is the most sensible way to go, because poll.[ch] (1) has proven itself to be useful outside the context of win32, and (2) the code is coming from gnulib anyway. I thought I already suggested going that route in my previous review, but perhaps I forgot to do so. No, I believe you did, but I had forgotten about it. Could/should that be a 2nd patch? Or a v3 of this one? Different, but related question: would poll.[ch] be allowed to #include git-compat-util.h? Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
Joachim Schmitz j...@schmitz-digital.de writes: Different, but related question: would poll.[ch] be allowed to #include git-compat-util.h? Seeing other existing generic wrappers directly under compat/, e.g. fopen.c, mkdtemp.c, doing so, I would say why not. Windows folks (I see Erik is already CC'ed, which is good ;-), please work with Joachim to make sure such a move won't break your builds. I believe that it should just be the matter of updating a couple of paths in the top-level Makefile. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
