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: