Merge lp:~canonical-ca-hackers/tarmac/jenkins into lp:tarmac

[Jonathan Lange]

WIP MP to make the delta visible

Unmerged revisions

410. By James Westby on 2012-11-15

Merge the check.py file.

409. By James Westby on 2012-11-15

Merge sidnei's fix for the timeout thing.

408. By James Westby on 2012-07-31

Fix typo.

407. By James Westby on 2012-07-31

Fix missing method.

406. By James Westby on 2012-07-31

Remove debugging.

405. By James Westby on 2012-07-31

Fix typo.

404. By James Westby on 2012-07-31

Add the commit plugin that sets [r=reviewer].

403. By James Westby on 2012-07-31

Ignore decode errors when decoding streams from executed commands.

402. By James Westby on 2012-07-26

Add commands for better jenkins integration.

=== modified file 'bin/tarmac'
--- bin/tarmac	2010-06-17 17:23:37 +0000
+++ bin/tarmac	2012-12-18 13:35:32 +0000
@@ -2,5 +2,7 @@
 # vim:filetype=python
 '''Main tarmac script.'''
 
+import sys
+
 from tarmac.bin import main
-main()
+sys.exit(main())

=== added file 'check.py'
--- check.py	1970-01-01 00:00:00 +0000
+++ check.py	2012-12-18 13:35:32 +0000
@@ -0,0 +1,89 @@
+#!/usr/bin/env python
+import httplib2
+import sys
+
+from tarmac.bin.commands import TarmacCommand
+from tarmac.bin.registry import CommandRegistry
+from tarmac.exceptions import TarmacCommandError
+from tarmac.log import set_up_debug_logging
+from tarmac.plugin import load_plugins
+
+
+registry = CommandRegistry()
+
+
+class cmd_check(TarmacCommand):
+
+    takes_args = ['branch_url?']
+
+    def _check_merges(self, branch_url):
+
+        lp_branch = self.launchpad.branches.getByUrl(url=branch_url)
+        if lp_branch is None:
+            self.logger.info('Not a valid branch: {0}'.format(branch_url))
+            return
+
+        proposals = self._get_mergable_proposals_for_branch(lp_branch)
+
+        if not proposals:
+            self.logger.info(
+                'No approved proposals found for %(branch_url)s' % {
+                    'branch_url': branch_url})
+            return 1
+        return 0
+
+    def _get_mergable_proposals_for_branch(self, lp_branch):
+        """Return a list of the mergable proposals for the given branch."""
+        proposals = []
+        for entry in lp_branch.landing_candidates:
+            self.logger.debug("Considering merge proposal: {0}".format(entry))
+
+            if entry.queue_status != u'Approved':
+                self.logger.debug(
+                    "  Skipping proposal: status is {0}, not "
+                    "'Approved'".format(entry.queue_status))
+                continue
+
+            if (not entry.commit_message):
+                self.logger.debug(
+                    "  Skipping proposal: proposal has no commit message")
+                continue
+
+            proposals.append(entry)
+        return proposals
+
+    def run(self, branch_url=None, launchpad=None, **kwargs):
+        for key, value in kwargs.iteritems():
+            self.config.set('Tarmac', key, value)
+
+        set_up_debug_logging()
+
+        self.launchpad = launchpad
+        if self.launchpad is None:
+            self.logger.debug('Loading launchpad object')
+            self.launchpad = self.get_launchpad_object()
+            self.logger.debug('launchpad object loaded')
+
+        if branch_url:
+            self.logger.debug(
+                '%(branch_url)s specified as branch_url' % {
+                    'branch_url': branch_url})
+            if not branch_url.startswith('lp:'):
+                raise TarmacCommandError('Branch urls must start with lp:')
+            return self._check_merges(branch_url)
+        else:
+            for branch in self.config.branches:
+                self.logger.debug(
+                    'Merging approved branches against %(branch)s' % {
+                        'branch': branch})
+                try:
+                    return self._check_merges(branch)
+                except Exception, error:
+                    self.logger.error(
+                        'An error occurred trying to merge %s: %s',
+                        branch, error)
+                    raise
+
+
+cmd = cmd_check(registry)
+sys.exit(cmd.run(sys.argv[1]))

=== modified file 'tarmac/bin/__init__.py'
--- tarmac/bin/__init__.py	2010-09-02 15:18:07 +0000
+++ tarmac/bin/__init__.py	2012-12-18 13:35:32 +0000
@@ -27,4 +27,4 @@
     args = sys.argv[1:]
     if not args:
         args = ['help']
-    registry.run(args)
+    return registry.run(args)

=== modified file 'tarmac/bin/commands.py'
--- tarmac/bin/commands.py	2012-06-29 20:03:55 +0000
+++ tarmac/bin/commands.py	2012-12-18 13:35:32 +0000
@@ -5,7 +5,7 @@
 import re
 
 from bzrlib.commands import Command
-from bzrlib.errors import PointlessMerge
+from bzrlib.errors import PointlessMerge, LockContention
 from bzrlib.help import help_commands
 from launchpadlib.launchpad import Launchpad
 from launchpadlib.uris import (LPNET_SERVICE_ROOT,
@@ -134,6 +134,55 @@
         help_commands(self.outf)
 
 
+def _get_mergable_proposals_for_branch(lp_branch, logger, imply_commit_message=False):
+    """Return a list of the mergable proposals for the given branch."""
+    proposals = []
+    for entry in lp_branch.landing_candidates:
+        logger.debug("Considering merge proposal: {0}".format(entry.web_link))
+
+        if entry.queue_status != u'Approved':
+            logger.debug(
+                "  Skipping proposal: status is {0}, not "
+                "'Approved'".format(entry.queue_status))
+            continue
+
+        if (not imply_commit_message and not entry.commit_message):
+            logger.debug(
+                "  Skipping proposal: proposal has no commit message")
+            continue
+
+        proposals.append(entry)
+    return proposals
+
+
+def _get_reviews(proposal):
+    """Get the set of reviews from the proposal."""
+    reviews = []
+    for vote in proposal.votes:
+        if not vote.comment:
+            continue
+        else:
+            reviews.append('%s;%s' % (vote.reviewer.display_name,
+                                      vote.comment.vote))
+
+    if len(reviews) == 0:
+        return None
+
+    return reviews
+
+
+def setup(logger, debug=False, http_debug=False):
+    if debug:
+        set_up_debug_logging()
+        logger.debug('Debug logging enabled')
+    if http_debug:
+        httplib2.debuglevel = 1
+        logger.debug('HTTP debugging enabled.')
+    logger.debug('Loading plugins')
+    load_plugins()
+    logger.debug('Plugins loaded')
+
+
 class cmd_merge(TarmacCommand):
     '''Automatically merge approved merge proposal branches.'''
 
@@ -142,7 +191,8 @@
     takes_options = [
         options.http_debug_option,
         options.debug_option,
-        options.imply_commit_message_option]
+        options.imply_commit_message_option,
+        options.one_option]
 
     def _do_merges(self, branch_url):
 
@@ -151,7 +201,8 @@
             self.logger.info('Not a valid branch: {0}'.format(branch_url))
             return
 
-        proposals = self._get_mergable_proposals_for_branch(lp_branch)
+        proposals = _get_mergable_proposals_for_branch(lp_branch, self.logger,
+                imply_commit_message=self.config.imply_commit_message)
 
         if not proposals:
             self.logger.info(
@@ -167,13 +218,10 @@
 
         try:
             for proposal in proposals:
-
+                target.cleanup()
                 self.logger.debug(
                     u'Preparing to merge %(source_branch)s' % {
-                        'source_branch': proposal.source_branch.bzr_identity})
-                source = Branch.create(
-                    proposal.source_branch, self.config, target=target)
-
+                        'source_branch': proposal.source_branch.web_link})
                 try:
                     prerequisite = proposal.prerequisite_branch
                     if prerequisite:
@@ -185,31 +233,37 @@
                                 u'No proposals of prerequisite branch.',
                                 u'No proposals found for merge of %s '
                                 u'into %s.' % (
-                                    prerequisite.display_name,
-                                    target.lp_branch.display_name))
+                                    prerequisite.web_link,
+                                    target.lp_branch.web_link))
                         elif len(merges) > 1:
                             raise TarmacMergeError(
                                 u'Too many proposals of prerequisite.',
                                 u'More than one proposal found for merge '
                                 u'of %s into %s, which is not Superseded.' % (
-                                    prerequisite.display_name,
-                                    target.lp_branch.display_name))
+                                    prerequisite.web_link,
+                                    target.lp_branch.web_link))
                         elif len(merges) == 1:
                             if merges[0].queue_status != u'Merged':
                                 raise TarmacMergeError(
-                                    u'Prerequsiite not yet merged.',
+                                    u'Prerequisite not yet merged.',
                                     u'The prerequisite %s has not yet been '
                                     u'merged into %s.' % (
-                                        prerequisite.display_name,
-                                        target.lp_branch.display_name))
+                                        prerequisite.web_link,
+                                        target.lp_branch.web_link))
 
                     if not proposal.reviewed_revid:
                         raise TarmacMergeError(
                             u'No approved revision specified.')
 
-                    approved = source.bzr_branch.revision_id_to_revno(
+
+                    source = Branch.create(
+                        proposal.source_branch, self.config, target=target)
+
+                    source_bzr_branch = source.get_bzr_branch()
+                    approved = source_bzr_branch.revision_id_to_revno(
                         str(proposal.reviewed_revid))
-                    tip = source.bzr_branch.revno()
+                    tip = source_bzr_branch.revno()
+
                     if tip > approved:
                         message = u'Unapproved changes made after approval'
                         lp_comment = (
@@ -220,12 +274,10 @@
 
                     self.logger.debug(
                         'Merging %(source)s at revision %(revision)s' % {
-                            'source': proposal.source_branch.display_name,
+                            'source': proposal.source_branch.web_link,
                             'revision': proposal.reviewed_revid})
 
-                    target.merge(
-                        source,
-                        str(proposal.reviewed_revid))
+                    target.merge(source, str(proposal.reviewed_revid))
 
                     self.logger.debug('Firing tarmac_pre_commit hook')
                     tarmac_hooks.fire('tarmac_pre_commit',
@@ -234,8 +286,8 @@
                 except TarmacMergeError, failure:
                     self.logger.warn(
                         u'Merging %(source)s into %(target)s failed: %(msg)s' %
-                        {'source': proposal.source_branch.display_name,
-                         'target': proposal.target_branch.display_name,
+                        {'source': proposal.source_branch.web_link,
+                         'target': proposal.target_branch.web_link,
                          'msg': str(failure)})
 
                     subject = u'Re: [Merge] %(source)s into %(target)s' % {
@@ -247,23 +299,25 @@
                     else:
                         comment = str(failure)
 
-                    proposal.createComment(subject=subject,
-                                           content=comment)
+                    proposal.createComment(subject=subject, content=comment)
                     try:
                         proposal.setStatus(
                             status=self.config.rejected_branch_status)
                     except AttributeError:
                         proposal.setStatus(status=u'Needs review')
                     proposal.lp_save()
-                    target.cleanup()
+
+                    # If we've been asked to only merge one branch, then exit.
+                    if self.config.one:
+                        return True
+
                     continue
                 except PointlessMerge:
                     self.logger.warn(
                         'Merging %(source)s into %(target)s would be '
                         'pointless.' % {
-                            'source': proposal.source_branch.display_name,
-                            'target': proposal.target_branch.display_name})
-                    target.cleanup()
+                            'source': proposal.source_branch.web_link,
+                            'target': proposal.target_branch.web_link})
                     continue
 
                 merge_url = get_review_url(proposal)
@@ -272,16 +326,17 @@
                 commit_message = proposal.commit_message
                 if commit_message is None and self.config.imply_commit_message:
                     commit_message = proposal.description
+
                 target.commit(commit_message,
-                             revprops=revprops,
-                             authors=source.authors,
-                             reviews=self._get_reviews(proposal))
+                              reviews=_get_reviews(proposal))
 
                 self.logger.debug('Firing tarmac_post_commit hook')
                 tarmac_hooks.fire('tarmac_post_commit',
                                   self, target, source, proposal)
 
-                target.cleanup()
+                # If we've been asked to only merge one branch, then exit.
+                if self.config.one:
+                    return True
 
         # This except is here because we need the else and can't have it
         # without an except as well.
@@ -294,55 +349,13 @@
         finally:
             target.cleanup()
 
-    def _get_mergable_proposals_for_branch(self, lp_branch):
-        """Return a list of the mergable proposals for the given branch."""
-        proposals = []
-        for entry in lp_branch.landing_candidates:
-            self.logger.debug("Considering merge proposal: {0}".format(entry))
-
-            if entry.queue_status != u'Approved':
-                self.logger.debug(
-                    "  Skipping proposal: status is {0}, not "
-                    "'Approved'".format(entry.queue_status))
-                continue
-
-            if (not self.config.imply_commit_message and
-                not entry.commit_message):
-                self.logger.debug(
-                    "  Skipping proposal: proposal has no commit message")
-                continue
-
-            proposals.append(entry)
-        return proposals
-
-    def _get_reviews(self, proposal):
-        """Get the set of reviews from the proposal."""
-        reviews = []
-        for vote in proposal.votes:
-            if not vote.comment:
-                continue
-            else:
-                reviews.append('%s;%s' % (vote.reviewer.display_name,
-                                          vote.comment.vote))
-
-        if len(reviews) == 0:
-            return None
-
-        return reviews
 
     def run(self, branch_url=None, launchpad=None, **kwargs):
         for key, value in kwargs.iteritems():
             self.config.set('Tarmac', key, value)
 
-        if self.config.debug:
-            set_up_debug_logging()
-            self.logger.debug('Debug logging enabled')
-        if self.config.http_debug:
-            httplib2.debuglevel = 1
-            self.logger.debug('HTTP debugging enabled.')
-        self.logger.debug('Loading plugins')
-        load_plugins()
-        self.logger.debug('Plugins loaded')
+        setup(self.logger, debug=self.config.debug,
+                http_debug=self.config.http_debug)
 
         self.launchpad = launchpad
         if self.launchpad is None:
@@ -351,9 +364,8 @@
             self.logger.debug('launchpad object loaded')
 
         if branch_url:
-            self.logger.debug(
-                '%(branch_url)s specified as branch_url' % {
-                    'branch_url': branch_url})
+            self.logger.debug('%(branch_url)s specified as branch_url' % {
+                'branch_url': branch_url})
             if not branch_url.startswith('lp:'):
                 raise TarmacCommandError('Branch urls must start with lp:')
             self._do_merges(branch_url)
@@ -364,9 +376,249 @@
                     'Merging approved branches against %(branch)s' % {
                         'branch': branch})
                 try:
-                    self._do_merges(branch)
+                    merged = self._do_merges(branch)
+
+                    # If we've been asked to only merge one branch, then exit.
+                    if merged and self.config.one:
+                        break
+                except LockContention:
+                    continue
                 except Exception, error:
                     self.logger.error(
                         'An error occurred trying to merge %s: %s',
                         branch, error)
                     raise
+
+
+class cmd_check(TarmacCommand):
+    '''Check whether there are any merge proposals ready to land.'''
+
+    takes_args = ['branch_url']
+    takes_options = [
+        options.http_debug_option,
+        options.debug_option]
+
+    def _any_merges(self, branch_url):
+
+        lp_branch = self.launchpad.branches.getByUrl(url=branch_url)
+        if lp_branch is None:
+            self.logger.info('Not a valid branch: {0}'.format(branch_url))
+            return
+
+        proposals = _get_mergable_proposals_for_branch(lp_branch, self.logger)
+
+        if not proposals:
+            self.logger.info(
+                'No approved proposals found for %(branch_url)s' % {
+                    'branch_url': branch_url})
+            return False
+        return True
+
+    def run(self, branch_url, launchpad=None, **kwargs):
+        for key, value in kwargs.iteritems():
+            self.config.set('Tarmac', key, value)
+
+        setup(self.logger, debug=self.config.debug,
+                http_debug=self.config.http_debug)
+
+        self.launchpad = launchpad
+        if self.launchpad is None:
+            self.logger.debug('Loading launchpad object')
+            self.launchpad = self.get_launchpad_object()
+            self.logger.debug('launchpad object loaded')
+
+        self.logger.debug('%(branch_url)s specified as branch_url' % {
+            'branch_url': branch_url})
+        if not branch_url.startswith('lp:'):
+            raise TarmacCommandError('Branch urls must start with lp:')
+        ret = self._any_merges(branch_url)
+        if ret:
+            return 0
+        return 1
+
+
+class cmd_merge_one(TarmacCommand):
+    '''Try and merge one branch.
+
+    Returns 0 if a branch was merged.
+    Return 1 if there were no branches to merge.
+    Returns 2 if the branch failed to merge.
+    '''
+
+    takes_args = ['branch_url']
+    takes_options = [
+        options.http_debug_option,
+        options.debug_option,
+        options.verify_command_option]
+
+    def _pick_merge(self, lp_branch, branch_url):
+        proposals = _get_mergable_proposals_for_branch(lp_branch, self.logger)
+
+        if not proposals:
+            self.logger.info(
+                'No approved proposals found for %(branch_url)s' % {
+                    'branch_url': branch_url})
+            return None
+        return proposals[0]
+
+    def _do_merge(self, lp_branch, proposal, verify_command=None):
+        target = Branch.create(lp_branch, self.config, create_tree=True)
+
+        if verify_command is not None:
+            setattr(target.config, 'verify_command', verify_command)
+
+        self.logger.debug('Firing tarmac_pre_merge hook')
+        tarmac_hooks['tarmac_pre_merge'].fire(self, target)
+
+        try:
+            target.cleanup()
+            self.logger.debug(
+                u'Preparing to merge %(source_branch)s' % {
+                    'source_branch': proposal.source_branch.web_link})
+            try:
+                prerequisite = proposal.prerequisite_branch
+                if prerequisite:
+                    merges = [x for x in  prerequisite.landing_targets
+                              if x.target_branch == target.lp_branch and
+                              x.queue_status != u'Superseded']
+                    if len(merges) == 0:
+                        raise TarmacMergeError(
+                            u'No proposals of prerequisite branch.',
+                            u'No proposals found for merge of %s '
+                            u'into %s.' % (
+                                prerequisite.web_link,
+                                target.lp_branch.web_link))
+                    elif len(merges) > 1:
+                        raise TarmacMergeError(
+                            u'Too many proposals of prerequisite.',
+                            u'More than one proposal found for merge '
+                            u'of %s into %s, which is not Superseded.' % (
+                                prerequisite.web_link,
+                                target.lp_branch.web_link))
+                    elif len(merges) == 1:
+                        if merges[0].queue_status != u'Merged':
+                            raise TarmacMergeError(
+                                u'Prerequisite not yet merged.',
+                                u'The prerequisite %s has not yet been '
+                                u'merged into %s.' % (
+                                    prerequisite.web_link,
+                                    target.lp_branch.web_link))
+
+                if not proposal.reviewed_revid:
+                    raise TarmacMergeError(
+                        u'No approved revision specified.')
+
+
+                source = Branch.create(
+                    proposal.source_branch, self.config, target=target)
+
+                approved = source.bzr_branch.revision_id_to_revno(
+                    str(proposal.reviewed_revid))
+                tip = source.bzr_branch.revno()
+
+                if tip > approved:
+                    message = u'Unapproved changes made after approval'
+                    lp_comment = (
+                        u'There are additional revisions which have not '
+                        u'been approved in review. Please seek review and '
+                        u'approval of these new revisions.')
+                    raise UnapprovedChanges(message, lp_comment)
+
+                self.logger.debug(
+                    'Merging %(source)s at revision %(revision)s' % {
+                        'source': proposal.source_branch.web_link,
+                        'revision': proposal.reviewed_revid})
+
+                target.merge(source, str(proposal.reviewed_revid))
+
+                self.logger.debug('Firing tarmac_pre_commit hook')
+                tarmac_hooks['tarmac_pre_commit'].fire(
+                    self, target, source, proposal)
+
+            except TarmacMergeError, failure:
+                self.logger.warn(
+                    u'Merging %(source)s into %(target)s failed: %(msg)s' %
+                    {'source': proposal.source_branch.web_link,
+                     'target': proposal.target_branch.web_link,
+                     'msg': str(failure)})
+
+                subject = u'Re: [Merge] %(source)s into %(target)s' % {
+                    "source": proposal.source_branch.display_name,
+                    "target": proposal.target_branch.display_name}
+
+                if failure.comment:
+                    comment = failure.comment
+                else:
+                    comment = str(failure)
+
+                proposal.createComment(subject=subject, content=comment)
+                try:
+                    proposal.setStatus(
+                        status=self.config.rejected_branch_status)
+                except AttributeError:
+                    proposal.setStatus(status=u'Needs review')
+                proposal.lp_save()
+
+                return False
+            except PointlessMerge:
+                self.logger.warn(
+                    'Merging %(source)s into %(target)s would be '
+                    'pointless.' % {
+                        'source': proposal.source_branch.web_link,
+                        'target': proposal.target_branch.web_link})
+                return False
+
+            merge_url = get_review_url(proposal)
+            revprops = {'merge_url': merge_url}
+
+            commit_message = proposal.commit_message
+            if commit_message is None and self.config.imply_commit_message:
+                commit_message = proposal.description
+            target.commit(commit_message,
+                         revprops=revprops,
+                         authors=source.authors,
+                         reviews=_get_reviews(proposal))
+
+            self.logger.debug('Firing tarmac_post_commit hook')
+            tarmac_hooks['tarmac_post_commit'].fire(
+                self, target, source, proposal)
+            return True
+
+        # This except is here because we need the else and can't have it
+        # without an except as well.
+        except:
+            raise
+        else:
+            self.logger.debug('Firing tarmac_post_merge hook')
+            tarmac_hooks['tarmac_post_merge'].fire(self, target)
+        finally:
+            target.cleanup()
+
+    def run(self, branch_url, launchpad=None, verify_command=None, **kwargs):
+        for key, value in kwargs.iteritems():
+            self.config.set('Tarmac', key, value)
+
+        setup(self.logger, debug=self.config.debug,
+                http_debug=self.config.http_debug)
+
+        self.launchpad = launchpad
+        if self.launchpad is None:
+            self.logger.debug('Loading launchpad object')
+            self.launchpad = self.get_launchpad_object()
+            self.logger.debug('launchpad object loaded')
+
+        self.logger.debug('%(branch_url)s specified as branch_url' % {
+            'branch_url': branch_url})
+        if not branch_url.startswith('lp:'):
+            raise TarmacCommandError('Branch urls must start with lp:')
+        lp_branch = self.launchpad.branches.getByUrl(url=branch_url)
+        if lp_branch is None:
+            self.logger.info('Not a valid branch: {0}'.format(branch_url))
+            return 3
+        proposal = self._pick_merge(lp_branch, branch_url)
+        if proposal is None:
+            return 1
+        successful = self._do_merge(lp_branch, proposal, verify_command=verify_command)
+        if successful:
+            return 0
+        return 2

=== modified file 'tarmac/bin/options.py'
--- tarmac/bin/options.py	2011-01-06 20:20:19 +0000
+++ tarmac/bin/options.py	2012-12-18 13:35:32 +0000
@@ -15,3 +15,9 @@
     'imply-commit-message',
     help=("Use the description as a commit message if the branch "
           "doesn't have a message"))
+one_option = Option(
+    'one', short_name='1',
+    help='Merge only one branch and exit.')
+verify_command_option = Option(
+    'verify-command', type=str,
+    help='The verify command to run.')

=== modified file 'tarmac/bin/registry.py'
--- tarmac/bin/registry.py	2010-10-25 20:20:18 +0000
+++ tarmac/bin/registry.py	2012-12-18 13:35:32 +0000
@@ -45,7 +45,7 @@
 
     def _run(self, args):
         '''Execute the command.'''
-        run_bzr(args)
+        return run_bzr(args)
 
     def install_hooks(self):
         '''Use the bzrlib Command support for running commands.'''
@@ -57,7 +57,7 @@
     def run(self, args):
         '''Execute the command.'''
         try:
-            self._run(args)
+            return self._run(args)
         except BzrCommandError, e:
             sys.exit('tarmac: ERROR: ' + str(e))
 

=== modified file 'tarmac/branch.py'
--- tarmac/branch.py	2012-06-29 20:03:55 +0000
+++ tarmac/branch.py	2012-12-18 13:35:32 +0000
@@ -37,7 +37,6 @@
 
     def __init__(self, lp_branch, config=False, target=None):
         self.lp_branch = lp_branch
-        self.bzr_branch = bzr_branch.Branch.open(self.lp_branch.bzr_identity)
         if config:
             self.config = BranchConfig(lp_branch.bzr_identity, config)
         else:
@@ -45,15 +44,27 @@
 
         self.target = target
         self.logger = logging.getLogger('tarmac')
+        self.temp_tree_dir = None
+
+    def get_bzr_branch(self):
+        return bzr_branch.Branch.open(self.lp_branch.bzr_identity)
+
+    # For backwards compatibility
+    bzr_branch = property(get_bzr_branch)
+
+    def get_tree(self):
+        if self.temp_tree_dir is not None:
+            return WorkingTree.open(self.temp_tree_dir)            
+        if os.path.exists(self.config.tree_dir):
+            return WorkingTree.open(self.config.tree_dir)
+
+    # For backwards compatibility
+    tree = property(get_tree)
 
     def __del__(self):
         """Do some potenetially necessary cleanup during deletion."""
-        try:
-            # If we were using a temp directory, then remove it
+        if self.temp_tree_dir is not None:
             shutil.rmtree(self.temp_tree_dir)
-        except AttributeError:
-            # Not using a tempdir
-            pass
 
     @classmethod
     def create(cls, lp_branch, config, create_tree=False, target=None):
@@ -62,50 +73,59 @@
             clazz.create_tree()
         return clazz
 
+
     def create_tree(self):
         '''Create the dir and working tree.'''
+        bzr_branch = self.get_bzr_branch()
         try:
-            self.logger.debug(
-                'Using tree in %(tree_dir)s' % {
-                    'tree_dir': self.config.tree_dir})
-            if os.path.exists(self.config.tree_dir):
-                self.tree = WorkingTree.open(self.config.tree_dir)
-            else:
+            tree = self.get_tree()
+            if tree is None:
                 self.logger.debug('Tree does not exist.  Creating dir')
                 # Create the path up to but not including tree_dir if it does
                 # not exist.
                 parent_dir = os.path.dirname(self.config.tree_dir)
                 if not os.path.exists(parent_dir):
                     os.makedirs(parent_dir)
-                self.tree = self.bzr_branch.create_checkout(
-                    self.config.tree_dir, lightweight=True)
+                tree = bzr_branch.create_checkout(
+                    self.config.tree_dir, lightweight=False)
+            self.logger.debug(
+                'Using tree in %(tree_dir)s' % {
+                    'tree_dir': self.config.tree_dir})
         except AttributeError:
             # Store this so we can rmtree later
             self.temp_tree_dir = tempfile.mkdtemp()
             self.logger.debug(
                 'Using temp dir at %(tree_dir)s' % {
                     'tree_dir': self.temp_tree_dir})
-            self.tree = self.bzr_branch.create_checkout(self.temp_tree_dir)
+            tree = bzr_branch.create_checkout(self.temp_tree_dir)
 
         self.cleanup()
 
     def cleanup(self):
         '''Remove the working tree from the temp dir.'''
-        assert self.tree
-        self.tree.revert()
-        for filename in [self.tree.abspath(f) for f in self.unmanaged_files]:
+        tree = self.get_tree()
+        assert tree
+        self.logger.info("Running cleanup in %s." % (
+            self.lp_branch.bzr_identity))
+        tree.revert()
+        self.logger.info("Reverted changes in %s." % (
+            self.lp_branch.bzr_identity))
+        for filename in [tree.abspath(f) for f in self.unmanaged_files]:
             if os.path.isdir(filename) and not os.path.islink(filename):
                 shutil.rmtree(filename)
             else:
                 os.remove(filename)
 
-        self.tree.update()
+        self.logger.info("Successfully removed extra files from %s." % (
+            self.lp_branch.bzr_identity))
+        tree.update()
 
     def merge(self, branch, revid=None):
         '''Merge from another tarmac.branch.Branch instance.'''
-        assert self.tree
-        conflict_list = self.tree.merge_from_branch(
-            branch.bzr_branch, to_revision=revid)
+        tree = self.get_tree()
+        assert tree
+        conflict_list = tree.merge_from_branch(
+            branch.get_bzr_branch(), to_revision=revid)
         if conflict_list:
             message = u'Conflicts merging branch.'
             lp_comment = (
@@ -118,24 +138,29 @@
     @property
     def unmanaged_files(self):
         """Get the list of ignored and unknown files in the tree."""
-        self.tree.lock_read()
-        unmanaged = [x for x in self.tree.unknowns()]
-        unmanaged.extend([x[0] for x in self.tree.ignored_files()])
-        self.tree.unlock()
+        tree = self.get_tree()
+        assert tree
+        tree.lock_read()
+        unmanaged = [x for x in tree.unknowns()]
+        unmanaged.extend([x[0] for x in tree.ignored_files()])
+        tree.unlock()
         return unmanaged
 
     @property
     def conflicts(self):
         '''Print the conflicts.'''
-        assert self.tree.conflicts()
+        tree = self.get_tree()
+        assert tree
         conflicts = []
-        for conflict in self.tree.conflicts():
+        for conflict in tree.conflicts():
             conflicts.append(
                 u'%s in %s' % (conflict.typestring, conflict.path))
         return '\n'.join(conflicts)
 
     def commit(self, commit_message, revprops=None, **kwargs):
         '''Commit changes.'''
+        tree = self.get_tree()
+        assert tree
         if not revprops:
             revprops = {}
 
@@ -152,8 +177,8 @@
                                            'review identity or vote.')
             revprops['reviews'] = '\n'.join(reviews)
 
-        self.tree.commit(commit_message, committer='Tarmac',
-                         revprops=revprops, authors=authors)
+        tree.commit(commit_message, committer='Tarmac',
+                    revprops=revprops, authors=authors)
 
     @property
     def landing_candidates(self):
@@ -164,18 +189,20 @@
     def authors(self):
         author_list = []
 
+        bzr_branch = self.get_bzr_branch()
         if self.target:
-            self.bzr_branch.lock_read()
-            self.target.bzr_branch.lock_read()
+            target_bzr_branch = self.target.get_bzr_branch()
+            bzr_branch.lock_read()
+            target_bzr_branch.lock_read()
 
-            graph = self.bzr_branch.repository.get_graph(
-                self.target.bzr_branch.repository)
+            graph = bzr_branch.repository.get_graph(
+                target_bzr_branch.repository)
 
             unique_ids = graph.find_unique_ancestors(
-                self.bzr_branch.last_revision(),
-                [self.target.bzr_branch.last_revision()])
+                bzr_branch.last_revision(),
+                [target_bzr_branch.last_revision()])
 
-            revs = self.bzr_branch.repository.get_revisions(unique_ids)
+            revs = bzr_branch.repository.get_revisions(unique_ids)
             for rev in revs:
                 apparent_authors = rev.get_apparent_authors()
                 for author in apparent_authors:
@@ -183,12 +210,12 @@
                     if author not in author_list:
                         author_list.append(author)
 
-            self.target.bzr_branch.unlock()
-            self.bzr_branch.unlock()
+            target_bzr_branch.unlock()
+            bzr_branch.unlock()
         else:
-            last_rev = self.bzr_branch.last_revision()
+            last_rev = bzr_branch.last_revision()
             if last_rev != 'null:':
-                rev = self.bzr_branch.repository.get_revision(last_rev)
+                rev = bzr_branch.repository.get_revision(last_rev)
                 apparent_authors = rev.get_apparent_authors()
                 author_list.extend(
                     [a.replace('\n', '') for a in apparent_authors])
@@ -200,12 +227,13 @@
         """Return the list of bugs fixed by the branch."""
         bugs_list = []
 
-        self.bzr_branch.lock_read()
-        oldrevid = self.bzr_branch.get_rev_id(self.lp_branch.revision_count)
-        for rev_info in self.bzr_branch.iter_merge_sorted_revisions(
+        bzr_branch = self.get_bzr_branch()
+        bzr_branch.lock_read()
+        oldrevid = bzr_branch.get_rev_id(self.lp_branch.revision_count)
+        for rev_info in bzr_branch.iter_merge_sorted_revisions(
             stop_revision_id=oldrevid):
             try:
-                rev = self.bzr_branch.repository.get_revision(rev_info[0])
+                rev = bzr_branch.repository.get_revision(rev_info[0])
                 for bug in rev.iter_bugs():
                     if bug[0].startswith('https://launchpad.net/bugs/'):
                         bugs_list.append(bug[0].replace(
@@ -213,5 +241,5 @@
             except NoSuchRevision:
                 continue
 
-        self.bzr_branch.unlock()
+        bzr_branch.unlock()
         return bugs_list

=== modified file 'tarmac/plugins/command.py'
--- tarmac/plugins/command.py	2011-09-02 04:06:25 +0000
+++ tarmac/plugins/command.py	2012-12-18 13:35:32 +0000
@@ -103,7 +103,8 @@
                                 shell=True,
                                 stdin=subprocess.PIPE,
                                 stdout=subprocess.PIPE,
-                                stderr=subprocess.PIPE)
+                                stderr=subprocess.PIPE,
+                                preexec_fn=os.setsid)
         proc.stdin.close()
         stdout = tempfile.TemporaryFile()
         stderr = tempfile.TemporaryFile()
@@ -125,7 +126,7 @@
                 killem(proc.pid, signal.SIGTERM)
                 time.sleep(5)
 
-                if proc.poll() is not None:
+                if proc.poll() is None:
                     self.logger.debug("SIGTERM did not work. Sending SIGKILL.")
                     killem(proc.pid, signal.SIGKILL)
 
@@ -184,7 +185,7 @@
                    u'%(output)s') % {
             'source': self.proposal.source_branch.display_name,
             'target': self.proposal.target_branch.display_name,
-            'output': u'\n'.join([stdout_value, stderr_value]),
+            'output': u'\n'.join([stdout_value.decode('utf-8', 'ignore'), stderr_value.decode('utf-8', 'ignore')]),
             }
         raise VerifyCommandFailed(message, comment)
 

=== added file 'tarmac/plugins/commit.py'
--- tarmac/plugins/commit.py	1970-01-01 00:00:00 +0000
+++ tarmac/plugins/commit.py	2012-12-18 13:35:32 +0000
@@ -0,0 +1,24 @@
+import re
+
+from tarmac.hooks import tarmac_hooks
+from tarmac.plugins import TarmacPlugin
+
+
+class CommitMessage(TarmacPlugin):
+
+    def run(self, command, target, source, proposal):
+        if not re.match(r'^\[.+?\].*$', proposal.commit_message):
+            params = ["[r=%s]" % proposal.reviewer.name]
+            for bug in proposal.source_branch.linked_bugs:
+                params.append("[bug=%s]" % bug.id)
+
+            proposal.commit_message = ("%s " % ",".join(params) +
+                                       proposal.commit_message)
+            self.logger.debug("Setting commit_message to %s",
+                              proposal.commit_message)
+        else:
+            self.logger.debug("%s has already right commit message",
+                              proposal)
+
+tarmac_hooks['tarmac_pre_commit'].hook(CommitMessage(),
+    'Commit messsage template editor.')