build: Add option to select environment for msbuild (vc2015 or vc2013)

[greenjava]

Add option in vcbuild.bat script to select Visual Studio environment to build Node on Windows.

• vc2013 to select Visual Studio 2013 environment
• vc2015 to select Visual Studio 2015 environment

[joaocgreis]

@greenjava thanks for contributing! When the option is passed, I think it should not look for any other version. Something like:

diff --git a/vcbuild.bat b/vcbuild.bat
index 924e3a3..1468ba5 100644
--- a/vcbuild.bat
+++ b/vcbuild.bat
@@ -112,10 +112,8 @@ call :getnodeversion || exit /b 1

 @rem Set environment for msbuild

-if "%target_env%"=="vc2015" goto vc-set-2015
-if "%target_env%"=="vc2013" goto vc-set-2013
-
 :vc-set-2015
+if defined target_env if "%target_env%" NEQ "vc2015" goto vc-set-2013
 @rem Look for Visual Studio 2015
 echo Looking for Visual Studio 2015
 if not defined VS140COMNTOOLS goto vc-set-2013
@@ -139,6 +137,7 @@ set PLATFORM_TOOLSET=v140
 goto msbuild-found

 :vc-set-2013
+if defined target_env if "%target_env%" NEQ "vc2013" goto msbuild-not-found
 @rem Look for Visual Studio 2013
 echo Looking for Visual Studio 2013
 if not defined VS120COMNTOOLS goto msbuild-not-found

Can you update if you agree?

[greenjava]

Thanks for your comment, i'll fix that (it's a smarter solution)

[joaocgreis]

LGTM, I'll land after the regulatory 48 hours pass.

The commit message has to be adjusted to obey the line sizes, when I land I'll change it to something like:

build: add option to select VS version

This changes vcbuild.bat to accept a new parameter (vc2015 or vc2013)
to select the version of Visual Studio to use.

Let me know or update if you disagree. Thanks!

[greenjava]

That sounds good :)
Thanks

[joaocgreis]

CI: https://ci.nodejs.org/job/node-test-pull-request/1233/ (failures unrelated - ref: #4679)

Landed in 8182ec0 . Thanks!

[rvagg]

Nice work @greenjava. It looks like this is your first commit to core, if that's right then welcome on board! It's great to have contributors to the Windows-specific parts of the repo because our collaborator base is bit heavily skewed towards POSIX environments. I hope you stick around and find other places to contribute.

[MylesBorins]

@nodejs/build is this something we want to see in LTS?

[jasnell]

hmmm.. not sure. @nodejs/lts @nodejs/build

[joaocgreis]

This could help somebody out there, but I doubt it. I see this more like a new feature and I wouldn't pick it, but it's fine either way.

[rvagg]

+1 for LTS from me, it might be a new feature but not in the semver-minor way since it's build-focused