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

Update gyp: Add Visual Studio 2017 support #11656

Merged
merged 1 commit into from Jan 17, 2018

Conversation

Projects
None yet
3 participants
@zcbenz
Contributor

zcbenz commented Jan 17, 2018

@zcbenz zcbenz requested a review from electron/reviewers as a code owner Jan 17, 2018

@zcbenz zcbenz closed this Jan 17, 2018

@zcbenz zcbenz reopened this Jan 17, 2018

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented Jan 17, 2018

The failing test is not related to this PR, I'm merging anyway.

@zcbenz zcbenz merged commit d69c17a into master Jan 17, 2018

8 of 9 checks passed

continuous-integration/appveyor/branch AppVeyor build failed
Details
ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@zcbenz zcbenz deleted the update-gyp branch Jan 17, 2018

@PhoebeHui

This comment has been minimized.

PhoebeHui commented Jan 17, 2018

Thanks for doing this change to support VS2017!

I encounter an issue 'could not locate Visual Studio installation' when execute ''python script\bootstrap.py -v --target_arch=ia32"

The script/update.py looks also need update. the GYP_MSVS_VERSION should be 2017, after I made this change, everything works well.
https://github.com/electron/electron/blob/master/script/update.py#L61

The failures like
Traceback (most recent call last):
File "vendor\gyp\gyp_main.py", line 16, in
sys.exit(gyp.script_main())
File "vendor\gyp\pylib\gyp_init_.py", line 545, in script_main
return main(sys.argv[1:])
File "vendor\gyp\pylib\gyp_init_.py", line 538, in main
return gyp_main(args)
File "vendor\gyp\pylib\gyp_init_.py", line 514, in gyp_main
options.duplicate_basename_check)
File "vendor\gyp\pylib\gyp_init_.py", line 98, in Load
generator.CalculateVariables(default_variables, params)
File "vendor\gyp\pylib\gyp\generator\ninja.py", line 1695, in CalculateVariables
gyp.msvs_emulation.CalculateCommonVariables(default_variables, params)
File "vendor\gyp\pylib\gyp\msvs_emulation.py", line 1097, in CalculateCommonVariables
msvs_version = gyp.msvs_emulation.GetVSVersion(generator_flags)
File "vendor\gyp\pylib\gyp\msvs_emulation.py", line 948, in GetVSVersion
allow_fallback=False)
File "vendor\gyp\pylib\gyp\MSVSVersion.py", line 485, in SelectVisualStudioVersion
raise ValueError('Could not locate Visual Studio installation.')
ValueError: Could not locate Visual Studio installation.

Thanks,
Phoebe

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented Jan 23, 2018

It seems that we should add a flag to specify Visual Studio version, or try to automatically find out it.

@alexeykuzmin

This comment has been minimized.

Contributor

alexeykuzmin commented Jan 23, 2018

@PhoebeHui

The script/update.py looks also need update. the GYP_MSVS_VERSION should be 2017, after I made this change, everything works well.

VS 2017 is only required by the Chromium 63, so we don't need to change the GYP_MSVS_VERSION in the master which uses libcc based on the Chromium 61.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment