[PATCH] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact

[PATCH] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact

[Joachim Schmitz]

Signed-off-by: Joachim Schmitz <jojo@schmitz-digital.de>
---
 Makefile            | 18 ++++++++++++++----
 compat/win32/poll.c |  8 ++++++--
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 6b0c961..2af4db3 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 find poll.h
+       COMPAT_OBJS += compat/win32/poll.c
+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

HP NonStop is using this, patch to add this is to follow.

Bye, Jojo

Re: [PATCH] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact

[Junio C Hamano]

"Joachim Schmitz" <jojo@schmitz-digital.de> writes:

> Signed-off-by: Joachim Schmitz <jojo@schmitz-digital.de>
> ---
>  Makefile            | 18 ++++++++++++++----
>  compat/win32/poll.c |  8 ++++++--
>  2 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 6b0c961..2af4db3 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.

Do you really need to have both?  I suspect "If you do not have a
usable sys/poll.h, set NO_SYS_POLL_H" may be a simpler alternative,
but there must be a reason why you had to add a new one instead of
going that route.  It would be a good idea to describe that reason
in the log message above, in the space before your Sign-off.

> @@ -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
> ...
> @@ -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
> ...
> @@ -1601,6 +1606,11 @@ ifdef NO_GETTEXT
> ...
> +ifdef NO_POLL
> +       NO_SYS_POLL_H = YesPlease
> +       COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it find poll.h
> +       COMPAT_OBJS += compat/win32/poll.c
> +endif

In general, I think this is a good direction to go.  If the existing
emulation in win32/poll.c turns out to be usable across platforms
and not windows specific, sharing it would be a good idea.

But if the emulation is no longer windows specific, shouldn't you
also move it outside compat/win32/ and somewhere more generic?

> 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

Hrm, are the Windows folks OK with this?  MINGW and MSVC are
affected; Cygwin should be OK, I think.

>  #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

RE: [PATCH] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact

[Joachim Schmitz]

> From: Junio C Hamano [mailto:gitster@pobox.com]
> Sent: Wednesday, August 22, 2012 7:13 PM
> To: Joachim Schmitz
> Cc: git@vger.kernel.org; Erik Faye-Lund; Johannes Sixt; Marius Storm-Olsen
> Subject: Re: [PATCH] Support non-WIN32 system lacking poll() while keeping
> the WIN32 part intact
> 
> "Joachim Schmitz" <jojo@schmitz-digital.de> writes:
> 
> > Signed-off-by: Joachim Schmitz <jojo@schmitz-digital.de>
> > ---
> >  Makefile            | 18 ++++++++++++++----
> >  compat/win32/poll.c |  8 ++++++--
> >  2 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 6b0c961..2af4db3 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.
> 
> Do you really need to have both?  I suspect "If you do not have a usable
> sys/poll.h, set NO_SYS_POLL_H" may be a simpler alternative, but there
must

Hmm, Not having <sys/poll.h> and not having poll() are different thinks,
aren't they?

Using NO_SYS_POL_H so far only affects BASIC_CFLAGS:
ifdef NO_SYS_POLL_H
        BASIC_CFLAGS += -DNO_SYS_POLL_H
endif

It does not add compat/win32/poll.c to COMPAT_OBJS, NO_POLL does that.

> be a reason why you had to add a new one instead of going that route.  It
would
> be a good idea to describe that reason in the log message above, in the
space
> before your Sign-off.
> 
> > @@ -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
> > ...
> > @@ -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
> > ...
> > @@ -1601,6 +1606,11 @@ ifdef NO_GETTEXT ...
> > +ifdef NO_POLL
> > +       NO_SYS_POLL_H = YesPlease
> > +       COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it find poll.h
> > +       COMPAT_OBJS += compat/win32/poll.c endif
> 
> In general, I think this is a good direction to go.  If the existing
emulation in
> win32/poll.c turns out to be usable across platforms and not windows
specific,
> sharing it would be a good idea.
> 
> But if the emulation is no longer windows specific, shouldn't you also
move it
> outside compat/win32/ and somewhere more generic?

Should be possible. Esp. as with the current setup make issues a warning:

Makefile:2329: target `compat/win32/poll.c' doesn't match the target pattern

Haven't yet been able to spot where that comes from.

> > 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
> 
> Hrm, are the Windows folks OK with this?  MINGW and MSVC are affected;
> Cygwin should be OK, I think.

I stole that define from another place in git (compat/nedmalloc/nedmalloc.c"
line 36), there too it was used to hide <malloc.h>, so I assumed it to be
the proper method?
 
> >  #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

Re: [PATCH] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact

[Junio C Hamano]

"Joachim Schmitz" <jojo@schmitz-digital.de> writes:

>> Do you really need to have both?  I suspect "If you do not have a usable
>> sys/poll.h, set NO_SYS_POLL_H" may be a simpler alternative, but there
> must
>
> Hmm, Not having <sys/poll.h> and not having poll() are different thinks,
> aren't they?

Ahh, OK, I forgot the other combination: working poll() but inclusion
of sys/poll.h is unnecessary.  OK.

RE: [PATCH] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact

[Joachim Schmitz]

> From: Joachim Schmitz [mailto:jojo@schmitz-digital.de]
> Sent: Wednesday, August 22, 2012 7:44 PM
> To: 'Junio C Hamano'
> Cc: 'git@vger.kernel.org'; 'Erik Faye-Lund'; 'Johannes Sixt'; 'Marius
Storm-
> Olsen'
> Subject: RE: [PATCH] Support non-WIN32 system lacking poll() while keeping
> the WIN32 part intact
> 
> > From: Junio C Hamano [mailto:gitster@pobox.com]
> > Sent: Wednesday, August 22, 2012 7:13 PM
> > To: Joachim Schmitz
> > Cc: git@vger.kernel.org; Erik Faye-Lund; Johannes Sixt; Marius
> > Storm-Olsen
> > Subject: Re: [PATCH] Support non-WIN32 system lacking poll() while
> > keeping the WIN32 part intact
> >
> > "Joachim Schmitz" <jojo@schmitz-digital.de> writes:
> >
> > > Signed-off-by: Joachim Schmitz <jojo@schmitz-digital.de>
> > > ---
> > >  Makefile            | 18 ++++++++++++++----
> > >  compat/win32/poll.c |  8 ++++++--
> > >  2 files changed, 20 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
...
> > > @@ -1601,6 +1606,11 @@ ifdef NO_GETTEXT ...
> > > +ifdef NO_POLL
> > > +       NO_SYS_POLL_H = YesPlease
> > > +       COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it find poll.h
> > > +       COMPAT_OBJS += compat/win32/poll.c

This is of course wrong! Needs to be poll.o, not poll.c

> > > endif
> >
> > In general, I think this is a good direction to go.  If the existing
> > emulation in win32/poll.c turns out to be usable across platforms and
> > not windows specific, sharing it would be a good idea.
> >
> > But if the emulation is no longer windows specific, shouldn't you also
> > move it outside compat/win32/ and somewhere more generic?
> 
> Should be possible. Esp. as with the current setup make issues a warning:
> 
> Makefile:2329: target `compat/win32/poll.c' doesn't match the target
pattern
> 
> Haven't yet been able to spot where that comes from.

See above.