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

Detect visual studio 2017 #1205

Closed
wants to merge 1 commit into from
Closed

Detect visual studio 2017 #1205

wants to merge 1 commit into from

Conversation

danielgindi
Copy link

This should have been done long ago...

@refack
Copy link
Contributor

refack commented May 22, 2017

@danielgindi thanks for the contribution! 🥇
As of [email protected] detection of VS2017 is already in.
(if you don't have > v3.6.0 you'll need to update npm)

@danielgindi
Copy link
Author

Actually 3.6.1 is installed, a fresh install - and do not have the VS2017 detection in...
It does use it somehow with an env called GYP_MSVS_OVERRIDE_PATH, which does not exist on my system so I have no idea where it comes from.
But on another system - also 3.6.1 just installed fresh - no VS2017 detection...

@refack
Copy link
Contributor

refack commented May 22, 2017

It does use it somehow with an env called GYP_MSVS_OVERRIDE_PATH

https://github.com/nodejs/node-gyp/blob/master/lib/configure.js#L156

We wanted to have VS2017 support before it landed in upstream GYP so we "lie" to it to make it work.
If you can't build with v3.6.1 please supply the error output. Also look if it's related to #1179

@danielgindi
Copy link
Author

Okay so I've debugged the configure.js, and findVS2017 says that "Error: Could not use PowerShell to find VS2017"
Looking further into it...

@refack
Copy link
Contributor

refack commented May 22, 2017

See if this #1198 helps

@danielgindi
Copy link
Author

'Exception calling "Query" with "0" argument(s): "Input string was not in a correct format."\r\nAt line:1 char:95\r\n+ ... e-gyp\\lib\\Find-VS2017.cs\'; [VisualStudioConfiguration.Main]::Query()}\r\n+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\r\n + CategoryInfo : NotSpecified: (:) [], MethodInvocationException\r\n + FullyQualifiedErrorId : FormatException\r\n \r\n'

@danielgindi
Copy link
Author

Yes, #1198 solves it. So we're waiting for v3.6.2, and for it to reach npm upstream...

@danielgindi
Copy link
Author

Seems like Windows Creators Update has this side effect of making vs2017 harder to detect by the old code.
This really needs to be released soon...

@refack
Copy link
Contributor

refack commented Jun 2, 2017

Will be in [email protected]

@refack
Copy link
Contributor

refack commented Jun 2, 2017

Will be in [email protected]
Ref: npm/npm#16894

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.

2 participants