Better Support for Visual Studio 2017 and 2019

[DeeDeeG]

Requirements

• Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
• All new code requires tests to ensure against regressions

Description of the Change

Main change:

• Detect Visual Studio 2017 and 2019

Related changes:

• Stop detecting unsupported versions, which node-gyp refuses to use (Visual Studio 2010 - 2013)
• Don't set the --msvs_version node-gyp flag, which sets a specific version of Visual Studio that node-gyp must find, or else it will error out.
  • Our detection for actually-working Visual Studio installs is substantially worse than node-gyp's. We should let them detect and use whatever usable version of Visual Studio they can find.

Explanation (click to expand):

Builds on apm's existing code to detect installs of Visual Studio, by adding the ability to detect Visual Studio 2017 and 2019.

Those versions of Visual Studio are now fully supported in the version of node-gyp we use (node-gyp@5).

As of node-gyp PR nodejs/node-gyp#1762, node-gyp is much better than us at finding Visual Studio installs and identifying whether on not they can be used. This isn't a simple matter of having Visual Studio or not, but also various components must be installed.

Given that our detection is worse, I have made it so apm no-longer sets the --msvs_version flag. This flag forces node-gyp to use the specified major version of Visual Studio/Build Tools. It can only restrict node-gyp within usable installs. There is no scenario where setting this causes any benefit to us. But if we get it wrong (and our detection is worse, so we very likely would), this can cause node-gyp to refuse to use a valid Visual Studio/Build Tools install and error out.

Alternate Designs

This, but with some of the changes reverted/taken out.

All commits in this PR are designed to be logical choices that are well-backed-up by the explanation and reasoning given.

That said, the commits are in order of how radical the change is, or how controversial I thought the change might be. If any of the commits/changes are objectionable, please let me know, and the rest of the PR still makes sense without that change. This is a few related changes bundled together, but any of the three bullet points is a positive step in the right direction, and works independent of the other two bullet points.

Benefits

• Lets apm and node-gyp use Visual Studio 2017 and 2019, even if 2015 or older is also installed
• Stops causing errors for our users, such as by rejecting Visual Studio 2017 and 2019 if a buggy 2015 or earlier install is the first one found
• Gets out of the way of a critical dependency detection task that node-gyp is doing better than us at
• More successful package builds, more happy customers?
• Keeps up with the latest Visual Studio major versions we support (via node-gyp).

Possible Drawbacks

None that I am aware of. Before this PR, we have been artificially limiting how good a job node-gyp could do to find Visual Studio, in certain cases. Now, with this PR, we stop doing that.

Reaching for a hypothetical problem: I suppose if Visual Studio 2017 or 2019 are buggy, this exposes our users to that. (In my experience, VS 2017 and 2019 are not buggy.)

Verification Process

Lengthy "Verification Process" section contents (click to expand):

This un-blocks apm from using a correctly-detected, working install of Visual Studio 2019 in (forked) Atom's CI.

• Before: https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=176&view=logs&j=97a617bf-bcbd-5dfa-bba2-cfba2747b693&t=1e1369e9-1c56-59b0-830c-f1606a680c8a&l=32
• After: https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=177&view=logs&j=97a617bf-bcbd-5dfa-bba2-cfba2747b693&t=1e1369e9-1c56-59b0-830c-f1606a680c8a

Breakdown of the "Before" case: The issue in this case was that apm detected an install of 2015 that turned out to be unusable. The error is: "could not find MSBuild in registry for this version". At the bottom of the error message, a message points out that 2019 would have been a valid version, had we allowed node-gyp to use it:

gyp ERR! find VS valid versions for msvs_version:
gyp ERR! find VS - "2019"
gyp ERR! find VS - "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise"

---

Furthermore, apm --version can now find Visual Studio 2017 and 2019.

Before and after on the same machine, with Visual Studio 2019 Community installed:

Before:

C:\Users\[User]\>apm-dev --version
apm  2.5.0
npm  6.14.5
node 10.20.1 x64
atom 1.50.0-dev-110b05baf
python 2.7.18
git 2.27.0.windows.1
visual studio

After:

C:\Users\[User]>apm-dev --version
apm  2.5.0
npm  6.14.5
node 10.20.1 x64
atom 1.51.0-dev-912dad331
python 2.7.18
git 2.27.0.windows.1
visual studio 2019

Note that Visual Studio 2019 is detected: visual studio 2019 (on the bottom line of output from apm --version).

Here's the before and after on another machine, this time with Visual Studio 2017:

Before

C:\Users\[User]>apm-dev --version
apm  2.5.0
npm  6.14.5
node 10.20.1 x64
atom 1.50.0-dev-9c16e5c67
python 2.7.18
git 2.27.0.windows.1
visual studio

After

C:\Users\[User]>apm-dev --version
apm  2.5.0
npm  6.14.5
node 10.20.1 x64
atom 1.51.0-dev-912dad331
python 2.7.18
git 2.27.0.windows.1
visual studio 2017

Note that Visual Studio 2017 is detected: visual studio 2017 (on the bottom line of output from apm --version).

Applicable Issues

Fixes #893

[DeeDeeG]

cc @darangi, @lkashef, @aminya so this is on your radar.

[DeeDeeG]

More research to confirm this PR is the right approach:

atom/flight-manual.atom.io#630 (comment)

Conclusions:

• Visual Studio 2015 works with all npm versions. (But with npm older than 6.10.1, you must do npm config set msvs_version=2015, OR in your environment variables set GYP_MSVS_VERSION=2015.)
• Visual Studio 2017 works with npm 4.5.0 or newer, no environment variable or config setting needed.
• Visual Studio 2019 works with npm 6.10.1 or newer, no environment variable or config settings needed.

We bundle/use the latest npm (npm 6.14.x), which is more than new enough to pick a Visual Studio installation automatically.

[aminya]

I created a PR for atom which depends on this.
atom/atom#21379

[aminya]

@sadick254 @darangi Will this ever be merged? I was interested in improving superstring, but it is not worth it without using std=C++17.

[DeeDeeG]

I hope this helps with reviewing this Pull Request; To summarize what this Pull Request does, here is the behavior before and after:

Before:

• apm checks on the disk for Visual Studio install folders, doing a so-so job detecting which Visual Studio version is installed
  • Picks the newest one out of 2015, 2013, 2012 and 2010
  • Forces that version to be used
  • If none of the above are detected, apm gives up trying to force a version

(This behavior was helpful way back years ago, with what are now very old versions of npm, but it isn't useful anymore, and hard-coding the wrong (too old) Visual Studio versions makes this worse than doing nothing. Modern npm/node-gyp do a really good job of picking a working copy of Visual Studio, so IMO we should just get out of the way and let node-gyp pick.)

With this PR:

• apm never forces a particular version of Visual Studio to be used (node-gyp picks the right version for us)
• apm generally doesn't try to detect Visual Studio installs on-disk anymore...
  • Except to find what version to print for apm --version.
    • For the sake of apm --version, apm knows to check for Visual Studio 2019, 2017 and 2015, which are all the versions modern node-gyp supports when used with Node > 8. We cannot use older Visual Studio versions, so I stopped checking for them.

[DeeDeeG]

In Azure Pipelines CI, there are copies of multiple Visual Studio installs, but not all of them are usable. Particularly, there is an unusable (for building Atom) partial install of Visual Studio 2015.

This PR makes it possible to use Visual Studio 2017 instead. Otherwise, apm will see the partial VS2015 install and force Visual Studio 2015. That errors out, since the VS2015 install is missing some components.

Please consider merging this PR so it is convenient to test with Visual Studio 2017 or 2019 in Atom's CI.

[sadick254]

This looks good to me @DeeDeeG. Thanks for all the investigation work. This really helps us do quicker reviews.

[DeeDeeG]

Thank you for merging! This is one of the contributions to apm I am proudest of, and that I think makes a big difference for compatibility with modern systems that have newer Visual Studio.