Thread: [PATCH] Fix double-inclusion of pg_config_os.h when building extensions with Visual Studio
[PATCH] Fix double-inclusion of pg_config_os.h when building extensions with Visual Studio
From
Craig Ringer
Date:
Hi all Related to the earlier comments about building extensions on Windows, I just noticed that we don't treat "WINDLL" as equivalent to "WIN32", and "WIN32" isn't set in a Visual Studio DLL project. We should actually be testing _WIN32, which is the compiler's pre-defined macro. The attached patch to c.h takes care of that - it tests _WIN32 in c.h and sets WIN32 early if it's found. _WIN32 is set for both win32 and win64, like we expect from WIN32. Without this patch, MSVC extension builds fail with: 9.3\include\server\pg_config_os.h(207): error C2011: 'timezone' : 'struct' type redefinition 1> c:\program files\postgresql\9.3\include\server\pg_config_os.h(207) : see declaration of 'timezone' 1>c:\program files\postgresql\9.3\include\server\pg_config_os.h(216): error C2011: 'itimerval' : 'struct' type redefinition 1> c:\program files\postgresql\9.3\include\server\pg_config_os.h(216) : see declaration of 'itimerval' due to double-inclusion of pg_config_os.h from c.h:57 then later on c.h:92 . WIN32 is not defined on line 57 (so we include pg_config_os.h), but gets defined by the include of crtdefs.h so we re-include it again later. This doesn't cause issues with our Perl build system because we set WIN32. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Attachment
Re: [PATCH] Fix double-inclusion of pg_config_os.h when building extensions with Visual Studio
From
Magnus Hagander
Date:
On Sat, Jan 11, 2014 at 8:57 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
Hi all
Related to the earlier comments about building extensions on Windows, I
just noticed that we don't treat "WINDLL" as equivalent to "WIN32", and
"WIN32" isn't set in a Visual Studio DLL project.
We should actually be testing _WIN32, which is the compiler's
pre-defined macro. The attached patch to c.h takes care of that - it
tests _WIN32 in c.h and sets WIN32 early if it's found.
_WIN32 is set for both win32 and win64, like we expect from WIN32.
Regardless of where the other thread goes, this seems like something we should fix. Thus - applied, with minor changes to the comment, thanks.
My understanding is that this change alone doesn't actually help us very much, so I haven't backpatched it anywhere. Let me know if that understanding was incorrect, and it would actually help as a backpatch.
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Re: [PATCH] Fix double-inclusion of pg_config_os.h when building extensions with Visual Studio
From
Craig Ringer
Date:
On 01/17/2014 07:41 PM, Magnus Hagander wrote: > Regardless of where the other thread goes, this seems like something we > should fix. Thus - applied, with minor changes to the comment, thanks. Thanks. > My understanding is that this change alone doesn't actually help us very > much, so I haven't backpatched it anywhere. Let me know if that > understanding was incorrect, and it would actually help as a backpatch. I don't think there's any point backpatching it - as you say, by its self it doesn't help tons. There's little or no evidence that anyone's doing standalone Visual Studio based builds at the moment, and if they are they'll have already had to define WIN32 themselves so they won't notice. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services