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.

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

GPII-4355: Updated build scripts and projects to use VS2017 build tools #181

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

christopher-rtf
Copy link
Contributor

This PR, combined with the windows/GPII-4355 PR, should fully enable Morphic for Windows to compile using modern (VS2017) developer tools and Node.js v10.16.3.

Please note that this branch updates package.json to pull in the respective windows GPII-4355 repo; that should of course be modified to point to the proper merged "windows" repo in github.com/GPII.

Critical: developers must upgrade their developer environment to Visual Studio 2017. The new build scripts use Microsoft's new "vswhere" utility (included with VS2017+) to locate MSBuild, CSC, environment variables, etc. This is a hard fork in the road...and we will need all the developers on our team (plus the CI environment) to move to VS2017 when this goes "live" in master.

For the future: The $visualStudioVersion PowerShell variable has been designed to support either a specific version (e.g. "15.0" for VS2017) or a range (e.g. "[15.0,16.0)" as an inclusive range for any VS2017'ish releases from v15.0 to v15.9999999999). This means that as we want to start adding support for VS2019 or other newer releases, we can simplly update the $visualStudioVersion variable to include a wider range of compiler versions. Our build may indeed be compatible with VS2019 as well, but I have not tested with VS2019 due to the fact that Electron still uses VS2017 in their documentation.

@gpii-bot
Copy link

Could one of the admins verify that these changes are reasonable to test? If so, please reply with "ok to test".

@amb26
Copy link
Member

amb26 commented Feb 16, 2020

ok to test

@gpii-bot
Copy link

CI job failed: https://ci.gpii.net/job/gpii-app-tests/981/

@christopher-rtf
Copy link
Contributor Author

While we're waiting to update CI to use VS2017 build tools (instead of VS2013/VS2015), here is the log of a manual VS2017 checkout/build from a local VM. [Build successful]

gpii-app_build_log_vs2017.txt

And a screenshot of the results of "npm start" (and then clicking on the Morphic icon to bring up the QuickStrip).

image

@gpii-bot
Copy link

CI job failed: https://ci.gpii.net/job/gpii-app-tests/982/

@stegru
Copy link
Member

stegru commented Feb 17, 2020

ok to test

THis happened the other day, too:

�[K==> default: Matching MAC address for NAT networking...
==> default: Checking if box 'inclusivedesign/windows10-eval-x64-Apps' is up to date...
==> default: Setting the name of the VM: gpii-app-tests_default_1581879463735_93217
Vagrant found a port collision for the specified port and virtual machine.
While this port was marked to be auto-corrected, the ports in the
auto-correction range are all also used.

VM: default
Forwarded port: 5985 => 5985
Build step 'Execute shell' marked build as failure

@gpii-bot
Copy link

CI job failed: https://ci.gpii.net/job/gpii-app-tests/983/

@stegru
Copy link
Member

stegru commented Feb 17, 2020

This can be fixed, in gpii-windows, with GPII/windows@29440be

    default: CSC : error CS0006: Metadata file 'C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5.1\mscorlib.dll' could not be found [V:\node_modules\gpii-windows\settingsHelper\SettingsHelper\SettingsHelper.csproj]
    default: Done Building Project "V:\node_modules\gpii-windows\settingsHelper\SettingsHelper\SettingsHelper.csproj" (default targets) -- FAILED.
    default: Done Building Project "V:\node_modules\gpii-windows\settingsHelper\SettingsHelper.sln" (default targets) -- FAILED.
    default: Build FAILED.
    default: "V:\node_modules\gpii-windows\settingsHelper\SettingsHelper.sln" (default target) (1) ->
    default: "V:\node_modules\gpii-windows\settingsHelper\SettingsHelper\SettingsHelper.csproj" (default target) (2) ->
    default: (ResolveAssemblyReferences target) -> 
    default:   C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(2110,5): warning MSB3270: There was a mismatch between the processor architecture of the project being built "MSIL" and the processor architecture of the reference "System.Data", "AMD64". This mismatch may cause runtime failures. Please consider changing the targeted processor architecture of your project through the Configuration Manager so as to align the processor architectures between your project and references, or take a dependency on references with a processor architecture that matches the targeted processor architecture of your project. [V:\node_modules\gpii-windows\settingsHelper\SettingsHelper\SettingsHelper.csproj]
    default: "V:\node_modules\gpii-windows\settingsHelper\SettingsHelper.sln" (default target) (1) ->
    default: "V:\node_modules\gpii-windows\settingsHelper\SettingsHelper\SettingsHelper.csproj" (default target) (2) ->
    default: (CoreCompile target) -> 
    default:   CSC : error CS0006: Metadata file 'C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5.1\mscorlib.dll' could not be found [V:\node_modules\gpii-windows\settingsHelper\SettingsHelper\SettingsHelper.csproj]
    default:     1 Warning(s)
    default:     1 Error(s)
    default: Time Elapsed 00:00:04.34

@christopher-rtf
Copy link
Contributor Author

SG> This can be fixed, in gpii-windows, with GPII/windows@29440be

On my test machine, the 4.5.1 SDK's mscorlib is at that location regardless of ia32, amd64 or arm64 architecture (with the "(x86)" missing on ia32...which the updated build script accounts for).

Are we missing the .NET 4.5.1 Framework SDK in the build machine install? Are the files located somewhere else in CI? What was the reason for the path override originally?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants