Unify subprocess.check_output for Py2/3.

This adds the *timeout* and *input* arguments for Py2.

This required a small doctest renormalizer to handle the byte string
difference.

.travis.yml

@@ -12,7 +12,7 @@ env:
 #      - TASK=test-py27-noembed
       - TASK=test-pypy
       - TASK=test-py36
-#      - TASK=lint-py27
+      - TASK=lint-py27
 #      - TASK=test-pypy3
 #      - TASK=test-py35
 #      - TASK=test-py278

changelog.rst

@@ -176,8 +176,12 @@ Other Changes
   exception; all instances where a ``Timeout`` exception would
   previously be thrown under Python 2 will now throw a
   ``TimeoutExpired`` exception.
-- :func:`gevent.subprocess.call` accepts the *timeout* keyword
-  argument on Python 2.
+- :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

@@ -233,7 +233,10 @@ def call(*popenargs, **kwargs):
             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.
@@ -250,102 +253,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.
-
+def check_output(*popenargs, **kwargs):
+    r"""
+    check_output(args, *, input=None, stdin=None, stderr=None, shell=False, universal_newlines=False, timeout=None) -> output
 
-        The arguments are the same as for the Popen constructor.  Example::
+    Run command with arguments and return its output.
 
-            >>> check_output(["ls", "-1", "/dev/null"])
-            b'/dev/null\n'
+    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 ``stdout`` argument is not allowed as it is used internally.
 
-        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()
 
@@ -632,7 +610,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
@@ -693,11 +673,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):
@@ -1442,7 +1418,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