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

Fix wrong subprocess call #16

Closed
wants to merge 1 commit into from
Closed

Fix wrong subprocess call #16

wants to merge 1 commit into from

Conversation

rakjin
Copy link
Contributor

@rakjin rakjin commented Jun 23, 2015

As you know, subprocess.call has some specific requirements on shell and the type of args.

On my windows machine with MINGW32:

>>> import subprocess
>>> subprocess.call(['ls', '-l'], shell=False)
total 0
-rw-r--r--  1 grampush  Administ  0  6 24 01:15 a.out
-rw-r--r--  1 grampush  Administ  0  6 24 01:14 sample.wav
-rw-r--r--  1 grampush  Administ  0  6 24 01:15 zigbee.bin
0
>>> subprocess.call(['ls', '-l'], shell=True)
-rw-r--r--  1 grampush  Administ  0  6 24 01:15 a.out
-rw-r--r--  1 grampush  Administ  0  6 24 01:14 sample.wav
-rw-r--r--  1 grampush  Administ  0  6 24 01:15 zigbee.bin
0
>>> 

However, on OS X:

>>> import subprocess
>>> subprocess.call(['ls', '-l'], shell=False)
total 0
-rw-r--r--  1 grampush  staff  0  6 24 01:15 a.out
-rw-r--r--  1 grampush  staff  0  6 24 01:14 sample.wav
-rw-r--r--  1 grampush  staff  0  6 24 01:15 zigbee.bin
0
>>> subprocess.call(['ls', '-l'], shell=True)
a.out       sample.wav  zigbee.bin
0
>>> 

Only the first item of args is applied to the actual execution, in this case.

I think we should do: call(a_list, shell=False) or call(a_str, shell=True)

@joeyespo
Copy link
Owner

Went with the fix in #18. Going to run some tests to make sure this still works on Windows, but that should take care of the non-Windows cases. Does that fix work for you too?

@joeyespo joeyespo closed this Jun 27, 2015
@rakjin
Copy link
Contributor Author

rakjin commented Jun 28, 2015

#18 works for win7 and OS X both, in my case. also I learnt about subprocess.mswindows 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants