Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
Gyp Python 3 fixes for Windows#29897
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
targos commented Oct 9, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
targos commented Oct 9, 2019
Note: with this change and #29898, I'm able to build Node on Windows with VS 2019 and Python 3.7. |
nodejs-github-bot commented Oct 9, 2019
Uh oh!
There was an error while loading. Please reload this page.
sam-github commented Oct 9, 2019
Does I'd RSLGTM it, but it failed in CI. I resumed in case the failures were ephemeral. |
nodejs-github-bot commented Oct 9, 2019
richardlau commented Oct 9, 2019
Is that a safe comparison? e.g. What if |
targos commented Oct 9, 2019
I have no idea. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
If GYP finds a string variable that can be converted to an integer, it will do it when the variable is expanded. Use "0.0" instead of "0" to force strings and be able to use comparison operations such as `gas_version >= "2.26"` in Python 3.
nodejs-github-bot commented Oct 11, 2019
sam-github commented Oct 11, 2019
I'd be willing to rubber stamp this once @cclauss or someone more up to date on python weighs in. Once question, @targos , is this CI-able? Do we have a windows CI machine with py3? I think that even if it did, we might currently "prefer" py2, so mere existence of py3 isn't enough yet. I wonder, can Travis do a py3-only windows build? |
targos commented Oct 11, 2019
I don't know. I could try to add a github action to this PR that runs on Windows and remove it before landing.
|
sam-github commented Oct 11, 2019
If py3 works for you, that's good enough for now -- adding and removing an action to prove it isn't necessary from my perspective. |
cclauss commented Oct 13, 2019
|
cclauss left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for this.
Refs: nodejs/node-gyp#1820 Refs: nodejs/node-gyp#1843 PR-URL: #29897 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
If GYP finds a string variable that can be converted to an integer, it will do it when the variable is expanded. Use "0.0" instead of "0" to force strings and be able to use comparison operations such as `gas_version >= "2.26"` in Python 3. PR-URL: #29897 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
PR-URL: #29897 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
targos commented Oct 13, 2019
Landed in 7de5a55...8728f86 |
Commits:
/cc @nodejs/python