Thread: VS 2015 support in src/tools/msvc

VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

03 March 2016, 17:03:00

Hi all,

Microsoft provides a set of VMs that one can use for testing and
Windows 10 is in the set:
https://dev.windows.com/en-us/microsoft-edge/tools/vms/windows/
I have grabbed one and installed the community version of Visual
Studio 2015 so I think that I am going to be able to compile Postgres
with VS2015 with a bit of black magic.

My goal is double:
1) As far as things have been discussed, VS2015 is making difficult
the compilation of Postgres, particularly for locales. So I'd like to
see what are the problems behind that and see if we can patch it
properly. This would facilitate the integration of cmake as well for
Windows.
2) src/tools/msvc stuff has support only up to VS2013. I think that it
would be nice to bump that a bit and get something for 9.5~.

So, would there be interest for a patch on the perl scripts in
src/tools/msvc or are they considered a lost cause? Having a look at
the failures could be done with the cmake work, but it seems a bit
premature to me to look at that for the moment, and having support for
VS2015 with 9.5 (support for new versions of VS won a backpatch the
last couple of years) would be a good thing I think.

Thoughts?
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

03 March 2016, 17:25:48

On 03/03/16 15:02, Michael Paquier wrote:
> Hi all,
>
> Microsoft provides a set of VMs that one can use for testing and
> Windows 10 is in the set:
> https://dev.windows.com/en-us/microsoft-edge/tools/vms/windows/
> I have grabbed one and installed the community version of Visual
> Studio 2015 so I think that I am going to be able to compile Postgres
> with VS2015 with a bit of black magic.
>
> My goal is double:
> 1) As far as things have been discussed, VS2015 is making difficult
> the compilation of Postgres, particularly for locales. So I'd like to
> see what are the problems behind that and see if we can patch it
> properly. This would facilitate the integration of cmake as well for
> Windows.

The locale problem is that:
a) the MS crt headers are broken for this particular part due to 
unfinished refactoring, even their msvcrt sources don't compile with 
them, if you read them it's obvious they forgot to put one variable in 
the struct
b) we are using somewhat legacy API there that's internally implemented 
as hacks over the new API (string parsing and generation and stuff is 
happening there)
c) the non-legacy API works only on Vista+ and does not support the old 
locale names (which is why the legacy api that is written on top of this 
has to do the string parsing and generation)

To me the least bad solution for MSVC 2015 and codepage detection seemed 
to use the new API when possible (GetLocaleInfoEx and friends) and fall 
back to our old string parsing that we did pre-VC2013 when it's not, 
since afaics it works correctly on all the locale names that are not 
supported by the new API. It means that MSVC 2015 builds would be Vista+ 
but I honestly don't see that as big issue given Microsoft's own policy 
about old Windows versions.

> 2) src/tools/msvc stuff has support only up to VS2013. I think that it
> would be nice to bump that a bit and get something for 9.5~.
>

Yeah, we'll also have to do something about perl 5.22 compatibility 
there as psed is not included in the core distribution anymore from that 
version and we use it to generate one header file IIRC.

> So, would there be interest for a patch on the perl scripts in
> src/tools/msvc or are they considered a lost cause? Having a look at
> the failures could be done with the cmake work, but it seems a bit
> premature to me to look at that for the moment, and having support for
> VS2015 with 9.5 (support for new versions of VS won a backpatch the
> last couple of years) would be a good thing I think.
>

+1, and I can help (at least review and testing if nothing else).

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

03 March 2016, 18:52:09

Petr Jelinek <petr@2ndquadrant.com> writes:
> On 03/03/16 15:02, Michael Paquier wrote:
>> So, would there be interest for a patch on the perl scripts in
>> src/tools/msvc or are they considered a lost cause? Having a look at
>> the failures could be done with the cmake work, but it seems a bit
>> premature to me to look at that for the moment, and having support for
>> VS2015 with 9.5 (support for new versions of VS won a backpatch the
>> last couple of years) would be a good thing I think.

> +1, and I can help (at least review and testing if nothing else).

Yeah.  At this point, the earliest the cmake work could land is 9.7,
and TBH I do not think we are committed to landing it at all.  So it'd
behoove us to fix at least HEAD, and probably older branches too,
to work with up-to-date MSVC.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Alvaro Herrera

Date:

03 March 2016, 19:00:25

Tom Lane wrote:

> Yeah.  At this point, the earliest the cmake work could land is 9.7,
> and TBH I do not think we are committed to landing it at all.  So it'd
> behoove us to fix at least HEAD, and probably older branches too,
> to work with up-to-date MSVC.

In that case we should probably add the configure check for IPC::Run
that was proposed in another thread.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

03 March 2016, 19:19:10

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> Yeah.  At this point, the earliest the cmake work could land is 9.7,
>> and TBH I do not think we are committed to landing it at all.  So it'd
>> behoove us to fix at least HEAD, and probably older branches too,
>> to work with up-to-date MSVC.

> In that case we should probably add the configure check for IPC::Run
> that was proposed in another thread.

Yeah, agreed.  I opposed it at the time because I thought we might be
considering cmake conversion for 9.6.  But now that that's missed the
final 9.6 commitfest, we'd better operate on the assumption that it's
not landing anytime soon.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

03 March 2016, 20:18:18


On 03/03/2016 09:02 AM, Michael Paquier wrote:
> Hi all,
>
> Microsoft provides a set of VMs that one can use for testing and
> Windows 10 is in the set:
> https://dev.windows.com/en-us/microsoft-edge/tools/vms/windows/
> I have grabbed one and installed the community version of Visual
> Studio 2015 so I think that I am going to be able to compile Postgres
> with VS2015 with a bit of black magic.
>
> My goal is double:
> 1) As far as things have been discussed, VS2015 is making difficult
> the compilation of Postgres, particularly for locales. So I'd like to
> see what are the problems behind that and see if we can patch it
> properly. This would facilitate the integration of cmake as well for
> Windows.
> 2) src/tools/msvc stuff has support only up to VS2013. I think that it
> would be nice to bump that a bit and get something for 9.5~.
>
> So, would there be interest for a patch on the perl scripts in
> src/tools/msvc or are they considered a lost cause? Having a look at
> the failures could be done with the cmake work, but it seems a bit
> premature to me to look at that for the moment, and having support for
> VS2015 with 9.5 (support for new versions of VS won a backpatch the
> last couple of years) would be a good thing I think.



I am not holding my breath on cmake. Until we have something pretty 
solid on that front I'm going to assume it's not happening. If we're 
going to support VS2015 (and I think we should) then it should be 
supported for all live branches if possible. I'm assuming the changes 
would be pretty localized, at least in src/tools/msvc, and adding a new 
compile shouldn't break anything with existing compilers.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Magnus Hagander

Date:

03 March 2016, 20:41:54

On Thu, Mar 3, 2016 at 6:18 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 03/03/2016 09:02 AM, Michael Paquier wrote:
Hi all,

Microsoft provides a set of VMs that one can use for testing and
Windows 10 is in the set:
https://dev.windows.com/en-us/microsoft-edge/tools/vms/windows/
I have grabbed one and installed the community version of Visual
Studio 2015 so I think that I am going to be able to compile Postgres
with VS2015 with a bit of black magic.

My goal is double:
1) As far as things have been discussed, VS2015 is making difficult
the compilation of Postgres, particularly for locales. So I'd like to
see what are the problems behind that and see if we can patch it
properly. This would facilitate the integration of cmake as well for
Windows.
2) src/tools/msvc stuff has support only up to VS2013. I think that it
would be nice to bump that a bit and get something for 9.5~.

So, would there be interest for a patch on the perl scripts in
src/tools/msvc or are they considered a lost cause? Having a look at
the failures could be done with the cmake work, but it seems a bit
premature to me to look at that for the moment, and having support for
VS2015 with 9.5 (support for new versions of VS won a backpatch the
last couple of years) would be a good thing I think.

I am not holding my breath on cmake. Until we have something pretty solid on that front I'm going to assume it's not happening. If we're going to support VS2015 (and I think we should) then it should be supported for all live branches if possible. I'm assuming the changes would be pretty localized, at least in src/tools/msvc, and adding a new compile shouldn't break anything with existing compilers.

+1.

Definitely do it for HEAD.

Then if it gets backpatched is going to depend on the locality I think. If it's just the build system then it should be no problem, but I thought Michael also suggested some API changes. If that's so, then it is going to depend on how invasive those are. But that part should be done for HEAD regardless, so it's definitely worth the effort to figure out exactly what it involves.

Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

04 March 2016, 01:21:35

On 03/03/16 18:41, Magnus Hagander wrote:
> On Thu, Mar 3, 2016 at 6:18 PM, Andrew Dunstan <andrew@dunslane.net
> <mailto:andrew@dunslane.net>> wrote:
>
>     On 03/03/2016 09:02 AM, Michael Paquier wrote:
>
>         Microsoft provides a set of VMs that one can use for testing and
>         Windows 10 is in the set:
>         https://dev.windows.com/en-us/microsoft-edge/tools/vms/windows/
>         I have grabbed one and installed the community version of Visual
>         Studio 2015 so I think that I am going to be able to compile
>         Postgres
>         with VS2015 with a bit of black magic.
>
>         So, would there be interest for a patch on the perl scripts in
>         src/tools/msvc or are they considered a lost cause? Having a look at
>         the failures could be done with the cmake work, but it seems a bit
>         premature to me to look at that for the moment, and having
>         support for
>         VS2015 with 9.5 (support for new versions of VS won a backpatch the
>         last couple of years) would be a good thing I think.
>
>
>
>
>     I am not holding my breath on cmake. Until we have something pretty
>     solid on that front I'm going to assume it's not happening. If we're
>     going to support VS2015 (and I think we should) then it should be
>     supported for all live branches if possible. I'm assuming the
>     changes would be pretty localized, at least in src/tools/msvc, and
>     adding a new compile shouldn't break anything with existing compilers.
>
>
> +1.
>
> Definitely do it for HEAD.
>
> Then if it gets backpatched is going to depend on the locality I think.
> If it's just the build system then it should be no problem, but I
> thought Michael also suggested some API changes. If that's so, then it
> is going to depend on how invasive those are. But that part should be
> done for HEAD regardless, so it's definitely worth the effort to figure
> out exactly what it involves.
>
>

Well the source code does not compile on MSVC2015, the perl changes 
needed are really tiny, there is some code that needs changes to work 
with 2015, particularly in the locale code-page detection area so it's 
definitely not just build system. But I think it should be fairly 
localized and fenced by ifdef anyway.

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

04 March 2016, 03:36:20

On Fri, Mar 4, 2016 at 7:21 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
> Well the source code does not compile on MSVC2015, the perl changes needed
> are really tiny, there is some code that needs changes to work with 2015,
> particularly in the locale code-page detection area so it's definitely not
> just build system. But I think it should be fairly localized and fenced by
> ifdef anyway.

Yeah, that's my first impression as well. We should not need any APIs
changes and the changes would be limited to if extra blocks with
_MSC_VER, if that would occur then I definitely agree that patching
only HEAD is the way to go. I'll look at that today and the next
couple of days, let's see what I can get out of it...
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

04 March 2016, 09:54:49

On Fri, Mar 4, 2016 at 9:36 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Yeah, that's my first impression as well. We should not need any APIs
> changes and the changes would be limited to if extra blocks with
> _MSC_VER, if that would occur then I definitely agree that patching
> only HEAD is the way to go. I'll look at that today and the next
> couple of days, let's see what I can get out of it...

So, I have finally been able to set up my environment correctly, and I
am at a stage where I can compile Postgres with VS 2015 thanks to the
patch attached that extends src/tools/msvc to do so. Unsurprisingly,
the first failures detected are related to locales :)

I still need to dig into that in more details. For the time being the
patch attached is useful IMO to plug in VS 2015 with the existing
infrastructure. So if anybody has a Windows environment, feel free to
play with it and dig into those problems. I'll update this thread once
I have a more advanced status.
Regards,
--
Michael

Attachment

vs-2015-support.patch

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

04 March 2016, 17:24:07

On Fri, Mar 4, 2016 at 3:54 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> I still need to dig into that in more details. For the time being the
> patch attached is useful IMO to plug in VS 2015 with the existing
> infrastructure. So if anybody has a Windows environment, feel free to
> play with it and dig into those problems. I'll update this thread once
> I have a more advanced status.

OK, attached are a set of patches that allowed me to compile Postgres
using VS2015, in more details:
- 0001, as mentioned by Petr upthread, psed is removed from the core
distribution of Perl in 5.22, so when installing ActivePerl it is not
possible to create probes.h, and the code compilation would fail. I
bumped into that so here is a patch. What I am proposing here is to
replace psed by sed, sed being available in MSYS like bison and flex,
so when building using MSVC the environment to set up is normally
already good to go even with this additional dependency. Now, it is
important to mention that probes.h is not part of a source tarball. I
think that we would want probes.h to be part of a source tarball so as
it would be possible to compile the code on Windows using MSVC without
having to install MSYS. I haven't done that in this patch, thoughts on
the matter are welcome.
- 0002, which adds support for VS2015 in src/tools/scripts
- 0003, to address a compilation failure that I bumped into when
compiling ecpg. In src/port, TIMEZONE_GLOBAL and TZNAME_GLOBAL refer
to respectively timezone and tzname, however for win32, those should
be _timezone and _tzname. See here:
https://msdn.microsoft.com/en-us/library/htb3tdkc.aspx
- 0004, which is to address the problem of the missing lc_codepage
from locale.h in src/port/. I have been pondering about the use of
more fancy routines like GetLocaleInfoEx as mentioned by Petr
upthread. However, I think that we had better avoid any kind of
complication and just fall back to the old code path should _MSC_VER
>= 1900. We could always reuse lc_codepage if it gets reintroduced in
a future version of VS.

This set of patches is clearly a work-in-progress, but I am at the
point where feedback is welcome, and the code can compile. The issue
wit psed is something I think could be backpatched a bit more than the
VS2015 core patches, support for perl 5.22 happening on all the
supported branches.
Note that I did not bump into the stdbool issues. Note as well that VS
has complained about a couple of warnings. I am attaching them in the
file named VS2015_warnings.txt. Those are quite interesting as well.
--
Michael

On 4 March 2016 at 22:23, Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, Mar 4, 2016 at 3:54 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> I still need to dig into that in more details. For the time being the
> patch attached is useful IMO to plug in VS 2015 with the existing
> infrastructure. So if anybody has a Windows environment, feel free to
> play with it and dig into those problems. I'll update this thread once
> I have a more advanced status.

OK, attached are a set of patches that allowed me to compile Postgres
using VS2015, in more details:
- 0001, as mentioned by Petr upthread, psed is removed from the core
distribution of Perl in 5.22, so when installing ActivePerl it is not
possible to create probes.h, and the code compilation would fail. I
bumped into that so here is a patch. What I am proposing here is to
replace psed by sed, sed being available in MSYS like bison and flex,
so when building using MSVC the environment to set up is normally
already good to go even with this additional dependency.

The assumption here is that we're using msys to provide bison and flex (probably via msysgit), so adding sed isn't any more intrusive.

I think that's reasonable, but wanted to spell it out since right now msys isn't actually a dependency of the MSVC builds, just a convenient way to get some of the dependencies. This still adds a new dependency, but it's one most people will have anyway. If they're using bison/flex from gnuwin32 or whatever instead they can get sed there too. So +1, sensible.

Now, it is
important to mention that probes.h is not part of a source tarball. I
think that we would want probes.h to be part of a source tarball so as
it would be possible to compile the code on Windows using MSVC without
having to install MSYS. I haven't done that in this patch, thoughts on
the matter are welcome.

That's consistent with how we include the generated scanner and lexer files etc in the source tarball, so +1.

Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

05 March 2016, 07:42:06

On 04/03/16 15:23, Michael Paquier wrote:
>
> OK, attached are a set of patches that allowed me to compile Postgres
> using VS2015, in more details:
> - 0001, as mentioned by Petr upthread, psed is removed from the core
> distribution of Perl in 5.22, so when installing ActivePerl it is not
> possible to create probes.h, and the code compilation would fail. I
> bumped into that so here is a patch. What I am proposing here is to
> replace psed by sed, sed being available in MSYS like bison and flex,
> so when building using MSVC the environment to set up is normally
> already good to go even with this additional dependency. Now, it is
> important to mention that probes.h is not part of a source tarball. I
> think that we would want probes.h to be part of a source tarball so as
> it would be possible to compile the code on Windows using MSVC without
> having to install MSYS. I haven't done that in this patch, thoughts on
> the matter are welcome.

I vote for just using sed considering we need flex and bison anyway.

> - 0003, to address a compilation failure that I bumped into when
> compiling ecpg. In src/port, TIMEZONE_GLOBAL and TZNAME_GLOBAL refer
> to respectively timezone and tzname, however for win32, those should
> be _timezone and _tzname. See here:
> https://msdn.microsoft.com/en-us/library/htb3tdkc.aspx

Yep I hit that one as well, looks good.

> - 0004, which is to address the problem of the missing lc_codepage
> from locale.h in src/port/. I have been pondering about the use of
> more fancy routines like GetLocaleInfoEx as mentioned by Petr
> upthread. However, I think that we had better avoid any kind of
> complication and just fall back to the old code path should _MSC_VER
>> = 1900. We could always reuse lc_codepage if it gets reintroduced in
> a future version of VS.

Well I am worried that this way we might break existing installs which 
means we can't backpatch this. The problem here is that the fallback 
code does not support the <language>-<REGION> format which Microsoft 
documents everywhere as recommended locale format. The good news is that 
our own initdb won't auto-generate those when no locale was specified as 
it uses the setlocale() which returns the legacy (and not recommended) 
locale names and our fallback code can handle those. But manually set 
locales can be a problem.

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

05 March 2016, 08:39:08

On Sat, Mar 5, 2016 at 1:41 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
> On 04/03/16 15:23, Michael Paquier wrote:
>> OK, attached are a set of patches that allowed me to compile Postgres
>> using VS2015, in more details:
>> - 0001, as mentioned by Petr upthread, psed is removed from the core
>> distribution of Perl in 5.22, so when installing ActivePerl it is not
>> possible to create probes.h, and the code compilation would fail. I
>> bumped into that so here is a patch. What I am proposing here is to
>> replace psed by sed, sed being available in MSYS like bison and flex,
>> so when building using MSVC the environment to set up is normally
>> already good to go even with this additional dependency. Now, it is
>> important to mention that probes.h is not part of a source tarball. I
>> think that we would want probes.h to be part of a source tarball so as
>> it would be possible to compile the code on Windows using MSVC without
>> having to install MSYS. I haven't done that in this patch, thoughts on
>> the matter are welcome.
>
> I vote for just using sed considering we need flex and bison anyway.

OK cool, we could go with something else than sed to generate probes.h
but that seems sensible considering that this should definitely be
back-patched. Not sure what the others think about adding a new file
in the source tarball by default though.

>> - 0003, to address a compilation failure that I bumped into when
>> compiling ecpg. In src/port, TIMEZONE_GLOBAL and TZNAME_GLOBAL refer
>> to respectively timezone and tzname, however for win32, those should
>> be _timezone and _tzname. See here:
>> https://msdn.microsoft.com/en-us/library/htb3tdkc.aspx
>
> Yep I hit that one as well, looks good.

MinGW would react to that correctly I think. If I look at
mingw/include/timezone.h, both timezone and _timezone are defined. I
would think that this is intentional to declare both there.

>> - 0004, which is to address the problem of the missing lc_codepage
>> from locale.h in src/port/. I have been pondering about the use of
>> more fancy routines like GetLocaleInfoEx as mentioned by Petr
>> upthread. However, I think that we had better avoid any kind of
>> complication and just fall back to the old code path should _MSC_VER
>>>
>>> = 1900. We could always reuse lc_codepage if it gets reintroduced in
>>
>> a future version of VS.
>
> Well I am worried that this way we might break existing installs which means
> we can't backpatch this. The problem here is that the fallback code does not
> support the <language>-<REGION> format which Microsoft documents everywhere
> as recommended locale format. The good news is that our own initdb won't
> auto-generate those when no locale was specified as it uses the setlocale()
> which returns the legacy (and not recommended) locale names and our fallback
> code can handle those. But manually set locales can be a problem.

I am open to more fancy solutions if it is possible to get reliably
the codepage in a different way, but I am not sure this is worth the
complication. The pre-VS2012 code has been able to live with that.
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

05 March 2016, 08:47:15

Michael Paquier <michael.paquier@gmail.com> writes:
> On Sat, Mar 5, 2016 at 1:41 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> I vote for just using sed considering we need flex and bison anyway.

> OK cool, we could go with something else than sed to generate probes.h
> but that seems sensible considering that this should definitely be
> back-patched. Not sure what the others think about adding a new file
> in the source tarball by default though.

AFAIK, sed flex and bison originate from three separate source projects;
there is no reason to suppose that the presence of flex and bison on a
particular system guarantee the presence of sed.  I thought the proposal
to get rid of the psed dependence in favor of some more perl code was
pretty sane.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

05 March 2016, 17:34:43


On 03/05/2016 12:46 AM, Tom Lane wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> On Sat, Mar 5, 2016 at 1:41 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>>> I vote for just using sed considering we need flex and bison anyway.
>> OK cool, we could go with something else than sed to generate probes.h
>> but that seems sensible considering that this should definitely be
>> back-patched. Not sure what the others think about adding a new file
>> in the source tarball by default though.
> AFAIK, sed flex and bison originate from three separate source projects;
> there is no reason to suppose that the presence of flex and bison on a
> particular system guarantee the presence of sed.  I thought the proposal
> to get rid of the psed dependence in favor of some more perl code was
> pretty sane.



Here is a translation into perl of the sed script, courtesy of the s2p 
incarnation of psed: 
<https://gist.github.com/adunstan/d61b1261a4b91496bdc6> The sed script 
appears to have been stable for a long time, so I don't think we need to 
be too concerned about possibly maintaining two versions.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

05 March 2016, 21:31:14

On Sat, Mar 5, 2016 at 11:34 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
> Here is a translation into perl of the sed script, courtesy of the s2p
> incarnation of psed: <https://gist.github.com/adunstan/d61b1261a4b91496bdc6>
> The sed script appears to have been stable for a long time, so I don't think
> we need to be too concerned about possibly maintaining two versions.

That's 95% of the work already done, nice! If I finish wrapping up a
patch for this issue at least would you backpatch? It would be saner
to get rid of this dependency everywhere I think regarding compilation
with perl 5.22.
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

05 March 2016, 23:55:13


On 03/05/2016 01:31 PM, Michael Paquier wrote:
> On Sat, Mar 5, 2016 at 11:34 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> Here is a translation into perl of the sed script, courtesy of the s2p
>> incarnation of psed: <https://gist.github.com/adunstan/d61b1261a4b91496bdc6>
>> The sed script appears to have been stable for a long time, so I don't think
>> we need to be too concerned about possibly maintaining two versions.
> That's 95% of the work already done, nice! If I finish wrapping up a
> patch for this issue at least would you backpatch? It would be saner
> to get rid of this dependency everywhere I think regarding compilation
> with perl 5.22.

Sure.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

07 March 2016, 16:40:27

On Sun, Mar 6, 2016 at 5:55 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
> On 03/05/2016 01:31 PM, Michael Paquier wrote:
>> On Sat, Mar 5, 2016 at 11:34 PM, Andrew Dunstan <andrew@dunslane.net>
>> wrote:
>>>
>>> Here is a translation into perl of the sed script, courtesy of the s2p
>>> incarnation of psed:
>>> <https://gist.github.com/adunstan/d61b1261a4b91496bdc6>
>>> The sed script appears to have been stable for a long time, so I don't
>>> think
>>> we need to be too concerned about possibly maintaining two versions.
>>
>> That's 95% of the work already done, nice! If I finish wrapping up a
>> patch for this issue at least would you backpatch? It would be saner
>> to get rid of this dependency everywhere I think regarding compilation
>> with perl 5.22.
>
> Sure.

OK, so after some re-lecture of the script and perltidy-ing I finish
with the attached. How does that look?
--
Michael

Attachment

psed-remove-v1.patch

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

08 March 2016, 16:32:33

On Mon, Mar 7, 2016 at 10:40 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Sun, Mar 6, 2016 at 5:55 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> On 03/05/2016 01:31 PM, Michael Paquier wrote:
>>> On Sat, Mar 5, 2016 at 11:34 PM, Andrew Dunstan <andrew@dunslane.net>
>>> wrote:
>>>>
>>>> Here is a translation into perl of the sed script, courtesy of the s2p
>>>> incarnation of psed:
>>>> <https://gist.github.com/adunstan/d61b1261a4b91496bdc6>
>>>> The sed script appears to have been stable for a long time, so I don't
>>>> think
>>>> we need to be too concerned about possibly maintaining two versions.
>>>
>>> That's 95% of the work already done, nice! If I finish wrapping up a
>>> patch for this issue at least would you backpatch? It would be saner
>>> to get rid of this dependency everywhere I think regarding compilation
>>> with perl 5.22.
>>
>> Sure.
>
> OK, so after some re-lecture of the script and perltidy-ing I finish
> with the attached. How does that look?

Attached is a new set for the support of MS 2015 + the psed issue,
please use those ones for testing:
- 0001 is the replacement of psed by a dedicated perl script to
generate probes.h
- 0002 Fix of the declaration of TIMEZONE_GLOBAL and TZNAME_GLOBAL for WIN32
- 0003 addresses the issue with lc_codepage missing from _locale_t.
- 0004 adds support for MS2015 in src/tools/scripts/
Regards,
--
Michael

On Wed, Mar 9, 2016 at 1:07 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
> Michael's patch proposes to replace the use of sed to generate probes.h with
> the perl equivalent everywhere. That has the advantage that we keep to one
> script to generate probes.h, but it does impose a perl dependency.

Good point. It did not occur to me that this would bring a hard
dependency for non-Windows builds. Let's keep both scripts then. The
attached is changed to do so.
--
Michael

On 09/03/16 05:31, Noah Misch wrote:
> On Tue, Mar 08, 2016 at 10:32:28PM +0900, Michael Paquier wrote:
>> Subject: [PATCH 3/4] Fix use of locales for VS 2015
>>
>> lc_codepage is a flag missing from locale.h, causing this code path
>> introduced in VS 2012 to fail. Perhaps there is a reason for this field
>> to have been clobbered, but let's fall back to the pre-VS-2012 code
>> parsing directly LC_TYPE to get the codepage wanted.
>> ---
>>   src/port/chklocale.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/port/chklocale.c b/src/port/chklocale.c
>> index a551fdc..a7d88fb 100644
>> --- a/src/port/chklocale.c
>> +++ b/src/port/chklocale.c
>> @@ -203,7 +203,16 @@ win32_langinfo(const char *ctype)
>>   {
>>       char       *r = NULL;
>>
>> -#if (_MSC_VER >= 1700)
>> +    /*
>> +     * lc_codepage is correctly declared in Visual Studio 2012 and 2013.
>> +     * However in VS 2015 this flag is missing from locale.h, visibly this
>> +     * is an error of refactoring from Microsoft that is at the origin of
>> +     * this missing field, causing a compilation failure in this code path.
>> +     * Hence, it is more reliable to fall back to other code path grabbing
>
> No, the other path can't handle a "locale name" like "initdb --locale=th-TH".
> That makes the other code path inadequate for VS2012 and later.  See the
> IsoLocaleName() header comment.
>

Agreed, that's what I was saying as well.

Something like attached is simplest way this would work correctly (note
that I didn't really test it and it's missing comments). Note that we
are falling back to the old parsing in case the GetLocaleInfoEx didn't
work, that's important because GetLocaleInfoEx does not support the
<Language>_<Country>.<CodePage> format but supports all the rest of
them. The problem with this is the binaries would need to be compiled
with target of vista/windows server 2008+. That would be of course only
the case with builds made with VS2015, so I am not sure if we care all
that much (especially given the fact that older windows are not
supported by MS anyway).

I don't currently know of any way of doing this in VS2015 that would
work with older versions of windows with the exception of having our own
definition of the locale_t struct so that the VS2013 code would still
work...

--
   Petr Jelinek                  http://www.2ndQuadrant.com/
   PostgreSQL Development, 24x7 Support, Training & Services

Attachment

chklocale.diff

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

20 March 2016, 02:06:48


On 03/08/2016 07:40 PM, Michael Paquier wrote:
> On Wed, Mar 9, 2016 at 1:07 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> Michael's patch proposes to replace the use of sed to generate probes.h with
>> the perl equivalent everywhere. That has the advantage that we keep to one
>> script to generate probes.h, but it does impose a perl dependency.
> Good point. It did not occur to me that this would bring a hard
> dependency for non-Windows builds. Let's keep both scripts then. The
> attached is changed to do so.


Actually, it wasn't, but I fixed it and committed it.

Still to do: the non-perl pieces.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

20 March 2016, 07:02:32

On Sun, Mar 20, 2016 at 8:06 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> On 03/08/2016 07:40 PM, Michael Paquier wrote:
>>
>> On Wed, Mar 9, 2016 at 1:07 AM, Andrew Dunstan <andrew@dunslane.net>
>> wrote:
>>>
>>> Michael's patch proposes to replace the use of sed to generate probes.h
>>> with
>>> the perl equivalent everywhere. That has the advantage that we keep to
>>> one
>>> script to generate probes.h, but it does impose a perl dependency.
>>
>> Good point. It did not occur to me that this would bring a hard
>> dependency for non-Windows builds. Let's keep both scripts then. The
>> attached is changed to do so.
>
>
>
> Actually, it wasn't, but I fixed it and committed it.

Ah, indeed, thanks. It looks like I have been bitten by an incorrect
rebase/cherry-pick.

> Still to do: the non-perl pieces.

The patch to address locales is the sensitive part. The patch from
Petr is taking the correct approach though I think that we had better
simplify it a bit and if possible we had better not rely on the else
block it introduces.
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

20 March 2016, 21:43:28


On 03/20/2016 12:02 AM, Michael Paquier wrote:
> On Sun, Mar 20, 2016 at 8:06 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>>
>> Still to do: the non-perl pieces.
> The patch to address locales is the sensitive part. The patch from
> Petr is taking the correct approach though I think that we had better
> simplify it a bit and if possible we had better not rely on the else
> block it introduces.


OK, the please send an updated set of patches for what remains.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

21 March 2016, 08:38:23

On 20/03/16 05:02, Michael Paquier wrote:
> On Sun, Mar 20, 2016 at 8:06 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>> Still to do: the non-perl pieces.
>
> The patch to address locales is the sensitive part. The patch from
> Petr is taking the correct approach though I think that we had better
> simplify it a bit and if possible we had better not rely on the else
> block it introduces.
>

That would be ideal, not particularly sure that both are possible at the 
same time. We can definitely remove the else block but it involves 
enumerating system locales which makes the patch orders of magnitude 
bigger and uglier. Do you have any better approaches in you mind?

As a side note, I have to say the current locale API in Windows is one 
huge mess and if it was on me I'd just redefine the locale_t struct 
correctly as that is simplest solution and the APIs involved are not 
deprecated or anything, it's just that the public headers are botched in 
current version and apparently nobody important is using them to force 
MS to fix them.

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

22 March 2016, 16:40:35

On Wed, Mar 9, 2016 at 11:10 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
> Something like attached is simplest way this would work correctly (note that
> I didn't really test it and it's missing comments). Note that we are falling
> back to the old parsing in case the GetLocaleInfoEx didn't work, that's
> important because GetLocaleInfoEx does not support the
> <Language>_<Country>.<CodePage> format but supports all the rest of them.
> The problem with this is the binaries would need to be compiled with target
> of vista/windows server 2008+. That would be of course only the case with
> builds made with VS2015, so I am not sure if we care all that much
> (especially given the fact that older windows are not supported by MS
> anyway).

Ah, OK. Of course.

> I don't currently know of any way of doing this in VS2015 that would work
> with older versions of windows with the exception of having our own
> definition of the locale_t struct so that the VS2013 code would still
> work...

There is no actual way to be sure if this is an intentional change or
not, if we see it back in the next version of VS, why not... I would
like to think like you that this is actually a merge mistake from
Redmond-side, but I think we had better be careful here, this could
lead to undetected errors in the future.
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

22 March 2016, 16:57:18

On 22/03/16 14:40, Michael Paquier wrote:
> On Wed, Mar 9, 2016 at 11:10 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> Something like attached is simplest way this would work correctly (note that
>> I didn't really test it and it's missing comments). Note that we are falling
>> back to the old parsing in case the GetLocaleInfoEx didn't work, that's
>> important because GetLocaleInfoEx does not support the
>> <Language>_<Country>.<CodePage> format but supports all the rest of them.
>> The problem with this is the binaries would need to be compiled with target
>> of vista/windows server 2008+. That would be of course only the case with
>> builds made with VS2015, so I am not sure if we care all that much
>> (especially given the fact that older windows are not supported by MS
>> anyway).
>
> Ah, OK. Of course.
>
>> I don't currently know of any way of doing this in VS2015 that would work
>> with older versions of windows with the exception of having our own
>> definition of the locale_t struct so that the VS2013 code would still
>> work...
>
> There is no actual way to be sure if this is an intentional change or
> not, if we see it back in the next version of VS, why not... I would
> like to think like you that this is actually a merge mistake from
> Redmond-side, but I think we had better be careful here, this could
> lead to undetected errors in the future.
>

Sure, I am looking at it from the perspective that they didn't even 
deprecate the function and changes to the struct it returns would break 
binary compatibility for existing apps.

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

23 March 2016, 10:17:21

On Mon, Mar 21, 2016 at 3:43 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> On 03/20/2016 12:02 AM, Michael Paquier wrote:
>>
>> On Sun, Mar 20, 2016 at 8:06 AM, Andrew Dunstan <andrew@dunslane.net>
>> wrote:
>>>
>>>
>>> Still to do: the non-perl pieces.
>>
>> The patch to address locales is the sensitive part. The patch from
>> Petr is taking the correct approach though I think that we had better
>> simplify it a bit and if possible we had better not rely on the else
>> block it introduces.
>
>
>
> OK, the please send an updated set of patches for what remains.

Here you go:
- 0001 fixes the global declarations of TIMEZONE_GLOBAL and
TZNAME_GLOBAL to be WIN32-compliant. I got bitten by that in the ECPG
compilation.
- 0002, support of VS2015 in the scripts of src/tools/msvc
- 0003, this is necessary in order to run the regression tests,
details are here:
http://www.postgresql.org/message-id/CAB7nPqTDiXxS8CdL8mOiVh6qFQ-1qV9mKN0AyjzYBvzv6WC0dA@mail.gmail.com
- 0004 is the fix for locales. This is actually Petr's work upthread,
I have updated comments in the code though and did a bit more
polishing. This still looks like the cleanest solution we have. Other
solutions are mentioned upthread: redeclaration of struct _locale_t in
backend code is one.
--
Michael


On 04/06/2016 04:50 PM, Andrew Dunstan wrote:
>
>
> On 03/29/2016 09:38 PM, Robert Haas wrote:
>> On Tue, Mar 29, 2016 at 9:29 PM, Andrew Dunstan <andrew@dunslane.net>
>> wrote:
>>> I am currently travelling, but my intention is to deal with the
>>> remaining
>>> patches when I'm back home this weekend, unless someone beats me to it.
>> Cool.
>>
>
> Progress report:
>
>
> I have spent way too much time on this and don't have it working yet.
> I'm setting up a sacrificial VM from scratch in a last ditch attempt
> to get it working.
>
> Things to note so far:
>
>  * VS2015 appears to create version 12 solution files, not version 14,
>    and the tools complained about version 14.
>  * Windows git (the successor to msysGit) apparently no longer ships
>    with bison and flex. So if you need those (i.e. to built from git,
>    not tarball) you're probably going to need to install the MsysDTK
>    even if you're not using its compiler.
>
>



Not out of the woods yet. Attached is what I got from VS2015 on a fresh
W10 VM, with Michael's patch 0002 and 0004 applied.


cheers

andrew

Attachment

vs2015.error_log

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

08 April 2016, 04:14:26

On Fri, Apr 8, 2016 at 9:29 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
> Not out of the woods yet. Attached is what I got from VS2015 on a fresh W10
> VM, with Michael's patch 0002 and 0004 applied.

Interesting, I have no idea what we are doing differently, and seeing
those errors it seems to me that Petr and I are actually getting it
wrong for some reason, because GetLocaleInfoEx should need a direct
declaration of windows.h. And similarly to some of the other files in
src/port I think that we should have it. Do you still get failures if
you add the following thing at the top of chklocale.c?

#if defined(WIN32) && (_MSC_VER >= 1900)
#include "windows.h"
#endif

Looking at the docs
(https://msdn.microsoft.com/en-us/library/windows/desktop/dd318103%28v=vs.85%29.aspx),
winnls.h would be enough, but I'd rather be consistent with the
approach taken by the other files.
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

08 April 2016, 14:16:10

* Michael Paquier wrote:
> On Fri, Apr 8, 2016 at 9:29 AM, Andrew Dunstan <andrew@dunslane.net> > wrote:

>> Not out of the woods yet. Attached is what I got from VS2015 on a fresh W10
>> VM, with Michael's patch 0002 and 0004 applied.
>
> Interesting, I have no idea what we are doing differently, and seeing
> those errors it seems to me that Petr and I are actually getting it
> wrong for some reason, because GetLocaleInfoEx should need a direct
> declaration of windows.h. And similarly to some of the other files in
> src/port I think that we should have it. Do you still get failures if
> you add the following thing at the top of chklocale.c?
>
> #if defined(WIN32) && (_MSC_VER >= 1900)
> #include "windows.h"
> #endif
>
> Looking at the docs
> (https://msdn.microsoft.com/en-us/library/windows/desktop/dd318103%28v=vs.85%29.aspx),
> winnls.h would be enough, but I'd rather be consistent with the
> approach taken by the other files.

GetLocaleInfoEx() is covered by #if (WINVER >= 0x0600), we define 
_WIN32_WINNT as 0x0501 (src/include/port/win32.h) and WINVER inherits 
that value.

Michael, none of your patches change this, so how does it ever build on 
your system?

MSDN also says this about the function: "Header: winnls.h (include 
windows.h)"; so there's probably something else needed to make it work 
that is not in <winnls.h> alone.

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

08 April 2016, 16:05:50


On 04/08/2016 07:15 AM, Christian Ullrich wrote:
> * Michael Paquier wrote:
>
> > On Fri, Apr 8, 2016 at 9:29 AM, Andrew Dunstan <andrew@dunslane.net> 
> > wrote:
>
>>> Not out of the woods yet. Attached is what I got from VS2015 on a 
>>> fresh W10
>>> VM, with Michael's patch 0002 and 0004 applied.
>>
>> Interesting, I have no idea what we are doing differently, and seeing
>> those errors it seems to me that Petr and I are actually getting it
>> wrong for some reason, because GetLocaleInfoEx should need a direct
>> declaration of windows.h. And similarly to some of the other files in
>> src/port I think that we should have it. Do you still get failures if
>> you add the following thing at the top of chklocale.c?
>>
>> #if defined(WIN32) && (_MSC_VER >= 1900)
>> #include "windows.h"
>> #endif
>>
>> Looking at the docs
>> (https://msdn.microsoft.com/en-us/library/windows/desktop/dd318103%28v=vs.85%29.aspx), 
>>
>> winnls.h would be enough, but I'd rather be consistent with the
>> approach taken by the other files.
>
> GetLocaleInfoEx() is covered by #if (WINVER >= 0x0600), we define 
> _WIN32_WINNT as 0x0501 (src/include/port/win32.h) and WINVER inherits 
> that value.
>
> Michael, none of your patches change this, so how does it ever build 
> on your system?
>
> MSDN also says this about the function: "Header: winnls.h (include 
> windows.h)"; so there's probably something else needed to make it work 
> that is not in <winnls.h> alone.


OK, at this stage it appears to me that if today is the deadline for 
getting this in for 9.6 then it's going to be missed.

And no, Michael's suggested inclusion didn't help, probably for the 
reasons Christian suggests.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

08 April 2016, 16:52:59

Andrew Dunstan <andrew@dunslane.net> writes:
> OK, at this stage it appears to me that if today is the deadline for 
> getting this in for 9.6 then it's going to be missed.

It would be unfortunate to go another year without support for that
toolchain.  My suggestion is that you continue to work away on the
problem, and when you have something that passes your testing, you
present it to -hackers and we can decide then whether it's too invasive
for a post-feature-freeze patch.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

08 April 2016, 17:06:06


On 04/08/2016 09:52 AM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> OK, at this stage it appears to me that if today is the deadline for
>> getting this in for 9.6 then it's going to be missed.
> It would be unfortunate to go another year without support for that
> toolchain.  My suggestion is that you continue to work away on the
> problem, and when you have something that passes your testing, you
> present it to -hackers and we can decide then whether it's too invasive
> for a post-feature-freeze patch.
>
>             


OK. Michael just told me in IM that he thinks he has a fix, so we'll see.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

08 April 2016, 17:13:09

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:
>> GetLocaleInfoEx() is covered by #if (WINVER >= 0x0600), we define
>> _WIN32_WINNT as 0x0501 (src/include/port/win32.h) and WINVER inherits that
>> value.

Yes, I had a look at winnls.h and that's true.

>> 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]
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?

> OK, at this stage it appears to me that if today is the deadline for getting
> this in for 9.6 then it's going to be missed.

Really? I thought that new VS support were done on HEAD and the last
stable version, aka 9.5 here. This was what was done previously for
VS2012 and VS2013.

> And no, Michael's suggested inclusion didn't help, probably for the reasons
> Christian suggests.

Yes.

List of _WIN32_WINNT is here:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa383745%28v=vs.85%29.aspx
And here is the compatibility grid of VS2015 with existing Windows OSes:
https://www.visualstudio.com/en-us/products/visual-studio-2015-compatibility-vs.aspx

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.
--
Michael

Attachment

Re: VS 2015 support in src/tools/msvc

From

Magnus Hagander

Date:

08 April 2016, 17:17:03

On Fri, Apr 8, 2016 at 4:12 PM, Michael Paquier <michael.paquier@gmail.com> 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:
>> GetLocaleInfoEx() is covered by #if (WINVER >= 0x0600), we define
>> _WIN32_WINNT as 0x0501 (src/include/port/win32.h) and WINVER inherits that
>> value.

Yes, I had a look at winnls.h and that's true.

>> 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]
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 that's easily acceptable for the build system, as long as we document it clearly.

I think it would probalby be a bad idea to use those binaries in our binary installers though, but that's a different discussion really. We have other compiler flags that we don't recommend for that :)

//Magnus

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

08 April 2016, 18:02:51

* 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
'constchar *' 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).

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

08 April 2016, 19:10:17


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

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

08 April 2016, 19:47:58

* Andrew Dunstan wrote:

> On 04/08/2016 11:02 AM, Christian Ullrich wrote:

>>   src/port/chklocale.c(233): warning C4133: 'function': incompatible
>>   types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]

> Do you have a fix for the LPCWSTR parameter issue?

As long as the locale short name cannot contain characters outside of 
ASCII, and I don't see how it could, just the typical 
measure-allocate-convert dance, add error handling to taste:

int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
WCHAR *wctype = malloc(res * sizeof(WCHAR));
memset(wctype, 0, res * sizeof(WCHAR));
res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);

If it is somehow guaranteed that ctype is only the most basic short name 
("xx-YY") with no code pages or anything, it becomes much simpler, of 
course, and I would just use a loop.

If the locale name can contain characters above 0x7f, we'd have to know 
the code page of the string we use to get the code page.

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

09 April 2016, 01:41:27

On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich <chris@chrullrich.net> wrote:
> * Andrew Dunstan wrote:
>
>> On 04/08/2016 11:02 AM, Christian Ullrich wrote:
>
>
>>>   src/port/chklocale.c(233): warning C4133: 'function': incompatible
>>>   types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>
>
>> Do you have a fix for the LPCWSTR parameter issue?
>
>
> As long as the locale short name cannot contain characters outside of ASCII,
> and I don't see how it could, just the typical measure-allocate-convert
> dance, add error handling to taste:
>
> int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
> WCHAR *wctype = malloc(res * sizeof(WCHAR));
> memset(wctype, 0, res * sizeof(WCHAR));
> res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);
>
> If it is somehow guaranteed that ctype is only the most basic short name
> ("xx-YY") with no code pages or anything, it becomes much simpler, of
> course, and I would just use a loop.
>
> If the locale name can contain characters above 0x7f, we'd have to know the
> code page of the string we use to get the code page.

Could somebody give a try instead of me? I could take a look on it,
but just in 12 hours or so, aka after the deadline if that matters for
this patch.
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

09 April 2016, 03:46:48

On 09/04/16 00:41, Michael Paquier wrote:
> On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich <chris@chrullrich.net> wrote:
>> * Andrew Dunstan wrote:
>>
>>> On 04/08/2016 11:02 AM, Christian Ullrich wrote:
>>
>>
>>>>    src/port/chklocale.c(233): warning C4133: 'function': incompatible
>>>>    types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>>
>>
>>> Do you have a fix for the LPCWSTR parameter issue?
>>
>>
>> As long as the locale short name cannot contain characters outside of ASCII,
>> and I don't see how it could, just the typical measure-allocate-convert
>> dance, add error handling to taste:
>>
>> int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
>> WCHAR *wctype = malloc(res * sizeof(WCHAR));
>> memset(wctype, 0, res * sizeof(WCHAR));
>> res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);
>>
>> If it is somehow guaranteed that ctype is only the most basic short name
>> ("xx-YY") with no code pages or anything, it becomes much simpler, of
>> course, and I would just use a loop.
>>
>> If the locale name can contain characters above 0x7f, we'd have to know the
>> code page of the string we use to get the code page.
>
> Could somebody give a try instead of me? I could take a look on it,
> but just in 12 hours or so, aka after the deadline if that matters for
> this patch.
>

I won't be able to get back to it (well mainly to windows environment) 
till Thursday due to travel, sorry.

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

09 April 2016, 15:37:36

On Sat, Apr 9, 2016 at 7:41 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich <chris@chrullrich.net> wrote:
>> * Andrew Dunstan wrote:
>>> On 04/08/2016 11:02 AM, Christian Ullrich wrote:
>>>>   src/port/chklocale.c(233): warning C4133: 'function': incompatible
>>>>   types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>>
>>> Do you have a fix for the LPCWSTR parameter issue?
>>
>> As long as the locale short name cannot contain characters outside of ASCII,
>> and I don't see how it could, just the typical measure-allocate-convert
>> dance, add error handling to taste:
>>
>> int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
>> WCHAR *wctype = malloc(res * sizeof(WCHAR));
>> memset(wctype, 0, res * sizeof(WCHAR));
>> res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);

I don't think that's good to use malloc in those code paths, and I
think that we cannot use palloc as well for a buffer passed directly
into this function, so it looks that we had better use a fix-sized
buffer and allocate the output into that. What would be a a correct
estimation of the maximum size we should allow? 80 (similar to
pg_locale.c)?
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

09 April 2016, 15:43:33

> Michael Paquier wrote:
>
> On Sat, Apr 9, 2016 at 7:41 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich <chris@chrullrich.net> wrote:
>>> * Andrew Dunstan wrote:
>>>>> On 04/08/2016 11:02 AM, Christian Ullrich wrote:
>>>>>  src/port/chklocale.c(233): warning C4133: 'function': incompatible
>>>>>  types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>>>
>>>> Do you have a fix for the LPCWSTR parameter issue?
>>>
>>> As long as the locale short name cannot contain characters outside of ASCII,
>>> and I don't see how it could, just the typical measure-allocate-convert
>>> dance, add error handling to taste:
>>>
>>> int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
>>> WCHAR *wctype = malloc(res * sizeof(WCHAR));
>>> memset(wctype, 0, res * sizeof(WCHAR));
>>> res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);
>
> I don't think that's good to use malloc in those code paths, and I
> think that we cannot use palloc as well for a buffer passed directly
> into this function, so it looks that we had better use a fix-sized
> buffer and allocate the output into that. What would be a a correct
> estimation of the maximum size we should allow? 80 (similar to
> pg_locale.c)?

I think it should not take more than 16 characters, but if you want to make sure the code can deal with long names as
well,MSDN gives an upper limit of 85 for those somewhere.  

--
Christian

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

10 April 2016, 17:58:45


On 04/09/2016 08:43 AM, Christian Ullrich wrote:
>> Michael Paquier wrote:
>>
>> On Sat, Apr 9, 2016 at 7:41 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich <chris@chrullrich.net> wrote:
>>>> * Andrew Dunstan wrote:
>>>>>> On 04/08/2016 11:02 AM, Christian Ullrich wrote:
>>>>>>   src/port/chklocale.c(233): warning C4133: 'function': incompatible
>>>>>>   types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>>>>> Do you have a fix for the LPCWSTR parameter issue?
>>>> As long as the locale short name cannot contain characters outside of ASCII,
>>>> and I don't see how it could, just the typical measure-allocate-convert
>>>> dance, add error handling to taste:
>>>>
>>>> int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
>>>> WCHAR *wctype = malloc(res * sizeof(WCHAR));
>>>> memset(wctype, 0, res * sizeof(WCHAR));
>>>> res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);
>> I don't think that's good to use malloc in those code paths, and I
>> think that we cannot use palloc as well for a buffer passed directly
>> into this function, so it looks that we had better use a fix-sized
>> buffer and allocate the output into that. What would be a a correct
>> estimation of the maximum size we should allow? 80 (similar to
>> pg_locale.c)?
> I think it should not take more than 16 characters, but if you want to make sure the code can deal with long names as
well,MSDN gives an upper limit of 85 for those somewhere.
 
>


I don't think we need to be too precious about saving a byte or two 
here. Can one or other of you please prepare a replacement patch based 
in this discussion?

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

10 April 2016, 21:48:59

* Andrew Dunstan:

> On 04/09/2016 08:43 AM, Christian Ullrich wrote:

>>> Michael Paquier wrote:

>>> I don't think that's good to use malloc in those code paths, and I

Oh?

+#if (_MSC_VER >= 1900)
+    uint32    cp;
+
+    if (GetLocaleInfoEx(ctype,
+            LOCALE_IDEFAULTANSICODEPAGE | LOCALE_RETURN_NUMBER,
+            (LPWSTR) &cp, sizeof(cp) / sizeof(WCHAR)) > 0)
+    {
+        r = malloc(16);            /* excess */
+        if (r != NULL)
+            sprintf(r, "CP%u", cp);
+    }
+    else
+    {
+#endif

> I don't think we need to be too precious about saving a byte or two
> here. Can one or other of you please prepare a replacement patch based
> in this discussion?

Sorry, I don't think I'm up to that (at least not for another week or 
so). I have to read up on the issue first; right now I'm not sure what 
exactly the problem is.

What I can say is that the existing patch needs more work, because 
GetLocaleInfoEx() expects a locale name ("de-DE") as its first argument, 
but the patched win32_langinfo() is giving it a locale identifier 
("German_Germany.1252"). At least it does for me.

I have not gone through the code sufficiently closely to find out where 
the argument to win32_langinfo() originates, but I will when I can.

As for missing code page information in the _locale_t type, ISTM it 
isn't hidden any better in UCRT than it was before:

int main()
{    /* Set locale with nonstandard code page */    _locale_t loc = _create_locale(LC_ALL, "German_Germany.850");
    __crt_locale_data_public* pub = 
(__crt_locale_data_public*)(loc->locinfo);    printf("CP: %d\n", pub->_locale_lc_codepage);    // "CP: 850"    return
0;
}

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

11 April 2016, 00:03:37

On 10/04/16 20:47, Christian Ullrich wrote:
> * Andrew Dunstan:
>
>> On 04/09/2016 08:43 AM, Christian Ullrich wrote:
>
>>>> Michael Paquier wrote:
>
>>>> I don't think that's good to use malloc in those code paths, and I
>
> Oh?
>
> +#if (_MSC_VER >= 1900)
> +    uint32    cp;
> +
> +    if (GetLocaleInfoEx(ctype,
> +            LOCALE_IDEFAULTANSICODEPAGE | LOCALE_RETURN_NUMBER,
> +            (LPWSTR) &cp, sizeof(cp) / sizeof(WCHAR)) > 0)
> +    {
> +        r = malloc(16);            /* excess */
> +        if (r != NULL)
> +            sprintf(r, "CP%u", cp);
> +    }
> +    else
> +    {
> +#endif
>

But r is return value so it has to be allocated. The intermediate 
variables are function local.

>> don't think we need to be too precious about saving a byte or two
>> here. Can one or other of you please prepare a replacement patch based
>> in this discussion?
>
> Sorry, I don't think I'm up to that (at least not for another week or
> so). I have to read up on the issue first; right now I'm not sure what
> exactly the problem is.
>
> What I can say is that the existing patch needs more work, because
> GetLocaleInfoEx() expects a locale name ("de-DE") as its first argument,
> but the patched win32_langinfo() is giving it a locale identifier
> ("German_Germany.1252"). At least it does for me.

That really depends on what you set in config/commandline. The current 
code supports both (that's why there is the else fallback to old code 
which handles the "German_Germany.1252" format).

> As for missing code page information in the _locale_t type, ISTM it
> isn't hidden any better in UCRT than it was before:
>
> int main()
> {
>      /* Set locale with nonstandard code page */
>      _locale_t loc = _create_locale(LC_ALL, "German_Germany.850");
>
>      __crt_locale_data_public* pub =
> (__crt_locale_data_public*)(loc->locinfo);
>      printf("CP: %d\n", pub->_locale_lc_codepage);    // "CP: 850"
>      return 0;
> }
>

This is basically same as the approach of fixing _locale_t that was also 
considered upthread but nobody was too happy with it. I guess the worry 
is that given that the header file is obviously unfinished in the area 
where this is defined and also the fact that this looks like something 
that's not meant to be used outside of that header makes people worry 
that it might change between point releases of the SDK.

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

11 April 2016, 09:25:26

On Mon, Apr 11, 2016 at 6:03 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
> On 10/04/16 20:47, Christian Ullrich wrote:
>>> don't think we need to be too precious about saving a byte or two
>>> here. Can one or other of you please prepare a replacement patch based
>>> in this discussion?

So, I am finally back on the battlefield.

>> Sorry, I don't think I'm up to that (at least not for another week or
>> so). I have to read up on the issue first; right now I'm not sure what
>> exactly the problem is.
>>
>> What I can say is that the existing patch needs more work, because
>> GetLocaleInfoEx() expects a locale name ("de-DE") as its first argument,
>> but the patched win32_langinfo() is giving it a locale identifier
>> ("German_Germany.1252"). At least it does for me.
>
>
> That really depends on what you set in config/commandline. The current code
> supports both (that's why there is the else fallback to old code which
> handles the "German_Germany.1252" format).

Yes, that's the whole point of falling back to the old code path
should the call to GetLocaleInfoEx() fail.

>> As for missing code page information in the _locale_t type, ISTM it
>> isn't hidden any better in UCRT than it was before:
>>
>> int main()
>> {
>>      /* Set locale with nonstandard code page */
>>      _locale_t loc = _create_locale(LC_ALL, "German_Germany.850");
>>
>>      __crt_locale_data_public* pub =
>> (__crt_locale_data_public*)(loc->locinfo);
>>      printf("CP: %d\n", pub->_locale_lc_codepage);    // "CP: 850"
>>      return 0;
>> }
>>
>
> This is basically same as the approach of fixing _locale_t that was also
> considered upthread but nobody was too happy with it.

I am one of them to be honest... Now if I look at that with one step
backward at this problem this requires just a ugly hack of a couple of
lines, and this does not require to bump __WIN32_WINNT... Neither does
it need an extra code hunk to handle the short locale name parsing
with GetLocaleInfoEx. We could even just handle _MSC_VER as an
exception, so as it is easy to check with future versions of VS if we
still have lc_codepage going missing:
+#if (_MSC_VER == 1900)
+               __crt_locale_data_public *pub =
+                       (__crt_locale_data_public *) loct->locinfo;
+               lc_codepage = pub->_locale_lc_codepage;
+#else
+               lc_codepage = loct->locinfo->lc_codepage;
+#endif

Another thing that I am wondering is that this would allow us to parse
even locale names that are not short, type ja-JP and not with the
COUNTRY_LANG.CODEPAGE format, though based on what I read on the docs
those do not exist.. But the world of Windows is full of surprises.

> I guess the worry is
> that given that the header file is obviously unfinished in the area where
> this is defined and also the fact that this looks like something that's not
> meant to be used outside of that header makes people worry that it might
> change between point releases of the SDK.

Yep.

Now, I have produced two patches:
- 0001-Support-for-VS-2015-locale-hack.patch, which makes use of
__crt_locale_data_public in ucrt/corecrt.h. This is definitely an ugly
hack, though I am coming to think that this may be the approach that
would us the less harm, and that's closer to what is done for VS 2012
and 2013.
- 0001-Support-for-VS-2015-getlocaleinfoex.patch, which make use of
GetLocaleInfoEx, this requires us to lower a bit the support grid for
Windows, basically that's throwing support for XP if compilation is
done with VS 2015.
Based on my tests, both are working with short and long local names,
testing via initdb --locale.
--
Michael

Attachment

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

11 April 2016, 10:47:31

On Mon, Apr 11, 2016 at 3:25 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Now, I have produced two patches:
> - 0001-Support-for-VS-2015-locale-hack.patch, which makes use of
> __crt_locale_data_public in ucrt/corecrt.h. This is definitely an ugly
> hack, though I am coming to think that this may be the approach that
> would us the less harm, and that's closer to what is done for VS 2012
> and 2013.
> - 0001-Support-for-VS-2015-getlocaleinfoex.patch, which make use of
> GetLocaleInfoEx, this requires us to lower a bit the support grid for
> Windows, basically that's throwing support for XP if compilation is
> done with VS 2015.
> Based on my tests, both are working with short and long local names,
> testing via initdb --locale.

The first patch is actually not what I wanted to send. Here are the
correct ones...
--
Michael

On 04/11/2016 03:47 AM, Michael Paquier wrote:
> On Mon, Apr 11, 2016 at 3:25 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> Now, I have produced two patches:
>> - 0001-Support-for-VS-2015-locale-hack.patch, which makes use of
>> __crt_locale_data_public in ucrt/corecrt.h. This is definitely an ugly
>> hack, though I am coming to think that this may be the approach that
>> would us the less harm, and that's closer to what is done for VS 2012
>> and 2013.
>> - 0001-Support-for-VS-2015-getlocaleinfoex.patch, which make use of
>> GetLocaleInfoEx, this requires us to lower a bit the support grid for
>> Windows, basically that's throwing support for XP if compilation is
>> done with VS 2015.
>> Based on my tests, both are working with short and long local names,
>> testing via initdb --locale.
> The first patch is actually not what I wanted to send. Here are the
> correct ones...

I think I prefer the less hacky solution.

Progress report:

1. My VS 2015 installations (I now have several) all generate solution
file with:

    Microsoft Visual Studio Solution File, Format Version 12.00

so I propose to set that as the solution file version.

2. The logic in win32.h is a bit convoluted. I'm going to simplify it.

3. I'm going to define _WINSOCK_DEPRECATED_NO_WARNINGS to silence some
compiler bleatings about the way we use the Winsock API

4. The compiler complains about one of Microsoft's own header files -
essentially it dislikes the=is construct:

    typedef enum { ... };

It would be nice to make it shut up about it.

5. It also complains about us casting a pid_t to a HANDLE in
pg_basebackup.c. Not sure what to do about that.

5. most importantly, on my Release/X64 build, I am getting a regression
failure on contrib/seg. regression diffs attached. Until that's fixed
this isn't going anywhere.

cheers

andrew

Attachment

seg-regression.diffs

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

21 April 2016, 22:56:10

* From: Andrew Dunstan [mailto:andrew@dunslane.net]

> 4. The compiler complains about one of Microsoft's own header files -
> essentially it dislikes the=is construct:
> 
>     typedef enum { ... };
> 
> It would be nice to make it shut up about it.

I doubt that's possible; the declaration *is* wrong after all. We could turn off the warning:

#pragma warning(push)
#pragma warning(disable : 1234, or whatever the number is)
#include <whatever.h>
#pragma warning(pop)

> 5. It also complains about us casting a pid_t to a HANDLE in
> pg_basebackup.c. Not sure what to do about that.

The thing that's being cast is not a PID, but a HANDLE to a process. pid_t is a typedef for int (in port/win32.h),
thereforeis always 32 bits, while HANDLE is actually void*. However, Microsoft guarantees that kernel32 HANDLEs (this
includesthose to threads and processes) fit into 32 bits on AMD64.
 

Source: https://msdn.microsoft.com/en-us/library/windows/desktop/ee872017(v=vs.85).aspx, third bullet point.

So we can simply silence the warning by casting explicitly.

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

22 April 2016, 00:15:36

Andrew Dunstan <andrew@dunslane.net> writes:
> 5. most importantly, on my Release/X64 build, I am getting a regression 
> failure on contrib/seg. regression diffs attached. Until that's fixed 
> this isn't going anywhere.

I'll bet a nickel that this means MSVC has broken the ABI rules that
allowed old-style (version 0) C functions to limp along without changes.
seg_same() and the other comparison functions that are misbehaving are
declared at the C level to return "bool".  I think what's happening is
they are now setting only one byte in the return register, or perhaps
only the low-order word, leaving the high-order bits as trash.  But
fmgr_oldstyle is coded as though V0 functions return "char *", and it's
going to be putting that whole pointer value into the result Datum, and
if the value isn't entirely zero then DatumGetBool will consider it
"true".  So this theory predicts a lot of intended "false" results will
read as "true", which is what your regression diffs show.

IIRC, we had intentionally left contrib/seg using mostly V0 calling
convention as a canary to let us know when that broke.  It's a bit
astonishing that it lasted this long, maybe.  But it looks like we're
going to have to convert at least the bool-returning functions to V1
if we want it to work on VS 2015.

Do the other contrib modules all pass?  I can't recall if seg was the
only one we'd left like this.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

22 April 2016, 06:51:39


On 04/21/2016 05:15 PM, Tom Lane wrote:
>
> IIRC, we had intentionally left contrib/seg using mostly V0 calling
> convention as a canary to let us know when that broke.  It's a bit
> astonishing that it lasted this long, maybe.  But it looks like we're
> going to have to convert at least the bool-returning functions to V1
> if we want it to work on VS 2015.
>
> Do the other contrib modules all pass?  I can't recall if seg was the
> only one we'd left like this.
>
>             


Only seg fails.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

22 April 2016, 07:31:11

Andrew Dunstan <andrew@dunslane.net> writes:
> On 04/21/2016 05:15 PM, Tom Lane wrote:
>> Do the other contrib modules all pass?  I can't recall if seg was the
>> only one we'd left like this.

> Only seg fails.

As a crosscheck, I put some code into fmgr_c_validator() to log a message
when creating a V0 function with a pass-by-val return type.  (Pass-by-ref
is no problem, according to my hypothesis, since it necessarily means
the C function returns a pointer.)  I get these hits in core + contrib
regression tests:

core:
LOG:  version-0 function widget_in returns type widget
LOG:  version-0 function oldstyle_length returns type integer

contrib:
LOG:  version-0 function seg_over_left returns type boolean
LOG:  version-0 function seg_over_right returns type boolean
LOG:  version-0 function seg_left returns type boolean
LOG:  version-0 function seg_right returns type boolean
LOG:  version-0 function seg_lt returns type boolean
LOG:  version-0 function seg_le returns type boolean
LOG:  version-0 function seg_gt returns type boolean
LOG:  version-0 function seg_ge returns type boolean
LOG:  version-0 function seg_contains returns type boolean
LOG:  version-0 function seg_contained returns type boolean
LOG:  version-0 function seg_overlap returns type boolean
LOG:  version-0 function seg_same returns type boolean
LOG:  version-0 function seg_different returns type boolean
LOG:  version-0 function seg_cmp returns type integer
LOG:  version-0 function gseg_consistent returns type boolean
LOG:  version-0 function gseg_compress returns type internal
LOG:  version-0 function gseg_decompress returns type internal
LOG:  version-0 function gseg_penalty returns type internal
LOG:  version-0 function gseg_picksplit returns type internal
LOG:  version-0 function gseg_same returns type internal

The widget_in gripe is a false positive, caused by the fact that we don't
know the properties of type widget when widget_in is declared.  We also
need not worry about the functions that return "internal", since that's
defined to be pointer-sized.

If we assume that oldstyle functions returning integer are still okay,
which the success of the regression test case involving oldstyle_length()
seems to prove, then indeed seg's bool-returning functions are the only
hazard.

Note though that this test fails to cover any contrib modules that
lack regression tests, since they wouldn't have gotten loaded by
"make installcheck".
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

22 April 2016, 08:21:31

On Fri, Apr 22, 2016 at 4:56 AM, Christian Ullrich <chris@chrullrich.net> wrote:
> * From: Andrew Dunstan [mailto:andrew@dunslane.net]
>
>> 4. The compiler complains about one of Microsoft's own header files -
>> essentially it dislikes the=is construct:
>>
>>     typedef enum { ... };
>>
>> It would be nice to make it shut up about it.
>
> I doubt that's possible; the declaration *is* wrong after all. We could turn off the warning:
>
> #pragma warning(push)
> #pragma warning(disable : 1234, or whatever the number is)
> #include <whatever.h>
> #pragma warning(pop)

Well, yes.. Even if that's not pretty that would not be the first one
caused by a VS header bug (float.c)..

>> 5. It also complains about us casting a pid_t to a HANDLE in
>> pg_basebackup.c. Not sure what to do about that.
>
> The thing that's being cast is not a PID, but a HANDLE to a process. pid_t is a typedef for int (in port/win32.h),
thereforeis always 32 bits, while HANDLE is actually void*. However, Microsoft guarantees that kernel32 HANDLEs (this
includesthose to threads and processes) fit into 32 bits on AMD64. 
>
> Source: https://msdn.microsoft.com/en-us/library/windows/desktop/ee872017(v=vs.85).aspx, third bullet point.
>
> So we can simply silence the warning by casting explicitly.

Yes, when casting things this way I think that a comment would be fine
in the code. We could do that as separate patches actually.
--
Michael

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

22 April 2016, 09:47:01

On Fri, Apr 22, 2016 at 4:39 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
> On 04/11/2016 03:47 AM, Michael Paquier wrote:
>> On Mon, Apr 11, 2016 at 3:25 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>>
>>> Now, I have produced two patches:
>>> - 0001-Support-for-VS-2015-locale-hack.patch, which makes use of
>>> __crt_locale_data_public in ucrt/corecrt.h. This is definitely an ugly
>>> hack, though I am coming to think that this may be the approach that
>>> would us the less harm, and that's closer to what is done for VS 2012
>>> and 2013.
>>> - 0001-Support-for-VS-2015-getlocaleinfoex.patch, which make use of
>>> GetLocaleInfoEx, this requires us to lower a bit the support grid for
>>> Windows, basically that's throwing support for XP if compilation is
>>> done with VS 2015.
>>> Based on my tests, both are working with short and long local names,
>>> testing via initdb --locale.
>>
>> The first patch is actually not what I wanted to send. Here are the
>> correct ones...
>
> I think I prefer the less hacky solution.

OK, at least we go in one direction.

> Progress report:
>
> 1. My VS 2015 installations (I now have several) all generate solution file
> with:
>    Microsoft Visual Studio Solution File, Format Version 12.00
> so I propose to set that as the solution file version.

I am wondering why it happens this way for you..

> 2. The logic in win32.h is a bit convoluted. I'm going to simplify it.

OK. Should I wait for a patch to look at then?

> 3. I'm going to define _WINSOCK_DEPRECATED_NO_WARNINGS to silence some
> compiler bleatings about the way we use the Winsock API

Agreed here. I saw those warnings as well.

> 6. most importantly, on my Release/X64 build, I am getting a regression
> failure on contrib/seg. regression diffs attached. Until that's fixed this
> isn't going anywhere.

Hm. I am now looking at that, and I can reproduce the failure... I
will send a patch soon.
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

22 April 2016, 09:54:42

On Fri, Apr 22, 2016 at 1:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> On 04/21/2016 05:15 PM, Tom Lane wrote:
>>> Do the other contrib modules all pass?  I can't recall if seg was the
>>> only one we'd left like this.
>
>> Only seg fails.
>
> As a crosscheck, I put some code into fmgr_c_validator() to log a message
> when creating a V0 function with a pass-by-val return type.  (Pass-by-ref
> is no problem, according to my hypothesis, since it necessarily means
> the C function returns a pointer.)  I get these hits in core + contrib
> regression tests:
>
> [...]
>
> If we assume that oldstyle functions returning integer are still okay,
> which the success of the regression test case involving oldstyle_length()
> seems to prove, then indeed seg's bool-returning functions are the only
> hazard.
>
> Note though that this test fails to cover any contrib modules that
> lack regression tests, since they wouldn't have gotten loaded by
> "make installcheck".

Your assumption is right. With the patch attached for contrib/seg/
that converts all those functions to use the V1 declaration, I am able
to make the tests pass. As the internal shape of the functions is not
changed and that there are no functional changes, I guess that it
would be fine to backpatch down to where VS2015 is intended to be
supported. Is anybody here foreseeing any problems for back-branches
if there is such a change?
--
Michael

Attachment

vs2015-seg-fix.patch

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

22 April 2016, 16:49:44

Michael Paquier <michael.paquier@gmail.com> writes:
> On Fri, Apr 22, 2016 at 1:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> If we assume that oldstyle functions returning integer are still okay,
>> which the success of the regression test case involving oldstyle_length()
>> seems to prove, then indeed seg's bool-returning functions are the only
>> hazard.

> Your assumption is right. With the patch attached for contrib/seg/
> that converts all those functions to use the V1 declaration, I am able
> to make the tests pass. As the internal shape of the functions is not
> changed and that there are no functional changes, I guess that it
> would be fine to backpatch down to where VS2015 is intended to be
> supported. Is anybody here foreseeing any problems for back-branches
> if there is such a change?

It should be fine, since converting a function to V1 makes no difference
at the SQL level --- we don't need an extension script modification.

How far back are we thinking of supporting VS2015, anyway?  I can check
and push this as a separate patch.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

22 April 2016, 16:58:12

On Fri, Apr 22, 2016 at 10:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> How far back are we thinking of supporting VS2015, anyway?  I can check
> and push this as a separate patch.

My guess is 9.5: HEAD + last stable branch. That's what has been done
when support for VS2012 or VS2013 has been added.
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

22 April 2016, 17:07:53


On 04/22/2016 02:46 AM, Michael Paquier wrote:
>
>> Progress report:
>>
>> 1. My VS 2015 installations (I now have several) all generate solution file
>> with:
>>     Microsoft Visual Studio Solution File, Format Version 12.00
>> so I propose to set that as the solution file version.
> I am wondering why it happens this way for you..


It's not just me. See the reply at 

<https://social.msdn.microsoft.com/Forums/sqlserver/en-US/ea671596-9e8d-4e80-bb1f-8c9bfb1738dc/2012-and-2015-solutions?forum=vcgeneral>

and notice that in both cases the Solution file format is version 12.00.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

22 April 2016, 18:56:56

Michael Paquier <michael.paquier@gmail.com> writes:
> On Fri, Apr 22, 2016 at 10:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> How far back are we thinking of supporting VS2015, anyway?  I can check
>> and push this as a separate patch.

> My guess is 9.5: HEAD + last stable branch. That's what has been done
> when support for VS2012 or VS2013 has been added.

OK.  Pushed with some adjustments --- mainly, I undid the conversion
of the non-bool-returning functions, since those aren't broken (yet)
and I think it's still valuable to test V0 as much as we can here.

Also, I static-ified some functions that weren't SQL-visible, and
thereby discovered that some of those were actually dead code ...
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

23 April 2016, 08:38:46

* Andrew Dunstan wrote:

> On 04/22/2016 02:46 AM, Michael Paquier wrote:

>>> Progress report:
>>>
>>> 1. My VS 2015 installations (I now have several) all generate
>>> solution file
>>> with:
>>>     Microsoft Visual Studio Solution File, Format Version 12.00
>>> so I propose to set that as the solution file version.>>
>> I am wondering why it happens this way for you..
>
> It's not just me. See the reply at
>
<https://social.msdn.microsoft.com/Forums/sqlserver/en-US/ea671596-9e8d-4e80-bb1f-8c9bfb1738dc/2012-and-2015-solutions?forum=vcgeneral>
> and notice that in both cases the Solution file format is version 12.00.

Try as I may, I cannot get my VS 2015 to write a version 14 .sln file. 
It's always "12.00". If I change it to 14 by hand, it still opens and 
appears to work fine.

I tried to find a real-world version 14 solution to see if I can spot a 
difference between it and the version 12 files, but there appears to be 
very little out there, and what there is, looks like it was either 
autogenerated or edited manually. Examples:

- 
<https://social.msdn.microsoft.com/Forums/sqlserver/en-US/2d08da92-f16a-4887-935a-e3159104ff2c/> 
- "converted to VS2015 in anticipation of the release"

- 

<https://connect.microsoft.com/VisualStudio/feedback/details/1909707/getting-empty-folder-in-project-root-after-opening-sln-file-dtar-08e86330-4835-4b5c-9e5a-61f37ae1a077-dtar>

- "but I changed the the contents of the .SLN to Microsoft Visual Studio 
Solution File, Format Version 14.00"

- 
<https://fossies.org/linux/ACE/examples/Logger/Acceptor-server/Acceptor_server_vc14.sln> 
- "# This file was generated by MPC."

Google returns not quite two pages of results for "Microsoft Visual 
Studio Solution File, Format Version 14.00"; it *has* to be nonstandard.

My best guess at this point is that the solution format really did not 
change between 2013 and 2015. Microsoft may just have added support for 
.sln version 14 for compatibility with people who generate solutions by 
other means and who are under the impression the .sln version has to be 
in step with the VS version, when in fact the VisualStudioVersion line 
in a version 12 .sln file controls which version of VS will open any 
given file by default, and the MinimumVisualStudioVersion line indicates 
which VS version supports the project types in the solution.

If my guess is wrong, and anyone knows a way to make VS 2015 write a 
version 14 .sln file, please tell me.

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

23 April 2016, 09:08:39

* Christian Ullrich wrote:

> * Andrew Dunstan wrote:
>
>> On 04/22/2016 02:46 AM, Michael Paquier wrote:
>
>>>> Progress report:
>>>>
>>>> 1. My VS 2015 installations (I now have several) all generate
>>>> solution file
>>>> with:
>>>>     Microsoft Visual Studio Solution File, Format Version 12.00
>>>> so I propose to set that as the solution file version.
>>>
>>> I am wondering why it happens this way for you..
>>
>> It's not just me. See the reply at
>>
<https://social.msdn.microsoft.com/Forums/sqlserver/en-US/ea671596-9e8d-4e80-bb1f-8c9bfb1738dc/2012-and-2015-solutions?forum=vcgeneral>
>>
>> and notice that in both cases the Solution file format is version 12.00.
>
> Try as I may, I cannot get my VS 2015 to write a version 14 .sln file.
> It's always "12.00". If I change it to 14 by hand, it still opens and
> appears to work fine.
>
> I tried to find a real-world version 14 solution to see if I can spot a
> difference between it and the version 12 files, but there appears to be
> very little out there, and what there is, looks like it was either
> autogenerated or edited manually. Examples:

OK, so searching on GitHub yielded a few more results, but I could not 
identify a single one that was clearly not created or edited by anything 
other than Visual Studio itself.

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

23 April 2016, 23:11:14


On 04/22/2016 01:21 AM, Michael Paquier wrote:
>>> 5. It also complains about us casting a pid_t to a HANDLE in
>>> pg_basebackup.c. Not sure what to do about that.
>> The thing that's being cast is not a PID, but a HANDLE to a process. pid_t is a typedef for int (in port/win32.h),
thereforeis always 32 bits, while HANDLE is actually void*. However, Microsoft guarantees that kernel32 HANDLEs (this
includesthose to threads and processes) fit into 32 bits on AMD64.
 
>>
>> Source: https://msdn.microsoft.com/en-us/library/windows/desktop/ee872017(v=vs.85).aspx, third bullet point.
>>
>> So we can simply silence the warning by casting explicitly.
> Yes, when casting things this way I think that a comment would be fine
> in the code. We could do that as separate patches actually.


We are already casting the pid_t to HANDLE and still getting a warning. 
Apparently we need to do something on win64 like
    (HANDLE) ((int64) bgchild)


cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

24 April 2016, 00:31:08

* Andrew Dunstan wrote:

> On 04/22/2016 01:21 AM, Michael Paquier wrote:

>>>> 5. It also complains about us casting a pid_t to a HANDLE in
>>>> pg_basebackup.c. Not sure what to do about that.
>>> The thing that's being cast is not a PID, but a HANDLE to a process.
>>> pid_t is a typedef for int (in port/win32.h), therefore is always 32
>>> bits, while HANDLE is actually void*. However, Microsoft guarantees
>>> that kernel32 HANDLEs (this includes those to threads and processes)
>>> fit into 32 bits on AMD64.

>> Yes, when casting things this way I think that a comment would be fine
>> in the code. We could do that as separate patches actually.
>
> We are already casting the pid_t to HANDLE and still getting a warning.
> Apparently we need to do something on win64 like
>
>     (HANDLE) ((int64) bgchild)

Ah, OK, it warns about a cast to a larger type because the value might 
get sign extended. Not unreasonable.

In this case, I would prefer this:

diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index ba8cf9d..b4086f1 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -256,7 +256,7 @@ typedef int gid_t; typedef long key_t;
 #ifdef WIN32_ONLY_COMPILER
-typedef int pid_t;
+typedef intptr_t pid_t; #endif
 /*

With this change, pg_basebackup -X stream works the same when built for 
64 and 32 bits.

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

24 April 2016, 00:46:47


On 04/23/2016 05:30 PM, Christian Ullrich wrote:
> * Andrew Dunstan wrote:
>
>> On 04/22/2016 01:21 AM, Michael Paquier wrote:
>
>>>>> 5. It also complains about us casting a pid_t to a HANDLE in
>>>>> pg_basebackup.c. Not sure what to do about that.
>>>> The thing that's being cast is not a PID, but a HANDLE to a process.
>>>> pid_t is a typedef for int (in port/win32.h), therefore is always 32
>>>> bits, while HANDLE is actually void*. However, Microsoft guarantees
>>>> that kernel32 HANDLEs (this includes those to threads and processes)
>>>> fit into 32 bits on AMD64.
>
>>> Yes, when casting things this way I think that a comment would be fine
>>> in the code. We could do that as separate patches actually.
>>
>> We are already casting the pid_t to HANDLE and still getting a warning.
>> Apparently we need to do something on win64 like
>>
>>     (HANDLE) ((int64) bgchild)
>
> Ah, OK, it warns about a cast to a larger type because the value might 
> get sign extended. Not unreasonable.
>
> In this case, I would prefer this:
>
> diff --git a/src/include/port/win32.h b/src/include/port/win32.h
> index ba8cf9d..b4086f1 100644
> --- a/src/include/port/win32.h
> +++ b/src/include/port/win32.h
> @@ -256,7 +256,7 @@ typedef int gid_t;
>  typedef long key_t;
>
>  #ifdef WIN32_ONLY_COMPILER
> -typedef int pid_t;
> +typedef intptr_t pid_t;
>  #endif
>
>  /*
>
> With this change, pg_basebackup -X stream works the same when built 
> for 64 and 32 bits.
>




That's a change that will have a pretty wide effect. Everything up to 
now has been pretty low risk, but this worries me rather more. Maybe 
it's safe, but I'd like to hear others' comments.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

24 April 2016, 00:54:38

Andrew Dunstan <andrew@dunslane.net> writes:
> On 04/23/2016 05:30 PM, Christian Ullrich wrote:
>> In this case, I would prefer this:
>>
>> #ifdef WIN32_ONLY_COMPILER
>> -typedef int pid_t;
>> +typedef intptr_t pid_t;
>> #endif

> That's a change that will have a pretty wide effect. Everything up to 
> now has been pretty low risk, but this worries me rather more. Maybe 
> it's safe, but I'd like to hear others' comments.

Yeah, it makes me a bit nervous too.  We know that most of the code is
agnostic as to the width of pid_t, because it works on Unixen where
pid_t is 64bit.  But I'm less sure about whether the Windows-specific
parts are equally flexible.

On the other hand, wouldn't you expect to get compiler warnings for
any code that does get affected?  The main hazard would be incorrect
printf format flags (cf 994f11257 for a recent example), and I'd
certainly expect any C compiler worth its salt to whine about
inconsistencies there.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

24 April 2016, 01:33:26

I wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> On 04/23/2016 05:30 PM, Christian Ullrich wrote:
>>> In this case, I would prefer this:
>>> 
>>> #ifdef WIN32_ONLY_COMPILER
>>> -typedef int pid_t;
>>> +typedef intptr_t pid_t;
>>> #endif

>> That's a change that will have a pretty wide effect. Everything up to 
>> now has been pretty low risk, but this worries me rather more. Maybe 
>> it's safe, but I'd like to hear others' comments.

> Yeah, it makes me a bit nervous too.

One other thought: even if this is safe for HEAD, I think we could
*not* back-patch it into 9.5, because it would amount to an ABI
break on Windows anywhere that pid_t is used in globally visible
structs or function signatures.  (Maybe there are no such places,
but I doubt it.)  So we'd need to go with the messy-cast solution
for 9.5.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

24 April 2016, 06:25:44


On 04/23/2016 06:33 PM, Tom Lane wrote:
> I wrote:
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>> On 04/23/2016 05:30 PM, Christian Ullrich wrote:
>>>> In this case, I would prefer this:
>>>>
>>>> #ifdef WIN32_ONLY_COMPILER
>>>> -typedef int pid_t;
>>>> +typedef intptr_t pid_t;
>>>> #endif
>>> That's a change that will have a pretty wide effect. Everything up to
>>> now has been pretty low risk, but this worries me rather more. Maybe
>>> it's safe, but I'd like to hear others' comments.
>> Yeah, it makes me a bit nervous too.
> One other thought: even if this is safe for HEAD, I think we could
> *not* back-patch it into 9.5, because it would amount to an ABI
> break on Windows anywhere that pid_t is used in globally visible
> structs or function signatures.  (Maybe there are no such places,
> but I doubt it.)  So we'd need to go with the messy-cast solution
> for 9.5.



It's not that messy. I'm inclined just to make minimal changed to 
pg_basebackup.c and be done with it. I don't think a compiler warning is 
worth doing more for.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

24 April 2016, 11:12:53

On 23/04/16 08:08, Christian Ullrich wrote:
> * Christian Ullrich wrote:
>
>> * Andrew Dunstan wrote:
>>
>>> On 04/22/2016 02:46 AM, Michael Paquier wrote:
>>
>>>>> Progress report:
>>>>>
>>>>> 1. My VS 2015 installations (I now have several) all generate
>>>>> solution file
>>>>> with:
>>>>>     Microsoft Visual Studio Solution File, Format Version 12.00
>>>>> so I propose to set that as the solution file version.
>>>>
>>>> I am wondering why it happens this way for you..
>>>
>>> It's not just me. See the reply at
>>>
<https://social.msdn.microsoft.com/Forums/sqlserver/en-US/ea671596-9e8d-4e80-bb1f-8c9bfb1738dc/2012-and-2015-solutions?forum=vcgeneral>
>>>
>>>
>>> and notice that in both cases the Solution file format is version 12.00.
>>
>> Try as I may, I cannot get my VS 2015 to write a version 14 .sln file.
>> It's always "12.00". If I change it to 14 by hand, it still opens and
>> appears to work fine.
>>
>> I tried to find a real-world version 14 solution to see if I can spot a
>> difference between it and the version 12 files, but there appears to be
>> very little out there, and what there is, looks like it was either
>> autogenerated or edited manually. Examples:
>
> OK, so searching on GitHub yielded a few more results, but I could not
> identify a single one that was clearly not created or edited by anything
> other than Visual Studio itself.
>

I did some checking too, and yes it seems having version 12 .sln is fine 
or maybe even desirable. We mainly need to make sure that the tools 
version is 14 and platform toolset is v140 for MSVC15.

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

24 April 2016, 18:31:56


On 04/23/2016 11:25 PM, Andrew Dunstan wrote:
>
>
> On 04/23/2016 06:33 PM, Tom Lane wrote:
>> I wrote:
>>> Andrew Dunstan <andrew@dunslane.net> writes:
>>>> On 04/23/2016 05:30 PM, Christian Ullrich wrote:
>>>>> In this case, I would prefer this:
>>>>>
>>>>> #ifdef WIN32_ONLY_COMPILER
>>>>> -typedef int pid_t;
>>>>> +typedef intptr_t pid_t;
>>>>> #endif
>>>> That's a change that will have a pretty wide effect. Everything up to
>>>> now has been pretty low risk, but this worries me rather more. Maybe
>>>> it's safe, but I'd like to hear others' comments.
>>> Yeah, it makes me a bit nervous too.
>> One other thought: even if this is safe for HEAD, I think we could
>> *not* back-patch it into 9.5, because it would amount to an ABI
>> break on Windows anywhere that pid_t is used in globally visible
>> structs or function signatures.  (Maybe there are no such places,
>> but I doubt it.)  So we'd need to go with the messy-cast solution
>> for 9.5.
>
>
>
> It's not that messy. I'm inclined just to make minimal changed to
> pg_basebackup.c and be done with it. I don't think a compiler warning
> is worth doing more for.
>
>


OK, here's my final version of the patch, which I will apply in 24 hours
or so unless there is an objection.

cheers

andrew

Attachment

VS2015-support.patch

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

24 April 2016, 18:58:53

Andrew Dunstan <andrew@dunslane.net> writes:
> OK, here's my final version of the patch, which I will apply in 24 hours 
> or so unless there is an objection.

> +#pragma warning(push)
> +#pragma warning(disable : 4091)
>  #include <dbghelp.h>
> +#pragma warning(pop)

Hmm ... does this pragma work on *every* compiler we're going to use
on Windows?  I'm afraid that trying to suppress a warning in VS2015
is going to result in outright failure with other compilers.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

24 April 2016, 19:14:49

Andrew Dunstan <andrew@dunslane.net> writes:
> OK, here's my final version of the patch, which I will apply in 24 hours 
> or so unless there is an objection.

BTW, in view of 9f633b404, shouldn't there be a similar addition to
win32env.c in this patch?
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

24 April 2016, 19:16:30

On 04/24/2016 11:58 AM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> OK, here's my final version of the patch, which I will apply in 24 hours
>> or so unless there is an objection.
>> +#pragma warning(push)
>> +#pragma warning(disable : 4091)
>>   #include <dbghelp.h>
>> +#pragma warning(pop)
> Hmm ... does this pragma work on *every* compiler we're going to use
> on Windows?  I'm afraid that trying to suppress a warning in VS2015
> is going to result in outright failure with other compilers.
>
>             

According to my research it works on all the MSVC versions we support. I 
didn't research the others (i.e. gcc), but we already use the pragma in 
float.c without ill effect. Isn't the way #pragma works that compilers 
that don't understand the particular pragma are just supposed to ignore it?

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

24 April 2016, 19:36:44

Andrew Dunstan <andrew@dunslane.net> writes:
> On 04/24/2016 11:58 AM, Tom Lane wrote:
>> Hmm ... does this pragma work on *every* compiler we're going to use
>> on Windows?

> According to my research it works on all the MSVC versions we support. I 
> didn't research the others (i.e. gcc), but we already use the pragma in 
> float.c without ill effect. Isn't the way #pragma works that compilers 
> that don't understand the particular pragma are just supposed to ignore it?

Well, the ones in float.c are guarded by "#if (_MSC_VER >= 1800)" and
will therefore not get compiled by any non-MSVC compiler.  I don't see
any entirely-unprotected #pragma uses in our tree, indicating that
we've not depended on any such assumption up to now.

Given that the whole file is platform-specific, it may well be fine;
I'm just voicing some concern.  I suppose the buildfarm will soon
tell us if it's not fine, though.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

24 April 2016, 19:38:55

On 04/24/2016 12:14 PM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> OK, here's my final version of the patch, which I will apply in 24 hours
>> or so unless there is an objection.
> BTW, in view of 9f633b404, shouldn't there be a similar addition to
> win32env.c in this patch?
>
>             

msvcr120.dll seems to be the highest numbered one on my system, and we 
already cover that. If you like we can add to the comments in that file.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

24 April 2016, 20:16:19

Andrew Dunstan <andrew@dunslane.net> writes:
> On 04/24/2016 12:14 PM, Tom Lane wrote:
>> BTW, in view of 9f633b404, shouldn't there be a similar addition to
>> win32env.c in this patch?

> msvcr120.dll seems to be the highest numbered one on my system, and we 
> already cover that. If you like we can add to the comments in that file.

Yeah, something like s/Visual Studio 2013/Visual Studio 2013 and 2105/
seems like it'd be appropriate.  The main thing I'm concerned about is
that this patch touch that list somehow, so that if someone uses it as
a reference for what to consider when adding support for VS N+1, they
know to check there.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

24 April 2016, 22:17:07

* Andrew Dunstan wrote:

> On 04/24/2016 12:14 PM, Tom Lane wrote:

>> Andrew Dunstan <andrew@dunslane.net> writes:
>>> OK, here's my final version of the patch, which I will apply in 24 hours
>>> or so unless there is an objection.

>> BTW, in view of 9f633b404, shouldn't there be a similar addition to
>> win32env.c in this patch?

> msvcr120.dll seems to be the highest numbered one on my system, and we
> already cover that. If you like we can add to the comments in that file.

There won't be a higher one, with VS 2015, CRT became a system 
component, namely, UCRT. However, ucrtbase.dll does export putenv, so it 
might need to be added.

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

24 April 2016, 22:24:18


On 04/24/2016 03:16 PM, Christian Ullrich wrote:
> * Andrew Dunstan wrote:
>
>> On 04/24/2016 12:14 PM, Tom Lane wrote:
>
>>> Andrew Dunstan <andrew@dunslane.net> writes:
>>>> OK, here's my final version of the patch, which I will apply in 24 
>>>> hours
>>>> or so unless there is an objection.
>
>>> BTW, in view of 9f633b404, shouldn't there be a similar addition to
>>> win32env.c in this patch?
>
>> msvcr120.dll seems to be the highest numbered one on my system, and we
>> already cover that. If you like we can add to the comments in that file.
>
> There won't be a higher one, with VS 2015, CRT became a system 
> component, namely, UCRT. However, ucrtbase.dll does export putenv, so 
> it might need to be added.
>


OK. Does that mean we won't have to add anything more for future VS 
releases?

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

24 April 2016, 22:27:42

* Andrew Dunstan wrote:

> On 04/24/2016 03:16 PM, Christian Ullrich wrote:

>> * Andrew Dunstan wrote:

>>> msvcr120.dll seems to be the highest numbered one on my system, and we
>>> already cover that. If you like we can add to the comments in that file.
>>
>> There won't be a higher one, with VS 2015, CRT became a system
>> component, namely, UCRT. However, ucrtbase.dll does export putenv, so
>> it might need to be added.

> OK. Does that mean we won't have to add anything more for future VS
> releases?

Microsoft is swearing up one side and down the other that they will keep 
binary compatibility forever, and ever, hallelujah, hallelujah, and 
hence the name of the DLL is not expected to change again.

OTOH, the next manager of the development tools division might have 
different ideas. You never know.

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Christian Ullrich

Date:

24 April 2016, 22:29:41

* Andrew Dunstan wrote:

> OK, here's my final version of the patch, which I will apply in 24 hours
> or so unless there is an objection.

+  <productname>Visual Studio 2008</productname> and above. Compilation
+  is supported down to <productname>Windows XP</productname> and
+  <productname>Windows Server 2003</> when building with
+  <productname>Visual Studio 2005</> to
+  <productname>Visual Studio 2013</productname>. Building with
+  <productname>Visual Studio 2015</productname> is supported down to
+  <productname>Windows Vista</> and <productname>Windows Server   2008</>.

This paragraph contradicts itself; it first says 2008 is the minimum 
version, right after that it supports 2005, too. The last version of
Visual Studio that will install on XP, 2003, Vista, and 2008 is VS 2010 
anyway, anything released after that requires at least 7/2008R2.

I would actually just say "is supported with Visual Studio 2005 [or 
possibly 2008] and higher" instead of listing versions. I don't think we 
need to recapitulate the system requirements of individual VS releases 
and anyone trying to install 2012+ on Vista will quickly find out it 
doesn't work. It would be different if we actually excluded particular 
VS versions or VS/OS combinations that are supported by VS itself, but 
we don't.

+        /*
+         * get a pointer sized version of bgchild to avoid                warnings about
+         * casting to a different size on WIN64.
+         */
+        intptr_t    bgchild_handle = bgchild;

If you're going to copy it anyway, why not just use a HANDLE rather than 
cast it again later? It's only used in two places, cast to HANDLE in 
both, and the special case of -1 does not matter in either.

+ * Leave a higher value in place. When building with at least Visual
+ * Studio 2015 the minimum requirement is Windows Vista (0x0600) to
+ * get support for GetLocaleInfoEx() with locales. For everything else
+ * the minumum version os Windows XP (0x0501).

s/os/is/ in the last line.

This one doesn't matter, but just for perfection's sake:

+#if (_MSC_VER >= 1900)
+    uint32        cp;
+    WCHAR        wctype[80];
+
+    memset(wctype, 0, 80 * sizeof(WCHAR));
+    MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, 80);

The maximum length is documented as 85 characters, also:

<https://msdn.microsoft.com/en-us/library/windows/desktop/dd373815(v=vs.85).aspx>: 
'Note   Your application must use the constant [LOCALE_NAME_MAX_LENGTH] 
for the maximum locale name length, instead of hard-coding the value "85".'

-- 
Christian

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

25 April 2016, 10:11:18

On Mon, Apr 25, 2016 at 4:29 AM, Christian Ullrich <chris@chrullrich.net> wrote:
> Andrew wrote:
>> OK, here's my final version of the patch, which I will apply in 24 hours
> or so unless there is an objection.

Thanks Andrew for the updated patch!

> This one doesn't matter, but just for perfection's sake:
>
> +#if (_MSC_VER >= 1900)
> +       uint32          cp;
> +       WCHAR           wctype[80];
> +
> +       memset(wctype, 0, 80 * sizeof(WCHAR));
> +       MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, 80);
>
> The maximum length is documented as 85 characters, also:
>
> <https://msdn.microsoft.com/en-us/library/windows/desktop/dd373815(v=vs.85).aspx>:
> 'Note   Your application must use the constant [LOCALE_NAME_MAX_LENGTH] for
> the maximum locale name length, instead of hard-coding the value "85".'

Just an addition on top of the comments of Christian..

+#pragma warning(push)
+#pragma warning(disable : 4091)#include <dbghelp.h>
+#pragma warning(pop)
It seems to me that we had better protect those pragmas with _MSC_VER
>= 1900. The crash dump facility could be used by MinGW as well, no?
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

29 April 2016, 07:15:15


On 04/25/2016 03:11 AM, Michael Paquier wrote:
> On Mon, Apr 25, 2016 at 4:29 AM, Christian Ullrich <chris@chrullrich.net> wrote:
>> Andrew wrote:
>>> OK, here's my final version of the patch, which I will apply in 24 hours
>> or so unless there is an objection.
> Thanks Andrew for the updated patch!
>
>> This one doesn't matter, but just for perfection's sake:
>>
>> +#if (_MSC_VER >= 1900)
>> +       uint32          cp;
>> +       WCHAR           wctype[80];
>> +
>> +       memset(wctype, 0, 80 * sizeof(WCHAR));
>> +       MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, 80);
>>
>> The maximum length is documented as 85 characters, also:
>>
>> <https://msdn.microsoft.com/en-us/library/windows/desktop/dd373815(v=vs.85).aspx>:
>> 'Note   Your application must use the constant [LOCALE_NAME_MAX_LENGTH] for
>> the maximum locale name length, instead of hard-coding the value "85".'
> Just an addition on top of the comments of Christian..
>
> +#pragma warning(push)
> +#pragma warning(disable : 4091)
>   #include <dbghelp.h>
> +#pragma warning(pop)
> It seems to me that we had better protect those pragmas with _MSC_VER
>> = 1900. The crash dump facility could be used by MinGW as well, no?



I think we're being overcautious here.  But to keep people happy I have
protected it with "#ifdef _MSC_VER".

latest patch attached. I have also cleaned up the docs some, and removed
references to the now redundant msysGit.

cheers

andrew

Attachment

VS2015-support2.patch

Re: VS 2015 support in src/tools/msvc

From

Tom Lane

Date:

29 April 2016, 07:39:41

Andrew Dunstan <andrew@dunslane.net> writes:
> latest patch attached. I have also cleaned up the docs some, and removed 
> references to the now redundant msysGit.

Please don't do stuff like this:

@@ -232,6 +265,10 @@ win32_langinfo(const char *ctype)        if (r != NULL)            sprintf(r, "CP%s", codepage);
}
 
+
+#if (_MSC_VER >= 1900)
+    }
+#endif#endif    return r;

I'm not very sure what pgindent will do with conditionally-included
indentation, but it's unlikely to be pleasing.

In this particular case, you could probably fix it by making the
other end of that be 

+    if (GetLocaleInfoEx(wctype,
+            LOCALE_IDEFAULTANSICODEPAGE | LOCALE_RETURN_NUMBER,
+            (LPWSTR) &cp, sizeof(cp) / sizeof(WCHAR)) > 0)
+    {
+        r = malloc(16);            /* excess */
+        if (r != NULL)
+            sprintf(r, "CP%u", cp);
+    }
+    else
+#endif
+    {
+

and omitting the #if/#endif around the trailing }.
        regards, tom lane

Re: VS 2015 support in src/tools/msvc

From

Andrew Dunstan

Date:

29 April 2016, 15:13:52


On 04/29/2016 12:39 AM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> latest patch attached. I have also cleaned up the docs some, and removed
>> references to the now redundant msysGit.
> Please don't do stuff like this:
>
> @@ -232,6 +265,10 @@ win32_langinfo(const char *ctype)
>           if (r != NULL)
>               sprintf(r, "CP%s", codepage);
>       }
> +
> +#if (_MSC_VER >= 1900)
> +    }
> +#endif
>   #endif
>   
>       return r;
>
> I'm not very sure what pgindent will do with conditionally-included
> indentation, but it's unlikely to be pleasing.
>
> In this particular case, you could probably fix it by making the
> other end of that be
>
> +    if (GetLocaleInfoEx(wctype,
> +            LOCALE_IDEFAULTANSICODEPAGE | LOCALE_RETURN_NUMBER,
> +            (LPWSTR) &cp, sizeof(cp) / sizeof(WCHAR)) > 0)
> +    {
> +        r = malloc(16);            /* excess */
> +        if (r != NULL)
> +            sprintf(r, "CP%u", cp);
> +    }
> +    else
> +#endif
> +    {
> +
>
> and omitting the #if/#endif around the trailing }.
>
>             



Yeah, I noticed the ugliness, should have fixed it. Applied your fix and 
committed.

cheers

andrew

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

29 April 2016, 17:02:43

On Fri, Apr 29, 2016 at 9:13 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
> Yeah, I noticed the ugliness, should have fixed it. Applied your fix and
> committed.

Thanks for the commit!

Nitpick comment:
+ * Also  for VS2015, add a define that stops compiler complaints about
Two spaces instead of one between "Also" and "for" :)
-- 
Michael

Re: VS 2015 support in src/tools/msvc

From

Petr Jelinek

Date:

29 April 2016, 18:22:37

On 29/04/16 16:02, Michael Paquier wrote:
> On Fri, Apr 29, 2016 at 9:13 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> Yeah, I noticed the ugliness, should have fixed it. Applied your fix and
>> committed.
>
> Thanks for the commit!
>

+1

After bit of fighting with the system the "caecilian" reported first 
successful build to the buildfarm.

--   Petr Jelinek                  http://www.2ndQuadrant.com/  PostgreSQL Development, 24x7 Support, Training &
Services

Re: VS 2015 support in src/tools/msvc

From

Michael Paquier

Date:

30 April 2016, 01:22:22

On Sat, Apr 30, 2016 at 12:22 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
> After bit of fighting with the system the "caecilian" reported first
> successful build to the buildfarm.

Thanks! The fight was there as well.
-- 
Michael