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

build,win,msi: support WiX with VS2017 #17101

Closed
wants to merge 1 commit into from
Closed

build,win,msi: support WiX with VS2017 #17101

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
2 changes: 2 additions & 0 deletions BUILDING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ Prerequisites:
* 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`.
* **Optional** (to build the MSI): the [WiX Toolset v3.11](http://wixtoolset.org/releases/)
and the [Wix Toolset Visual Studio 2017 Extension](https://marketplace.visualstudio.com/items?itemName=RobMensching.WixToolsetVisualStudio2017Extension).
Copy link
Contributor

Choose a reason for hiding this comment

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

Building MSI was previously undocumented, correct?

Copy link
Member

Choose a reason for hiding this comment

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

Correct.


If the path to your build directory contains a space, the build will likely fail.

Expand Down
20 changes: 17 additions & 3 deletions vcbuild.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,24 @@ if %target_arch%==x64 if %msvs_host_arch%==amd64 set vcvarsall_arg=amd64
:vs-set-2017
if defined target_env if "%target_env%" NEQ "vs2017" goto vs-set-2015
echo Looking for Visual Studio 2017
call tools\msvs\vswhere_usability_wrapper.cmd
if "_%VCINSTALLDIR%_" == "__" goto vs-set-2015
if defined msi (
echo Looking for WiX installation for Visual Studio 2017...
if not exist "%WIX%\SDK\VS2017" (
echo Failed to find WiX install for Visual Studio 2017
echo VS2017 support for WiX is only present starting at version 3.11
goto vs-set-2015
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we really fall back to VS2015 (or exit once #16969 lands) if the extension is not installed? Perhaps msi should just be skipped instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

MSI is not enabled by default, and this only applies if vcbuild was explicitly called with msi. So, skipping the msi here would be ignoring a parameter. Given that this is for releases, not just a nice to have, I don't think we should skip.

)
if not exist "%VCINSTALLDIR%\..\MSBuild\Microsoft\WiX" (
echo Failed to find the Wix Toolset Visual Studio 2017 Extension
goto vs-set-2015
)
)
@rem check if VS2017 is already setup, and for the requested arch
if "_%VisualStudioVersion%_" == "_15.0_" if "_%VSCMD_ARG_TGT_ARCH%_"=="_%target_arch%_" goto found_vs2017
@rem need to clear VSINSTALLDIR for vcvarsall to work as expected
set "VSINSTALLDIR="
call tools\msvs\vswhere_usability_wrapper.cmd
if "_%VCINSTALLDIR%_" == "__" goto vs-set-2015
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the idea was to avoid calling vswhere_usability_wrapper.cmd if it's already setup? This will call it every time.

Copy link
Member Author

Choose a reason for hiding this comment

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

vcvarsall.bat below is the one to avoid. vswhere is not a problem and is needed for the WiX checks.

@rem prevent VsDevCmd.bat from changing the current working directory
set "VSCMD_START_DIR=%CD%"
set vcvars_call="%VCINSTALLDIR%\Auxiliary\Build\vcvarsall.bat" %vcvarsall_arg%
Expand Down Expand Up @@ -354,7 +366,9 @@ if not defined msi goto run

:msibuild
echo Building node-v%FULLVERSION%-%target_arch%.msi
msbuild "%~dp0tools\msvs\msi\nodemsi.sln" /m /t:Clean,Build /p:PlatformToolset=%PLATFORM_TOOLSET% /p:GypMsvsVersion=%GYP_MSVS_VERSION% /p:Configuration=%config% /p:Platform=%target_arch% /p:NodeVersion=%NODE_VERSION% /p:FullVersion=%FULLVERSION% /p:DistTypeDir=%DISTTYPEDIR% %noetw_msi_arg% %noperfctr_msi_arg% /clp:NoSummary;NoItemAndPropertyList;Verbosity=minimal /nologo
set "msbsdk="
if defined WindowsSDKVersion set "msbsdk=/p:WindowsTargetPlatformVersion=%WindowsSDKVersion:~0,-1%"
msbuild "%~dp0tools\msvs\msi\nodemsi.sln" /m /t:Clean,Build %msbsdk% /p:PlatformToolset=%PLATFORM_TOOLSET% /p:GypMsvsVersion=%GYP_MSVS_VERSION% /p:Configuration=%config% /p:Platform=%target_arch% /p:NodeVersion=%NODE_VERSION% /p:FullVersion=%FULLVERSION% /p:DistTypeDir=%DISTTYPEDIR% %noetw_msi_arg% %noperfctr_msi_arg% /clp:NoSummary;NoItemAndPropertyList;Verbosity=minimal /nologo
if errorlevel 1 goto exit

if not defined sign goto upload
Expand Down