Re: [PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI
Hello, On Mon, May 05, 2014 at 12:55:52AM +0400, Marat Radchenko wrote: On Sun, May 04, 2014 at 08:52:44PM +0200, Stepan Kasal wrote: is really a work around: it would be in effect only for MinGW-W64, and the comment would explain that this is a hack to work around the bug. Workarounds do not have to be ugly and full of #ifdef's. I'm afraid they have to. If you just select one of the reasonable variants, without noting that the other ones would trigger a bug, you are lying a trap for future contributors. If you manage to change the defs for poll.c without changing its content, no one could tell you to report to gnulib first. v1 does exactly this. Yes, but it changes the define for other configurations as well (MSVC, mingw 32bit). I would suggest something along the change below. What do you think? Stepan diff --git a/config.mak.uname b/config.mak.uname index 82b8dff..446dd41 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -508,7 +508,11 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_POSIX_GOODIES = UnfortunatelyYes DEFAULT_HELP_FORMAT = html NO_D_INO_IN_DIRENT = YesPlease - COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32 + COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -Icompat -Icompat/win32 +ifneq ($(uname_M),x86_64) + # MinGW-W64 x.y headers do not provide MsgWaitForMultipleObjects with NOGDI + COMPAT_CFLAGS += -DNOGDI +endif COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\ COMPAT_OBJS += compat/mingw.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ diff --git a/git-compat-util.h b/git-compat-util.h index e6de32c..29a8afd 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -92,6 +92,9 @@ #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ #include winsock2.h #include windows.h +/* We cannot define NOGDI on MinGW-W64, so we unfortunately include + wingdi.h. It then defines ERROR=0, undef it to avoid conflicts */ +#undef ERROR #define GIT_WINDOWS_NATIVE #endif -- 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 10/12] MINGW: compat/poll/poll.c: undef NOGDI
Stepan Kasal wrote: diff --git a/config.mak.uname b/config.mak.uname index 82b8dff..446dd41 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -508,7 +508,11 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_POSIX_GOODIES = UnfortunatelyYes DEFAULT_HELP_FORMAT = html NO_D_INO_IN_DIRENT = YesPlease - COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32 + COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -Icompat -Icompat/win32 +ifneq ($(uname_M),x86_64) + # MinGW-W64 x.y headers do not provide MsgWaitForMultipleObjects with NOGDI MinGW-w64 != x86_64; it provides a i686 compiler as well. -- Felipe Contreras -- 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: [msysGit] Re: [PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI
On Mon, May 05, 2014 at 02:32:11AM -0500, Felipe Contreras wrote: Stepan Kasal wrote: +ifneq ($(uname_M),x86_64) + # MinGW-W64 x.y headers do not provide MsgWaitForMultipleObjects with NOGDI MinGW-w64 != x86_64; it provides a i686 compiler as well. thanks for correcting me. The diff was just a quick sketch. Every workaround should say what bug is working around, otherwise it is destined to become a superstition in the future. Stepan -- 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 10/12] MINGW: compat/poll/poll.c: undef NOGDI
Hello Marat, On Sat, May 03, 2014 at 11:00:51AM +0400, Marat Radchenko wrote: On Wed, Apr 30, 2014 at 01:41:25PM +0200, Stepan Kasal wrote: On Tue, Apr 29, 2014 at 01:12:04PM +0400, Marat Radchenko wrote: On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI. Removal -DNOGDI=1 from config.mak.uname has an undesirable effect of [..] compat/poll/poll.c comes from Gnulib, so it would be better to submit [..] That's why v1 of this patch [1] didn't touch poll.c at all. ouch! It looks like you everyone sending you elsewhere. I apologize for being part of that. (I was not aware about the previous version.) I don't think it's gnulib problem that combination of two third-parties (git and mingw-w64) set up such conditions where poll.c fails to compile. Well, yes and no: gnulib is mostly a collection of compatibility reimplementaions of functions that should be available on an ideal system. If one wants to dig deeper, I'd say the problem is in MinGW-W64 headers because their behavior of hiding MsgWaitForMultipleObjects doesn't match behavior of MSVC headers. Thank you very much for this analysis. It enables us to redirect you the third time: to report this as a bug in MinGW-W64 ! ;-) Seriously, it looks you found the best description of the problem, and it would be nice if you could modify your patch so that it is really a work around: it would be in effect only for MinGW-W64, and the comment would explain that this is a hack to work around the bug. If you manage to change the defs for poll.c without changing its content, no one could tell you to report to gnulib first. OTOH, if MsgWaitForMultipleObjects is present ustream (in gnulib's poll.c, sorry that I cannot check right now), it still might be better to submit the work-around there first. Thanks for your work, Stepan Kasal -- 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 10/12] MINGW: compat/poll/poll.c: undef NOGDI
Marat Radchenko wrote: If one wants to dig deeper, I'd say the problem is in MinGW-W64 headers because their behavior of hiding MsgWaitForMultipleObjects doesn't match behavior of MSVC headers. I agree with that. Can you file a bug report? -- Felipe Contreras -- 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 10/12] MINGW: compat/poll/poll.c: undef NOGDI
On Sun, May 04, 2014 at 08:52:44PM +0200, Stepan Kasal wrote: Thank you very much for this analysis. It enables us to redirect you the third time: to report this as a bug in MinGW-W64 ! ;-) I'll report this to MinGW-W64 soon, though even if/when they fix the issue on their side, I'd still like to have a workaround in Git to be able to use older MinGW-W64 versions that didn't receive a fix. Seriously, it looks you found the best description of the problem, and it would be nice if you could modify your patch so that it is really a work around: it would be in effect only for MinGW-W64, and the comment would explain that this is a hack to work around the bug. Workarounds do not have to be ugly and full of #ifdef's. If you manage to change the defs for poll.c without changing its content, no one could tell you to report to gnulib first. v1 does exactly this. OTOH, if MsgWaitForMultipleObjects is present ustream (in gnulib's poll.c, sorry that I cannot check right now), it still might be better to submit the work-around there first. Workaround is just don't pass -DNOGDI on MinGW-W64 if you want MsgWaitForMultipleObjects, there's nothing to send to gnulib. After all, was there a strong reason why Git started passing it? What is there was no option to disable part of windows.h? -- 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 10/12] MINGW: compat/poll/poll.c: undef NOGDI
On Wed, Apr 30, 2014 at 01:41:25PM +0200, Stepan Kasal wrote: Hello, On Tue, Apr 29, 2014 at 01:12:04PM +0400, Marat Radchenko wrote: On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI. Removal -DNOGDI=1 from config.mak.uname has an undesirable effect of bringing in wingdi.h with weird #define ERROR 0 that conflicts with internal Git enums. So, just #undef NOGDI in compat/poll/poll.c. compat/poll/poll.c comes from Gnulib, so it would be better to submit the patch there and then backport so that the divergence of the two versions does not get worse. That's why v1 of this patch [1] didn't touch poll.c at all. I don't think it's gnulib problem that combination of two third-parties (git and mingw-w64) set up such conditions where poll.c fails to compile. If one wants to dig deeper, I'd say the problem is in MinGW-W64 headers because their behavior of hiding MsgWaitForMultipleObjects doesn't match behavior of MSVC headers. -- 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 10/12] MINGW: compat/poll/poll.c: undef NOGDI
Hello, On Tue, Apr 29, 2014 at 01:12:04PM +0400, Marat Radchenko wrote: On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI. Removal -DNOGDI=1 from config.mak.uname has an undesirable effect of bringing in wingdi.h with weird #define ERROR 0 that conflicts with internal Git enums. So, just #undef NOGDI in compat/poll/poll.c. compat/poll/poll.c comes from Gnulib, so it would be better to submit the patch there and then backport so that the divergence of the two versions does not get worse. Stepan -- 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
