Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

win: enable build with vs2017 #11852

Closed
wants to merge 1 commit into from
+60 −6
Closed

win: enable build with vs2017 #11852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion BUILDING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Support is divided into three tiers:
|--------------|--------------|----------------------------------|----------------------|------------------|
| GNU/Linux | Tier 1 | kernel >= 2.6.18, glibc >= 2.5 | x86, x64, arm, arm64 | |
| macOS | Tier 1 | >= 10.10 | x64 | |
| Windows | Tier 1 | >= Windows 7 or >= Windows2008R2 | x86, x64 | |
| Windows | Tier 1 | >= Windows 7 / 2008 R2 | x86, x64 | vs2015 or vs2017 |
| SmartOS | Tier 2 | >= 15 < 16.4 | x86, x64 | see note1 |
| FreeBSD | Tier 2 | >= 10 | x64 | |
| GNU/Linux | Tier 2 | kernel >= 3.13.0, glibc >= 2.19 | ppc64le >=power8 | |
Expand Down Expand Up @@ -175,6 +175,8 @@ Prerequisites:
* [Visual Studio 2015 Update 3](https://www.visualstudio.com/), all editions
including the Community edition (remember to select
"Common Tools for Visual C++ 2015" feature during installation).
* [Visual Studio 2017](https://www.visualstudio.com/downloads/), any edition (including the Build Tools SKU).
__Required Components:__ "MSbuild", "VC++ 2017 v141 toolset" and one of the Windows SDKs (10 or 8.1).
* Basic Unix tools required for some tests,
[Git for Windows](http://git-scm.com/download/win) includes Git Bash
and tools which can be included in the global `PATH`.
Expand Down
20 changes: 20 additions & 0 deletions tools/msvs/vswhere_usability_wrapper.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
:: Copyright 2017 - Refael Ackermann
:: Distributed under MIT style license
:: See accompanying file LICENSE at https://github.com/node4good/windows-autoconf
:: version: 1.14.0

@if not defined DEBUG_HELPER @ECHO OFF
setlocal
set VSWHERE_REQ=-requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64
set VSWHERE_PRP=-property installationPath
set VSWHERE_LMT=-version "[15.0,16.0)"
SET VSWHERE_ARGS=-latest -products * %VSWHERE_REQ% %VSWHERE_PRP% %VSWHERE_LMT%
set "VSWHERE=%ProgramFiles(x86)%\Microsoft Visual Studio\Installer"
if not exist "%VSWHERE%" set "VSWHERE=%ProgramFiles%\Microsoft Visual Studio\Installer"
if not exist "%VSWHERE%" exit /B 1
set Path=%Path%;%VSWHERE%
for /f "usebackq tokens=*" %%i in (`vswhere %VSWHERE_ARGS%`) do (
endlocal
set "VCINSTALLDIR=%%i\VC\"
set "VS150COMNTOOLS=%%i\Common7\Tools\"
exit /B 0)
42 changes: 37 additions & 5 deletions vcbuild.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ if /i "%1"=="/?" goto help
set config=Release
set target=Build
set target_arch=x64
set target_env=
set target_env=vs2015
set noprojgen=
set nobuild=
set sign=
Expand Down Expand Up @@ -53,7 +53,10 @@ if /i "%1"=="clean" set target=Clean&goto arg-ok
if /i "%1"=="ia32" set target_arch=x86&goto arg-ok
if /i "%1"=="x86" set target_arch=x86&goto arg-ok
if /i "%1"=="x64" set target_arch=x64&goto arg-ok
if /i "%1"=="vc2015" set target_env=vc2015&goto arg-ok
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be kept. vcbuild.bat should search for VS2017 and, only if that is not found, VS2015. But a user with both installed should be able to use vc2015/2017 to specifically select the version to use or fail if that version is not found. This mechanism was introduced in #4645, I don't think it should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.
It has been a blind switch since build: remove VS 2013 switch from vcbuild.bat but now it makes sense again...

@rem args should be vs2017 and vs2015. keeping vc2015 for backward combatibility (undocumented)
if /i "%1"=="vc2015" set target_env=vs2015&goto arg-ok
if /i "%1"=="vs2015" set target_env=vs2015&goto arg-ok
if /i "%1"=="vs2017" set target_env=vs2017&goto arg-ok
if /i "%1"=="noprojgen" set noprojgen=1&goto arg-ok
if /i "%1"=="nobuild" set nobuild=1&goto arg-ok
if /i "%1"=="nosign" set "sign="&echo Note: vcbuild no longer signs by default. "nosign" is redundant.&goto arg-ok
Expand Down Expand Up @@ -88,7 +91,7 @@ if /i "%1"=="upload" set upload=1&goto arg-ok
if /i "%1"=="small-icu" set i18n_arg=%1&goto arg-ok
if /i "%1"=="full-icu" set i18n_arg=%1&goto arg-ok
if /i "%1"=="intl-none" set i18n_arg=%1&goto arg-ok
if /i "%1"=="without-intl" set i18n_arg=%1&goto arg-ok
if /i "%1"=="without-intl" set i18n_arg=%1&goto arg-ok
if /i "%1"=="download-all" set download_arg="--download=all"&goto arg-ok
if /i "%1"=="ignore-flaky" set test_args=%test_args% --flaky-tests=dontcare&goto arg-ok
if /i "%1"=="enable-vtune" set enable_vtune_arg=1&goto arg-ok
Expand Down Expand Up @@ -149,7 +152,33 @@ if defined noprojgen if defined nobuild if not defined sign if not defined msi g

@rem Set environment for msbuild

set msvs_host_arch=x86
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this go inside :vs-set-2017? It looks like it is only used there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually should be added to the VS2015 case as well, but that's for a different PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it's kinda consistent with the rem two lines above ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing the implication on calling the VS2016 with a host_target arch

if _%PROCESSOR_ARCHITECTURE%_==_AMD64_ set msvs_host_arch=amd64
if _%PROCESSOR_ARCHITEW6432%_==_AMD64_ set msvs_host_arch=amd64
@rem usualy vcvarsall takes an argument: host + '_' + target
set vcvarsall_arg=%msvs_host_arch%_%target_arch%
@rem unless both host and taget are x64
if %target_arch%==x64 if %msvs_host_arch%==amd64 set vcvarsall_arg=amd64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use VsDevCmd to avoid having to detect the host arch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda don't trust VsDevCmd 🤷‍♀️ I think it does too much.
Also both of them, for x86 it choose a native compiler over a 64bit host cross compiler:

c:\code$ "c:\bin\dev\VS\Microsoft Visual Studio 2017 Community\Common7\Tools\VsDevCmd.bat" -arch=x86
**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0.26403.7
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************

c:\code$ where cl
c:\bin\dev\VS\Microsoft Visual Studio 2017 Community\VC\Tools\MSVC\14.10.25017\bin\HostX86\x86\cl.exe

And apparently that matters: https://chromium-review.googlesource.com/c/486400/#message-04d641d86728a1b1832181033b692eb9a85e55de

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*"does too much" == "runs longer" (probably does .NET setup)


@rem Look for Visual Studio 2017
:vs-set-2017
if "%target_env%" NEQ "vs2017" goto vs-set-2015
echo Looking for Visual Studio 2017
if "_%VSCMD_ARG_TGT_ARCH%_"=="_%target_arch%_" goto found_vs2017
call tools\msvs\vswhere_usability_wrapper.cmd
if "_%VCINSTALLDIR%_" == "__" goto vs-set-2015
set vcvars_call="%VCINSTALLDIR%\Auxiliary\Build\vcvarsall.bat" %vcvarsall_arg%
echo calling: %vcvars_call%
call %vcvars_call%
:found_vs2017
echo Found MSVS version %VisualStudioVersion%
set GYP_MSVS_VERSION=2017
set PLATFORM_TOOLSET=v141
goto msbuild-found

@rem Look for Visual Studio 2015
:vs-set-2015
if "%target_env%" NEQ "vs2015" goto msbuild-not-found
echo Looking for Visual Studio 2015
if not defined VS140COMNTOOLS goto msbuild-not-found
if not exist "%VS140COMNTOOLS%\..\..\vc\vcvarsall.bat" goto msbuild-not-found
Expand All @@ -172,7 +201,9 @@ set PLATFORM_TOOLSET=v140
goto msbuild-found

:msbuild-not-found
echo Failed to find Visual Studio installation.
echo Failed to find a suitable Visual Studio installation.
echo Try to run in a "Developer Command Prompt" or consult
echo https://github.com/nodejs/node/blob/master/BUILDING.md#windows-1
goto exit

:wix-not-found
Expand Down Expand Up @@ -298,6 +329,7 @@ if not defined SSHCONFIG (
echo SSHCONFIG is not set for upload
exit /b 1
)

if not defined STAGINGSERVER set STAGINGSERVER=node-www
ssh -F %SSHCONFIG% %STAGINGSERVER% "mkdir -p nodejs/%DISTTYPEDIR%/v%FULLVERSION%/win-%target_arch%"
scp -F %SSHCONFIG% Release\node.exe %STAGINGSERVER%:nodejs/%DISTTYPEDIR%/v%FULLVERSION%/win-%target_arch%/node.exe
Expand Down Expand Up @@ -455,7 +487,7 @@ echo Failed to create vc project files.
goto exit

:help
echo vcbuild.bat [debug/release] [msi] [test-all/test-uv/test-inspector/test-internet/test-pummel/test-simple/test-message] [clean] [noprojgen] [small-icu/full-icu/without-intl] [nobuild] [sign] [x86/x64] [vc2015] [download-all] [enable-vtune] [lint/lint-ci] [no-NODE-OPTIONS]
echo vcbuild.bat [debug/release] [msi] [test-all/test-uv/test-inspector/test-internet/test-pummel/test-simple/test-message] [clean] [noprojgen] [small-icu/full-icu/without-intl] [nobuild] [sign] [x86/x64] [vs2015/vs2017] [download-all] [enable-vtune] [lint/lint-ci] [no-NODE-OPTIONS]
echo Examples:
echo vcbuild.bat : builds release build
echo vcbuild.bat debug : builds debug build
Expand Down