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

Hi,

Isn't it better to check the value of macros itsef rather than checking for
system dependent macros that does not directly relate to the issue?
specifically for getaddrinfo.c case I think
#if EAI_NODATA != EAI_NONAME
is a better check than checking for
#if !defined(__MINGW64_VERSION_MAJOR) && !defined(WIN32_ONLY_COMPILER) /* MSVC/WIN64 duplicate */

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

+/* __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?

On 2011/11/24, at 17:24, Lars Kanis wrote:

Hello, Lars.

You wrote:

LK> Hi PostgreSQL hackers,
LK>
LK> support for Mingw-w64 compiler was added to postgres with commit
LK> 91812df. Unfortunately only the 64 bit output is working right
LK> now. This issue was already highlighted with initial patch in
LK> http://archives.postgresql.org/pgsql-bugs/2011-07/msg00059.php
LK>
LK> Mingw-w64 uses the same header files for 32 and 64 bit compiles.
LK> So the same conditions apply to mingw-w32 bit as for the WIN64
LK> case. In WIN64 "WSAAPI" is defined to nothing, but in 32 bit to
LK> stdcall, so it needs to be used in the accept-parameter check,
LK> too. Maybe you prefer PASCAL instead of WSAAPI in configure.
LK>
LK> I tested successful compilation for the following platforms:
LK> - i686-w64-mingw32 - gcc v4.6.1
LK> - x86_64-w64-mingw32 - gcc v4.6.1
LK> - i586-mingw32msvc - gcc v4.4.4
LK> - x86_64-linux-gnu - gcc v4.6.1
LK>

Can you please provide me with some howto on building PG sources with
mingw-w64?

--
With best wishes,
Pavel mailto:pavel@gf.microolap.com

Hi,

2011-11-24 09:55 keltezéssel, Pavel Golub írta:

Can you please provide me with some howto on building PG sources with
mingw-w64?

Install Fedora 15 or 16, add this repo file into /etc/yum.repos.d :

http://build1.openftd.org/fedora-cross/fedora-cross.repo

Then "yum install mingw*". This will install mingw64 both 32 and 64-bit.
Replaces a lot of official mingw32 packages from Fedora.

Then you can do for 32-bit:

cd postgresql-9.1.1
mingw32-configure
mingw32-make
sudo mingw32-make install

or for 64-bit:

mingw64-configure
mingw64-make
sudo mingw64-make install

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/

Can you please provide me with some howto on building PG sources with
mingw-w64?

For 32/64 bit mingw-v4.6.1 on ubuntu 11.10:
apt-get install flex gcc-mingw-w64
./configure --host=i686-w64-mingw32 --build=x86_64-linux --without-zlib && make
and
./configure --host=x86_64-w64-mingw32 --build=x86_64-linux --without-zlib && make

For 32 bit mingw-v4.4.4 on ubuntu 11.10:
apt-get install flex gcc-mingw32
./configure --host=i586-mingw32msvc --build=x86_64-linux --without-zlib && make

Regards,
Lars Kanis

Isn't it better to check the value of macros itsef rather than checking for
system dependent macros that does not directly relate to the issue?
specifically for getaddrinfo.c case I think
#if EAI_NODATA != EAI_NONAME
is a better check than checking for
#if !defined(__MINGW64_VERSION_MAJOR) && !defined(WIN32_ONLY_COMPILER) /* MSVC/WIN64 duplicate */

Yes it's better and it works for all described test environments.

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

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?

At least in mingw32 v4.4.4 there is no crtdefs.h. I couldn't find a proper define that relates directly
to that issue, so attached is a somewhat cumbersome MINGW version check.

--
Regards,
Lars Kanis

On 11/24/2011 04:39 AM, Lars Kanis wrote:

Can you please provide me with some howto on building PG sources with

mingw-w64?

For 32/64 bit mingw-v4.6.1 on ubuntu 11.10:

apt-get install flex gcc-mingw-w64

./configure --host=i686-w64-mingw32 --build=x86_64-linux
--without-zlib && make

and

./configure --host=x86_64-w64-mingw32 --build=x86_64-linux
--without-zlib && make

For 32 bit mingw-v4.4.4 on ubuntu 11.10:

apt-get install flex gcc-mingw32

./configure --host=i586-mingw32msvc --build=x86_64-linux
--without-zlib && make

The only thing I at least am going to be interested in supporting is a
native compiler, not a cross-compiler. It's impossible to automate
cross-compiler testing.

The way to build natively with a mingw-w64 compiler is doumented fairly
simply at
<http://www.postgresql.org/docs/current/static/installation-platform-notes.html#INSTALLATION-NOTES-MINGW&gt;:

To build 64 bit binaries using MinGW, install the 64 bit tool set
from http://mingw-w64.sourceforge.net/, put its bin directory in the
PATH, and run configure with the --host=x86_64-w64-mingw option.

This is exactly how the buildfarm member pitta was set up.

The only issue here is how to add support for using the 32bit mingw-w64
compiler in pretty much the same way.

cheers

andrew

Hi,

The way to build natively with a mingw-w64 compiler is doumented fairly simply at <http://www.postgresql.org/docs/current/static/installation-platform-notes.html#INSTALLATION-NOTES-MINGW&gt;:

To build 64 bit binaries using MinGW, install the 64 bit tool set
from http://mingw-w64.sourceforge.net/, put its bin directory in the
PATH, and run configure with the --host=x86_64-w64-mingw option.

This is exactly how the buildfarm member pitta was set up.

The only issue here is how to add support for using the 32bit mingw-w64 compiler in pretty much the same way.

I hope that configure --host=i686-w64-mingw use 32 bit tool sets and
construct a 32 bit binary. It is mostly the same procedure and should have
sufficient information to distinguish them. Both 64 bit and 32 bit binaries
should be runnable on the same 64 bit system.

Best regards,

On 11/24/2011 06:29 AM, Lars Kanis wrote:

Isn't it better to check the value of macros itsef rather than checking for
system dependent macros that does not directly relate to the issue?
specifically for getaddrinfo.c case I think
#if EAI_NODATA != EAI_NONAME
is a better check than checking for
#if !defined(__MINGW64_VERSION_MAJOR)&& !defined(WIN32_ONLY_COMPILER) /* MSVC/WIN64 duplicate */

Yes it's better and it works for all described test environments.

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

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.

cheers

andrew

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.

Hi,

+/* __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?

This really breaks mingw gcc 4.6.1 :( it does not have crtdefs.h)
If moving downward do not break MSVC, perhaps its the good way.
Otherwise, we might check for the presence of crtdefs.h with configure?

On 11/27/2011 09:18 AM, NISHIYAMA Tomoaki wrote:

Hi,

+/* __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?

This really breaks mingw gcc 4.6.1 :( it does not have crtdefs.h)
If moving downward do not break MSVC, perhaps its the good way.
Otherwise, we might check for the presence of crtdefs.h with configure?

I have looked at this a bit. It's fairly ugly, and the only moderately
clean way I see forward is a configure test to check for a mingw-w64
compiler, e.g. by running gcc -E -dM over a file which just includes
stdio.h and checking for the definedness of __MINGW64__VERSION_MAJOR, or
something similar.

cheers

andrew

Hi,

A new patch:
check for the presence of crtdefs.h in configure

-#if _MSC_VER >= 1400 || defined(WIN64)
+#if _MSC_VER >= 1400 || HAVE_CRTDEFS_H
#define errcode __msvc_errcode
#include <crtdefs.h>
#undef errcode

Perhaps there is no guarantee that mingw (not -w64) may not have crtdefs.h in the future versions.
the 3 lines
#define errcode __msvc_errcode
#include <crtdefs.h>
#undef errcode
should be valid as far as crtdefs.h exists and errcode is not defined previously.
Because this is the first system include file, we can be sure errcode is not
defined by this point.
So, I believe its better to just test for the presence of crtdefs.h rather than
try to figure out the name of compiler.

check for fseeko and ftello macro definition before defining to avoid
warning in mingw-w64 4.7.0 20110827

This patch was tested to build successfully on
mingw gcc version 4.6.1
mingw-w64 i686-w64-mingw32 gcc version 4.5.4 20110812
mingw-w64 x86_64-w64-mingw32 gcc version 4.7.0 20110827

On Sat, Dec 3, 2011 at 09:24, NISHIYAMA Tomoaki
<tomoakin@staff.kanazawa-u.ac.jp> wrote:

Hi,

A new patch:
check for the presence of crtdefs.h in configure

-#if _MSC_VER >= 1400 || defined(WIN64)
+#if _MSC_VER >= 1400 || HAVE_CRTDEFS_H
 #define errcode __msvc_errcode
 #include <crtdefs.h>
 #undef errcode

Have you verified if tihs affects _MSC_VER < 1400? Suddently that
branch would care about HAVE_CRTDEFS_H, and I'm not sure if that's
something we need to worry about.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Hi,

Have you verified if tihs affects _MSC_VER < 1400? Suddently that
branch would care about HAVE_CRTDEFS_H, and I'm not sure if that's
something we need to worry about.

I have no MSVC. In that sense it is not verified in fact, and I hope
those who knows well would kindly comment on it.

However, it appears that pg_config.h is not created through
configure, but just copied from pg_config.h.win32 in those
compilers and thus HAVE_CRTDEFS_H will not be defined.
So, I think this code fragment will not be enabled in
_MSC_VER < 1400

In addition, the code fragment should have no harm as far as they
have crtdefs.h.
(Again, this is I think it should, and not tested with real tools.)

On 2011/12/03, at 23:20, Magnus Hagander wrote:

On Sat, Dec 3, 2011 at 15:49, NISHIYAMA Tomoaki
<tomoakin@staff.kanazawa-u.ac.jp> wrote:

Hi,

Have you verified if tihs affects _MSC_VER < 1400? Suddently that
branch would care about HAVE_CRTDEFS_H, and I'm not sure if that's
something we need to worry about.

I have no MSVC. In that sense it is not verified in fact, and I hope
those who knows well would kindly comment on it.

However, it appears that pg_config.h is not created through
configure, but just copied from pg_config.h.win32 in those
compilers and thus HAVE_CRTDEFS_H will not be defined.
So, I think this code fragment will not be enabled in
_MSC_VER < 1400

Hmm, true. Unless HAVE_CRTDEFS_H is defined by the sytem, which it
likely isn't - I was confusing it with the kind of defines that MSVC
tends to stick in their own headerfiles, and thought that's what you
were testing for.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On 12/03/2011 09:59 AM, Magnus Hagander wrote:

On Sat, Dec 3, 2011 at 15:49, NISHIYAMA Tomoaki
<tomoakin@staff.kanazawa-u.ac.jp> wrote:

Hi,

Have you verified if tihs affects _MSC_VER< 1400? Suddently that
branch would care about HAVE_CRTDEFS_H, and I'm not sure if that's
something we need to worry about.

I have no MSVC. In that sense it is not verified in fact, and I hope
those who knows well would kindly comment on it.

However, it appears that pg_config.h is not created through
configure, but just copied from pg_config.h.win32 in those
compilers and thus HAVE_CRTDEFS_H will not be defined.
So, I think this code fragment will not be enabled in
_MSC_VER< 1400

Hmm, true. Unless HAVE_CRTDEFS_H is defined by the sytem, which it
likely isn't - I was confusing it with the kind of defines that MSVC
tends to stick in their own headerfiles, and thought that's what you
were testing for.

Yes, but there's a deal more work to do here. This whole thing is
falling over in my build environment (64 bit Windows 7, MinGW/MSys, the
machine that runs pitta on the buildfarm.)

This is a long way from a done deal.

cheers

andrew

On 12/03/2011 06:12 PM, Andrew Dunstan wrote:

On 12/03/2011 09:59 AM, Magnus Hagander wrote:

On Sat, Dec 3, 2011 at 15:49, NISHIYAMA Tomoaki
<tomoakin@staff.kanazawa-u.ac.jp> wrote:

Hi,

Have you verified if tihs affects _MSC_VER< 1400? Suddently that
branch would care about HAVE_CRTDEFS_H, and I'm not sure if that's
something we need to worry about.

I have no MSVC. In that sense it is not verified in fact, and I hope
those who knows well would kindly comment on it.

However, it appears that pg_config.h is not created through
configure, but just copied from pg_config.h.win32 in those
compilers and thus HAVE_CRTDEFS_H will not be defined.
So, I think this code fragment will not be enabled in
_MSC_VER< 1400

Hmm, true. Unless HAVE_CRTDEFS_H is defined by the sytem, which it
likely isn't - I was confusing it with the kind of defines that MSVC
tends to stick in their own headerfiles, and thought that's what you
were testing for.

Yes, but there's a deal more work to do here. This whole thing is
falling over in my build environment (64 bit Windows 7, MinGW/MSys,
the machine that runs pitta on the buildfarm.)

This is a long way from a done deal.

In particular, it's a major mess because it does this (or at least the
version I'm using does):

#define stat _stat64

which plays merry hell with pgwin32_safestat(). Working around that
looks very unpleasant indeed.

cheers

andrew

Hi,

On 2011/12/04, at 9:45, Andrew Dunstan wrote:

Yes, but there's a deal more work to do here. This whole thing is falling over in my build environment (64 bit Windows 7, MinGW/MSys, the machine that runs pitta on the buildfarm.)

This is a long way from a done deal.

In particular, it's a major mess because it does this (or at least the version I'm using does):

#define stat _stat64

which plays merry hell with pgwin32_safestat(). Working around that looks very unpleasant indeed.

Thank you for testing and reporting.
Would you mind giving me a bit more information on the environment?
--especially for the MinGW/MSYS versions and any other component that is required.

I tested on a virtual machine running 64 bit Windows 7 SP1,
installing MinGW/MSYS in the clean state and then compile.
The versions coming with pre-packaged repository catalogues of 20110802
(gcc 4.5.2) and latest catalogue (gcc-4.6.1-2) compiles successfully.

On 2011/12/04, at 9:45, Andrew Dunstan wrote:

Hi,

I found error on #define stat _stat64 occurs on Mingw-w64 (x86_64-w64-mingw32)
gcc version 4.7.0 20111203 (experimental) (GCC)

The code can be compiled with

diff --git a/src/include/port.h b/src/include/port.h
index eceb4bf..78d5c92 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -332,7 +332,7 @@ extern bool rmtree(const char *path, bool rmtopdir);
  * Some frontends don't need the size from stat, so if UNSAFE_STAT_OK
  * is defined we don't bother with this.
  */
-#if defined(WIN32) && !defined(__CYGWIN__) && !defined(UNSAFE_STAT_OK)
+#if defined(WIN32_ONLY_COMPILER) && !defined(UNSAFE_STAT_OK)
 #include <sys/stat.h>
 extern int     pgwin32_safestat(const char *path, struct stat * buf);

but, surely we need to know if it is ok or not
as the comment before says:
* stat() is not guaranteed to set the st_size field on win32, so we
* redefine it to our own implementation that is.

Is there any simple test program that determines if the pgwin32_safestat
is required or the library stat is sufficient?
I presume the stat is a library function and therefore it depends on the
compiler rather than the WIN32 platform as a whole.

On 2011/12/04, at 12:55, NISHIYAMA Tomoaki wrote: