From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 22 Aug 2012 17:13:15 GMT
Subject: Re: [PATCH] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
Message-ID: <7vd32ialzo.fsf@alter.siamese.dyndns.org>
In-Reply-To: <002101cd8078$eb2eb4f0$c18c1ed0$@schmitz-digital.de>

"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