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

Raise ValueError on no executable and empty args #103233

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

maximxlss
Copy link

Windows throws a cryptic OSError: [WinError 87] when trying to create a process with no executable and empty args, which confused me a lot. Doesn't work on Linux too and contradicts the purpose of Popen (as I understand it), so probably should be disallowed completely.

Windows throws a cryptic error when trying to create a process with no executable and empty args. Make it obvious with a ValueError.
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Apr 4, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@maximxlss maximxlss marked this pull request as draft April 4, 2023 01:20
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

Windows throws a cryptic error when trying to create a process with no executable and empty args. Make it obvious with a ValueError.
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@maximxlss maximxlss marked this pull request as ready for review April 4, 2023 02:01
@arhadthedev arhadthedev added the stdlib Python modules in the Lib dir label Apr 4, 2023
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@arhadthedev
Copy link
Member

Thank you for a patch!

Could you post your exception message with a stack trace, please? On my Windows 10 Home 22H2, I get WinError 3 (no path found):

Running Release|x64 interpreter...
Python 3.12.0a7+ (heads/main-dirty:de18267685, Apr  5 2023, 22:28:44) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from subprocess import Popen
>>> Popen(args=[], executable="")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "D:\Oleg\cpython\Lib\subprocess.py", line 1024, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "D:\Oleg\cpython\Lib\subprocess.py", line 1509, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [WinError 3] Системе не удается найти указанный путь

@maximxlss
Copy link
Author

OSError: [WinError 87] is raised when you do subprocess.Popen([]), which is subprocess.Popen(args=[], executable=None). Happens because CreateProcess gets called with null executable and null args. FileNotFoundError: [WinError 3] does make sense in case of an empty path, I think.

In case of Linux, it causes an IndexError at trying to get the executable from the args.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review OS-windows stdlib Python modules in the Lib dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants