build,win: replace run-python subroutine with single find_python call #18621
Conversation
A subroutine does not work as a replacement for the `python` command since one cannot use a subroutine call in a `for /F` loop.
| @@ -165,6 +165,9 @@ if "%target%"=="Clean" echo deleting %~dp0deps\icu | |||
| if "%target%"=="Clean" rmdir /S /Q %~dp0deps\icu | |||
| :no-depsicu | |||
|
|
|||
| call tools\msvs\find_python.cmd | |||
There was a problem hiding this comment.
Maybe redirect to NUL, to avoid ERROR: The system was unable to find the specified registry key or value.?
There was a problem hiding this comment.
I don't really like combining unrelated fixes in a single PR (this line was just moved from :run-python). Plus, I think it would be better to fix this inside find_python.cmd itself.
|
Failures are unrelated. |
|
...but the commit title is to too long. @seishun, could you make the commit title <50 chars long? |
|
I don't think so, and it's not a requirement: https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines
|
|
Landed in cfad441 |
A subroutine does not work as a replacement for the `python` command since one cannot use a subroutine call in a `for /F` loop. PR-URL: #18621 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
A subroutine does not work as a replacement for the `python` command since one cannot use a subroutine call in a `for /F` loop. PR-URL: #18621 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
A subroutine does not work as a replacement for the `python` command since one cannot use a subroutine call in a `for /F` loop. PR-URL: #18621 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
A subroutine does not work as a replacement for the `python` command since one cannot use a subroutine call in a `for /F` loop. PR-URL: nodejs#18621 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
|
Should this be backported to |
|
@MylesBorins seems pointless. |
PR-URL: nodejs#41235 Refs: nodejs#18621 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
A subroutine does not work as a replacement for the
pythoncommandsince one cannot use a subroutine call in a
for /Floop.This is an alternative to #17293 and #17804. I realized that
find_python.cmdadds Python toPATH, so there is no need to introduce a variable.I still slightly prefer #17293 for the reasons outlined in #17804 (basically, extra maintenance), but I prefer this PR to #17804 since it doesn't introduce an extra variable.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
build