Fixed handling of missing python on Windows. by mvidmar · Pull Request #1325 · nodejs/node-gyp

mvidmar · 2017-11-04T21:04:10Z

Checklist

npm install && npm test passes
tests are included
documentation is changed or added
commit message follows commit guidelines

Description of change

configure: fixing to better handle python badVersion on windows platform

invalid error message when python is found but is invalid version
error returned: file not found but with valid file path which is
confusing. error expected: invalid python version.
in case of invalid version on guessed path (python27) it goes into
an endless loop (unlikely but an ugly error)

Tar version 3 performs better and is more well tested than its predecessor. npm will be using this in the near future, so there is no benefit in shipping a node-gyp that uses the slower and less reliable fstream-based tar. This drops support for node 0.x, and thus should be considered a breaking semver-major change. PR-URL: #1212 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

* dropping support for node < 4 * signal the CI not to test node < 4

If you're providing a path to a header tarball to install, you probably want it to always be re-installed. PR-URL: #1220Fixes: #1216 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Refael Ackermann <refack@gmail.com>

* test: build simple addon in path with non-ascii characters * test: add test-charmap.py PR-URL: #1203 Reviewed-By: Refael Ackermann <refack@gmail.com>

Enable linking to the platform specific installation instructions PR-URL: #1225 Reviewed-By: Refael Ackermann <refack@gmail.com>

Lifted verbatim from https://github.com/nodejs/node/blob/master/CONTRIBUTING.md then `s/Node.js/node-gyp/`. PR-URL: #1229 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Give users reporting bugs a clearer idea of the info that will be helpful when reporting issues. PR-URL: #1228 Refs: https://github.com/nodejs/node/tree/master/.github Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: #1292 Refs: #1195 (comment) Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Initial work to add z/OS support to node-gyp. PR-URL: #1276 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

GYP automatically turns variables ending in _dir, _file or _path into absolute paths but didn't check for empty strings. It interacted badly with variables inherited through the environment from npm, the `scripts-prepend-node-path=false` setting in particular because it is turned into `npm_config_script_prepend_node_path=`. Fixes: #1217 PR-URL: #1267 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

… error message when python is found but invalid version (error returned: file not found with valid file path, expected: invalid python version). 2. in case of invalid version on guessed path (python27) it goes into an endless loop (unlikely but an ugly error)

refack · 2017-11-05T14:53:57Z

CI: https://ci.nodejs.org/job/nodegyp-test-pull-request/41/

refack · 2017-11-05T14:58:29Z

Hello @mvidmar and welcome! Thank you for the fix 🥇

Could you describe the situation that causes the endless recursion (I want to try to design a test around that)?

Also If you could adjust the commit message to our style guide (which is roughly based on node-core's)

mvidmar · 2017-11-05T15:15:44Z

Tried to fix the commit message according to the style guide, let me know if I missed something.

The loop happens on Windows when c:\Python27\python.exe is not a valid version (like I mentioned - unlikely but possible. Happened to me during trying to get an understanding of this code). The loop happens from checkPythonVersion, call to checkPythonLauncher, then guessPython, which finds the executable and then reruns checkPythonVersion. That's why I added an increase of checkPythonLauncherDepth within guessPython so it skips the fallback to checkPythonLauncher. If you remove that line and put an invalid version in c:\Python27\python.exe it loops.

I also merged our fixes in case you want to pull that instead of this fix. Since I noticed you modified some other stuff as well. Here is the merged fix with PR 1269 https://github.com/mvidmar/node-gyp/commit/9c7ad90d5caeb809580ec1620ef3853a677cb503

rvagg · 2019-06-21T03:00:08Z

hi @mvidmar do you still want to pursue this? if so, rebase against master and let's sort out a few style nits and get some windows people involved again

joaocgreis · 2019-07-17T13:44:25Z

The current code to find Python already includes both these features, I think this can be closed. If I missed something, please rebase on current master. Thanks!

mvidmar · 2017-11-04T21:04:10Z

Checklist

npm install && npm test passes
tests are included
documentation is changed or added
commit message follows commit guidelines

Description of change

configure: fixing to better handle python badVersion on windows platform

invalid error message when python is found but is invalid version
error returned: file not found but with valid file path which is
confusing. error expected: invalid python version.
in case of invalid version on guessed path (python27) it goes into
an endless loop (unlikely but an ugly error)

refs #1269