Skip to content
This repository was archived by the owner on Jan 1, 2021. It is now read-only.

add forgotten shell=True to Popen#1

Closed
yaniv-aknin wants to merge 1 commit intodoloopwhile:masterfrom
yaniv-aknin:patch-1
Closed

add forgotten shell=True to Popen#1
yaniv-aknin wants to merge 1 commit intodoloopwhile:masterfrom
yaniv-aknin:patch-1

Conversation

@yaniv-aknin
Copy link

'command' is a shell builtin, and is not available as an external binary on all platforms; you must pass shell=True if you want it to work.

'command' is a shell builtin, and is not available as an external binary on all platforms; you must pass shell=True if you want it to work.
@doloopwhile
Copy link
Owner

Thank you for your fixing. I merged it into my branch and PyPI

@yaniv-aknin
Copy link
Author

Thanks!

@mattjmorrison
Copy link

I'm sorry that I don't know any technical details as to why, but having shell=True breaks the "command" command for OS X.

I'm not sure if there is a more system independent way to check for the existence of a JavaScript runtime, or if there would need to be some system detection logic to determine how to execute the "command" command.

@yaniv-aknin yaniv-aknin reopened this Mar 10, 2012
@yaniv-aknin
Copy link
Author

ugh. To be honest, I ran into further issues with finding runtimes, but didn't have time to author a clean fix and offer a pull request. I forked PyExecJs and did what local hacks I needed to get things going and left it as it is. If @doloopwhile (or anyone else) is interested in fixing this, I suggest implementing a pure-Python implementation of which/command - all it has to do is iterate environ['PATH'].split(':'), looking for the runtime in each.

@mattjmorrison
Copy link

I took a stab at it...

https://github.com/mattjmorrison/PyExecJS/commit/146465ac7ac6d2ee52da4720e1c17b702fb3ab5d

I tried it on Mac OS X and Ubuntu 11.10 and it seemed to work with Node.js installed for both.

@doloopwhile
Copy link
Owner

I pushed new commit. It includes a implementation pure Python "which" based on your idea.
Thanks you !

@yaniv-aknin
Copy link
Author

I really did very little, no sweat.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants