Обсуждение: Issues with MinGW W64
Hi all,
I am using MingW W64 from SVN, version 3.0 beta-ish.
There are few issues when compiling Postgres with MingW W64. There was
a patch submitted to this list in July 2011 but it does not address
these issues (that I can tell). The following applies to both 32 and 64
bit builds.
----
The header file crtdefs.h in MinGW typedefs errcode which conflicts with
Postgres' elog.h.
#ifndef __ERRCODE_DEFINED_MS
#define __ERRCODE_DEFINED_MS
typedef int errcode;
#endif
The previous patch #undef'ed errcode, which is probably not going to
help here. I added CPPFLAGS=-D__ERRCODE_DEFINED_MS to the configure
line to work around this.
How would a proper patch deal with this? Add an explicit #define when
MinGW W64 is detected?
----
MinGW W64's sys/stat.h #defines stat to be _stati64 and there is
subsequently a compilation error in port.h:
note: expected 'struct _stati64 *' but argument is of type 'struct stat *'
error: conflicting types for 'pgwin32_safestat'
As stupid as it is, I added #undef stat immediately after including
sys/stat.h in port.h:
#if defined(WIN32) && !defined(__CYGWIN__) && !defined(UNSAFE_STAT_OK)
#include <sys/stat.h>
#undef stat
extern int pgwin32_safestat(const char *path, struct stat * buf);
#define stat(a,b) pgwin32_safestat(a,b)
#endif
Though I might have tried -DUNSAFE_STAT_OK too, but didn't.
NOTE: I'm compiling Postgres exclusively to cross compile PL/Java and
run time behaviour is not a concern of mine.
I am not sure what macro magic would be proper here. Comments welcome.
----
There are series of redefined macros from the MinGW W64 CRT.
In pg_config_os:
warning: "_WIN32_WINNT" redefined [enabled by default]
warning: "fseeko" redefined [enabled by default]
warning: "ftello" redefined [enabled by default]
warning: "EMSGSIZE" redefined [enabled by default]
warning: "EAFNOSUPPORT" redefined [enabled by default]
warning: "EWOULDBLOCK" redefined [enabled by default]
warning: "ECONNRESET" redefined [enabled by default]
warning: "EINPROGRESS" redefined [enabled by default]
warning: "ENOBUFS" redefined [enabled by default]
warning: "EPROTONOSUPPORT" redefined [enabled by default]
warning: "ECONNREFUSED" redefined [enabled by default]
warning: "EOPNOTSUPP" redefined [enabled by default]
In port.h:
warning: "popen" redefined [enabled by default]
warning: "pclose" redefined [enabled by default]
And possibly some more. Do we need these redefines?
----
I'm willing to work on some (if not all) of these issues with proper
guidance.
-- Johann Oskarsson http://www.2ndquadrant.com/ |[] PostgreSQL Development, 24x7 Support, Training
andServices --+-- | Blog:
http://my.opera.com/myrkraverk/blog/
On Tue, May 29, 2012 at 9:04 AM, Johann 'Myrkraverk' Oskarsson <johann@2ndquadrant.com> wrote: > The header file crtdefs.h in MinGW typedefs errcode which conflicts with > Postgres' elog.h. > > #ifndef __ERRCODE_DEFINED_MS > #define __ERRCODE_DEFINED_MS > typedef int errcode; > #endif Eep. Maybe this is not directly relevant, but I guess my first question is: why is MinGW doing that? It seems like pretty bad form to go around defining identifiers that user code might be using for other purposes. We could fix this in our code by, say, changing errcode to pgerrcode everywhere, but that would be fairly invasive, and I don't in general want to start putting "pg" in front of every identifier we use just in case the operating system happens to want to grab that symbol unannounced for some unrelated purpose. > MinGW W64's sys/stat.h #defines stat to be _stati64 and there is > subsequently a compilation error in port.h: > > note: expected 'struct _stati64 *' but argument is of type 'struct stat *' > error: conflicting types for 'pgwin32_safestat' In this case, I really think we ought to change all backend calls that hit stat() to use something like pgstat() instead. Just as the operating system shouldn't define random symbols that might be in use by us, we should not be defining, or redefining, symbols that may be provided by the operating system. pgstat() might not be the best choice of name, though, because we've got a file pgstat.c which does something unrelated. > There are series of redefined macros from the MinGW W64 CRT. > > In pg_config_os: > > warning: "_WIN32_WINNT" redefined [enabled by default] > warning: "fseeko" redefined [enabled by default] > warning: "ftello" redefined [enabled by default] > warning: "EMSGSIZE" redefined [enabled by default] > warning: "EAFNOSUPPORT" redefined [enabled by default] > warning: "EWOULDBLOCK" redefined [enabled by default] > warning: "ECONNRESET" redefined [enabled by default] > warning: "EINPROGRESS" redefined [enabled by default] > warning: "ENOBUFS" redefined [enabled by default] > warning: "EPROTONOSUPPORT" redefined [enabled by default] > warning: "ECONNREFUSED" redefined [enabled by default] > warning: "EOPNOTSUPP" redefined [enabled by default] > > In port.h: > > warning: "popen" redefined [enabled by default] > warning: "pclose" redefined [enabled by default] > > And possibly some more. Do we need these redefines? We probably need them somewhere, or they wouldn't have been added. But maybe we don't need them on the exact platform you're using. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, May 29, 2012 at 9:04 AM, Johann 'Myrkraverk' Oskarsson
> <johann@2ndquadrant.com> wrote:
>> The header file crtdefs.h in MinGW typedefs errcode which conflicts with
>> Postgres' elog.h.
> Eep. Maybe this is not directly relevant, but I guess my first
> question is: why is MinGW doing that?
I concur with Robert here: your first step should be to push back on the
MinGW developers about this nonstandard intrusion on application
namespace. We've been using errcode() as a function name since 2003,
and it's never been a problem before on any platform, including previous
versions of MinGW.
If they won't change it, then we could consider some other hack, but
that should really be the first attempt.
>> MinGW W64's sys/stat.h #defines stat to be _stati64 and there is
>> subsequently a compilation error in port.h:
>>
>> note: expected 'struct _stati64 *' but argument is of type 'struct stat *'
>> error: conflicting types for 'pgwin32_safestat'
> In this case, I really think we ought to change all backend calls that
> hit stat() to use something like pgstat() instead.
I disagree with this conclusion. That'd be an unnecessarily nonstandard
notation, which all existing and future developers would have to learn.
I'd rather work around this in port.h if at all possible. I'm not quite
sure why the existing code fails, though --- is there a conflict between
"#define stat" and "#define stat(a,b)"?
>> There are series of redefined macros from the MinGW W64 CRT.
>> ...
>> And possibly some more. Do we need these redefines?
> We probably need them somewhere, or they wouldn't have been added.
> But maybe we don't need them on the exact platform you're using.
Can we deal with this by just wrapping each #define in #ifndef?
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, May 29, 2012 at 9:04 AM, Johann 'Myrkraverk' Oskarsson >> <johann@2ndquadrant.com> wrote: >>> The header file crtdefs.h in MinGW typedefs errcode which conflicts >>> with Postgres' elog.h. > >> Eep. Maybe this is not directly relevant, but I guess my first >> question is: why is MinGW doing that? > > I concur with Robert here: your first step should be to push back on > the MinGW developers about this nonstandard intrusion on application > namespace. We've been using errcode() as a function name since 2003, > and it's never been a problem before on any platform, including > previous versions of MinGW. I have contacted the MinGW W64 team on this. > If they won't change it, then we could consider some other hack, but > that should really be the first attempt. >>> MinGW W64's sys/stat.h #defines stat to be _stati64 and there is >>> subsequently a compilation error in port.h: >>> >>> note: expected 'struct _stati64 *' but argument is of type 'struct >>> stat *' error: conflicting types for 'pgwin32_safestat' > >> In this case, I really think we ought to change all backend calls >> that hit stat() to use something like pgstat() instead. > > I disagree with this conclusion. That'd be an unnecessarily > nonstandard notation, which all existing and future developers would > have to learn. I'd rather work around this in port.h if at all > possible. I'm not quite sure why the existing code fails, though --- > is there a conflict between "#define stat" and "#define stat(a,b)"? I wouldn't know, the compiler is GCC 4.6.3 here (any 4.5+ will do I think) so all the usal GCC macro magic should be working. Is this something to discuss with the MinGW W64 team? >>> There are series of redefined macros from the MinGW W64 CRT. >>> ... >>> And possibly some more. Do we need these redefines? > >> We probably need them somewhere, or they wouldn't have been added. >> But maybe we don't need them on the exact platform you're using. > > Can we deal with this by just wrapping each #define in #ifndef? I'll take a look and make sure the #defines end up with the same values. If so I'll attach a patch for this. -- Johann Oskarsson http://www.2ndquadrant.com/ |[] PostgreSQL Development, 24x7 Support, Training andServices --+-- | Blog: http://my.opera.com/myrkraverk/blog/
On Thu, May 31, 2012 at 8:15 AM, Johann 'Myrkraverk' Oskarsson <johann@2ndquadrant.com> wrote: >>>> MinGW W64's sys/stat.h #defines stat to be _stati64 and there is >>>> subsequently a compilation error in port.h: >>>> >>>> note: expected 'struct _stati64 *' but argument is of type 'struct >>>> stat *' error: conflicting types for 'pgwin32_safestat' >> >>> In this case, I really think we ought to change all backend calls >>> that hit stat() to use something like pgstat() instead. >> >> I disagree with this conclusion. That'd be an unnecessarily >> nonstandard notation, which all existing and future developers would >> have to learn. I'd rather work around this in port.h if at all >> possible. I'm not quite sure why the existing code fails, though --- >> is there a conflict between "#define stat" and "#define stat(a,b)"? > > I wouldn't know, the compiler is GCC 4.6.3 here (any 4.5+ will do I > think) so all the usal GCC macro magic should be working. > > Is this something to discuss with the MinGW W64 team? My viewpoint on this (which is different than Tom's) is that we're probably not entitled to assume anything about what the system header files do with respect to stat. On some systems, they might just have a function prototype, while others might define stat or stat() as a macro. It seems to me that our source code is hoping for a function definition rather than a macro definition and falling over when that's not how it is. I don't see that as very reasonable, unless we have some basis for believing that the OS isn't entitled to define stat as a macro rather than a function, and maybe not even then. We have plenty of other places where we use our own wrapper function in lieu of OS facilities for various reasons (e.g. BasicOpenFile) and I don't think adding one more is a big deal. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Thu, May 31, 2012 at 8:15 AM, Johann 'Myrkraverk' Oskarsson > <johann@2ndquadrant.com> wrote: >> Is this something to discuss with the MinGW W64 team? > My viewpoint on this (which is different than Tom's) is that we're > probably not entitled to assume anything about what the system header > files do with respect to stat. On some systems, they might just have > a function prototype, while others might define stat or stat() as a > macro. It seems to me that our source code is hoping for a function > definition rather than a macro definition and falling over when that's > not how it is. I don't see that as very reasonable, unless we have > some basis for believing that the OS isn't entitled to define stat as > a macro rather than a function, and maybe not even then. I quote from the POSIX:2008 specification for <sys/stat.h>: The following shall be declared as functions and may also bedefined as macros. Function prototypes shall be provided. ...int stat(const char *restrict, struct stat *restrict); I do not believe that the standard intends the word "shall" to have any wiggle room. I would also read this to mean that if the header defines "stat" as a macro, that macro ought to be an alternative way of invoking the function. Now we are messing up by failing to #undef the macro before redefining it, but if we do that and it still doesn't work, the header is not conformant to POSIX. You can read this yourself at http://pubs.opengroup.org/onlinepubs/9699919799/ > We have > plenty of other places where we use our own wrapper function in lieu > of OS facilities for various reasons (e.g. BasicOpenFile) and I don't > think adding one more is a big deal. Well, I think it is. Where we use a wrapper function, it's because it has somewhat different semantics from the underlying standard function. I do not think it's good for readability to define pgfoo() when that's only meant to be exactly foo(). I'm especially not keen on doing that just because one variant of MinGW has managed to break their conformance with POSIX. regards, tom lane
On Thu, May 31, 2012 at 2:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Thu, May 31, 2012 at 8:15 AM, Johann 'Myrkraverk' Oskarsson >> <johann@2ndquadrant.com> wrote: >>> Is this something to discuss with the MinGW W64 team? > >> My viewpoint on this (which is different than Tom's) is that we're >> probably not entitled to assume anything about what the system header >> files do with respect to stat. On some systems, they might just have >> a function prototype, while others might define stat or stat() as a >> macro. It seems to me that our source code is hoping for a function >> definition rather than a macro definition and falling over when that's >> not how it is. I don't see that as very reasonable, unless we have >> some basis for believing that the OS isn't entitled to define stat as >> a macro rather than a function, and maybe not even then. > > I quote from the POSIX:2008 specification for <sys/stat.h>: > > The following shall be declared as functions and may also be > defined as macros. Function prototypes shall be provided. > ... > int stat(const char *restrict, struct stat *restrict); > > I do not believe that the standard intends the word "shall" to have any > wiggle room. I would also read this to mean that if the header defines > "stat" as a macro, that macro ought to be an alternative way of invoking > the function. There's nothing in the passage you quote that says the macro definition can't do anything other than invoke the eponymous function, but... > Now we are messing up by failing to #undef the macro > before redefining it, but if we do that and it still doesn't work, the > header is not conformant to POSIX. ...this is probably still true. >> We have >> plenty of other places where we use our own wrapper function in lieu >> of OS facilities for various reasons (e.g. BasicOpenFile) and I don't >> think adding one more is a big deal. > > Well, I think it is. Where we use a wrapper function, it's because it > has somewhat different semantics from the underlying standard function. > I do not think it's good for readability to define pgfoo() when that's > only meant to be exactly foo(). I'm especially not keen on doing that > just because one variant of MinGW has managed to break their conformance > with POSIX. In this case, I feel like we've kind of already stepped in it, because we've defined stat() to be pgwin32_safestat(), which is, in fact, not exactly stat(). So right now ISTM that a naive backend hacker might think that stat() means "the stat provided by the OS", but, on Windows, it doesn't. I would exactly describe myself as "keen" on adding wrapper functions to things like stat(), but until non-POSIX operating systems go the way of the dodo bird, I'm not sure there's any way around it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company