Skip to content
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

Provide fallback to Python launcher on Windows when python.exe cannot be found #890

Closed
poke opened this issue Mar 7, 2016 · 6 comments
Closed

Comments

@poke
Copy link
Contributor

poke commented Mar 7, 2016

I regularly get the following error:

Error: Can't find Python executable "python", you can set the PYTHON env variable.

This is because Python on Windows by default does not place a Python executable into the PATH (and for what it’s worth, I would personally also opt out of that). Instead, Python has this great thing on Windows called the Python launcher. It’s an executable that is placed into the WINDIR directory that allows to run any installed Python version on the computer. Even more importantly, the installation of this Python launcher is the default and has been for a number of CPython releases. So for Windows, it’s a much better idea to depend on the Python launcher than on a python executable.

So I’d like to ask for a fallback in node-gyp that attempts to call py.exe -2 before ultimately failing with the above error message. So the logic would be something like this:

  1. Check for PYTHON environment variable; if set, use that executable
  2. Otherwise attempt to call the python executable
  3. Otherwise, if on Windows, attempt to call the py -2 executable
  4. Otherwise show the error, and ask the user to set the PYTHON environment variable

Note that the py.exe launcher should be called with a -2 as its first argument. This ensures that a Python 2 executable is used, regardless of what has been set as the system default. Since node-gyp does not appear to support Python 3 at all yet, this would be the safest solution.

Btw. I also attempted to set the PYTHON environment variable to py -2 to make it use the Python launcher, but that didn’t work: It sadly couldn’t find the py -2 executable (i.e. it didn’t interpret the argument properly). Just using py did work, but called the wrong Python version on my system.

@DbauchdSloth
Copy link

I am at a loss to understand why the added external dependency on a binary installation of python would be required just to build this app? That added dependency adds a huge amount of hassle when trying to deploy (particularly on windows), and there is literally no runtime code that demands it. Is there some reason this is absolutely needed or can I just refactor out the dependency on python using javascript equivalents? Anyone mind or is there something I am missing here? This seems a whole lot easier than trying to solve the problem of deploying python along with the app just to run the build.

@poke
Copy link
Contributor Author

poke commented Mar 12, 2016

I’m not sure what you mean. All I am suggesting here that when a Python executable is required to build things (which seems to be the way pretty often), that instead of just looking for python in the PATH or using the PYTHON environment variable, node-gyp should also attempt to call py -2 on Windows systems before failing. Because that’s how Python will usually be available when it’s installed.

@bnoordhuis
Copy link
Member

@poke What you propose seems reasonable to me but if you want to see it happen, you'll have to do the work. Most of the maintainers don't use Windows regularly or at all.

@DbauchdSloth You either seem to misunderstand the proposal or you're posting what amounts to an off-topic rant.

@poke
Copy link
Contributor Author

poke commented Mar 14, 2016

@bnoordhuis Glad I got some support for the idea, so here is my pull request for this functionality: #894!

@DbauchdSloth
Copy link

Sorry guys! I somehow responded to the node-gyp repository, rather than the correct one, which was using it as a dependency. It was not my intent to rant at all, and least of all to the wrong people ;) Please forgive my confusion

@poke
Copy link
Contributor Author

poke commented Mar 31, 2016

The pull request #894 implementing this was merged in 3bcb172. So I’m going to close this issue :)

@poke poke closed this as completed Mar 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants