procutil: rewrite popen() as a subprocess.Popen wrapper (issue4746) (API)
authorYuya Nishihara <yuya@tcha.org>
Sat, 07 Apr 2018 21:09:21 +0900
changeset 37459 90c5ca718781
parent 37458 00e4bd97b095
child 37460 a6c6b7beb025
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.
mercurial/posix.py
mercurial/utils/procutil.py
mercurial/windows.py
tests/test-patch.t
--- a/mercurial/posix.py	Sat Apr 07 20:50:38 2018 +0900
+++ b/mercurial/posix.py	Sat Apr 07 21:09:21 2018 +0900
@@ -469,9 +469,6 @@
 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':
--- a/mercurial/utils/procutil.py	Sat Apr 07 20:50:38 2018 +0900
+++ b/mercurial/utils/procutil.py	Sat Apr 07 21:09:21 2018 +0900
@@ -58,7 +58,6 @@
 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 @@
 
 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
--- a/mercurial/windows.py	Sat Apr 07 20:50:38 2018 +0900
+++ b/mercurial/windows.py	Sat Apr 07 21:09:21 2018 +0900
@@ -311,13 +311,6 @@
         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
 
--- a/tests/test-patch.t	Sat Apr 07 20:50:38 2018 +0900
+++ b/tests/test-patch.t	Sat Apr 07 21:09:21 2018 +0900
@@ -89,4 +89,12 @@
   # 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 ..