classification
Title: incorrect path in subprocess.Popen() FileNotFoundError message
Type: behavior Stage: commit review
Components: Library (Lib) Versions: Python 3.4, Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: 16115 Superseder:
Assigned To: gregory.p.smith Nosy List: Arfrever, asvetlov, chris.jerdonek, gregory.p.smith, python-dev
Priority: normal Keywords: patch

Created on 2012-10-03 04:16 by chris.jerdonek, last changed 2013-06-26 00:47 by gregory.p.smith. This issue is now closed.

Files
File name Uploaded Description Edit
issue-16114-1-tests-default.patch chris.jerdonek, 2012-10-03 07:35 review
Messages (19)
msg171850 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-03 04:16
The error message in the FileNotFoundError error raised by subprocess.Popen() displays the wrong path when the bad path is due to the executable argument rather than args.  The message gives the path for args[0] rather than for the executable argument.

For example, this--

import subprocess, sys
python_path = sys.executable
p = subprocess.Popen([python_path, "-c", "import sys; sys.exit(1)"])
p.wait()
p = subprocess.Popen([python_path, "-c", "import sys; sys.exit(1)"],
                     executable="foo")
p.wait()

gives--

Traceback (most recent call last):
  File "test-subprocess.py", line 6, in <module>
    executable="foo")
  File ".../Lib/subprocess.py", line 818, in __init__
    restore_signals, start_new_session)
  File ".../Lib/subprocess.py", line 1416, in _execute_child
    raise child_exception_type(errno_num, err_msg)
FileNotFoundError: [Errno 2] No such file or directory: '.../python.exe'

The path in the last line should read "foo" since '.../python.exe' is obviously found as evidenced from the previous Popen() invocation.
msg171851 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-03 04:18
2.7 is not affected because 2.7 makes no attempt to display the path:

OSError: [Errno 2] No such file or directory
msg171852 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-03 04:40
It looks like the error is here:

if issubclass(child_exception_type, OSError) and hex_errno:
    errno_num = int(hex_errno, 16)
    if errno_num != 0:
        err_msg = os.strerror(errno_num)
        if errno_num == errno.ENOENT:
            err_msg += ': ' + repr(args[0])
    raise child_exception_type(errno_num, err_msg)
raise child_exception_type(err_msg)

http://hg.python.org/cpython/file/b40025e37bcd/Lib/subprocess.py#l1415

The "args[0]" should be "executable" when appropriate.
msg171857 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-03 06:23
Adding issue 16115 as a dependency so that the more general case can be settled and committed before dealing with the current issue's more specific (and platform-specific) case.
msg171864 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-03 07:35
Attaching proposed tests.
msg172125 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-05 20:07
Maybe better to fix Windows behavior for unifying FileNotFoundError?
msg172128 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-05 20:15
I will be opening a separate issue to have the same behavior in a future version after this issue is closed.  For existing releases, we don't want to break working code that could be relying on the difference.
msg172129 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-05 20:18
Sorry, I was confusing this issue with issue 15533.  Yes, I support adding the file path to the error message in the Windows implementation, though my preference would be for that to be addressed as part of a separate issue.
msg172135 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-05 20:40
I'm ok with your patch if it will be applied to 3.2 etc and after that new issue will fix Windows problem.
BTW, the patch fails for 3.2 and 3.3 but works for 3.4
msg172456 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-09 07:53
> BTW, the patch fails for 3.2 and 3.3 but works for 3.4

By "patch" do you mean "test"?  And by "works", do you mean fails or succeeds? :)

I haven't prepared a patch yet, but I just started working on it.  On my machine, I found that the test fails as is on 3.3 and 3.4, but on 3.2 it fails for a different reason: the FileNotFoundError needs to be switched to OSError (which was to be expected).
msg172556 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2012-10-10 09:32
replacing repr(args[0]) with repr(executable) in the identified python should be sufficient for this bug as originally reported.

BUT it goes deeper:  I just ran into this error in a different case.  It also happens when cwd is passed and the chdir(cwd) fails in the child process.  args[0] is reported as not found rather than cwd in the error message.

I have a test and fix for that.  I'll take care of both.
msg172561 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-10 10:48
> replacing repr(args[0]) with repr(executable) in the identified python should be sufficient for this bug as originally reported.

Yes, almost.  The executable variable is a bytes object so it needs to be fsdecoded first.

> I have a test and fix for that.  I'll take care of both.

Let me at least upload what I already prepared.  You can add to it or modify it as you see fit.
msg172562 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-10 10:52
New changeset e938fa6be509 by Gregory P. Smith in branch '3.2':
Fixes Issue #16114: The subprocess module no longer provides a
http://hg.python.org/cpython/rev/e938fa6be509

New changeset ee30d7ef70be by Gregory P. Smith in branch '3.3':
Fixes Issue #16114: The subprocess module no longer provides a
http://hg.python.org/cpython/rev/ee30d7ef70be
msg172563 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-10 10:53
New changeset 543bb0e0afb9 by Gregory P. Smith in branch 'default':
Fixes Issue #16114: The subprocess module no longer provides a
http://hg.python.org/cpython/rev/543bb0e0afb9
msg172564 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2012-10-10 10:54
I'm keeping this open until I backport this to subprocess32 for use on Python 2.
msg172567 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-10 11:25
I made some comments on the changes as committed here:

http://mail.python.org/pipermail/python-dev/2012-October/122125.html

"""
It would be cleaner to use the self.assertRaises() pattern here and
also probably better to share code across the three test methods which
are nearly identical to one another (there is a fourth scenario I
would also add of shell=True).

I would also check for FileNotFoundError instead of OSError in the 3.3
and later versions.
"""
msg172570 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-10 12:23
> Maybe better to fix Windows behavior for unifying FileNotFoundError?

I created issue 16185 to include the path in the messages of the corresponding errors on Windows.
msg175323 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2012-11-11 05:20
fyi - I agree with your comments about the test and assertRaises.  This code is old, there's a lot that could be improved in there.  I chose to maintain a style equivalent to the existing surrounding code.

Feel free to clean that up.
msg191893 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2013-06-26 00:47
I backported the fix to this in the subprocess32 3.2.5rc1 release I made a week or two ago.
History
Date User Action Args
2013-06-26 00:47:09gregory.p.smithsetstatus: open -> closed
resolution: fixed
messages: + msg191893
2012-11-11 05:20:16gregory.p.smithsetmessages: + msg175323
2012-10-10 12:23:55chris.jerdoneksetmessages: + msg172570
2012-10-10 11:25:39chris.jerdoneksetmessages: + msg172567
2012-10-10 10:54:48gregory.p.smithsetmessages: + msg172564
stage: needs patch -> commit review
2012-10-10 10:53:22python-devsetmessages: + msg172563
2012-10-10 10:52:44python-devsetnosy: + python-dev
messages: + msg172562
2012-10-10 10:48:25chris.jerdoneksetmessages: + msg172561
2012-10-10 09:32:43gregory.p.smithsetassignee: chris.jerdonek -> gregory.p.smith

messages: + msg172556
nosy: + gregory.p.smith
2012-10-09 07:56:00chris.jerdoneksetassignee: chris.jerdonek
2012-10-09 07:54:00chris.jerdoneksetmessages: + msg172456
2012-10-05 20:40:05asvetlovsetmessages: + msg172135
2012-10-05 20:18:33chris.jerdoneksetmessages: + msg172129
2012-10-05 20:15:16chris.jerdoneksetmessages: + msg172128
2012-10-05 20:07:26asvetlovsetmessages: + msg172125
2012-10-03 12:51:03Arfreversetnosy: + Arfrever
2012-10-03 07:35:40chris.jerdoneksetfiles: + issue-16114-1-tests-default.patch
keywords: + patch
messages: + msg171864

stage: test needed -> needs patch
2012-10-03 06:23:32chris.jerdoneksetdependencies: + test the executable arg to Popen() and improve related docs
messages: + msg171857
2012-10-03 04:40:11chris.jerdoneksetmessages: + msg171852
2012-10-03 04:18:55chris.jerdoneksetmessages: + msg171851
versions: + Python 3.2
2012-10-03 04:16:47chris.jerdonekcreate