Replace os.system() by subprocess.Popen()

10 files changed, 123 insertions, 92 deletions

diff --git a/CHANGES b/CHANGES
index f6ab303..10b4371 100644
--- a/CHANGES
+++ b/CHANGES
@@ -8,6 +8,8 @@ Version TODO
      Former now shows a usage summary while latter now reports
      an error.  Both of them kept quiet before.
 
+ - Replace os.system() by subprocess.Popen()
+
 
 Version 1.3.4 - Released 2010-07-05
 ===================================
diff --git a/layman/overlays/bzr.py b/layman/overlays/bzr.py
index d5a5321..e73c2f4 100644
--- a/layman/overlays/bzr.py
+++ b/layman/overlays/bzr.py
@@ -50,17 +50,18 @@ class BzrOverlay(OverlaySource):
 
         self.supported()
 
-        return self.cmd(self.command() + ' get "' + self.src + '/" "' +\
-                        path([base, self.parent.name]) + '"')
+        # bzr get SOURCE TARGET
+        args = ['get', self.src + '/', path([base, self.parent.name])]
+        return self.run_command(*args)
 
     def sync(self, base, quiet = False):
         '''Sync overlay.'''
 
         self.supported()
 
-        return self.cmd('cd "' + path([base, self.parent.name]) + '" && ' +          \
-                        self.command() + ' pull --overwrite "' + self.src \
-                        + '"')
+        # bzr pull --overwrite SOURCE
+        args = ['pull', '--overwrite', self.src]
+        return self.run_command(*args, cwd=path([base, self.parent.name]))
 
     def supported(self):
         '''Overlay type supported?'''
diff --git a/layman/overlays/cvs.py b/layman/overlays/cvs.py
index ba7c0a9..f8fc277 100644
--- a/layman/overlays/cvs.py
+++ b/layman/overlays/cvs.py
@@ -74,27 +74,29 @@ class CvsOverlay(OverlaySource):
 
         self.supported()
 
+        # cvs [-q] co -d SOURCE SCOPE
+        args = []
         if quiet:
-            quiet_option = ' -q'
-        else:
-            quiet_option = ''
+            args.append('-q')
+        args.append('co')
+        args.append('-d')
+        args.append(self.parent.name)
+        args.append(self.subpath)
 
-        return self.cmd('cd "' + base + '" && CVSROOT="' + self.src + '" ' +
-                        self.command() + quiet_option + ' co -d "' + self.parent.name
-                        + '" "' + self.subpath + '"' )
+        return self.run_command(*args, cwd=base, env=dict(CVSROOT=self.src))
 
     def sync(self, base, quiet = False):
         '''Sync overlay.'''
 
         self.supported()
 
+        # cvs [-q] update -d
+        args = []
         if quiet:
-            quiet_option = ' -q'
-        else:
-            quiet_option = ''
-
-        return self.cmd('cd "' + path([base, self.parent.name]) + '" && ' +
-                        self.command() + quiet_option + ' update -d')
+            args.append('-q')
+        args.append('update')
+        args.append('-d')
+        return self.run_command(*args, cwd=path([base, self.parent.name]))
 
     def supported(self):
         '''Overlay type supported?'''
diff --git a/layman/overlays/darcs.py b/layman/overlays/darcs.py
index 33f7c6c..7d89477 100644
--- a/layman/overlays/darcs.py
+++ b/layman/overlays/darcs.py
@@ -49,16 +49,18 @@ class DarcsOverlay(OverlaySource):
 
         self.supported()
 
-        return self.cmd(self.command() + ' get --partial "' + self.src +
-                        '/" "' + path([base, self.parent.name]) + '"')
+        # darcs get --partial SOURCE TARGET
+        args = ['get', '--partial', self.src + '/', path([base, self.parent.name])]
+        return self.run_command(*args)
 
     def sync(self, base, quiet = False):
         '''Sync overlay.'''
 
         self.supported()
 
-        return self.cmd('cd "' + path([base, self.parent.name]) + '" && ' +
-                        self.command() + ' pull --all "' + self.src + '"')
+        # darcs pull --all SOURCE
+        args = ['pull', '--all', self.src]
+        return self.run_command(*args, cwd=path([base, self.parent.name]))
 
     def supported(self):
         '''Overlay type supported?'''
diff --git a/layman/overlays/git.py b/layman/overlays/git.py
index 40cf2d1..caa73f4 100644
--- a/layman/overlays/git.py
+++ b/layman/overlays/git.py
@@ -48,30 +48,29 @@ class GitOverlay(OverlaySource):
 
         self.supported()
 
+        def fix_git_source(source):
+            # http:// should get trailing slash, other protocols shouldn't
+            if source.split(':')[0] == 'http':
+                return source + '/'
+            return source
+
+        # git clone [-q] SOURCE TARGET
+        args = ['clone']
         if quiet:
-            quiet_option = '-q '
-        else:
-            quiet_option = ''
- 
-        # http:// should get trailing slash, other protocols shouldn't
-        slash = ''
-        if self.src.split(':')[0] == 'http':
-            slash = '/'
-        return self.cmd(self.command() + ' clone ' + quiet_option + '"' + self.src + slash
-                        + '" "' + path([base, self.parent.name]) + '"')
+            args.append('-q')
+        args.append(fix_git_source(self.src))
+        args.append(path([base, self.parent.name]))
+        return self.run_command(*args)
 
     def sync(self, base, quiet = False):
         '''Sync overlay.'''
 
         self.supported()
 
+        args = ['pull']
         if quiet:
-            quiet_option = ' -q'
-        else:
-            quiet_option = ''
- 
-        return self.cmd('cd "' + path([base, self.parent.name]) + '" && '
-                        + self.command() + ' pull' + quiet_option)
+            args.append('-q')
+        return self.run_command(*args, cwd=path([base, self.parent.name]))
 
     def supported(self):
         '''Overlay type supported?'''
diff --git a/layman/overlays/mercurial.py b/layman/overlays/mercurial.py
index 651e452..06cf59d 100644
--- a/layman/overlays/mercurial.py
+++ b/layman/overlays/mercurial.py
@@ -49,16 +49,18 @@ class MercurialOverlay(OverlaySource):
 
         self.supported()
 
-        return self.cmd(self.command() + ' clone "' + self.src + '/" "' +
-                        path([base, self.parent.name]) + '"')
+        # hg clone SOURCE TARGET
+        args = ['clone', self.src + '/', path([base, self.parent.name])]
+        return self.run_command(*args)
 
     def sync(self, base, quiet = False):
         '''Sync overlay.'''
 
         self.supported()
 
-        return self.cmd('cd "' + path([base, self.parent.name]) + '" && ' +
-                        self.command() + ' pull -u "' + self.src + '"')
+        # hg pull -u SOURCE
+        args = ['pull', '-u', self.src]
+        return self.run_command(*args, cwd=path([base, self.parent.name]))
 
     def supported(self):
         '''Overlay type supported?'''
diff --git a/layman/overlays/rsync.py b/layman/overlays/rsync.py
index 495d2ba..3a5a342 100644
--- a/layman/overlays/rsync.py
+++ b/layman/overlays/rsync.py
@@ -58,17 +58,15 @@ class RsyncOverlay(OverlaySource):
 
         self.supported()
 
+        # rsync OPTIONS [-q] SOURCE TARGET
+        args = ['-rlptDvz', '--progress', '--delete', '--delete-after', '--timeout=180',
+            '--exclude=distfiles/*', '--exclude=local/*', '--exclude=packages/*']
         if quiet:
-            quiet_option = '-q '
-        else:
-            quiet_option = ''
+            args.append('-q')
+        args.append(self.src + '/')
+        args.append(path([base, self.parent.name]))
 
-        _command = self.command() + ' -rlptDvz --progress --delete --delete-after ' +\
-                '--timeout=180 --exclude="distfiles/*" --exclude="local/*" ' +\
-                '--exclude="packages/*" '
-
-        return self.cmd(_command + quiet_option + '"' + self.src + '/" "' +
-                        path([base, self.parent.name]) + '"')
+        return self.run_command(*args)
 
     def supported(self):
         '''Overlay type supported?'''
diff --git a/layman/overlays/source.py b/layman/overlays/source.py
index 5ff2b15..621cf4c 100644
--- a/layman/overlays/source.py
+++ b/layman/overlays/source.py
@@ -14,6 +14,7 @@
 #             Sebastian Pipping <sebastian@pipping.org>
 
 import os
+import copy
 import sys
 import shutil
 import subprocess
@@ -21,21 +22,26 @@ from layman.debug import OUT
 from layman.utils import path
 
 
+def _resolve_command(command):
+    if os.path.isabs(command):
+        if not os.path.exists(command):
+            raise Exception('Program "%s" not found' % command)
+        return ('File', command)
+    else:
+        kind = 'Command'
+        env_path = os.environ['PATH']
+        for d in env_path.split(os.pathsep):
+            f = os.path.join(d, command)
+            if os.path.exists(f):
+                return ('Command', f)
+        raise Exception('Cound not resolve command "%s" based on PATH "%s"' % (command, env_path))
+
+
 def require_supported(binaries):
     for command, mtype, package in binaries:
         found = False
-        if os.path.isabs(command):
-            kind = 'Binary'
-            found = os.path.exists(command)
-        else:
-            kind = 'Command'
-            for d in os.environ['PATH'].split(os.pathsep):
-                f = os.path.join(d, command)
-                if os.path.exists(f):
-                    found = True
-                    break
-
-        if not found:
+        kind, path = _resolve_command(command)
+        if not path:
             raise Exception(kind + ' ' + command + ' seems to be missing!'
                             ' Overlay type "' + mtype + '" not support'
                             'ed. Did you emerge ' + package + '?')
@@ -101,25 +107,43 @@ class OverlaySource(object):
     def command(self):
         return self.config['%s_command' % self.__class__.type_key]
 
-    def cmd(self, command):
-        '''Run a command.'''
+    def run_command(self, *args, **kwargs):
+        file_to_run = _resolve_command(self.command())[1]
+        args = (file_to_run, ) + args
+        assert('pwd' not in kwargs)  # Bug detector
+
+        cwd = kwargs.get('cwd', None)
+        env = None
+        env_updates = None
+        if 'env' in kwargs:
+            # Build actual env from surrounding plus updates
+            env_updates = kwargs['env']
+            env = copy.copy(os.environ)
+            env.update(env_updates)
+
+        command_repr = ' '.join(args)
+        if env_updates:
+            command_repr = '%s %s' % (' '.join('%s=%s' % (k, v) for (k, v) in sorted(env_updates.items())), command_repr)
+        if cwd:
+            command_repr = '( cd %s  && %s )' % (cwd, command_repr)
+
+        OUT.info('Running... # %s' % command_repr, 2)
+
+        if self.quiet:
+            output_target = open('/dev/null', 'w')
+        else:
+            output_target = None  # i.e. re-use parent file descriptors
 
-        OUT.info('Running command "' + command + '"...', 2)
+        proc = subprocess.Popen(args,
+            stdout=output_target,
+            stderr=output_target,
+            cwd=cwd,
+            env=env)
 
-        if hasattr(sys.stdout,'encoding'):
-            enc = sys.stdout.encoding or sys.getfilesystemencoding()
-            if enc:
-                command = command.encode(enc)
+        if self.quiet:
+            output_target.close()
 
-        if not self.quiet:
-            return os.system(command)
-        else:
-            cmd = subprocess.Popen([command], shell = True,
-                                   stdout = subprocess.PIPE,
-                                   stderr = subprocess.PIPE,
-                                   close_fds = True)
-            result = cmd.wait()
-            return result
+        return proc.wait()
 
     def to_xml_hook(self, repo_elem):
         pass
diff --git a/layman/overlays/svn.py b/layman/overlays/svn.py
index d9f90c5..2a30198 100644
--- a/layman/overlays/svn.py
+++ b/layman/overlays/svn.py
@@ -50,24 +50,19 @@ class SvnOverlay(OverlaySource):
 
         super(SvnOverlay, self).add(base)
 
+        args = ['co']
         if quiet:
-            quiet_option = '-q '
-        else:
-            quiet_option = ''
+            args.append('-q')
+        args.append(self.src + '/@')
+        args.append(path([base, self.parent.name]))
 
-        return self.cmd(self.command() + ' co ' + quiet_option +
-                        '"' + self.src + '/@" "' + path([base, self.parent.name]) + '"')
+        return self.run_command(*args)
 
     def sync(self, base, quiet = False):
         '''Sync overlay.'''
 
         self.supported()
 
-        if quiet:
-            quiet_option = '-q '
-        else:
-            quiet_option = ''
-
         def checkout_location():
             # Append '@' iff needed
             # Keeps users of SVN <1.6.5 happy in more cases (bug #313303)
@@ -76,8 +71,13 @@ class SvnOverlay(OverlaySource):
                 repo_part = repo_part + '@'
             return path([base, repo_part])
 
-        return self.cmd(self.command() + ' up ' + quiet_option +
-                        '"' + checkout_location() + '"')
+        # svn up [-q] TARGET
+        args = ['up']
+        if quiet:
+            args.append('-q')
+        args.append(checkout_location())
+
+        return self.run_command(*args)
 
     def supported(self):
         '''Overlay type supported?'''
diff --git a/layman/overlays/tar.py b/layman/overlays/tar.py
index 71369a6..b86f7d3 100644
--- a/layman/overlays/tar.py
+++ b/layman/overlays/tar.py
@@ -126,8 +126,9 @@ class TarOverlay(OverlaySource):
             raise Exception('Failed to store tar package in '
                             + pkg + '\nError was:' + str(error))
 
-        result = self.cmd(self.command() + u' -v -x' + u' -f "' + pkg
-                          + u'" -C "' + dest_dir + u'"')
+        # tar -v -x -f SOURCE -C TARGET
+        args = ['-v', '-x', '-f', pkg, '-C', dest_dir]
+        result = self.run_command(*args)
 
         os.unlink(pkg)
         return result