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

#890 Add support for the Python launcher on Windows #894

Closed
wants to merge 1 commit into from

Conversation

poke
Copy link
Contributor

@poke poke commented Mar 14, 2016

This is a pull request for the issue #890: “Provide fallback to Python launcher on Windows when python.exe cannot be found”.

The changes here attempt to have very little impact. The new check is just inserted between the checkPython call (looking for the 'python' executable) and the guessPython call on Windows. Since the Python launcher will give an explicit Python 2 executable, it is more explicit than if we found a match using guessPython, so I gave the launcher solution more priority.

As explained in the commit message, we cannot make a call to py.exe -2 since that wouldn’t work with execFile, so I had to make an actual call to the Python launcher to get the full path of the Python executable. But if the command succeeds, then that reliably gives us a valid Python 2 executable.

If there is no Python 2 version registered with the launcher, the launcher will fail when attempting to call it with -2. When there is no Python launcher, execFile will fail. In both cases, the old next step guessPython will run.

When looking for a Python executable on Windows, before falling back to guessing the default location or failing completely, attempt to use the Python launcher to figure out the location of the Python executable.

The Python launcher is being distributed by default with Python distributions on Windows, and is placed in the %WINDIR% folder (which is in the PATH). This allows us to locate a Python installation even if it was installed without putting the python.exe executable itself into the PATH.

Because the Python launcher supports all versions of Python, we have to explicitly request a Python 2 version. This is done by supplying "-2" as the first command line argument. Since "py.exe -2" would be an invalid executable for "execFile", we have to use the launcher to figure out where the actual "python.exe" executable is located.

When looking for a Python executable on Windows, before falling back to
guessing the default location or failing completely, attempt to use the
Python launcher to figure out the location of the Python executable.

The Python launcher is being distributed by default with Python
distributions on Windows, and is placed in the %WINDIR% folder (which is
in the PATH). This allows us to locate a Python installation even if it
was installed without putting the python.exe executable itself into the
PATH.

Because the Python launcher supports all versions of Python, we have to
explicitly request a Python 2 version. This is done by supplying "-2" as
the first command line argument. Since "py.exe -2" would be an invalid
executable for "execFile", we have to use the launcher to figure out where
the actual "python.exe" executable is located.
var env = extend({}, process.env)
env.TERM = 'dumb'

execFile('py.exe', ['-2', '-c', 'import sys; print sys.executable'], { env: env }, function (err, stdout) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you wrap this line at 80 columns?

@bnoordhuis
Copy link
Member

Mostly LGTM, thanks. Can you verify that npm test still passes and maybe add one or two tests to test/test-find-python.js if possible?

@poke poke force-pushed the py-launcher branch 2 times, most recently from 1292e91 to a228a31 Compare March 24, 2016 17:48
@poke
Copy link
Contributor Author

poke commented Mar 24, 2016

Updated my pull request to wrap at 80 characters; for what it’s worth, the checkPythonVersion a few lines below of my code is happily breaking the 80 characters limit too – do you want me to adjust that too?

As for the test, with my changes, npm test does pass for me. It did not before since the find python executable failed on my machine (for the exact reasons why I am submitting this pull request). As such I’m also not sure how I could add additional tests for finding the python executable since it highly depends on the environment the test is run on.

bnoordhuis pushed a commit that referenced this pull request Mar 31, 2016
When looking for a Python executable on Windows, before falling back to
guessing the default location or failing completely, attempt to use the
Python launcher to figure out the location of the Python executable.

The Python launcher is being distributed by default with Python
distributions on Windows, and is placed in the %WINDIR% folder (which is
in the PATH). This allows us to locate a Python installation even if it
was installed without putting the python.exe executable itself into the
PATH.

Because the Python launcher supports all versions of Python, we have to
explicitly request a Python 2 version. This is done by supplying "-2" as
the first command line argument. Since "py.exe -2" would be an invalid
executable for "execFile", we have to use the launcher to figure out where
the actual "python.exe" executable is located.

PR-URL: #894
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bnoordhuis
Copy link
Member

LGTM and, seeing how there are no comments from other maintainers, I'll go ahead and land this. Thanks Patrick, merged in 3bcb172.

@bnoordhuis bnoordhuis closed this Mar 31, 2016
@poke
Copy link
Contributor Author

poke commented Mar 31, 2016

Glad to see this land, thanks a lot! :)

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

Successfully merging this pull request may close these issues.

2 participants