-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
add msvs 2017 support #1964
add msvs 2017 support #1964
Conversation
Based on #1952 (comment) this isn't necessary? cc @joaocgreis |
after opening the PR I saw that these changes already there in GYP3. However, as long as you use the node-gyp this is needed. |
This shouldn't be needed, VS detection is not done by GYP but in https://github.com/nodejs/node-gyp/blob/master/lib/find-visualstudio.js. @dothebart what issue did you see that needs this? Is the detection in node-gyp not working for you? Can you paste the output of a failing |
Ah, I don't compile V8 for nodejs - its https://github.com/ArangoDB/ArangoDB - so I don't have this file. |
I guess we'll have to land something like this or remove autodetection from the GYP side, but I'd rather wait until we figure out how GYP will be managed here. @dothebart thanks for opening this PR and trying to help. |
hm, asking a heretic question - how do you run |
I'm not sure I understand your question... Here in node-gyp, Node.js is required to run, so V8 is always already compiled. For compiling Node.js itself in https://github.com/nodejs/node/ a different mechanism is used. It uses |
ah, nice. a shellscript to tell the environment autodetection infrastructure about the infrastructure to work around the autodetection. |
No description provided.