Merge pull request #884 from gevent/simplify-subprocess

Simplify subprocess.py: Unify many py2/py3 branches

changelog.rst

@@ -14,7 +14,8 @@ Incompatible Changes
   and has been deprecated since 1.0b2.
 - The internal implementation modules ``gevent.corecext`` and
   ``gevent.corecffi`` have been moved. Please import from
-  ``gevent.core`` instead.
+  ``gevent.core`` instead; this has always been the only documented place to
+  import from.
 
 Libraries
 ---------
@@ -31,7 +32,8 @@ Libraries
   installation time. Previously, if it wasn't available, a build was
   attempted at every import. This could lead to scattered "gevent"
   directories and undependable results.
-- Update Cython to 0.24.
+- Update Cython to 0.24. Cython 0.25 beta is known to work and will
+  probably be used by a future 1.2 release.
 - setuptools is now required at build time on all platforms.
   Previously it was only required for Windows and PyPy.
 - POSIX: Don't hardcode ``/bin/sh`` into the configuration command
@@ -76,6 +78,14 @@ Stdlib Compatibility
 - The modules :mod:`gevent.os`, :mod:`gevent.signal` and
   :mod:`gevent.select` export all the attributes from their
   corresponding standard library counterpart.
+- Python 2: ``reload(site)`` no longer fails with a ``TypeError`` if
+  gevent has been imported. Reported in :issue:`805` by Jake Hilton.
+- Python 2: ``sendall`` on a non-blocking socket could spuriously fail
+  with a timeout.
+
+select/poll
+~~~~~~~~~~~
+
 - If :func:`gevent.select.select` is given a negative *timeout*
   argument, raise an exception like the standard library does.
 - If :func:`gevent.select.select` is given closed or invalid
@@ -92,10 +102,11 @@ Stdlib Compatibility
 - :meth:`gevent.select.poll.poll` returns an event with
   ``POLLNVAL`` for registered fds that are invalid. Previously it
   would tend to report both read and write events.
-- Python 2: ``reload(site)`` no longer fails with a ``TypeError`` if
-  gevent has been imported. Reported in :issue:`805` by Jake Hilton.
-- Python 2: ``sendall`` on a non-blocking socket could spuriously fail
-  with a timeout.
+
+
+File objects
+~~~~~~~~~~~~
+
 - ``FileObjectPosix`` exposes the ``read1`` method when in read mode,
   and generally only exposes methods appropriate to the mode it is in.
 - ``FileObjectPosix`` supports a *bufsize* of 0 in binary write modes.
@@ -105,18 +116,7 @@ Stdlib Compatibility
   returning the errno due to the refactoring of the exception
   hierarchy in Python 3.3. Now the errno is returned. Reported in
   :issue:`841` by Dana Powers.
-- Setting SIGCHLD to SIG_IGN or SIG_DFL after :mod:`gevent.subprocess`
-  had been used previously could not be reversed, causing
-  ``Popen.wait`` and other calls to hang. Now, if SIGCHLD has been
-  ignored, the next time :mod:`gevent.subprocess` is used this will be
-  detected and corrected automatically. (This potentially leads to
-  issues with :func:`os.popen` on Python 2, but the signal can always
-  be reset again. Mixing the low-level process handling calls,
-  low-level signal management and high-level use of
-  :mod:`gevent.subprocess` is tricky.) Reported in :issue:`857` by
-  Chris Utz.
-- ``Popen.kill`` and ``send_signal`` no longer attempt to send signals
-  to processes that are known to be exited.
+
 
 Other Changes
 -------------
@@ -124,11 +124,33 @@ Other Changes
 - :class:`~.Group` and :class:`~.Pool` now return whether
   :meth:`~.Group.join` returned with an empty group. Suggested by Filippo Sironi in
   :pr:`503`.
-- Servers: Default to AF_INET6 when binding to all addresses (e.g.,
-  ""). This supports both IPv4 and IPv6 connections (except on
-  Windows). Original change in :pr:`495` by Felix Kaiser.
 - Unhandled exception reports that kill a greenlet now include a
   timestamp. See :issue:`137`.
+- :class:`~.PriorityQueue` now ensures that an initial items list is a
+  valid heap. Fixed in :pr:`793` by X.C.Dong.
+- :class:`gevent.hub.signal` (aka :func:`gevent.signal`) now verifies
+  that its `handler` argument is callable, raising a :exc:`TypeError`
+  if it isn't. Reported in :issue:`818` by Peter Renström.
+- If ``sys.stderr`` has been monkey-patched (not recommended),
+  exceptions that the hub reports aren't lost and can still be caught.
+  Reported in :issue:`825` by Jelle Smet.
+- The :func:`gevent.os.waitpid` function is cooperative in more
+  circumstances. Reported in :issue:`878` by Heungsub Lee.
+- The various ``FileObject`` implementations are more consistent with
+  each other. **Note:** Writing to the *io* property of a FileObject should be
+  considered deprecated.
+
+Servers
+~~~~~~~
+- Default to AF_INET6 when binding to all addresses (e.g.,
+  ""). This supports both IPv4 and IPv6 connections (except on
+  Windows). Original change in :pr:`495` by Felix Kaiser.
+- pywsgi/performance: Chunks of data the application returns are no longer copied
+  before being sent to the socket when the transfer-encoding is
+  chunked, potentially reducing overhead for large responses.
+
+Threads
+~~~~~~~
 - Add :class:`gevent.threadpool.ThreadPoolExecutor` (a
   :class:`concurrent.futures.ThreadPoolExecutor` variant that always
   uses native threads even when the system has been monkey-patched)
@@ -139,11 +161,9 @@ Other Changes
 - Native threads created before monkey-patching threading can now be
   joined. Previously on Python < 3.4, doing so would raise a
   ``LoopExit`` error. Reported in :issue:`747` by Sergey Vasilyev.
-- pywsgi/performance: Chunks of data the application returns are no longer copied
-  before being sent to the socket when the transfer-encoding is
-  chunked, potentially reducing overhead for large responses.
-- :class:`~.PriorityQueue` now ensures that an initial items list is a
-  valid heap. Fixed in :pr:`793` by X.C.Dong.
+
+SSL
+~~~
 - On Python 2.7.9 and above (more generally, when the SSL backport is
   present in Python 2), :func:`gevent.ssl.get_server_certificate`
   would raise a :exc:`ValueError` if the system wasn't monkey-patched.
@@ -152,30 +172,48 @@ Other Changes
   while it's being read from or written to in a different greenlet is
   less likely to raise a :exc:`TypeError` instead of a
   :exc:`ValueError`. Reported in :issue:`800` by Kevin Chen.
-- :class:`gevent.hub.signal` (aka :func:`gevent.signal`) now verifies
-  that its `handler` argument is callable, raising a :exc:`TypeError`
-  if it isn't. Reported in :issue:`818` by Peter Renström.
-- If ``sys.stderr`` has been monkey-patched (not recommended),
-  exceptions that the hub reports aren't lost and can still be caught.
-  Reported in :issue:`825` by Jelle Smet.
-- The various ``FileObject`` implementations are more consistent with
-  each other.
 
-  .. note:: Writing to the *io* property of a FileObject should be
-            considered deprecated after it is constructed.
-- The :func:`gevent.os.waitpid` function is cooperative in more
-  circumstances. Reported in :issue:`878` by Heungsub Lee.
+
+subprocess module
+~~~~~~~~~~~~~~~~~
+
+- Setting SIGCHLD to SIG_IGN or SIG_DFL after :mod:`gevent.subprocess`
+  had been used previously could not be reversed, causing
+  ``Popen.wait`` and other calls to hang. Now, if SIGCHLD has been
+  ignored, the next time :mod:`gevent.subprocess` is used this will be
+  detected and corrected automatically. (This potentially leads to
+  issues with :func:`os.popen` on Python 2, but the signal can always
+  be reset again. Mixing the low-level process handling calls,
+  low-level signal management and high-level use of
+  :mod:`gevent.subprocess` is tricky.) Reported in :issue:`857` by
+  Chris Utz.
+- ``Popen.kill`` and ``send_signal`` no longer attempt to send signals
+  to processes that are known to be exited.
+
+Several backwards compatible updates to the subprocess module have
+been backported from Python 3 to Python 2, making
+:mod:`gevent.subprocess` smaller, easier to maintain and in some cases
+safer.
+
+- Popen objects can be used as context managers even on Python 2. The
+  high-level API functions (``call``, etc) use this for added safety.
 - The :mod:`gevent.subprocess` module now provides the
   :func:`gevent.subprocess.run` function in a cooperative way even
   when the system is not monkey patched, on all supported versions of
   Python. (It was added officially in Python 3.5.)
-- Popen objects can be used as context managers even on Python 2.
 - Popen objects save their *args* attribute even on Python 2.
 - :exc:`gevent.subprocess.TimeoutExpired` is defined even on Python 2,
   where it is a subclass of the :exc:`gevent.timeout.Timeout`
   exception; all instances where a ``Timeout`` exception would
   previously be thrown under Python 2 will now throw a
   ``TimeoutExpired`` exception.
+- :func:`gevent.subprocess.call` (and ``check_call``) accepts the
+  *timeout* keyword argument on Python 2. This is standard on Python
+  3, but a gevent extension on Python 2.
+- :func:`gevent.subprocess.check_output` accepts the *timeout* and
+  *input* arguments on Python 2. This is standard on Python 3, but a
+  gevent extension on Python 2.
+
 
 1.1.2 (Jul 21, 2016)
 ====================

src/gevent/subprocess.py

@@ -12,11 +12,13 @@ Cooperative ``subprocess`` module.
    signal is sent to a different thread.
 
 .. note:: The interface of this module is intended to match that of
-   the standard library :mod:`subprocess` module. There are some small
-   differences between the Python 2 and Python 3 versions of that
-   module and between the POSIX and Windows versions. The HTML
-   documentation here can only describe one version; for definitive
-   documentation, see the standard library or the source code.
+   the standard library :mod:`subprocess` module (with many backwards
+   compatible extensions from Python 3 backported to Python 2). There
+   are some small differences between the Python 2 and Python 3
+   versions of that module and between the POSIX and Windows versions.
+   The HTML documentation here can only describe one version; for
+   definitive documentation, see the standard library or the source
+   code.
 
 .. _is not defined: http://www.linuxprogrammingblog.com/all-about-linux-signals?page=11
 """
@@ -92,6 +94,7 @@ __extra__ = [
     '_winapi',
     # Python 2.5 does not have _subprocess, so we don't use it
     # XXX We don't run on Py 2.5 anymore; can/could/should we use _subprocess?
+    # It's only used on mswindows
     'WAIT_OBJECT_0',
     'WaitForSingleObject',
     'GetExitCodeProcess',
@@ -206,37 +209,36 @@ else:
     fork = monkey.get_original('os', 'fork')
     from gevent.os import fork_and_watch
 
-if PY3:
-    def call(*popenargs, **kwargs):
-        """Run command with arguments.  Wait for command to complete or
-        timeout, then return the returncode attribute.
-
-        The arguments are the same as for the Popen constructor.  Example::
+def call(*popenargs, **kwargs):
+    """
+    call(args, *, stdin=None, stdout=None, stderr=None, shell=False, timeout=None) -> returncode
 
-            retcode = call(["ls", "-l"])
-        """
-        timeout = kwargs.pop('timeout', None)
-        with Popen(*popenargs, **kwargs) as p:
-            try:
-                return p.wait(timeout=timeout)
-            except:
-                p.kill()
-                p.wait()
-                raise
-else:
-    def call(*popenargs, **kwargs):
-        """Run command with arguments.  Wait for command to complete, then
-        return the returncode attribute.
+    Run command with arguments. Wait for command to complete or
+    timeout, then return the returncode attribute.
 
-        The arguments are the same as for the Popen constructor.  Example::
+    The arguments are the same as for the Popen constructor.  Example::
 
-            retcode = call(["ls", "-l"])
-        """
-        return Popen(*popenargs, **kwargs).wait()
+        retcode = call(["ls", "-l"])
 
+    .. versionchanged:: 1.2a1
+       The ``timeout`` keyword argument is now accepted on all supported
+       versions of Python (not just Python 3) and if it expires will raise a
+       :exc:`TimeoutExpired` exception (under Python 2 this is a subclass of :exc:`~.Timeout`).
+    """
+    timeout = kwargs.pop('timeout', None)
+    with Popen(*popenargs, **kwargs) as p:
+        try:
+            return p.wait(timeout=timeout, _raise_exc=True)
+        except:
+            p.kill()
+            p.wait()
+            raise
 
 def check_call(*popenargs, **kwargs):
-    """Run command with arguments.  Wait for command to complete.  If
+    """
+    check_call(args, *, stdin=None, stdout=None, stderr=None, shell=False, timeout=None) -> 0
+
+    Run command with arguments.  Wait for command to complete.  If
     the exit code was zero then return, otherwise raise
     :exc:`CalledProcessError`.  The ``CalledProcessError`` object will have the
     return code in the returncode attribute.
@@ -253,102 +255,77 @@ def check_call(*popenargs, **kwargs):
         raise CalledProcessError(retcode, cmd)
     return 0
 
-if PY3:
-    def check_output(*popenargs, **kwargs):
-        r"""Run command with arguments and return its output.
-
-        If the exit code was non-zero it raises a :exc:`CalledProcessError`.  The
-        ``CalledProcessError`` object will have the return code in the returncode
-        attribute and output in the output attribute.
-
-
-        The arguments are the same as for the Popen constructor.  Example::
+def check_output(*popenargs, **kwargs):
+    r"""
+    check_output(args, *, input=None, stdin=None, stderr=None, shell=False, universal_newlines=False, timeout=None) -> output
 
-            >>> check_output(["ls", "-1", "/dev/null"])
-            b'/dev/null\n'
+    Run command with arguments and return its output.
 
-        The ``stdout`` argument is not allowed as it is used internally.
+    If the exit code was non-zero it raises a :exc:`CalledProcessError`.  The
+    ``CalledProcessError`` object will have the return code in the returncode
+    attribute and output in the output attribute.
 
-        To capture standard error in the result, use ``stderr=STDOUT``::
 
-            >>> check_output(["/bin/sh", "-c",
-            ...               "ls -l non_existent_file ; exit 0"],
-            ...              stderr=STDOUT)
-            b'ls: non_existent_file: No such file or directory\n'
+    The arguments are the same as for the Popen constructor.  Example::
 
-        There is an additional optional argument, "input", allowing you to
-        pass a string to the subprocess's stdin.  If you use this argument
-        you may not also use the Popen constructor's "stdin" argument, as
-        it too will be used internally.  Example::
+        >>> check_output(["ls", "-1", "/dev/null"])
+        '/dev/null\n'
 
-            >>> check_output(["sed", "-e", "s/foo/bar/"],
-            ...              input=b"when in the course of fooman events\n")
-            b'when in the course of barman events\n'
+    The ``stdout`` argument is not allowed as it is used internally.
 
-        If ``universal_newlines=True`` is passed, the return value will be a
-        string rather than bytes.
-        """
-        timeout = kwargs.pop('timeout', None)
-        if 'stdout' in kwargs:
-            raise ValueError('stdout argument not allowed, it will be overridden.')
-        if 'input' in kwargs:
-            if 'stdin' in kwargs:
-                raise ValueError('stdin and input arguments may not both be used.')
-            inputdata = kwargs['input']
-            del kwargs['input']
-            kwargs['stdin'] = PIPE
-        else:
-            inputdata = None
-        with Popen(*popenargs, stdout=PIPE, **kwargs) as process:
-            try:
-                output, unused_err = process.communicate(inputdata, timeout=timeout)
-            except TimeoutExpired:
-                process.kill()
-                output, unused_err = process.communicate()
-                raise TimeoutExpired(process.args, timeout, output=output)
-            except:
-                process.kill()
-                process.wait()
-                raise
-            retcode = process.poll()
-            if retcode:
-                raise CalledProcessError(retcode, process.args, output=output)
-        return output
-else:
-    def check_output(*popenargs, **kwargs):
-        r"""Run command with arguments and return its output as a byte string.
+    To capture standard error in the result, use ``stderr=STDOUT``::
 
-        If the exit code was non-zero it raises a CalledProcessError.  The
-        CalledProcessError object will have the return code in the returncode
-        attribute and output in the output attribute.
+        >>> check_output(["/bin/sh", "-c",
+        ...               "ls -l non_existent_file ; exit 0"],
+        ...              stderr=STDOUT)
+        'ls: non_existent_file: No such file or directory\n'
 
-        The arguments are the same as for the Popen constructor.  Example:
+    There is an additional optional argument, "input", allowing you to
+    pass a string to the subprocess's stdin.  If you use this argument
+    you may not also use the Popen constructor's "stdin" argument, as
+    it too will be used internally.  Example::
 
-        >>> print(check_output(["ls", "-1", "/dev/null"]).decode('ascii'))
-        /dev/null
-        <BLANKLINE>
+        >>> check_output(["sed", "-e", "s/foo/bar/"],
+        ...              input=b"when in the course of fooman events\n")
+        'when in the course of barman events\n'
 
-        The stdout argument is not allowed as it is used internally.
-        To capture standard error in the result, use stderr=STDOUT.
+    If ``universal_newlines=True`` is passed, the return value will be a
+    string rather than bytes.
 
-        >>> print(check_output(["/bin/sh", "-c", "echo hello world"], stderr=STDOUT).decode('ascii'))
-        hello world
-        <BLANKLINE>
-        """
-        if 'stdout' in kwargs:
-            raise ValueError('stdout argument not allowed, it will be overridden.')
-        process = Popen(stdout=PIPE, *popenargs, **kwargs)
-        output = process.communicate()[0]
+    .. versionchanged:: 1.2a1
+       The ``timeout`` keyword argument is now accepted on all supported
+       versions of Python (not just Python 3) and if it expires will raise a
+       :exc:`TimeoutExpired` exception (under Python 2 this is a subclass of :exc:`~.Timeout`).
+    .. versionchanged:: 1.2a1
+       The ``input`` keyword argument is now accepted on all supported
+       versions of Python, not just Python 3
+    """
+    timeout = kwargs.pop('timeout', None)
+    if 'stdout' in kwargs:
+        raise ValueError('stdout argument not allowed, it will be overridden.')
+    if 'input' in kwargs:
+        if 'stdin' in kwargs:
+            raise ValueError('stdin and input arguments may not both be used.')
+        inputdata = kwargs['input']
+        del kwargs['input']
+        kwargs['stdin'] = PIPE
+    else:
+        inputdata = None
+    with Popen(*popenargs, stdout=PIPE, **kwargs) as process:
+        try:
+            output, unused_err = process.communicate(inputdata, timeout=timeout)
+        except TimeoutExpired:
+            process.kill()
+            output, unused_err = process.communicate()
+            raise TimeoutExpired(process.args, timeout, output=output)
+        except:
+            process.kill()
+            process.wait()
+            raise
         retcode = process.poll()
         if retcode:
-            cmd = kwargs.get("args")
-            if cmd is None:
-                cmd = popenargs[0]
-            ex = CalledProcessError(retcode, cmd)
-            # on Python 2.6 and older CalledProcessError does not accept 'output' argument
-            ex.output = output
-            raise ex
-        return output
+            raise CalledProcessError(retcode, process.args, output=output)
+    return output
 
 _PLATFORM_DEFAULT_CLOSE_FDS = object()
 
@@ -635,7 +612,9 @@ class Popen(object):
         communicate() returns a tuple (stdout, stderr).
 
         :keyword timeout: Under Python 2, this is a gevent extension; if
-           given and it expires, we will raise :class:`gevent.timeout.Timeout`.
+           given and it expires, we will raise :exc:`TimeoutExpired`, which
+           extends :exc:`gevent.timeout.Timeout` (note that this only extends :exc:`BaseException`,
+           *not* :exc:`Exception`)
            Under Python 3, this raises the standard :exc:`TimeoutExpired` exception.
 
         .. versionchanged:: 1.1a2
@@ -696,11 +675,7 @@ class Popen(object):
         # RunFuncTestCase.test_timeout). Instead, we go directly to
         # self.wait
         if not greenlets and timeout is not None:
-            result = self.wait(timeout=timeout)
-            # Python 3 would have already raised, but Python 2 would not
-            # so we need to do that manually
-            if result is None:
-                raise TimeoutExpired(self.args, timeout)
+            self.wait(timeout=timeout, _raise_exc=True)
 
         done = joinall(greenlets, timeout=timeout)
         if timeout is not None and len(done) != len(greenlets):
@@ -747,6 +722,13 @@ class Popen(object):
             # blocks forever.
             self.wait()
 
+    def _gevent_result_wait(self, timeout=None, raise_exc=PY3):
+        result = self.result.wait(timeout=timeout)
+        if raise_exc and timeout is not None and not self.result.ready():
+            raise TimeoutExpired(self.args, timeout)
+        return result
+
+
     if mswindows:
         #
         # Windows methods
@@ -968,17 +950,14 @@ class Popen(object):
         def _wait(self):
             self.threadpool.spawn(self._blocking_wait).rawlink(self.result)
 
-        def wait(self, timeout=None):
+        def wait(self, timeout=None, _raise_exc=PY3):
             """Wait for child process to terminate.  Returns returncode
             attribute."""
             if self.returncode is None:
                 if not self._waiting:
                     self._waiting = True
                     self._wait()
-            result = self.result.wait(timeout=timeout)
-            if PY3 and timeout is not None and not self.result.ready():
-                raise TimeoutExpired(self.args, timeout)
-            return result
+            return self._gevent_result_wait(timeout, _raise_exc)
 
         def send_signal(self, sig):
             """Send a signal to the process
@@ -1347,7 +1326,7 @@ class Popen(object):
                         sleep(0.00001)
             return self.returncode
 
-        def wait(self, timeout=None):
+        def wait(self, timeout=None, _raise_exc=PY3):
             """
             Wait for child process to terminate.  Returns :attr:`returncode`
             attribute.
@@ -1358,10 +1337,7 @@ class Popen(object):
                 this time elapses without finishing the process,
                 :exc:`TimeoutExpired` is raised.
             """
-            result = self.result.wait(timeout=timeout)
-            if PY3 and timeout is not None and not self.result.ready():
-                raise TimeoutExpired(self.args, timeout)
-            return result
+            return self._gevent_result_wait(timeout, _raise_exc)
 
         def send_signal(self, sig):
             """Send a signal to the process
@@ -1416,6 +1392,10 @@ class CompletedProcess(object):
       - returncode: The exit code of the process, negative for signals.
       - stdout: The standard output (None if not captured).
       - stderr: The standard error (None if not captured).
+
+    .. versionadded:: 1.2a1
+       This first appeared in Python 3.5 and is available to all
+       Python versions in gevent.
     """
     def __init__(self, args, returncode, stdout=None, stderr=None):
         self.args = args
@@ -1440,7 +1420,7 @@ class CompletedProcess(object):
 
 def run(*popenargs, **kwargs):
     """
-    `subprocess.run(args, *, stdin=None, input=None, stdout=None, stderr=None, shell=False, timeout=None, check=False)`
+    run(args, *, stdin=None, input=None, stdout=None, stderr=None, shell=False, timeout=None, check=False) -> CompletedProcess
 
     Run command with arguments and return a CompletedProcess instance.
 

src/greentest/test__doctests.py

@@ -68,6 +68,9 @@ if __name__ == '__main__':
             # 'cannot access'
             (re.compile('cannot access non_existent_file: No such file or directory'),
              'non_existent_file: No such file or directory'),
+            # Python 3 bytes add a "b".
+            (re.compile(r'b(".*?")'), r"\1"),
+            (re.compile(r"b('.*?')"), r"\1"),
         ))
 
         tests_count = 0