* RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
@ 2012-08-24 13:05 Joachim Schmitz
2012-08-24 16:07 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Joachim Schmitz @ 2012-08-24 13:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Erik Faye-Lund
> From: Joachim Schmitz [mailto:jojo@schmitz-digital.de]
> Sent: Friday, August 24, 2012 11:45 AM
> To: Junio C Hamano (gitster@pobox.com)
> Cc: git@vger.kernel.org; Erik Faye-Lund (kusmabite@gmail.com)
> Subject: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
>
>
> Signed-off-by: Joachim Schmitz <jojo@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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
2012-08-24 13:05 [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact Joachim Schmitz
@ 2012-08-24 16:07 ` Junio C Hamano
2012-08-24 18:58 ` Joachim Schmitz
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2012-08-24 16:07 UTC (permalink / raw)
To: Joachim Schmitz; +Cc: git, Erik Faye-Lund
"Joachim Schmitz" <jojo@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
2012-08-24 16:07 ` Junio C Hamano
@ 2012-08-24 18:58 ` Joachim Schmitz
2012-08-24 19:46 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Joachim Schmitz @ 2012-08-24 18:58 UTC (permalink / raw)
To: 'Junio C Hamano'; +Cc: git, 'Erik Faye-Lund'
> From: Junio C Hamano [mailto:gitster@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" <jojo@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
2012-08-24 18:58 ` Joachim Schmitz
@ 2012-08-24 19:46 ` Junio C Hamano
2012-09-04 11:49 ` Joachim Schmitz
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2012-08-24 19:46 UTC (permalink / raw)
To: Joachim Schmitz; +Cc: git, 'Erik Faye-Lund'
"Joachim Schmitz" <jojo@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
2012-08-24 19:46 ` Junio C Hamano
@ 2012-09-04 11:49 ` Joachim Schmitz
0 siblings, 0 replies; 6+ messages in thread
From: Joachim Schmitz @ 2012-09-04 11:49 UTC (permalink / raw)
To: 'Junio C Hamano'; +Cc: git, 'Erik Faye-Lund'
> From: Junio C Hamano [mailto:gitster@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" <jojo@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
@ 2012-08-24 9:45 Joachim Schmitz
0 siblings, 0 replies; 6+ messages in thread
From: Joachim Schmitz @ 2012-08-24 9:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Erik Faye-Lund
Signed-off-by: Joachim Schmitz <jojo@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-04 11:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-24 13:05 [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact Joachim Schmitz
2012-08-24 16:07 ` Junio C Hamano
2012-08-24 18:58 ` Joachim Schmitz
2012-08-24 19:46 ` Junio C Hamano
2012-09-04 11:49 ` Joachim Schmitz
-- strict thread matches above, loose matches on Subject: below --
2012-08-24 9:45 Joachim Schmitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).