changeset 628:05686657e989

0.6dev: Implement a `subprocess` implementation of `execute()` that is now the default for all slaves where this is available - essentially all slaves with Python 2.4 and higher, or where module is installed separately. This fixes: * #256 - correct interleaving of `stderr` and `stdout` on Windows * #257 - missing timeout on Windows * #425 - `popen2` is deprecated * #346 - trapping execution errors (where process cannot be spawned at all) * #311 - trapping argument errors (graceful message when arguments don't match) * #262 - add a 1 second delay if a step finish in same second as previous step * #226 - cleaned up log messages, and remove some ambiguity that could arise * #101 - better logging for os errors * old Windows `execute()` forgot to reset `cwd`
author osimons
date Tue, 11 Aug 2009 22:05:53 +0000
parents 0a9f2af1f711
children f3bb52da9e3c
files bitten/build/api.py bitten/build/tests/api.py bitten/recipe.py bitten/slave.py bitten/slave_tests/recipe.py
diffstat 5 files changed, 170 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/bitten/build/api.py
+++ b/bitten/build/api.py
@@ -17,6 +17,11 @@
 import time
 import sys
 
+try:
+    import subprocess
+except ImportError:
+    subprocess = None
+
 log = logging.getLogger('bitten.build.api')
 
 __docformat__ = 'restructuredtext en'
@@ -82,14 +87,110 @@
             assert os.path.isdir(self.cwd)
         self.returncode = None
 
-    if os.name == 'nt': # windows
+    if subprocess:
 
         def execute(self, timeout=None):
             """Execute the command, and return a generator for iterating over
             the output written to the standard output and error streams.
             
             :param timeout: number of seconds before the external process
-                            should be aborted (not supported on Windows)
+                            should be aborted (not supported on Windows without
+                            ``subprocess`` module / Python 2.4+)
+            """
+            from threading import Thread
+            from Queue import Queue, Empty
+
+            class ReadThread(Thread):
+                def __init__(self, pipe, pipe_name, queue):
+                    self.pipe = pipe
+                    self.pipe_name = pipe_name
+                    self.queue = queue
+                    Thread.__init__(self)
+                def run(self):
+                    while self.pipe and not self.pipe.closed:
+                        line = self.pipe.readline()
+                        if line == '':
+                            break
+                        self.queue.put((self.pipe_name, line))
+                    if not self.pipe.closed:
+                        self.pipe.close()
+
+            class WriteThread(Thread):
+                def __init__(self, pipe, data):
+                    self.pipe = pipe
+                    self.data = data
+                    Thread.__init__(self)
+                def run(self):
+                    if self.data and self.pipe and not self.pipe.closed:
+                        self.pipe.write(self.data)
+                    if not self.pipe.closed:
+                        self.pipe.close()
+
+            args = [self.executable] + self.arguments
+            try:
+                p = subprocess.Popen(args, bufsize=1, # Line buffered
+                            stdin=subprocess.PIPE,
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE,
+                            cwd=(self.cwd or None),
+                            shell=(os.name == 'nt' and True or False),
+                            universal_newlines=True,
+                            env=None)
+            except Exception, e:
+                # NT executes through shell and will not raise BuildError
+                raise BuildError('Error executing %s: %s %s' % (args,
+                                            e.__class__.__name__, str(e)))
+
+            log.debug('Executing %s, (pid = %s)', args, p.pid)
+
+            if self.input:
+                if isinstance(self.input, basestring):
+                    in_data = self.input
+                else:
+                    in_data = self.input.read()
+            else:
+                in_data = None
+            
+            queue = Queue()
+            limit = timeout and timeout + time.time() or 0
+
+            pipe_in = WriteThread(p.stdin, in_data)
+            pipe_out = ReadThread(p.stdout, 'stdout', queue)
+            pipe_err = ReadThread(p.stderr, 'stderr', queue)
+            pipe_err.start(); pipe_out.start(); pipe_in.start()
+
+            while True:
+                if limit and limit < time.time():
+                    if hasattr(subprocess, 'kill'): # Python 2.6+
+                        p.kill()
+                    raise TimeoutError('Command %s timed out' % self.executable)
+                if p.poll() != None and self.returncode == None:
+                    self.returncode = p.returncode
+                try:
+                    name, line = queue.get(block=True, timeout=.01)
+                    line = line and line.rstrip().replace('\x00', '')
+                    if name == 'stderr':
+                        yield (None, line)
+                    else:
+                        yield (line, None)
+                except Empty:
+                    if self.returncode != None:
+                        break
+
+            pipe_out.join(); pipe_in.join(); pipe_err.join()
+
+            log.debug('%s exited with code %s', self.executable,
+                      self.returncode)
+
+    elif os.name == 'nt': # windows
+
+        def execute(self, timeout=None):
+            """Execute the command, and return a generator for iterating over
+            the output written to the standard output and error streams.
+            
+            :param timeout: number of seconds before the external process
+                            should be aborted (not supported on Windows without
+                            ``subprocess`` module / Python 2.4+)
             """
             args = [self.executable] + self.arguments
             for idx, arg in enumerate(args):
@@ -118,29 +219,24 @@
             out_file, out_name = tempfile.mkstemp(prefix='bitten_',
                                                   suffix='.pipe')
             os.close(out_file)
-            err_file, err_name = tempfile.mkstemp(prefix='bitten_',
-                                                  suffix='.pipe')
-            os.close(err_file)
 
             try:
-                cmd = '( %s ) > "%s" %s 2> "%s"' % (' '.join(args), out_name,
-                                                    in_redirect, err_name)
+                # NT without subprocess joins output from stdout & stderr
+                cmd = '( %s ) > "%s" %s 2>&1' % (' '.join(args), out_name,
+                                                    in_redirect)
+                log.info("running: %s", cmd)
                 self.returncode = os.system(cmd)
                 log.debug('Exited with code %s', self.returncode)
 
                 out_file = file(out_name, 'r')
-                err_file = file(err_name, 'r')
                 out_lines = out_file.readlines()
-                err_lines = err_file.readlines()
+                err_lines = []
                 out_file.close()
-                err_file.close()
             finally:
                 if in_name:
                     os.unlink(in_name)
                 if out_name:
                     os.unlink(out_name)
-                if err_name:
-                    os.unlink(err_name)
                 if self.cwd:
                     os.chdir(old_cwd)
 
@@ -150,6 +246,9 @@
                       err_line and _decode(
                                     err_line.rstrip().replace('\x00', ''))
 
+            if self.cwd:
+                os.chdir(old_cwd)
+
     else: # posix
 
         def execute(self, timeout=None):
@@ -157,7 +256,8 @@
             the output written to the standard output and error streams.
             
             :param timeout: number of seconds before the external process
-                            should be aborted (not supported on Windows)
+                            should be aborted (not supported on Windows without
+                            ``subprocess`` module / Python 2.4+)
             """
             import popen2, select
             if self.cwd:
--- a/bitten/build/tests/api.py
+++ b/bitten/build/tests/api.py
@@ -14,7 +14,12 @@
 import tempfile
 import unittest
 
-from bitten.build import CommandLine, FileSet, TimeoutError
+try:
+    has_subprocess = __import__('subprocess') and True
+except:
+    has_subprocess = False
+
+from bitten.build import CommandLine, FileSet, TimeoutError, BuildError
 from bitten.build.api import _combine
 
 
@@ -72,6 +77,11 @@
                 stderr.append(err)
         py_version = '.'.join([str(v) for (v) in sys.version_info[:3]]
                                                             ).rstrip('.0')
+
+        # NT without subprocess doesn't split stderr and stdout. See #256.
+        if not has_subprocess and os.name == "nt":
+            return
+
         self.assertEqual(['Python %s' % py_version], stderr)
         self.assertEqual([], stdout)
         self.assertEqual(0, cmdline.returncode)
@@ -110,7 +120,7 @@
             stderr.append(err)
         py_version = '.'.join([str(v) for (v) in sys.version_info[:3]])
         # nt doesn't properly split stderr and stdout. See ticket #256.
-        if os.name != "nt":
+        if has_subprocess or os.name != "nt":
             self.assertEqual(['Hello', 'world!', None], stdout)
         self.assertEqual(0, cmdline.returncode)
 
@@ -163,10 +173,26 @@
 """)
         cmdline = CommandLine('python', [script_file])
         iterable = iter(cmdline.execute(timeout=.5))
-        if os.name != "nt":
+        if has_subprocess or os.name != "nt":
             # commandline timeout not implemented on windows. See #257
             self.assertRaises(TimeoutError, iterable.next)
 
+    def test_nonexisting_command(self):
+        if not has_subprocess:
+            # Test only valid for subprocess execute()
+            return
+        cmdline = CommandLine('doesnotexist', [])
+        iterable = iter(cmdline.execute())
+        try:
+            out, err = iterable.next()
+            if os.name == 'nt':
+                # NT executes through shell and will not raise BuildError
+                self.failUnless("'doesnotexist' is not recognized" in err)
+            else:
+                self.fail("Expected BuildError")
+        except BuildError, e:
+            self.failUnless("Error executing ['doesnotexist']" in str(e))
+
 class FileSetTestCase(unittest.TestCase):
 
     def setUp(self):
--- a/bitten/recipe.py
+++ b/bitten/recipe.py
@@ -14,9 +14,11 @@
 most importantly the `Recipe` class.
 """
 
+import inspect
 import keyword
 import logging
 import os
+import time
 try:
     set
 except NameError:
@@ -90,6 +92,12 @@
             args = dict([(escape(name),
                           self.config.interpolate(attr[name], **self.vars))
                          for name in attr])
+            function_args, has_kwargs = inspect.getargspec(function)[0:3:2]
+            for arg in args:
+                if not arg in function_args or not has_kwargs:
+                    raise InvalidRecipeError(
+                            "Unsupported argument '%s' for command %s" % \
+                            (arg, qname))
 
             self.generator = qname
             log.debug('Executing %s with arguments: %s', function, args)
@@ -188,8 +196,15 @@
         :param ctxt: the build context
         :type ctxt: `Context`
         """
+        last_finish = time.time()
         for child in self._elem:
-            ctxt.run(self, child.namespace, child.name, child.attr)
+            try:
+                ctxt.run(self, child.namespace, child.name, child.attr)
+            except (BuildError, InvalidRecipeError), e:
+                ctxt.error(e)
+        if time.time() < last_finish + 1:
+            # Add a delay to make sure steps appear in correct order
+            time.sleep(1)
 
         errors = []
         while ctxt.output:
@@ -198,10 +213,11 @@
             if type == Recipe.ERROR:
                 errors.append((generator, output))
         if errors:
+            for _t, error in errors:
+                log.error(error)
             if self.onerror != 'ignore':
-                raise BuildError('Build step %s failed' % self.id)
-            log.warning('Continuing despite errors in step %s (%s)', self.id,
-                        ', '.join([error[1] for error in errors]))
+                raise BuildError("Build step '%s' failed" % self.id)
+            log.warning("Continuing despite errors in step '%s'", self.id)
 
 
 class Recipe(object):
--- a/bitten/slave.py
+++ b/bitten/slave.py
@@ -298,7 +298,7 @@
             log.warning('Build interrupted')
             self._cancel_build(build_url)
         except BuildError, e:
-            log.error('Build step %r failed (%s)', step.id, e)
+            log.error('Build step %r failed', step.id)
             failed = True
         except Exception, e:
             log.error('Internal error in build step %r', step.id, exc_info=True)
@@ -306,7 +306,6 @@
         xml.attr['duration'] = (datetime.utcnow() - started).seconds
         if failed:
             xml.attr['status'] = 'failure'
-            log.warning('Build step %r failed', step.id)
         else:
             xml.attr['status'] = 'success'
             log.info('Build step %s completed successfully', step.id)
--- a/bitten/slave_tests/recipe.py
+++ b/bitten/slave_tests/recipe.py
@@ -34,6 +34,13 @@
         self.assertEquals(os.path.realpath('/foo/bar/baz'),
                         ctxt.vars['basedir'])
 
+    def test_run_wrong_arg(self):
+        ctxt = Context(self.basedir)
+        try:
+            ctxt.run(1, 'http://bitten.cmlenz.net/tools/sh', 'exec', {'foo':'bar'})
+            self.fail("InvalidRecipeError expected")
+        except InvalidRecipeError, e:
+            self.failUnless("Unsupported argument 'foo'" in str(e))
 
 class RecipeTestCase(unittest.TestCase):
 
Copyright (C) 2012-2017 Edgewall Software