[PATCH] Fix double-inclusion of pg_config_os.h when building extensions with Visual Studio

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

On Sat, Jan 11, 2014 at 8:57 AM, Craig Ringer <craig(at)2ndquadrant(dot)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/

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