classification
Title: invalid docs for subprocess exceptions with shell=True
Type: Stage:
Components: Documentation, Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: cvrebert, docs@python, eric.araujo, ncoghlan, r.david.murray, techtonik
Priority: normal Keywords:

Created on 2012-05-22 06:09 by techtonik, last changed 2012-07-07 10:42 by ncoghlan.

Messages (6)
msg161329 - (view) Author: anatoly techtonik (techtonik) Date: 2012-05-22 06:09
http://docs.python.org/library/subprocess.html#exceptions documentation is wrong at least for the case when shell=True on Linux. An attempt to execute a non-existent file with:

  process = subprocess.Popen("sdfsdf", shell=True, stdout=subprocess.PIPE, stderr=errpipe)
  out, err = process.communicate()
  print out

Results in a message "/bin/sh: sfs: command not found" with no exceptions. Other statements may be invalid too.
msg161384 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-22 22:07
Eh, just needs clarification along the lines of:

    "Exceptions raised in the child ++Python++ process"

    "A ValueError will be raised if Popen is called with invalid arguments
    ++whose validity is not dependent upon the state of the environment++."
msg161385 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-05-22 22:24
I don't think that those additions make anything clearer, I'm afraid.

"child python process" would seem to imply raised in the new python running in the child process, which is clearly not true.

Hmm.  I seem to remember having this discussion before.  The current statement is in fact exactly correct.  When shell=True the new program being run is 'sh' (or similar).  So the error message is coming from the program being run successfully by subprocess.  End of story.

So, we could add a clarfication: "If shell=True, the 'new program' being run is the shell, which may generate its own errors based on the shell command passed to it (such as command not found).  These will not result in exceptions."
msg161394 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-23 05:04
The term "invalid arguments" in "A ValueError will be raised if Popen is called with invalid arguments." is still vague. One could well argue that a nonexistent executable or bad command is "invalid". Anything resulting in an OSError can be considered "invalid" in a sense. The ValueError sentence should use a different descriptor or else include a qualifier.

+1 on calling out the shell=True case.
(My but how the `shell` argument complicates everything...)
msg161404 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-05-23 12:34
Heh.  Maybe what we ought to do is drop the shell argument and make everyone build their own shell invocations :)

Actually, an API refactor where a shell call looks like this might be kind of cool:

  Popen(shell_cmd('echo magic'))

where shell would return a list.

As for the ValueError, yeah, it's vague, but I think that the only information that sentence is trying to convey is that ValueError is one of the exceptions you might get when calling Popen.  Exactly what you get it for is pretty much an implementation detail.  If someone wants to grovel through the code and figure out all the things that can raise ValueError, we could see if there's any sensible way to turn that into a sentence, but it might not be easy.  Short of that, perhaps we could  drop the existing qualifier and just say "Popen may also raise ValueError."
msg164814 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-07-07 10:42
I would add a simple note to the exceptions section:

"Note, when "shell=True", OSError will be raised by the child only if the selected shell itself cannot be found. To determine if the shell failed to find the requested application, it is necessary to check the return code or output from the subprcess."
History
Date User Action Args
2012-07-07 10:42:18ncoghlansetmessages: + msg164814
2012-05-25 18:48:10eric.araujosetnosy: + ncoghlan, eric.araujo
2012-05-23 12:34:36r.david.murraysetmessages: + msg161404
2012-05-23 05:04:58cvrebertsetmessages: + msg161394
2012-05-22 22:24:33r.david.murraysetnosy: + r.david.murray

messages: + msg161385
versions: - Python 2.6, Python 3.1, Python 3.4
2012-05-22 22:07:14cvrebertsetnosy: + cvrebert
messages: + msg161384
2012-05-22 06:09:53techtonikcreate