# HG changeset patch # User osimons # Date 1250028353 0 # Node ID 05686657e98953f40a06afb90601afe4f0e34de1 # Parent 0a9f2af1f711aa8853de07f31af74996971e0617 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` diff --git a/bitten/build/api.py b/bitten/build/api.py --- 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: diff --git a/bitten/build/tests/api.py b/bitten/build/tests/api.py --- 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): diff --git a/bitten/recipe.py b/bitten/recipe.py --- 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): diff --git a/bitten/slave.py b/bitten/slave.py --- 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) diff --git a/bitten/slave_tests/recipe.py b/bitten/slave_tests/recipe.py --- 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):