changeset 17413:97f1f22c2dba stable

convert: use subprocess for all commandline calls

Avoid mixing popen and subprocess calls, it simplifies the command line
generation and quoting issues with redirections.

In practice, it fixes the subversion sink on Windows and probably helps
with monotone and darcs sources.

--- a/hgext/convert/common.py	Fri Aug 03 21:05:01 2012 +0200
+++ b/hgext/convert/common.py	Fri Aug 03 21:37:33 2012 +0200
@@ -5,8 +5,7 @@
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
-import base64, errno
-import os
+import base64, errno, subprocess, os
 import cPickle as pickle
 from mercurial import util
 from mercurial.i18n import _
@@ -260,7 +259,7 @@
     def postrun(self):
         pass
 
-    def _cmdline(self, cmd, closestdin, *args, **kwargs):
+    def _cmdline(self, cmd, *args, **kwargs):
         cmdline = [self.command, cmd] + list(args)
         for k, v in kwargs.iteritems():
             if len(k) == 1:
@@ -277,19 +276,22 @@
         cmdline = [util.shellquote(arg) for arg in cmdline]
         if not self.ui.debugflag:
             cmdline += ['2>', os.devnull]
-        if closestdin:
-            cmdline += ['<', os.devnull]
         cmdline = ' '.join(cmdline)
         return cmdline
 
     def _run(self, cmd, *args, **kwargs):
-        return self._dorun(util.popen, cmd, True, *args, **kwargs)
+        def popen(cmdline):
+            p = subprocess.Popen(cmdline, shell=True, bufsize=-1,
+                    close_fds=util.closefds,
+                    stdout=subprocess.PIPE)
+            return p
+        return self._dorun(popen, cmd, *args, **kwargs)
 
     def _run2(self, cmd, *args, **kwargs):
-        return self._dorun(util.popen2, cmd, False, *args, **kwargs)
+        return self._dorun(util.popen2, cmd, *args, **kwargs)
 
-    def _dorun(self, openfunc, cmd, closestdin, *args, **kwargs):
-        cmdline = self._cmdline(cmd, closestdin, *args, **kwargs)
+    def _dorun(self, openfunc, cmd,  *args, **kwargs):
+        cmdline = self._cmdline(cmd, *args, **kwargs)
         self.ui.debug('running: %s\n' % (cmdline,))
         self.prerun()
         try:
@@ -298,16 +300,17 @@
             self.postrun()
 
     def run(self, cmd, *args, **kwargs):
-        fp = self._run(cmd, *args, **kwargs)
-        output = fp.read()
+        p = self._run(cmd, *args, **kwargs)
+        output = p.communicate()[0]
         self.ui.debug(output)
-        return output, fp.close()
+        return output, p.returncode
 
     def runlines(self, cmd, *args, **kwargs):
-        fp = self._run(cmd, *args, **kwargs)
-        output = fp.readlines()
+        p = self._run(cmd, *args, **kwargs)
+        output = p.stdout.readlines()
+        p.wait()
         self.ui.debug(''.join(output))
-        return output, fp.close()
+        return output, p.returncode
 
     def checkexit(self, status, output=''):
         if status:

--- a/hgext/convert/darcs.py	Fri Aug 03 21:05:01 2012 +0200
+++ b/hgext/convert/darcs.py	Fri Aug 03 21:37:33 2012 +0200
@@ -104,9 +104,10 @@
         # possible, etree will still raise an exception if any
         # non-printable characters are in the XML changelog.
         parser = XMLParser(encoding='latin-1')
-        fp = self._run(cmd, **kwargs)
-        etree.parse(fp, parser=parser)
-        self.checkexit(fp.close())
+        p = self._run(cmd, **kwargs)
+        etree.parse(p.stdout, parser=parser)
+        p.wait()
+        self.checkexit(p.returncode)
         return etree.getroot()
 
     def format(self):