Allow building with other Visual Studio 2017 editions (eg. Professional)#8940
Conversation
AAClause
left a comment
There was a problem hiding this comment.
Successfully built using Microsoft Visual Studio Community 2017 Version 15.0.28306.52 D15.9 and Microsoft Visual Studio Enterprise 2017 Version 15.0.28306.52 D15.9.
|
Hi, can we get a second look from @michaelDCurran? Thanks.
|
| # VS 2017 keeps changing the path to reflect the latest major.minor.build version which we canot easily find out. | ||
| # Therefore Search these versioned directories from newest to oldest to collect all the files we need. | ||
| vcRedistDirs = glob.glob(os.path.join(progFilesX86, r"Microsoft Visual Studio\2017\Community\VC\Redist\MSVC\14.1*\x86\Microsoft.VC141.CRT")) | ||
| vcRedistDirs = glob.glob(os.path.join(progFilesX86, r"Microsoft Visual Studio\2017\*\VC\Redist\MSVC\14.1*\x86\Microsoft.VC141.CRT")) |
There was a problem hiding this comment.
If two different variants of Visual Studio 2017 are installed, it is unclear as to which version of the dlls it will found. We really need to some how get the real path from Scons when it set up the visual studio paths. Though having a quick look myself so far this seems impossible. Unless we hack the vs took code in scons to save off a path during set-up.
There was a problem hiding this comment.
@michaelDCurran you are right, versions could mismatch.
SCons does not keep track of MSVC installation path once it used it to setup the environment.
Still, we can ask it to retrieve it again.
Pushed a second commit, using SCons.Tool.MSCommon.vc.find_vc_pdir which itself relies on calling vswhere.exe for 14.1.
If two different editions of Visual Studio 2017 are installed, I am still unsure as to which is used by SCons, but it will be the exact same for the compiler and the CRT.
Would you prefer I squash those commits together?
| # VS 2017 keeps changing the path to reflect the latest major.minor.build version which we canot easily find out. | ||
| # Therefore Search these versioned directories from newest to oldest to collect all the files we need. | ||
| vcRedistDirs = glob.glob(os.path.join(progFilesX86, r"Microsoft Visual Studio\2017\Community\VC\Redist\MSVC\14.1*\x86\Microsoft.VC141.CRT")) | ||
| vcProductDir = find_vc_pdir("14.1") |
There was a problem hiding this comment.
Can you please fetch the vc version from env.get('MSVC_VERSION') and use that rather than hardcoding 14.1 on this line and the next line also. Otherwise, we may forget to update the version string here if we ever move to a later version of Visual Studio.
There was a problem hiding this comment.
Done.
Please note Microsoft.VC141.CRT, msvcp140.dll, vccorlib140.dllandvcruntime140.dllstill hold version numbers in their very names, but I doubt there is much we can do except maybe externalizing these strings. Appart from these, the only remaining hard-coded reference to "14.1" is innvdaHelper/archBuild_sconscript` in which we check if the latest available version meets our requirement rather than instructing which version we need. This is safe only as long as we do not pull a newer SCons supporting a newer MSVS.
Link to issue number:
Fixes #8939
Summary of the issue:
Currently, NVDA requires Visual Studio 2017 Community edition in order to build from source.
This PR allows to also build with other edition, like Professional.
Description of how this pull request fixes the issue:
The build process requires the file
Microsoft.VC141.CRT, which currently cannot be found with other editions of Visual Studio 2017.In
nvdaHelper/localWin10/sconscript, replace the globMicrosoft Visual Studio\2017\Community\VC\Redist\MSVC\14.1*\x86\Microsoft.VC141.CRTwithMicrosoft Visual Studio\2017\*\VC\Redist\MSVC\14.1*\x86\Microsoft.VC141.CRT, that is, replacingCommunitywith*, to allow resolving the required file.Testing performed:
Successfully built using Visual Studio 2017 Professional version 15.6.1
Known issues with pull request:
Could someone please validate the build using Visual Studio 2017 Community as well?
EDIT: Seems like AppVeyor volunteered…
Change log entry:
Section: Changes for Developpers
NVDA can now also be built with other editions of Microsoft Visual Studio 2017 (used to require Community edition).