Build requirement checking #4185
Conversation
build.psm1
Outdated
| @@ -222,7 +222,15 @@ Fix steps: | |||
| throw 'Win 10 SDK not found. Run Start-PSBootstrap or install Microsoft Windows 10 SDK from https://developer.microsoft.com/en-US/windows/downloads/windows-10-sdk' | |||
| } | |||
|
|
|||
| $vcVarsPath = (Get-Item(Join-Path -Path "$env:VS140COMNTOOLS" -ChildPath '../../vc')).FullName | |||
| # atlbase.h is included in at least one project | |||
| $vcPath = (Get-Item(Join-Path -Path "$env:VS140COMNTOOLS" -ChildPath '../../vc')).FullName | |||
There was a problem hiding this comment.
More build diagnostics are good in my book. I just wish this weren't tied specifically to VS 2015. On at least one of my machines I have only VS 2017 installed. @KirkMunro this is not your issue. Just an observation on this build script.
There was a problem hiding this comment.
I completely agree. I kept it tied to 2015 to follow the script's current design, but it would be nice to see a separate PR that would allow it to work on the latest release version of VS, or a specific version if you tell it to.
There was a problem hiding this comment.
+1 on a separate PR to support VS 2017. I have both installed so I never noticed this pain point, but it keeps coming up more and more. If either of you guys want to tackle it, it would be a welcome contribution.
It is tracked as issue #3400
build.psm1
Outdated
| @@ -222,7 +222,15 @@ Fix steps: | |||
| throw 'Win 10 SDK not found. Run Start-PSBootstrap or install Microsoft Windows 10 SDK from https://developer.microsoft.com/en-US/windows/downloads/windows-10-sdk' | |||
| } | |||
|
|
|||
| $vcVarsPath = (Get-Item(Join-Path -Path "$env:VS140COMNTOOLS" -ChildPath '../../vc')).FullName | |||
| # atlbase.h is included in at least one project | |||
| $vcPath = (Get-Item(Join-Path -Path "$env:VS140COMNTOOLS" -ChildPath '../../vc')).FullName | |||
There was a problem hiding this comment.
+1 on a separate PR to support VS 2017. I have both installed so I never noticed this pain point, but it keeps coming up more and more. If either of you guys want to tackle it, it would be a welcome contribution.
It is tracked as issue #3400
build.psm1
Outdated
| @@ -222,7 +222,15 @@ Fix steps: | |||
| throw 'Win 10 SDK not found. Run Start-PSBootstrap or install Microsoft Windows 10 SDK from https://developer.microsoft.com/en-US/windows/downloads/windows-10-sdk' | |||
| } | |||
|
|
|||
| $vcVarsPath = (Get-Item(Join-Path -Path "$env:VS140COMNTOOLS" -ChildPath '../../vc')).FullName | |||
| # atlbase.h is included in at least one project | |||
There was a problem hiding this comment.
Please move this comment down to the IF check. Also, can you please specify the project that requires it? It is most likely the native remoting plugin (pwershplugin.dll).
build.psm1
Outdated
| } | ||
|
|
||
| # vcvarsall.bat is used to setup environment variables | ||
| $vcVarsPath = $vcPath |
There was a problem hiding this comment.
The renaming and reassignment of this variable seems unnecessary. If you leave the name as-is, then this line is not required.
There was a problem hiding this comment.
If I leave the name as vcVarsPath, then the variable is not appropriately named anymore. But two variables are not needed, you're right about that. I reduced/simplified to one variable for that path, and stuck with the vcPath variable name.
After fighting with the build for a while, I finally figured out what was wrong -- my VS 2015 install did not have the MFC for C++ feature installed, so I was missing atlbase.h. Since that feature is required for the Windows build, I updated build.psm1 to check for it ahead of time and notify the user when it is missing.