diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 2680a73603..9cadd1bf8e 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -43,6 +43,7 @@ import builtins import errno import io +import locale import os import time import signal @@ -65,16 +66,19 @@ # NOTE: We intentionally exclude list2cmdline as it is # considered an internal implementation detail. issue10838. +# use presence of msvcrt to detect Windows-like platforms (see bpo-8110) try: import msvcrt - import _winapi - _mswindows = True except ModuleNotFoundError: _mswindows = False - import _posixsubprocess - import select - import selectors else: + _mswindows = True + +# wasm32-emscripten and wasm32-wasi do not support processes +_can_fork_exec = sys.platform not in {"emscripten", "wasi"} + +if _mswindows: + import _winapi from _winapi import (CREATE_NEW_CONSOLE, CREATE_NEW_PROCESS_GROUP, STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, STD_ERROR_HANDLE, SW_HIDE, @@ -95,6 +99,24 @@ "NORMAL_PRIORITY_CLASS", "REALTIME_PRIORITY_CLASS", "CREATE_NO_WINDOW", "DETACHED_PROCESS", "CREATE_DEFAULT_ERROR_MODE", "CREATE_BREAKAWAY_FROM_JOB"]) +else: + if _can_fork_exec: + from _posixsubprocess import fork_exec as _fork_exec + # used in methods that are called by __del__ + _waitpid = os.waitpid + _waitstatus_to_exitcode = os.waitstatus_to_exitcode + _WIFSTOPPED = os.WIFSTOPPED + _WSTOPSIG = os.WSTOPSIG + _WNOHANG = os.WNOHANG + else: + _fork_exec = None + _waitpid = None + _waitstatus_to_exitcode = None + _WIFSTOPPED = None + _WSTOPSIG = None + _WNOHANG = None + import select + import selectors # Exception classes used by this module. @@ -207,8 +229,7 @@ def Detach(self): def __repr__(self): return "%s(%d)" % (self.__class__.__name__, int(self)) - # XXX: RustPython; OSError('The handle is invalid. (os error 6)') - # __del__ = Close + __del__ = Close else: # When select or poll has indicated that the file is writable, # we can write up to _PIPE_BUF bytes without risk of blocking. @@ -303,12 +324,14 @@ def _args_from_interpreter_flags(): args.append('-E') if sys.flags.no_user_site: args.append('-s') + if sys.flags.safe_path: + args.append('-P') # -W options warnopts = sys.warnoptions[:] - bytes_warning = sys.flags.bytes_warning xoptions = getattr(sys, '_xoptions', {}) - dev_mode = ('dev' in xoptions) + bytes_warning = sys.flags.bytes_warning + dev_mode = sys.flags.dev_mode if bytes_warning > 1: warnopts.remove("error::BytesWarning") @@ -335,6 +358,26 @@ def _args_from_interpreter_flags(): return args +def _text_encoding(): + # Return default text encoding and emit EncodingWarning if + # sys.flags.warn_default_encoding is true. + if sys.flags.warn_default_encoding: + f = sys._getframe() + filename = f.f_code.co_filename + stacklevel = 2 + while f := f.f_back: + if f.f_code.co_filename != filename: + break + stacklevel += 1 + warnings.warn("'encoding' argument not specified.", + EncodingWarning, stacklevel) + + if sys.flags.utf8_mode: + return "utf-8" + else: + return locale.getencoding() + + def call(*popenargs, timeout=None, **kwargs): """Run command with arguments. Wait for command to complete or timeout, then return the returncode attribute. @@ -406,13 +449,15 @@ def check_output(*popenargs, timeout=None, **kwargs): decoded according to locale encoding, or by "encoding" if set. Text mode is triggered by setting any of text, encoding, errors or universal_newlines. """ - if 'stdout' in kwargs: - raise ValueError('stdout argument not allowed, it will be overridden.') + for kw in ('stdout', 'check'): + if kw in kwargs: + raise ValueError(f'{kw} argument not allowed, it will be overridden.') if 'input' in kwargs and kwargs['input'] is None: # Explicitly passing input=None was previously equivalent to passing an # empty string. That is maintained here for backwards compatibility. - if kwargs.get('universal_newlines') or kwargs.get('text'): + if kwargs.get('universal_newlines') or kwargs.get('text') or kwargs.get('encoding') \ + or kwargs.get('errors'): empty = '' else: empty = b'' @@ -464,7 +509,8 @@ def run(*popenargs, The returned instance will have attributes args, returncode, stdout and stderr. By default, stdout and stderr are not captured, and those attributes - will be None. Pass stdout=PIPE and/or stderr=PIPE in order to capture them. + will be None. Pass stdout=PIPE and/or stderr=PIPE in order to capture them, + or pass capture_output=True to capture both. If check is True and the exit code was non-zero, it raises a CalledProcessError. The CalledProcessError object will have the return code @@ -600,7 +646,7 @@ def list2cmdline(seq): # Various tools for executing commands and looking at their output and status. # -def getstatusoutput(cmd): +def getstatusoutput(cmd, *, encoding=None, errors=None): """Return (exitcode, output) of executing cmd in a shell. Execute the string 'cmd' in a shell with 'check_output' and @@ -622,7 +668,8 @@ def getstatusoutput(cmd): (-15, '') """ try: - data = check_output(cmd, shell=True, text=True, stderr=STDOUT) + data = check_output(cmd, shell=True, text=True, stderr=STDOUT, + encoding=encoding, errors=errors) exitcode = 0 except CalledProcessError as ex: data = ex.output @@ -631,7 +678,7 @@ def getstatusoutput(cmd): data = data[:-1] return exitcode, data -def getoutput(cmd): +def getoutput(cmd, *, encoding=None, errors=None): """Return output (stdout or stderr) of executing cmd in a shell. Like getstatusoutput(), except the exit status is ignored and the return @@ -641,7 +688,8 @@ def getoutput(cmd): >>> subprocess.getoutput('ls /bin/ls') '/bin/ls' """ - return getstatusoutput(cmd)[1] + return getstatusoutput(cmd, encoding=encoding, errors=errors)[1] + def _use_posix_spawn(): @@ -736,6 +784,8 @@ class Popen: start_new_session (POSIX only) + process_group (POSIX only) + group (POSIX only) extra_groups (POSIX only) @@ -761,8 +811,14 @@ def __init__(self, args, bufsize=-1, executable=None, startupinfo=None, creationflags=0, restore_signals=True, start_new_session=False, pass_fds=(), *, user=None, group=None, extra_groups=None, - encoding=None, errors=None, text=None, umask=-1, pipesize=-1): + encoding=None, errors=None, text=None, umask=-1, pipesize=-1, + process_group=None): """Create new Popen instance.""" + if not _can_fork_exec: + raise OSError( + errno.ENOTSUP, f"{sys.platform} does not support processes." + ) + _cleanup() # Held while anything is calling waitpid before returncode has been # updated to prevent clobbering returncode if wait() or poll() are @@ -848,15 +904,8 @@ def __init__(self, args, bufsize=-1, executable=None, errread = msvcrt.open_osfhandle(errread.Detach(), 0) self.text_mode = encoding or errors or text or universal_newlines - - # PEP 597: We suppress the EncodingWarning in subprocess module - # for now (at Python 3.10), because we focus on files for now. - # This will be changed to encoding = io.text_encoding(encoding) - # in the future. if self.text_mode and encoding is None: - # TODO: RUSTPYTHON; encoding `locale` is not supported yet - pass - # self.encoding = encoding = "locale" + self.encoding = encoding = _text_encoding() # How long to resume waiting on a child after the first ^C. # There is no right value for this. The purpose is to be polite @@ -874,6 +923,9 @@ def __init__(self, args, bufsize=-1, executable=None, else: line_buffering = False + if process_group is None: + process_group = -1 # The internal APIs are int-only + gid = None if group is not None: if not hasattr(os, 'setregid'): @@ -977,7 +1029,7 @@ def __init__(self, args, bufsize=-1, executable=None, errread, errwrite, restore_signals, gid, gids, uid, umask, - start_new_session) + start_new_session, process_group) except: # Cleanup if the child failed starting. for f in filter(None, (self.stdin, self.stdout, self.stderr)): @@ -1285,11 +1337,7 @@ def _get_handles(self, stdin, stdout, stderr): else: # Assuming file-like object p2cread = msvcrt.get_osfhandle(stdin.fileno()) - # XXX RUSTPYTHON TODO: figure out why closing these old, non-inheritable - # pipe handles is necessary for us, but not CPython - old = p2cread p2cread = self._make_inheritable(p2cread) - if stdin == PIPE: _winapi.CloseHandle(old) if stdout is None: c2pwrite = _winapi.GetStdHandle(_winapi.STD_OUTPUT_HANDLE) @@ -1307,11 +1355,7 @@ def _get_handles(self, stdin, stdout, stderr): else: # Assuming file-like object c2pwrite = msvcrt.get_osfhandle(stdout.fileno()) - # XXX RUSTPYTHON TODO: figure out why closing these old, non-inheritable - # pipe handles is necessary for us, but not CPython - old = c2pwrite c2pwrite = self._make_inheritable(c2pwrite) - if stdout == PIPE: _winapi.CloseHandle(old) if stderr is None: errwrite = _winapi.GetStdHandle(_winapi.STD_ERROR_HANDLE) @@ -1331,11 +1375,7 @@ def _get_handles(self, stdin, stdout, stderr): else: # Assuming file-like object errwrite = msvcrt.get_osfhandle(stderr.fileno()) - # XXX RUSTPYTHON TODO: figure out why closing these old, non-inheritable - # pipe handles is necessary for us, but not CPython - old = errwrite errwrite = self._make_inheritable(errwrite) - if stderr == PIPE: _winapi.CloseHandle(old) return (p2cread, p2cwrite, c2pread, c2pwrite, @@ -1373,7 +1413,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, unused_restore_signals, unused_gid, unused_gids, unused_uid, unused_umask, - unused_start_new_session): + unused_start_new_session, unused_process_group): """Execute program (MS Windows version)""" assert not pass_fds, "pass_fds not supported on Windows." @@ -1705,7 +1745,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, errread, errwrite, restore_signals, gid, gids, uid, umask, - start_new_session): + start_new_session, process_group): """Execute program (POSIX version)""" if isinstance(args, (str, bytes)): @@ -1741,6 +1781,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, and (c2pwrite == -1 or c2pwrite > 2) and (errwrite == -1 or errwrite > 2) and not start_new_session + and process_group == -1 and gid is None and gids is None and uid is None @@ -1790,7 +1831,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, for dir in os.get_exec_path(env)) fds_to_keep = set(pass_fds) fds_to_keep.add(errpipe_write) - self.pid = _posixsubprocess.fork_exec( + self.pid = _fork_exec( args, executable_list, close_fds, tuple(sorted(map(int, fds_to_keep))), cwd, env_list, @@ -1798,8 +1839,8 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, errread, errwrite, errpipe_read, errpipe_write, restore_signals, start_new_session, - gid, gids, uid, umask, - preexec_fn) + process_group, gid, gids, uid, umask, + preexec_fn, _USE_VFORK) self._child_created = True finally: # be sure the FD is closed no matter what @@ -1862,19 +1903,19 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, def _handle_exitstatus(self, sts, - waitstatus_to_exitcode=os.waitstatus_to_exitcode, - _WIFSTOPPED=os.WIFSTOPPED, - _WSTOPSIG=os.WSTOPSIG): + _waitstatus_to_exitcode=_waitstatus_to_exitcode, + _WIFSTOPPED=_WIFSTOPPED, + _WSTOPSIG=_WSTOPSIG): """All callers to this function MUST hold self._waitpid_lock.""" # This method is called (indirectly) by __del__, so it cannot # refer to anything outside of its local scope. if _WIFSTOPPED(sts): self.returncode = -_WSTOPSIG(sts) else: - self.returncode = waitstatus_to_exitcode(sts) + self.returncode = _waitstatus_to_exitcode(sts) - def _internal_poll(self, _deadstate=None, _waitpid=os.waitpid, - _WNOHANG=os.WNOHANG, _ECHILD=errno.ECHILD): + def _internal_poll(self, _deadstate=None, _waitpid=_waitpid, + _WNOHANG=_WNOHANG, _ECHILD=errno.ECHILD): """Check if child process has terminated. Returns returncode attribute. @@ -2105,7 +2146,7 @@ def send_signal(self, sig): try: os.kill(self.pid, sig) except ProcessLookupError: - # Supress the race condition error; bpo-40550. + # Suppress the race condition error; bpo-40550. pass def terminate(self): diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 2d263fa865..0a72ffade8 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -48,6 +48,9 @@ if support.PGO: raise unittest.SkipTest("test is not helpful for PGO") +if not support.has_subprocess_support: + raise unittest.SkipTest("test module requires subprocess") + mswindows = (sys.platform == "win32") # @@ -171,6 +174,14 @@ def test_check_output(self): [sys.executable, "-c", "print('BDFL')"]) self.assertIn(b'BDFL', output) + with self.assertRaisesRegex(ValueError, + "stdout argument not allowed, it will be overridden"): + subprocess.check_output([], stdout=None) + + with self.assertRaisesRegex(ValueError, + "check argument not allowed, it will be overridden"): + subprocess.check_output([], check=False) + def test_check_output_nonzero(self): # check_call() function with non-zero return code with self.assertRaises(subprocess.CalledProcessError) as c: @@ -227,6 +238,12 @@ def test_check_output_input_none_universal_newlines(self): input=None, universal_newlines=True) self.assertNotIn('XX', output) + def test_check_output_input_none_encoding_errors(self): + output = subprocess.check_output( + [sys.executable, "-c", "print('foo')"], + input=None, encoding='utf-8', errors='ignore') + self.assertIn('foo', output) + def test_check_output_stdout_arg(self): # check_output() refuses to accept 'stdout' argument with self.assertRaises(ValueError) as c: @@ -719,6 +736,8 @@ def test_pipesizes(self): # However, this function is not yet in _winapi. p.stdin.write(b"pear") p.stdin.close() + p.stdout.close() + p.stderr.close() finally: p.kill() p.wait() @@ -746,6 +765,8 @@ def test_pipesize_default(self): # On other platforms we cannot test the pipe size (yet). But above # code using pipesize=-1 should not crash. p.stdin.close() + p.stdout.close() + p.stderr.close() finally: p.kill() p.wait() @@ -1547,6 +1568,22 @@ def test_class_getitems(self): self.assertIsInstance(subprocess.Popen[bytes], types.GenericAlias) self.assertIsInstance(subprocess.CompletedProcess[str], types.GenericAlias) + @unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"), + "vfork() not enabled by configure.") + @mock.patch("subprocess._fork_exec") + def test__use_vfork(self, mock_fork_exec): + self.assertTrue(subprocess._USE_VFORK) # The default value regardless. + mock_fork_exec.side_effect = RuntimeError("just testing args") + with self.assertRaises(RuntimeError): + subprocess.run([sys.executable, "-c", "pass"]) + mock_fork_exec.assert_called_once() + self.assertTrue(mock_fork_exec.call_args.args[-1]) + with mock.patch.object(subprocess, '_USE_VFORK', False): + with self.assertRaises(RuntimeError): + subprocess.run([sys.executable, "-c", "pass"]) + self.assertFalse(mock_fork_exec.call_args_list[-1].args[-1]) + + class RunFuncTestCase(BaseTestCase): def run_python(self, code, **kwargs): """Run Python code in a subprocess using subprocess.run""" @@ -1721,27 +1758,20 @@ def test_run_with_shell_timeout_and_capture_output(self): msg="TimeoutExpired was delayed! Bad traceback:\n```\n" f"{stacks}```") - @unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"), - "vfork() not enabled by configure.") - def test__use_vfork(self): - # Attempts code coverage within _posixsubprocess.c on the code that - # probes the subprocess module for the existence and value of this - # attribute in 3.10.5. - self.assertTrue(subprocess._USE_VFORK) # The default value regardless. - with mock.patch.object(subprocess, "_USE_VFORK", False): - self.assertEqual(self.run_python("pass").returncode, 0, - msg="False _USE_VFORK failed") - - class RaisingBool: - def __bool__(self): - raise RuntimeError("force PyObject_IsTrue to return -1") - - with mock.patch.object(subprocess, "_USE_VFORK", RaisingBool()): - self.assertEqual(self.run_python("pass").returncode, 0, - msg="odd bool()-error _USE_VFORK failed") - del subprocess._USE_VFORK - self.assertEqual(self.run_python("pass").returncode, 0, - msg="lack of a _USE_VFORK attribute failed") + # TODO: RUSTPYTHON + @unittest.expectedFailure + def test_encoding_warning(self): + code = textwrap.dedent("""\ + from subprocess import * + run("echo hello", shell=True, text=True) + check_output("echo hello", shell=True, text=True) + """) + cp = subprocess.run([sys.executable, "-Xwarn_default_encoding", "-c", code], + capture_output=True) + lines = cp.stderr.splitlines() + self.assertEqual(len(lines), 2, lines) + self.assertTrue(lines[0].startswith(b":2: EncodingWarning: ")) + self.assertTrue(lines[1].startswith(b":3: EncodingWarning: ")) def _get_test_grp_name(): @@ -1837,7 +1867,7 @@ class PopenNoDestructor(subprocess.Popen): def __del__(self): pass - @mock.patch("subprocess._posixsubprocess.fork_exec") + @mock.patch("subprocess._fork_exec") def test_exception_errpipe_normal(self, fork_exec): """Test error passing done through errpipe_write in the good case""" def proper_error(*args): @@ -1854,7 +1884,7 @@ def proper_error(*args): with self.assertRaises(IsADirectoryError): self.PopenNoDestructor(["non_existent_command"]) - @mock.patch("subprocess._posixsubprocess.fork_exec") + @mock.patch("subprocess._fork_exec") def test_exception_errpipe_bad_data(self, fork_exec): """Test error passing done through errpipe_write where its not in the expected format""" @@ -1910,14 +1940,34 @@ def test_start_new_session(self): output = subprocess.check_output( [sys.executable, "-c", "import os; print(os.getsid(0))"], start_new_session=True) - except OSError as e: + except PermissionError as e: if e.errno != errno.EPERM: - raise + raise # EACCES? else: parent_sid = os.getsid(0) child_sid = int(output) self.assertNotEqual(parent_sid, child_sid) + # TODO: RUSTPYTHON + @unittest.expectedFailure + @unittest.skipUnless(hasattr(os, 'setpgid') and hasattr(os, 'getpgid'), + 'no setpgid or getpgid on platform') + def test_process_group_0(self): + # For code coverage of calling setpgid(). We don't care if we get an + # EPERM error from it depending on the test execution environment, that + # still indicates that it was called. + try: + output = subprocess.check_output( + [sys.executable, "-c", "import os; print(os.getpgid(0))"], + process_group=0) + except PermissionError as e: + if e.errno != errno.EPERM: + raise # EACCES? + else: + parent_pgid = os.getpgid(0) + child_pgid = int(output) + self.assertNotEqual(parent_pgid, child_pgid) + # TODO: RUSTPYTHON @unittest.expectedFailure @unittest.skipUnless(hasattr(os, 'setreuid'), 'no setreuid on platform') @@ -2158,7 +2208,7 @@ def raise_it(): preexec_fn=raise_it) except subprocess.SubprocessError as e: self.assertTrue( - subprocess._posixsubprocess, + subprocess._fork_exec, "Expected a ValueError from the preexec_fn") except ValueError as e: self.assertIn("coconut", e.args[0]) @@ -2654,11 +2704,11 @@ def prepare(): preexec_fn=prepare) except ValueError as err: # Pure Python implementations keeps the message - self.assertIsNone(subprocess._posixsubprocess) + self.assertIsNone(subprocess._fork_exec) self.assertEqual(str(err), "surrogate:\uDCff") except subprocess.SubprocessError as err: # _posixsubprocess uses a default message - self.assertIsNotNone(subprocess._posixsubprocess) + self.assertIsNotNone(subprocess._fork_exec) self.assertEqual(str(err), "Exception occurred in preexec_fn.") else: self.fail("Expected ValueError or subprocess.SubprocessError") @@ -3165,9 +3215,9 @@ def test_fork_exec(self): True, (), cwd, env_list, -1, -1, -1, -1, 1, 2, 3, 4, - True, True, + True, True, 0, False, [], 0, -1, - func) + func, False) # Attempt to prevent # "TypeError: fork_exec() takes exactly N arguments (M given)" # from passing the test. More refactoring to have us start @@ -3214,9 +3264,9 @@ def __int__(self): True, fds_to_keep, None, [b"env"], -1, -1, -1, -1, 1, 2, 3, 4, - True, True, + True, True, 0, None, None, None, -1, - None) + None, "no vfork") self.assertIn('fds_to_keep', str(c.exception)) finally: if not gc_enabled: @@ -3319,6 +3369,7 @@ def test_send_signal_race2(self): with mock.patch.object(p, 'poll', new=lambda: None): p.returncode = None p.send_signal(signal.SIGTERM) + p.kill() def test_communicate_repeated_call_after_stdout_close(self): proc = subprocess.Popen([sys.executable, '-c', diff --git a/stdlib/src/posixsubprocess.rs b/stdlib/src/posixsubprocess.rs index c60cd8f3ac..5f8cb3f5b9 100644 --- a/stdlib/src/posixsubprocess.rs +++ b/stdlib/src/posixsubprocess.rs @@ -48,6 +48,7 @@ mod _posixsubprocess { macro_rules! gen_args { ($($field:ident: $t:ty),*$(,)?) => { + #[allow(dead_code)] #[derive(FromArgs)] struct ForkExecArgs { $(#[pyarg(positional)] $field: $t,)* @@ -66,14 +67,31 @@ impl TryFromObject for CStrPathLike { } gen_args! { - args: ArgSequence /* list */, exec_list: ArgSequence /* list */, - close_fds: bool, fds_to_keep: ArgSequence, - cwd: Option, env_list: Option>, - p2cread: i32, p2cwrite: i32, c2pread: i32, c2pwrite: i32, - errread: i32, errwrite: i32, errpipe_read: i32, errpipe_write: i32, - restore_signals: bool, call_setsid: bool, - gid: Option>, groups_list: Option, uid: Option>, child_umask: i32, + args: ArgSequence /* list */, + exec_list: ArgSequence /* list */, + close_fds: bool, + fds_to_keep: ArgSequence, + cwd: Option, + env_list: Option>, + p2cread: i32, + p2cwrite: i32, + c2pread: i32, + c2pwrite: i32, + errread: i32, + errwrite: i32, + errpipe_read: i32, + errpipe_write: i32, + restore_signals: bool, + call_setsid: bool, + // TODO: Difference between gid_to_set and gid_object. + // One is a `gid_t` and the other is a `PyObject` in CPython. + gid_to_set: Option>, + gid_object: PyObjectRef, + groups_list: Option, + uid: Option>, + child_umask: i32, preexec_fn: Option, + use_vfork: bool, } // can't reallocate inside of exec(), so we reallocate prior to fork() and pass this along @@ -160,7 +178,7 @@ fn exec_inner(args: &ForkExecArgs, procargs: ProcArgs) -> nix::Result { // unistd::setgroups(groups_size, groups); } - if let Some(_gid) = args.gid.as_ref() { + if let Some(_gid) = args.gid_to_set.as_ref() { // TODO: setgid // unistd::setregid(gid, gid)?; }