-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Gyp Python 3 fixes for Windows #29897
Conversation
Note: with this change and #29898, I'm able to build Node on Windows with VS 2019 and Python 3.7. |
Does I'd RSLGTM it, but it failed in CI. I resumed in case the failures were ephemeral. |
Is that a safe comparison? e.g. What if |
I have no idea. |
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.
4c03164
to
83dc1d7
Compare
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? |
I don't know. I could try to add a github action to this PR that runs on Windows and remove it before landing.
|
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. |
|
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 <cclauss@me.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
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 <cclauss@me.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
PR-URL: #29897 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Landed in 7de5a55...8728f86 |
Commits:
/cc @nodejs/python