Horton

Thread

Commits

  1. Add support for Visual Studio 2019 in build scripts

  2. Support building with Visual Studio 2017

  3. Require C99 (and thus MSCV 2013 upwards).

  1. MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-03-21T00:36:42Z

    Hi Hackers,

Here I attached a patch that supports building of PostgreSQL with VS 2019.
VS 2019 is going to release on Apr 2nd 2019, it will be good if version 12
supports compiling. The attached for is for review, it may needs some
updates
once the final version is released.

Commit d9dd406fe281d22d5238d3c26a7182543c711e74 has reduced the
minimum visual studio support to 2013 to support C99 standards, because of
this
reason, the current attached patch cannot be backpatched as it is.

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.

comments?

Regards,
Haribabu Kommi
Fujitsu Australia

  2. Re: MSVC Build support with visual studio 2019

    Michael Paquier <michael@paquier.xyz> — 2019-03-21T00:47:02Z

    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

  3. Re: MSVC Build support with visual studio 2019

    Michael Paquier <michael@paquier.xyz> — 2019-03-21T01:31:45Z

    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

  4. Re: MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-03-21T01:45:57Z

    On Thu, Mar 21, 2019 at 12:31 PM Michael Paquier <michael@paquier.xyz>
wrote:

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

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.

Regards,
Haribabu Kommi
Fujitsu Australia

  5. Re: MSVC Build support with visual studio 2019

    Michael Paquier <michael@paquier.xyz> — 2019-03-21T07:13:55Z

    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

  6. Re: MSVC Build support with visual studio 2019

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-03-25T19:44:45Z

    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

  7. Re: MSVC Build support with visual studio 2019

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-03-25T21:32:42Z

    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

  8. Re: MSVC Build support with visual studio 2019

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-03-26T16:03:05Z

    On 3/20/19 8:36 PM, Haribabu Kommi wrote:
> Hi Hackers,
>
> Here I attached a patch that supports building of PostgreSQL with VS 2019.
> VS 2019 is going to release on Apr 2nd 2019, it will be good if version 12
> supports compiling. The attached for is for review, it may needs some
> updates
> once the final version is released.
>
> Commit d9dd406fe281d22d5238d3c26a7182543c711e74 has reduced the
> minimum visual studio support to 2013 to support C99 standards,
> because of this
> reason, the current attached patch cannot be backpatched as it is.
>
> 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.
>
> comments?
>
>


I have verified that this works with VS2019.


There are a few typos in the comments that need cleaning up.


cheers


andrew


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

  9. Re: MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-03-27T00:42:02Z

    On Wed, Mar 27, 2019 at 3:03 AM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:

>
> On 3/20/19 8:36 PM, Haribabu Kommi wrote:
> > Hi Hackers,
> >
> > Here I attached a patch that supports building of PostgreSQL with VS
> 2019.
> > VS 2019 is going to release on Apr 2nd 2019, it will be good if version
> 12
> > supports compiling. The attached for is for review, it may needs some
> > updates
> > once the final version is released.
> >
> > Commit d9dd406fe281d22d5238d3c26a7182543c711e74 has reduced the
> > minimum visual studio support to 2013 to support C99 standards,
> > because of this
> > reason, the current attached patch cannot be backpatched as it is.
> >
> > 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.
> >
> > comments?
> >
> >
>
>
> I have verified that this works with VS2019.
>
>
> There are a few typos in the comments that need cleaning up.
>

Thanks for the review.

I corrected the typos in the comments, hopefully I covered everything.
Attached is the updated patch. Once the final VS 2019, I will check the
patch if it needs any more updates.

Regards,
Haribabu Kommi
Fujitsu Australia

  10. Re: MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-04-09T07:46:56Z

    On Wed, Mar 27, 2019 at 11:42 AM Haribabu Kommi <kommi.haribabu@gmail.com>
wrote:

>
> On Wed, Mar 27, 2019 at 3:03 AM Andrew Dunstan <
> andrew.dunstan@2ndquadrant.com> wrote:
>
>>
>> On 3/20/19 8:36 PM, Haribabu Kommi wrote:
>> > Hi Hackers,
>> >
>> > Here I attached a patch that supports building of PostgreSQL with VS
>> 2019.
>> > VS 2019 is going to release on Apr 2nd 2019, it will be good if version
>> 12
>> > supports compiling. The attached for is for review, it may needs some
>> > updates
>> > once the final version is released.
>> >
>> > Commit d9dd406fe281d22d5238d3c26a7182543c711e74 has reduced the
>> > minimum visual studio support to 2013 to support C99 standards,
>> > because of this
>> > reason, the current attached patch cannot be backpatched as it is.
>> >
>> > 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.
>> >
>> > comments?
>> >
>> >
>>
>>
>> I have verified that this works with VS2019.
>>
>>
>> There are a few typos in the comments that need cleaning up.
>>
>
> Thanks for the review.
>
> I corrected the typos in the comments, hopefully I covered everything.
> Attached is the updated patch. Once the final VS 2019, I will check the
> patch if it needs any more updates.
>

Visual Studio 2019 is officially released. There is no major change in the
patch, except some small comments update.

Also attached patches for the back branches also.

Regards,
Haribabu Kommi
Fujitsu Australia

  11. Re: MSVC Build support with visual studio 2019

    Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> — 2019-05-21T21:35:24Z

    I have gone through path '0001-Support-building-with-visual-studio-2019.patch' only, but I am sure some comments will also apply 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 supported version 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

  12. Re: MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-05-23T01:44:34Z

    On Wed, May 22, 2019 at 7:36 AM Juanjo Santamaria Flecha <
juanjo.santamaria@gmail.com> wrote:

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

Thanks for the review.



> 1. The VisualStudioVersion value looks odd:
>
> +       $self->{VisualStudioVersion}        = '16.0.32.32432';
>
> Are you using a pre-release version [1]?
>

I first developed this patch on the preview version.
I updated it to version 16.0.28729.10.


> 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 supported version instead.";
>         return '14.00';
>         }
>
> Shouldn't the returned value be '14.20' for Visual Studio 2019?
>

Yes, that will be good to return Visual Studio 2019, updated.

Updated patches are attached for all branches.

Regards,
Haribabu Kommi
Fujitsu Australia

  13. Re: MSVC Build support with visual studio 2019

    Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> — 2019-05-27T10:14:02Z

    On Thu, May 23, 2019 at 3:44 AM Haribabu Kommi <kommi.haribabu@gmail.com>
wrote:

>
> Updated patches are attached for all branches.
>
>
I have gone through all patches and there are a couple of typos:

    1. s/prodcutname/productname/

    1.1 In file: 0001-support-building-with-visual-studio-2019_v9.4.patch
@@ -97,8 +97,9 @@
   <productname>Visual Studio 2013</productname>. Building with
   <productname>Visual Studio 2015</productname> is supported down to
   <productname>Windows Vista</> and <productname>Windows Server 2008</>.
-   Building with <productname>Visual Studio 2017</productname> is supported
-   down to <productname>Windows 7 SP1</> and <productname>Windows Server
2008 R2 SP1</>.
+   Building with <productname>Visual Studio 2017</productname> and
+   <prodcutname>Visual Studio 2019</prodcutname> are supported down to

    1.2 In file:
0001-support-building-with-visual-studio-2019_v10_to_v9.5.patch
@@ -97,8 +97,9 @@
   <productname>Visual Studio 2013</productname>. Building with
   <productname>Visual Studio 2015</productname> is supported down to
   <productname>Windows Vista</> and <productname>Windows Server 2008</>.
-   Building with <productname>Visual Studio 2017</productname> is supported
-   down to <productname>Windows 7 SP1</> and <productname>Windows Server
2008 R2 SP1</>.
+   Building with <productname>Visual Studio 2017</productname> and
+   <prodcutname>Visual Studio 2019</prodcutname> are supported down to

    2. s/stuido/studio/

     2.1 In file: 0001-support-building-with-visual-studio-2019_v9.4.patch
 @@ -143,12 +173,12 @@ sub DetermineVisualStudioVersion
 sub _GetVisualStudioVersion
 {
  my ($major, $minor) = @_;
- # visual 2017 hasn't changed the nmake version to 15, so still using the
older version for comparison.
+ # The major visual stuido that is suppored has nmake version >= 14.20 and
< 15.

     2.2 In file:
0001-support-building-with-visual-studio-2019_v10_to_v9.5.patch
@@ -132,12 +162,12 @@ sub DetermineVisualStudioVersion
 sub _GetVisualStudioVersion
 {
  my ($major, $minor) = @_;
- # visual 2017 hasn't changed the nmake version to 15, so still using the
older version for comparison.
+ # The major visual stuido that is suppored has nmake version >= 14.20 and
< 15.

     2.3 In file:  0001-support-building-with-visual-studio-2019_v11.patch
@@ -139,12 +165,12 @@ sub _GetVisualStudioVersion
 {
  my ($major, $minor) = @_;

- # visual 2017 hasn't changed the nmake version to 15, so still using the
older version for comparison.
+ # The major visual stuido that is suppored has nmake version >= 14.20 and
< 15.

     2.4 In file:  0001-Support-building-with-visual-studio-2019_HEAD.patch
@@ -106,17 +132,17 @@ sub _GetVisualStudioVersion
 {
  my ($major, $minor) = @_;

- # visual 2017 hasn't changed the nmake version to 15, so still using the
older version for comparison.
+ # The major visual stuido that is suppored has nmake version >= 14.20 and
< 15.


Other than that, since this is affects comments and docs, I have already
tested that the patches build and pass tests on all the intended versions.

Regards,

Juan José Santamaría Flecha

  14. Re: MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-05-29T08:30:20Z

    On Mon, May 27, 2019 at 8:14 PM Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> wrote:

>
> On Thu, May 23, 2019 at 3:44 AM Haribabu Kommi <kommi.haribabu@gmail.com>
> wrote:
>
>>
>> Updated patches are attached for all branches.
>>
>>
> I have gone through all patches and there are a couple of typos:
>

Thanks for the review.


>     1. s/prodcutname/productname/
>
>     1.1 In file: 0001-support-building-with-visual-studio-2019_v9.4.patch
> @@ -97,8 +97,9 @@
>    <productname>Visual Studio 2013</productname>. Building with
>    <productname>Visual Studio 2015</productname> is supported down to
>    <productname>Windows Vista</> and <productname>Windows Server 2008</>.
> -   Building with <productname>Visual Studio 2017</productname> is
> supported
> -   down to <productname>Windows 7 SP1</> and <productname>Windows Server
> 2008 R2 SP1</>.
> +   Building with <productname>Visual Studio 2017</productname> and
> +   <prodcutname>Visual Studio 2019</prodcutname> are supported down to
>
>     1.2 In file:
> 0001-support-building-with-visual-studio-2019_v10_to_v9.5.patch
> @@ -97,8 +97,9 @@
>    <productname>Visual Studio 2013</productname>. Building with
>    <productname>Visual Studio 2015</productname> is supported down to
>    <productname>Windows Vista</> and <productname>Windows Server 2008</>.
> -   Building with <productname>Visual Studio 2017</productname> is
> supported
> -   down to <productname>Windows 7 SP1</> and <productname>Windows Server
> 2008 R2 SP1</>.
> +   Building with <productname>Visual Studio 2017</productname> and
> +   <prodcutname>Visual Studio 2019</prodcutname> are supported down to
>

Corrected.


>     2. s/stuido/studio/
>
>      2.1 In file: 0001-support-building-with-visual-studio-2019_v9.4.patch
>  @@ -143,12 +173,12 @@ sub DetermineVisualStudioVersion
>  sub _GetVisualStudioVersion
>  {
>   my ($major, $minor) = @_;
> - # visual 2017 hasn't changed the nmake version to 15, so still using
> the older version for comparison.
> + # The major visual stuido that is suppored has nmake version >= 14.20
> and < 15.
>
>      2.2 In file:
> 0001-support-building-with-visual-studio-2019_v10_to_v9.5.patch
> @@ -132,12 +162,12 @@ sub DetermineVisualStudioVersion
>  sub _GetVisualStudioVersion
>  {
>   my ($major, $minor) = @_;
> - # visual 2017 hasn't changed the nmake version to 15, so still using
> the older version for comparison.
> + # The major visual stuido that is suppored has nmake version >= 14.20
> and < 15.
>
>      2.3 In file:  0001-support-building-with-visual-studio-2019_v11.patch
> @@ -139,12 +165,12 @@ sub _GetVisualStudioVersion
>  {
>   my ($major, $minor) = @_;
>
> - # visual 2017 hasn't changed the nmake version to 15, so still using
> the older version for comparison.
> + # The major visual stuido that is suppored has nmake version >= 14.20
> and < 15.
>
>      2.4 In file:  0001-Support-building-with-visual-studio-2019_HEAD.patch
> @@ -106,17 +132,17 @@ sub _GetVisualStudioVersion
>  {
>   my ($major, $minor) = @_;
>
> - # visual 2017 hasn't changed the nmake version to 15, so still using
> the older version for comparison.
> + # The major visual stuido that is suppored has nmake version >= 14.20
> and < 15.
>

Corrected. And also 'supported' spelling is also wrong.

Updated patches are attached.

Regards,
Haribabu Kommi
Fujitsu Australia

  15. Re: MSVC Build support with visual studio 2019

    Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> — 2019-06-05T07:22:09Z

    On Wed, May 29, 2019 at 10:30 AM Haribabu Kommi <kommi.haribabu@gmail.com>
wrote:

>
> Updated patches are attached.
>
>
All patches apply, build and pass tests. The patch
'0001-support-building-with-visual-studio-2019_v10_to_v9.6_v3.patch'
applies on version 9.5.

Not sure if more review is needed before moving to 'ready for commite'r,
but I have no more comments to make on current patches.

Regards,

Juan José Santamaría Flecha

  16. Re: MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-06-26T12:29:05Z

    On Wed, 5 Jun 2019 at 17:22, Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> wrote:

>
> On Wed, May 29, 2019 at 10:30 AM Haribabu Kommi <kommi.haribabu@gmail.com>
> wrote:
>
>>
>> Updated patches are attached.
>>
>>
> All patches apply, build and pass tests. The patch
> '0001-support-building-with-visual-studio-2019_v10_to_v9.6_v3.patch'
> applies on version 9.5.
>
> Not sure if more review is needed before moving to 'ready for commite'r,
> but I have no more comments to make on current patches.
>

Thanks for the review. Yes, that patch applies till 9.5, it is my mistake
in naming the patch.

Regards,
Haribabu Kommi

  17. Re: MSVC Build support with visual studio 2019

    Michael Paquier <michael@paquier.xyz> — 2019-06-27T07:27:25Z

    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

  18. Re: MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-07-01T09:56:29Z

    On Thu, 27 Jun 2019 at 17:28, Michael Paquier <michael@paquier.xyz> wrote:

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

Thanks for the review.


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

We stopped the support of building with all the visual studio versions less
than 2013.
I updated the SDK versions accordingly.



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

The change in the patch is good.


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

Yes, that is a good change, I thought of doing the same, but I don't know
how to do it.

The similar change is required for the CreateProject also.



> +   # 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.
>

The change is good, but the comment is wrong.

+ # 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

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

Regards,
Haribabu Kommi

  19. Re: MSVC Build support with visual studio 2019

    Michael Paquier <michael@paquier.xyz> — 2019-07-02T05:10:11Z

    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

  20. Re: MSVC Build support with visual studio 2019

    Michael Paquier <michael@paquier.xyz> — 2019-07-03T00:01:23Z

    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

  21. Re: MSVC Build support with visual studio 2019

    Haribabu Kommi <kommi.haribabu@gmail.com> — 2019-07-03T08:51:59Z

    On Wed, 3 Jul 2019 at 10:01, Michael Paquier <michael@paquier.xyz> wrote:

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


Thanks Michael.

Regards,
Haribabu Kommi