-
Notifications
You must be signed in to change notification settings - Fork 29.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
Fix Python detection when depot_tools are in PATH in Windows #22539
Conversation
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.
Can confirm: python.bat in PATH breaks vcbuild, this fixes it.
The commit message has to be fixed to adhere to the guidelines (e.g., adding @nodejs/platform-windows @nodejs/build-files PTAL |
@guybedford could you add a inline comment as to why the added |
(This was not |
@addaleax sorry, I missed to start the CI. |
@guybedford You’ll need to rebase this to get rid of the CI failures here, sorry. |
Ping @guybedford |
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/18851/ |
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/18852/ |
PR-URL: nodejs#22539 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Landed in 1d3e40d |
PR-URL: #22539 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: #22539 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: #22539 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: nodejs#22539 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: #22539 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: #22539 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
This fixes the detection of Python for me in Windows.
The issue is I've installed "depot_tools" in path for building v8, and as a result "depot_tools/python.bat" is being found instead and that then causes Node to say it can't find Python.
It's probably only affecting my case, but that is good enough for a PR for me :)
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes