Patchwork [2,of,7] procutil: rewrite popen() as a subprocess.Popen wrapper (issue4746) (API)

login
register
mail settings
Submitter Yuya Nishihara
Date April 7, 2018, 1:39 p.m.
Message ID <2564c8fbfd8d501efade.1523108378@mimosa>
Download mbox | patch
Permalink /patch/30543/
State Accepted
Headers show

Comments

Yuya Nishihara - April 7, 2018, 1:39 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1523102961 -32400
#      Sat Apr 07 21:09:21 2018 +0900
# Node ID 2564c8fbfd8d501efade8cbaf1ecf689f956311e
# Parent  7a2e591c5eec11d8cf946d8032b6245f4099a161
procutil: rewrite popen() as a subprocess.Popen wrapper (issue4746) (API)

os.popen() of Python 3 is not the popen() we want. First, it doesn't accept
command in bytes. Second, a returned stream is always wrapped by TextIO.
So we have to reimplement our popen(). Fortunately, this fixes the bug 4746
since ours returns an exit code compatible with explainexit().

.. api::

   ``procutil.popen()`` no longer supports text mode I/O.

Patch

diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -469,9 +469,6 @@  def shellsplit(s):
 def quotecommand(cmd):
     return cmd
 
-def popen(command, mode='r'):
-    return os.popen(command, mode)
-
 def testpid(pid):
     '''return False if pid dead, True if running or not sure'''
     if pycompat.sysplatform == 'OpenVMS':
diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
--- a/mercurial/utils/procutil.py
+++ b/mercurial/utils/procutil.py
@@ -58,7 +58,6 @@  findexe = platform.findexe
 getuser = platform.getuser
 getpid = os.getpid
 hidewindow = platform.hidewindow
-popen = platform.popen
 quotecommand = platform.quotecommand
 readpipe = platform.readpipe
 setbinary = platform.setbinary
@@ -80,6 +79,49 @@  except AttributeError:
 
 closefds = pycompat.isposix
 
+class _pfile(object):
+    """File-like wrapper for a stream opened by subprocess.Popen()"""
+
+    def __init__(self, proc, fp):
+        self._proc = proc
+        self._fp = fp
+
+    def close(self):
+        # unlike os.popen(), this returns an integer in subprocess coding
+        self._fp.close()
+        return self._proc.wait()
+
+    def __iter__(self):
+        return iter(self._fp)
+
+    def __getattr__(self, attr):
+        return getattr(self._fp, attr)
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_value, exc_tb):
+        self.close()
+
+def popen(cmd, mode='rb', bufsize=-1):
+    if mode == 'rb':
+        return _popenreader(cmd, bufsize)
+    elif mode == 'wb':
+        return _popenwriter(cmd, bufsize)
+    raise error.ProgrammingError('unsupported mode: %r' % mode)
+
+def _popenreader(cmd, bufsize):
+    p = subprocess.Popen(quotecommand(cmd), shell=True, bufsize=bufsize,
+                         close_fds=closefds,
+                         stdout=subprocess.PIPE)
+    return _pfile(p, p.stdout)
+
+def _popenwriter(cmd, bufsize):
+    p = subprocess.Popen(quotecommand(cmd), shell=True, bufsize=bufsize,
+                         close_fds=closefds,
+                         stdin=subprocess.PIPE)
+    return _pfile(p, p.stdin)
+
 def popen2(cmd, env=None, newlines=False):
     # Setting bufsize to -1 lets the system decide the buffer size.
     # The default for bufsize is 0, meaning unbuffered. This leads to
diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -311,13 +311,6 @@  def quotecommand(cmd):
         return '"' + cmd + '"'
     return cmd
 
-def popen(command, mode='r'):
-    # Work around "popen spawned process may not write to stdout
-    # under windows"
-    # http://bugs.python.org/issue1366
-    command += " 2> %s" % pycompat.bytestr(os.devnull)
-    return os.popen(quotecommand(command), mode)
-
 def explainexit(code):
     return _("exited with status %d") % code, code
 
diff --git a/tests/test-patch.t b/tests/test-patch.t
--- a/tests/test-patch.t
+++ b/tests/test-patch.t
@@ -89,4 +89,12 @@  Clone and apply patch:
   # User lines looks like this - but it _is_ just a comment
   
   
+
+Error exit (issue4746)
+
+  $ hg import ../c/p --config ui.patch='sh -c "exit 1"'
+  applying ../c/p
+  abort: patch command failed: exited with status 1
+  [255]
+
   $ cd ..