Re: [PATCH] PostgreSQL fails to build with 32bit MinGW-w64

Hi,

>>> For the win32.h, I really don't understand why _WINSOCKAPI_ was defined before
>>> <winsock2.h>
>>> some google suggests that defining _WINSOCKAPI_ before<windows.h> prevents
>>> inclusion of winsock.h but that does not have relation to inclusion of
>>> <winsock2.h> and if<winsock2.h> is included first, it should be ok.
>>>
>>> If this guess is right, perhaps it could be better to remove the three lines.
>>> #if !defined(WIN64) || defined(WIN32_ONLY_COMPILER)
>>> #define _WINSOCKAPI_
>>> #endif
>
> No, this broke some compilers, IIRC (probably the native mingw compiler, which is in use by several buildfarm members). Getting this right was very tricky and time-consuming when I was adding support for the 64 bit mingw-w64 compiler, and there were a couple of rounds of breakage.
>
> I'm therefore much more inclined to go the way of your earlier patch, which seems much less risky.

I agree that original patch could be less risky.
However, it doesn't match what Microsoft says:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms737629%28v=vs.85%29.aspx

So, I think the standard way is not defining _WINSOCKAPI_, and if any compiler requires that,
which I think unlikely, then it should be defined as a exceptional case.
At least, native mingw GCC 4.5.2 (20110802 catalogue) and
4.6.1 (latest catalogue) does compile happily without the three lines.

>> I only changed this for consistency. For me, it works without that define in all test
>> environments, too.
>>
>>> +/* __MINGW64_VERSION_MAJOR is related to both 32/64 bit gcc compiles by
>>> + * mingw-w64, however it gots defined only after
>>> Why not use __MINGW32__, which is defined without including any headers?
>
> Because it's defined by other than mingw-w64 compilers.

I see. That's because mingw (not -w64).
Should it be ok if mingw is ok with that condition?

> We have a bunch of compilers to support here. There are LOTS of compiler scenarios on Windows (several versions of MSVC, 32bit and 64bit mingw-w64, native mingw gcc, and a couple of Cygwin based compilers), and keeping track of them all and making sure they don't break can be a pain.

Yes, that really is a pain.

The code block
#if _MSC_VER >= 1400 || defined(WIN64)
#define errcode __msvc_errcode
#include <crtdefs.h>
#undef errcode
#endif

looks as if there is no real need to crtdefs.h but, they wanted to prevent
definition of errcode and therefor put to the first place.

So, I was afraid moving the include downwards might break by
by including a header that internally includes crtdefs.h.
If this is not problematic for MSVC (perhaps you know better on that),
I have no objection in moving the order.