# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 2018-04-07 12:09:21
# Node ID 90c5ca718781708dec4aa21587ffcf8952e475ef
# Parent  00e4bd97b095b81e4dae4a1783c0cb89bbaea7ae

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.

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 ..