> From: Junio C Hamano [mailto:gitster@xxxxxxxxx] > Sent: Wednesday, August 22, 2012 7:13 PM > To: Joachim Schmitz > Cc: git@xxxxxxxxxxxxxxx; 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@xxxxxxxxxxxxxxxxxx> writes: > > > Signed-off-by: Joachim Schmitz <jojo@xxxxxxxxxxxxxxxxxx> > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html