Обсуждение: MSVC Build support with visual studio 2019

On Thu, Mar 21, 2019 at 11:36:42AM +1100, Haribabu Kommi wrote:
> I can provide a separate back branches patch later once this patch comes to
> a stage of commit. Currently all the supported branches are possible to
> compile with VS 2017.

When it comes to support newer versions of MSVC, we have come up
lately to backpatch that down to two stable versions but not further
down (see f2ab389 for v10 and v9.6), so it looks sensible to target
v11 and v10 as well if we were to do it today, and v11/v12 if we do it
in six months per the latest trends.
--
Michael

On Thu, Mar 21, 2019 at 09:47:02AM +0900, Michael Paquier wrote:
> When it comes to support newer versions of MSVC, we have come up
> lately to backpatch that down to two stable versions but not further
> down (see f2ab389 for v10 and v9.6), so it looks sensible to target
> v11 and v10 as well if we were to do it today, and v11/v12 if we do it
> in six months per the latest trends.

By the way, you mentioned upthread that all the branches can compile
with MSVC 2017, but that's not actually the case for 9.5 and 9.4 if
you don't back-patch f2ab389 further down.  Or did you mean that the
code as-is is compilable if the scripts are patched manually?
--
Michael

On Thu, Mar 21, 2019 at 12:45:57PM +1100, Haribabu Kommi wrote:
> The commit f2ab389 is later back-patch to version till 9.3 in commit
> 19acfd65.  I guess that building the windows installer for all the
> versions using the same visual studio is may be the reason behind
> that back-patch.

I did not remember this one, thanks for pointing it out.  So my
memories on that were incorrect.  If it is possible to get the code to
build with MSVC 2019 on all the supported branches, we could do so.
--
Michael

On 3/21/19 3:13 AM, Michael Paquier wrote:
> On Thu, Mar 21, 2019 at 12:45:57PM +1100, Haribabu Kommi wrote:
>> The commit f2ab389 is later back-patch to version till 9.3 in commit
>> 19acfd65.  I guess that building the windows installer for all the
>> versions using the same visual studio is may be the reason behind
>> that back-patch.
> I did not remember this one, thanks for pointing it out.  So my
> memories on that were incorrect.  If it is possible to get the code to
> build with MSVC 2019 on all the supported branches, we could do so.




VS2019 is currently in preview. I think we'd probably be better off
waiting until the full release. I don't know of any pressing urgency for
us to support it.


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I

On 3/25/19 3:44 PM, Andrew Dunstan wrote:
> On 3/21/19 3:13 AM, Michael Paquier wrote:
>> On Thu, Mar 21, 2019 at 12:45:57PM +1100, Haribabu Kommi wrote:
>>> The commit f2ab389 is later back-patch to version till 9.3 in commit
>>> 19acfd65.  I guess that building the windows installer for all the
>>> versions using the same visual studio is may be the reason behind
>>> that back-patch.
>> I did not remember this one, thanks for pointing it out.  So my
>> memories on that were incorrect.  If it is possible to get the code to
>> build with MSVC 2019 on all the supported branches, we could do so.
>
>
>
> VS2019 is currently in preview. I think we'd probably be better off
> waiting until the full release. I don't know of any pressing urgency for
> us to support it.
>
>

I see Haribabu mentioned that in his original email.


I'll take a look at verifying the patch.


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I have gone through path '0001-Support-building-with-visual-studio-2019.patch' only, but I am sure some comments will
alsoapply to back branches.
 

1. The VisualStudioVersion value looks odd:

+    $self->{VisualStudioVersion}        = '16.0.32.32432';

Are you using a pre-release version [1]?

2. There is a typo: s/stuido/studio/:

+    # The major visual stuido that is suppored has nmake version >= 14.20 and < 15.

There is something in the current code that I think should be also updated. The code for _GetVisualStudioVersion
contains:

  if ($major > 14)
     {
     carp
      "The determined version of Visual Studio is newer than the latest supported version. Returning the latest
supportedversion instead.";
 
     return '14.00';
     }

Shouldn't the returned value be '14.20' for Visual Studio 2019?

Regards,

Juan José Santamaría Flecha

[1] https://docs.microsoft.com/en-us/visualstudio/releases/2019/history#release-dates-and-build-numbers

On Wed, Jun 26, 2019 at 10:29:05PM +1000, Haribabu Kommi wrote:
> Thanks for the review. Yes, that patch applies till 9.5, it is my mistake
> in naming the patch.

I have been able to finally set up an environment with VS 2019 (as
usual this stuff needs time, anyway..), and I can confirm that the
patch is able to compile properly.

-  <productname>Visual Studio 2017</productname> (including Express editions),
-  as well as standalone Windows SDK releases 6.0 to 8.1.
+  <productname>Visual Studio 2019</productname> (including Express editions),
+  as well as standalone Windows SDK releases 8.1a to 10.
I would like to understand why this range of requirements is updated.
Is there any reason to do so.  If we change these docs, what does it
mean in terms of versions of Visual Studio supported?

-or a VS2015Solution or a VS2017Solution, all in Solution.pm, depending on
-the user's build environment) and adding objects implementing the corresponding
-Project interface (VC2013Project or VC2015Project or VC2017Project from
-MSBuildProject.pm) to it.
+or a VS2015Solution or a VS2017Solution or a VS2019Solution, all in Solution.pm,
+depending on the user's build environment) and adding objects implementing
+the corresponding Project interface (VC2013Project or VC2015Project or VC2017Project
+or VC2019Project from MSBuildProject.pm) to it.
This formulation is weird the more we accumulate new objects, let's
put that in a proper list of elements separated with commas except
for the two last ones which should use "or".

s/greather/greater/.

The patch still has typos, and the format is not satisfying yet, so I
have done a set of fixes as per the attached.

-   elsif ($major < 6)
+   elsif ($major < 12)
    {
        croak
-         "Unable to determine Visual Studio version:
            Visual Studio versions before 6.0 aren't supported.";
+         "Unable to determine Visual Studio version:
        Visual Studio versions before 12.0 aren't supported.";
Well, this is a separate bug fix, still I don't mind fixing that in
the same patch as we meddle with those code paths now.  Good catch.

-       croak $visualStudioVersion;
+       carp $visualStudioVersion;
Same here.  Just wouldn't it be better to print the version found in
the same message?

+   # The major visual studio that is supported has nmake version >=
 14.20 and < 15.
    if ($major > 14)
Comment line is too long.  It seems to me that the condition here
should be ($major >= 14 && $minor >= 30).  That's not completely
correct either as we have a version higher than 14.20 for VS 2019 but
that's better than just using 14.29 or a fake number I guess.

So for now I have the attached which applies to HEAD.  The patch is
not indented yet because the conditions in CreateProject() and
CreateSolution() get messed up, but I'll figure out something.

Any comments?  I am wondering the update related to the version range
of the standalone SDKs though.  No need for backpatched versions yet,
first let's agree about the shape of what we want on HEAD.
--
Michael

On Mon, Jul 01, 2019 at 07:56:29PM +1000, Haribabu Kommi wrote:
> We stopped the support of building with all the visual studio versions less
> than 2013.  I updated the SDK versions accordingly.

I have spent some time looking around, and wikipedia-sensei has proved
to be helpful to grasp the release references:
https://en.wikipedia.org/wiki/Microsoft_Windows_SDK

So the suggestions from the patch are fine.  This one was actually
forgotten:
src/tools/msvc/README:from www.microsoft.com (v6.0 or greater).

> The similar change is required for the CreateProject also.

I have changed both messages so as the version of VS attempted to be
used is reported in the error message directly.

> + # The major visual studio that is supported has nmake
> + # version >= 14.30, so stick with it as the latest version
>
> The major visual studio version that is supported has nmake version
> <=14.30

Damn.  Thanks for pointing out that.

> Except for the above two changes, overall the patch is in good shape.

OK, committed to HEAD for now after perltidy'ing the patch.  Let's see
what the buildfarm has to say about it first.  Once we are sure that
the thing is stable, I'll try to backpatch it.  This works on my own
dev machines with VS 2015 and 2019, but who knows what hides in the
shadows...
--
Michael

On Tue, Jul 02, 2019 at 02:10:11PM +0900, Michael Paquier wrote:
> OK, committed to HEAD for now after perltidy'ing the patch.  Let's see
> what the buildfarm has to say about it first.  Once we are sure that
> the thing is stable, I'll try to backpatch it.  This works on my own
> dev machines with VS 2015 and 2019, but who knows what hides in the
> shadows...

The buildfarm did not have much to say, so backpatched down to 9.4,
adjusting things on the way.
--
Michael