Re: VS 2015 support in src/tools/msvc - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: VS 2015 support in src/tools/msvc
Date
Msg-id 5707D7DC.2030508@dunslane.net
Whole thread Raw
In response to Re: VS 2015 support in src/tools/msvc  (Christian Ullrich <chris@chrullrich.net>)
Responses Re: VS 2015 support in src/tools/msvc  (Christian Ullrich <chris@chrullrich.net>)
List pgsql-hackers

On 04/08/2016 11:02 AM, Christian Ullrich wrote:
> * Michael Paquier wrote:
>
>> On Fri, Apr 8, 2016 at 10:05 PM, Andrew Dunstan <andrew@dunslane.net> 
>> wrote:
>> ¥> On 04/08/2016 07:15 AM, Christian Ullrich wrote:
>
>>>> Michael, none of your patches change this, so how does it ever 
>>>> build on
>>>> your system?
>>
>> Luck. I am getting a warning but the code is able to somewhat compile:
>>    src/port/chklocale.c(230): warning C4013: 'GetLocaleInfoEx'
>> undefined; assuming extern returning int
>> [C:\Users\IEUser\git\postgres\libpgport.vcxproj]
>
> Weird. This assumed declaration is __cdecl, the actual function is 
> __stdcall, and I think this should be guaranteed to crash.
>
>> But that's clearly incorrect to get that. As you are saying, what we
>> actually just need to do is bumping _WIN32_WINNT to 0x0600 when
>> compiling with VS2015, meaning that the minimum build requirement for
>> Postgres with VS2015 would be Windows Vista, and it would not be
>> possible to compile it on XP or Windows server 2k3. As XP is already
>> out of support, I think that this is an acceptable tradeoff, and it
>> would still be possible to build Postgres on XP with older versions of
>> Visual. Thoughts?
>
> I think you confuse two things here, let's call them "build 
> environment" and "build platform". The build environment is whichever 
> Windows SDK (among other things) is installed; if it is a version for 
> Vista or later, that just means it has the declaration in the first 
> place, and has the import in kernel32.lib. The build platform is the 
> OS the compiler is run on; as long as you find a compiler that 
> understands the headers from your chosen SDK version, you can run it 
> on Windows 95 if both of you want.
>
> Changing _WIN32_WINNT also affects, indirectly, on which platforms the 
> resulting binaries can run. Assume a macro that has an alternative 
> definition, conditioned on _WIN32_WINNT >= _WIN32_WINNT_VISTA, that 
> uses a function added in Vista. A binary built using this declaration, 
> no matter where and when, will not run on anything older.
>
>> Anyway, attached are updated patches. This makes the warning go away
>> from my side, so I guess that it should allow Andrew to compile the
>> code.
>
> Which brings us to the next problem:
>
>   src/port/chklocale.c(233): warning C4133: 'function': incompatible
>   types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>
> I have no idea why I get this warning; I would have expected something 
> more like this:
>
>   localetest.cpp(26): error C2664: 'int
>   GetLocaleInfoEx(LPCWSTR,LCTYPE,LPWSTR,int)': cannot convert
>   argument 1 from 'const char *' to 'LPCWSTR'
>
> Apparently the warning is triggered by type mismatches in pointer 
> arithmetic, although I can't see any here. Anyway, it concerns the 
> first argument in this call to GetLocaleInfoEx(), which here is a 
> const char*.
>
> According to the documentation and the prototype, however, it should 
> be an LPCWSTR, because this function is Unicode-only (has no A/W 
> variants). Unless LOCALE_IDEFAULTANSICODEPAGE also changes the 
> interpretation of this first argument to a single-byte string, which 
> is not mentioned anywhere in the documentation and makes no sense to 
> begin with, I don't think this has ever worked either.
>
> I just tested it, and, of course, if I pass '(LPCWSTR)"de-DE"' (narrow 
> string cast to LPCWSTR), the call fails with ERROR_INVALID_PARAMETER. 
> With a wide string, I get the correct code page for the locale.
>
>
> Also, while you're at it, could you improve the comments a bit? I have 
> not yet tried following the code to see which locale formats it uses 
> where ("German_Germany", "de-DE", etc.), but GetLocaleInfoEx() takes 
> the short form and there is a comment about the long form right below 
> that call once patched (in the old code that gets turned into an else 
> branch).
>



OK, well, we're making progress. I can confirm that using _WIN32_WINNT = 
0x0600 fixes my problems - I can build and run the regression tests. I'm 
inclined to define _WINSOCK_DEPRECATED_NO_WARNINGS to silence a few 
compiler bleatings.

Do you have a fix for the LPCWSTR parameter issue?

cheers

andrew



pgsql-hackers by date:

Previous
From: David Steele
Date:
Subject: Re: Batch update of indexes
Next
From: Christian Ullrich
Date:
Subject: Re: Lower msvc build verbosity level