subprocess: use subprocessio helper to run various subprocess commands.

vcsserver/hooks.py

@@ -377,30 +377,6 b' def git_pre_receive(unused_repo_path, re'
     return _call_hook('pre_push', extras, GitMessageWriter())
 
 
-def _run_command(arguments):
-    """
-    Run the specified command and return the stdout.
-
-    :param arguments: sequence of program arguments (including the program name)
-    :type arguments: list[str]
-    """
-
-    cmd = arguments
-    try:
-        gitenv = os.environ.copy()
-        _opts = {'env': gitenv, 'shell': False, 'fail_on_stderr': False}
-        p = subprocessio.SubprocessIOChunker(cmd, **_opts)
-        stdout = ''.join(p)
-    except (EnvironmentError, OSError) as err:
-        cmd = ' '.join(cmd)  # human friendly CMD
-        tb_err = ("Couldn't run git command (%s).\n"
-                  "Original error was:%s\n" % (cmd, err))
-        log.exception(tb_err)
-        raise Exception(tb_err)
-
-    return stdout
-
-
 def git_post_receive(unused_repo_path, revision_lines, env):
     """
     Post push hook.
@@ -437,21 +413,26 b' def git_post_receive(unused_repo_path, r'
                 # Fix up head revision if needed
                 cmd = [settings.GIT_EXECUTABLE, 'show', 'HEAD']
                 try:
-                    _run_command(cmd)
+                    subprocessio.run_command(cmd, env=os.environ.copy())
                 except Exception:
                     cmd = [settings.GIT_EXECUTABLE, 'symbolic-ref', 'HEAD',
                            'refs/heads/%s' % push_ref['name']]
                     print("Setting default branch to %s" % push_ref['name'])
-                    _run_command(cmd)
+                    subprocessio.run_command(cmd, env=os.environ.copy())
 
-                cmd = [settings.GIT_EXECUTABLE, 'for-each-ref', '--format=%(refname)',
-                       'refs/heads/*']
-                heads = _run_command(cmd)
+                cmd = [settings.GIT_EXECUTABLE, 'for-each-ref',
+                       '--format=%(refname)', 'refs/heads/*']
+                stdout, stderr = subprocessio.run_command(
+                    cmd, env=os.environ.copy())
+                heads = stdout
                 heads = heads.replace(push_ref['ref'], '')
                 heads = ' '.join(head for head in heads.splitlines() if head)
-                cmd = [settings.GIT_EXECUTABLE, 'log', '--reverse', '--pretty=format:%H',
-                        '--', push_ref['new_rev'], '--not', heads]
-                git_revs.extend(_run_command(cmd).splitlines())
+                cmd = [settings.GIT_EXECUTABLE, 'log', '--reverse',
+                       '--pretty=format:%H', '--', push_ref['new_rev'],
+                       '--not', heads]
+                stdout, stderr = subprocessio.run_command(
+                    cmd, env=os.environ.copy())
+                git_revs.extend(stdout.splitlines())
             elif push_ref['new_rev'] == empty_commit_id:
                 # delete branch case
                 git_revs.append('delete_branch=>%s' % push_ref['name'])
@@ -462,7 +443,9 b' def git_post_receive(unused_repo_path, r'
                 cmd = [settings.GIT_EXECUTABLE, 'log',
                        '{old_rev}..{new_rev}'.format(**push_ref),
                        '--reverse', '--pretty=format:%H']
-                git_revs.extend(_run_command(cmd).splitlines())
+                stdout, stderr = subprocessio.run_command(
+                    cmd, env=os.environ.copy())
+                git_revs.extend(stdout.splitlines())
         elif type_ == 'tags':
             if push_ref['name'] not in tags:
                 tags.append(push_ref['name'])

vcsserver/subprocessio.py

@@ -482,3 +482,30 b' class SubprocessIOChunker(object):'
 
     def __del__(self):
         self.close()
+
+
+def run_command(arguments, env=None):
+    """
+    Run the specified command and return the stdout.
+
+    :param arguments: sequence of program arguments (including the program name)
+    :type arguments: list[str]
+    """
+
+    cmd = arguments
+    log.debug('Running subprocessio command %s', cmd)
+    try:
+        _opts = {'shell': False, 'fail_on_stderr': False}
+        if env:
+            _opts.update({'env': env})
+        p = SubprocessIOChunker(cmd, **_opts)
+        stdout = ''.join(p)
+        stderr = ''.join(''.join(p.error))
+    except (EnvironmentError, OSError) as err:
+        cmd = ' '.join(cmd)  # human friendly CMD
+        tb_err = ("Couldn't run subprocessio command (%s).\n"
+                  "Original error was:%s\n" % (cmd, err))
+        log.exception(tb_err)
+        raise Exception(tb_err)
+
+    return stdout, stderr

vcsserver/svn.py

@@ -22,7 +22,6 b' from urllib2 import URLError'
 import logging
 import posixpath as vcspath
 import StringIO
-import subprocess
 import urllib
 import traceback
 
@@ -33,11 +32,9 b' import svn.diff'
 import svn.fs
 import svn.repos
 
-from vcsserver import svn_diff
-from vcsserver import exceptions
+from vcsserver import svn_diff, exceptions, subprocessio
 from vcsserver.base import RepoFactory, raise_from_original
 
-
 log = logging.getLogger(__name__)
 
 
@@ -164,10 +161,9 b' class SvnRemote(object):'
             raise Exception(
                 "Path %s is not a valid Subversion repository." % repo_path)
 
-        load = subprocess.Popen(
-            ['svnadmin', 'info', repo_path],
-            stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        return ''.join(load.stdout)
+        cmd = ['svnadmin', 'info', repo_path]
+        stdout, stderr = subprocessio.run_command(cmd)
+        return stdout
 
     def lookup(self, wire, revision):
         if revision not in [-1, None, 'HEAD']:
@@ -343,7 +339,9 b' class SvnRemote(object):'
         if not self.is_path_valid_repository(wire, repo_path):
             raise Exception(
                 "Path %s is not a valid Subversion repository." % repo_path)
+
         # TODO: johbo: URL checks ?
+        import subprocess
         rdump = subprocess.Popen(
             ['svnrdump', 'dump', '--non-interactive', src_url],
             stdout=subprocess.PIPE, stderr=subprocess.PIPE)