Merge lp:~ubuntuone-hackers/tarmac/trunk into lp:tarmac

[Jonathan Lange]

WIP MP to make the delta visible.

[lp:~ubuntuone-hackers/tarmac/trunk]

415. By Jonathan Lange on 2012-12-19

Fix the test failures in u1's tarmac fork

416. By Jonathan Lange on 2012-12-19

Oops. Really fix the tests.

417. By Jonathan Lange on 2012-12-19

Handle unicode output from tests. (~james-w)

418. By Jonathan Lange on 2012-12-19

Commit message plugin that sets review & bugs fixed (james-w)

419. By Jonathan Lange on 2012-12-20

Make the tests pass.

420. By Jonathan Lange on 2012-12-20

Merge James's branch to break up _do_merges, plus changes from my review.

421. By Jonathan Lange on 2012-12-20

Merge exit-status

422. By Jonathan Lange on 2012-12-20

Add "check" command.

423. By Jonathan Lange on 2012-12-20

Allow verify command to be specified on command line.

424. By Sidnei da Silva on 2012-12-20

- Fix to not turn 'proposals' into a single Entry object.

425. By James Westby on 2013-01-03

Also rename the other call to set_up.

426. By dobey on 2013-01-04

Remove the broken commit plug-in.

427. By Jonathan Lange on 2013-01-15

Better error message when private branches attack

428. By Sidnei da Silva on 2013-01-16

Use HTTPError directly to make it work in Lucid.

429. By James Westby on 2013-01-23

Merge trunk.

430. By James Westby on 2013-03-13

Allow the branch name to be used in the commit message template. (David Britton)

Unmerged revisions

431. By Vincent Ladeuil on 2016-07-11

Set the MP status to Merged after committing instead of waiting for lp to do so.

430. By James Westby on 2013-03-13

Allow the branch name to be used in the commit message template. (David Britton)

429. By James Westby on 2013-01-23

Merge trunk.

428. By Sidnei da Silva on 2013-01-16

Use HTTPError directly to make it work in Lucid.

427. By Jonathan Lange on 2013-01-15

Better error message when private branches attack

426. By dobey on 2013-01-04

Remove the broken commit plug-in.

425. By James Westby on 2013-01-03

Also rename the other call to set_up.

424. By Sidnei da Silva on 2012-12-20

- Fix to not turn 'proposals' into a single Entry object.

423. By Jonathan Lange on 2012-12-20

Allow verify command to be specified on command line.

422. By Jonathan Lange on 2012-12-20

Add "check" command.

=== modified file 'bin/tarmac'
--- bin/tarmac	2010-06-17 17:23:37 +0000
+++ bin/tarmac	2013-03-13 16:57:22 +0000
@@ -2,5 +2,7 @@
 # vim:filetype=python
 '''Main tarmac script.'''
 
+import sys
+
 from tarmac.bin import main
-main()
+sys.exit(main())
 
=== modified file 'docs/introduction.txt'
--- docs/introduction.txt	2012-06-25 21:05:28 +0000
+++ docs/introduction.txt	2013-03-13 16:57:22 +0000
@@ -151,6 +151,11 @@
 **reviewer**
   The display name of the merge proposal reviewer.
 
+**branch_name**
+  The short branch name.  i.e.: branch_name in "lp:~user/project/branch_name"
+
+**\n**
+  A \n in the commit message template will be rendered as a newline character.
 
 Command
 =======
 
=== modified file 'tarmac/bin/__init__.py'
--- tarmac/bin/__init__.py	2010-09-02 15:18:07 +0000
+++ tarmac/bin/__init__.py	2013-03-13 16:57:22 +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	2013-01-31 20:03:27 +0000
+++ tarmac/bin/commands.py	2013-03-13 16:57:22 +0000
@@ -10,6 +10,7 @@
 from launchpadlib.launchpad import Launchpad
 from launchpadlib.uris import (LPNET_SERVICE_ROOT,
     STAGING_SERVICE_ROOT)
+from lazr.restfulclient.errors import HTTPError
 
 from tarmac.bin import options
 from tarmac.branch import Branch
@@ -134,6 +135,237 @@
         help_commands(self.outf)
 
 
+def _get_login_name(lp):
+    """Return the name of the user `lp` is logged in as.
+
+    `None` if `lp` is an anonymous connection.
+    """
+    try:
+        me = lp.me
+    except HTTPError, e:
+        # XXX Newer lazr.restfulclient has a proper Unauthorized exception, but
+        # the version in Lucid does not.
+        if e.status == 401:
+            return None
+        raise
+    if me:
+        return me.name
+    return None
+
+
+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_branch(branch_url, launchpad, logger):
+    lp_branch = launchpad.branches.getByUrl(url=branch_url)
+    if lp_branch is None:
+        logger.info(
+            'User {0} could not find {1} branch on Launchpad'.format(
+                _get_login_name(launchpad), branch_url))
+    return lp_branch
+
+
+def _get_branch_and_mps(branch_url, launchpad, logger,
+                        imply_commit_message=False):
+    """Get branch and merge proposals from Launchpad.
+
+    :param branch_url: The Launchpad URL of the branch. e.g `lp:foo`.
+    :param launchpad: A Launchpad API object.
+    :param logger: A Python logger.
+    :param imply_commit_message: Whether to make up a commit message
+        if the merge proposal lacks an explicit one.  Defaults to False.
+    :return: `(lp_branch, [mergable_mp, ...])`
+    """
+    lp_branch = _get_branch(branch_url, launchpad, logger)
+    proposals = []
+
+    if lp_branch:
+        proposals = _get_mergable_proposals_for_branch(
+            lp_branch, logger,
+            imply_commit_message=imply_commit_message)
+
+    if not proposals:
+        logger.info(
+            'No approved proposals found for %(branch_url)s' % {
+                'branch_url': branch_url})
+
+    return lp_branch, proposals
+
+
+
+def _get_reviews(proposal):
+    """Get the set of reviews from the proposal."""
+    votes = [vote for vote in proposal.votes if vote.comment]
+    if not votes:
+        return None
+    return [
+        '%s;%s' % (vote.reviewer.display_name, vote.comment.vote)
+        for vote in votes]
+
+
+def set_up(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')
+
+
+def merge_proposals(target, proposals, logger, config, command):
+    logger.debug('Firing tarmac_pre_merge hook')
+    tarmac_hooks.fire('tarmac_pre_merge',
+                      command, target)
+    try:
+        statuses = [
+            merge_proposal(target, proposal, logger, config, command)
+            for proposal in proposals]
+        logger.debug('Firing tarmac_post_merge hook')
+        tarmac_hooks.fire('tarmac_post_merge',
+                          command, target, success_count=sum(statuses))
+    finally:
+        target.cleanup()
+    return statuses
+
+
+def merge_proposal(target, proposal, logger, config, command):
+    target.cleanup()
+    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, 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()
+
+        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)
+
+        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))
+
+        logger.debug('Firing tarmac_pre_commit hook')
+        tarmac_hooks.fire('tarmac_pre_commit',
+                          command, target, source, proposal)
+
+    except TarmacMergeError, failure:
+        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=config.rejected_branch_status)
+        except AttributeError:
+            proposal.setStatus(status=u'Needs review')
+        proposal.lp_save()
+        return False
+
+    except PointlessMerge:
+        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 config.imply_commit_message:
+        commit_message = proposal.description
+
+    target.commit(commit_message,
+                  revprops=revprops,
+                  authors=source.authors,
+                  reviews=_get_reviews(proposal))
+
+    logger.debug('Firing tarmac_post_commit hook')
+    tarmac_hooks.fire('tarmac_post_commit',
+                      command, target, source, proposal)
+    return True
+
+
 class cmd_merge(TarmacCommand):
     '''Automatically merge approved merge proposal branches.'''
 
@@ -142,6 +374,7 @@
     takes_options = [
         options.http_debug_option,
         options.debug_option,
+<<<<<<< TREE
         options.imply_commit_message_option,
         options.one_option]
 
@@ -153,14 +386,26 @@
             return
 
         proposals = self._get_mergable_proposals_for_branch(lp_branch)
+=======
+        options.imply_commit_message_option,
+        options.one_option,
+        options.verify_command_option,
+        ]
+
+    def _do_merges(self, branch_url, verify_command=None):
+        lp_branch, proposals = _get_branch_and_mps(
+            branch_url, self.launchpad, self.logger,
+            self.config.imply_commit_message)
+>>>>>>> MERGE-SOURCE
 
         if not proposals:
-            self.logger.info(
-                'No approved proposals found for %(branch_url)s' % {
-                    'branch_url': branch_url})
-            return
+            return []
+
+        if self.config.one:
+            proposals = [proposals[0]]
 
         target = Branch.create(lp_branch, self.config, create_tree=True)
+<<<<<<< TREE
 
         self.logger.debug('Firing tarmac_pre_merge hook')
         tarmac_hooks.fire('tarmac_pre_merge',
@@ -337,18 +582,19 @@
         return reviews
 
     def run(self, branch_url=None, launchpad=None, **kwargs):
+=======
+        if verify_command is not None:
+            setattr(target.config, 'verify_command', verify_command)
+        statuses = merge_proposals(target, proposals, self.logger, self.config, self)
+        return statuses
+
+    def run(self, branch_url=None, launchpad=None, verify_command=None, **kwargs):
+>>>>>>> MERGE-SOURCE
         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')
+        set_up(self.logger, debug=self.config.debug,
+               http_debug=self.config.http_debug)
 
         self.launchpad = launchpad
         if self.launchpad is None:
@@ -356,19 +602,21 @@
             self.launchpad = self.get_launchpad_object()
             self.logger.debug('launchpad object loaded')
 
+        statuses = []
+
         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:')
-            self._do_merges(branch_url)
-
+            statuses.extend(self._do_merges(branch_url, verify_command=verify_command))
         else:
             for branch in self.config.branches:
                 self.logger.debug(
                     'Merging approved branches against %(branch)s' % {
                         'branch': branch})
                 try:
+<<<<<<< TREE
                     merged = self._do_merges(branch)
 
                     # If we've been asked to only merge one branch, then exit.
@@ -376,8 +624,56 @@
                         break
                 except LockContention:
                     continue
+=======
+                    statuses.extend(self._do_merges(branch, verify_command=verify_command))
+
+                    # If we've been asked to only merge one branch, then exit.
+                    if statuses and self.config.one:
+                        break
+                except LockContention:
+                    continue
+>>>>>>> MERGE-SOURCE
                 except Exception, error:
                     self.logger.error(
                         'An error occurred trying to merge %s: %s',
                         branch, error)
                     raise
+        if not all(statuses):
+            return 2
+        return 0
+
+
+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, proposals = _get_branch_and_mps(
+            branch_url, self.launchpad, self.logger)
+        return bool(proposals)
+
+    def run(self, branch_url, launchpad=None, **kwargs):
+        for key, value in kwargs.iteritems():
+            self.config.set('Tarmac', key, value)
+
+        set_up(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
 
=== modified file 'tarmac/bin/options.py'
--- tarmac/bin/options.py	2012-05-26 04:14:45 +0000
+++ tarmac/bin/options.py	2013-03-13 16:57:22 +0000
@@ -15,6 +15,15 @@
     '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.')
+<<<<<<< TREE
+one_option = Option(
+    'one', short_name='1',
+    help='Merge only one branch and exit.')
+=======
+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.')
+>>>>>>> MERGE-SOURCE
 
=== modified file 'tarmac/bin/registry.py'
--- tarmac/bin/registry.py	2010-10-25 20:20:18 +0000
+++ tarmac/bin/registry.py	2013-03-13 16:57:22 +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	2013-02-04 21:07:51 +0000
+++ tarmac/branch.py	2013-03-13 16:57:22 +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,12 +73,25 @@
             clazz.create_tree()
         return clazz
 
+
     def create_tree(self):
         '''Create the dir and working tree.'''
+        bzr_branch = self.get_bzr_branch()
         try:
+            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)
+                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})
+<<<<<<< TREE
             if os.path.exists(self.config.tree_dir):
                 self.tree = WorkingTree.open(self.config.tree_dir)
             else:
@@ -79,33 +103,43 @@
                     os.makedirs(parent_dir)
                 self.tree = self.bzr_branch.create_checkout(
                     self.config.tree_dir, lightweight=True)
+=======
+>>>>>>> MERGE-SOURCE
         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,6 +152,7 @@
     @property
     def unmanaged_files(self):
         """Get the list of ignored and unknown files in the tree."""
+<<<<<<< TREE
         unmanaged = []
         try:
             self.tree.lock_read()
@@ -125,20 +160,31 @@
             unmanaged.extend([x[0] for x in self.tree.ignored_files()])
         finally:
             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()
+>>>>>>> MERGE-SOURCE
         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 = {}
 
@@ -155,8 +201,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):
@@ -167,7 +213,9 @@
     def authors(self):
         author_list = []
 
+        bzr_branch = self.get_bzr_branch()
         if self.target:
+<<<<<<< TREE
             try:
                 self.bzr_branch.lock_read()
                 self.target.bzr_branch.lock_read()
@@ -190,10 +238,33 @@
             finally:
                 self.target.bzr_branch.unlock()
                 self.bzr_branch.unlock()
+=======
+            target_bzr_branch = self.target.get_bzr_branch()
+            bzr_branch.lock_read()
+            target_bzr_branch.lock_read()
+
+            graph = bzr_branch.repository.get_graph(
+                target_bzr_branch.repository)
+
+            unique_ids = graph.find_unique_ancestors(
+                bzr_branch.last_revision(),
+                [target_bzr_branch.last_revision()])
+
+            revs = bzr_branch.repository.get_revisions(unique_ids)
+            for rev in revs:
+                apparent_authors = rev.get_apparent_authors()
+                for author in apparent_authors:
+                    author.replace('\n', '')
+                    if author not in author_list:
+                        author_list.append(author)
+
+            target_bzr_branch.unlock()
+            bzr_branch.unlock()
+>>>>>>> MERGE-SOURCE
         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])
@@ -205,6 +276,7 @@
         """Return the list of bugs fixed by the branch."""
         bugs_list = []
 
+<<<<<<< TREE
         try:
             self.bzr_branch.lock_read()
             oldrevid = self.bzr_branch.get_rev_id(self.lp_branch.revision_count)
@@ -218,7 +290,26 @@
                                     'https://launchpad.net/bugs/', ''))
                 except NoSuchRevision:
                     continue
+=======
+        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 = 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(
+                                'https://launchpad.net/bugs/', ''))
+            except NoSuchRevision:
+                continue
+>>>>>>> MERGE-SOURCE
 
+<<<<<<< TREE
         finally:
             self.bzr_branch.unlock()
+=======
+        bzr_branch.unlock()
+>>>>>>> MERGE-SOURCE
         return bugs_list
 
=== modified file 'tarmac/plugins/command.py'
--- tarmac/plugins/command.py	2011-09-02 04:06:25 +0000
+++ tarmac/plugins/command.py	2013-03-13 16:57:22 +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)
 
 
=== modified file 'tarmac/plugins/commitmessage.py'
--- tarmac/plugins/commitmessage.py	2010-10-25 21:27:39 +0000
+++ tarmac/plugins/commitmessage.py	2013-03-13 16:57:22 +0000
@@ -30,18 +30,17 @@
 
     def run(self, command, target, source, proposal):
     # pylint: disable-msg=W0613
-
         try:
-            template = target.config.commit_message_template
-            template = template.replace('<', '%(').replace('>', ')s')
+            proposal.commit_message = self.render(
+                target.config.commit_message_template,
+                CommitMessageTemplateInfo(proposal))
         except AttributeError:
             return
 
-        proposal.commit_message = self.render(
-            template, CommitMessageTemplateInfo(proposal))
-
     def render(self, template, info):
         """Render a template using the given information."""
+        template = template.replace('<', '%(').replace('>', ')s')
+        template = template.replace('\\n', '\n')
         return template % info
 
 
@@ -77,6 +76,11 @@
         return self._proposal.commit_message
 
     @property
+    def branch_name(self):
+        """The branch name under review."""
+        return self._proposal.source_branch.name
+
+    @property
     def reviewer(self):
         """The display name of the merge proposal reviewer.
 
 
=== modified file 'tarmac/plugins/tests/test_commitmessage.py'
--- tarmac/plugins/tests/test_commitmessage.py	2010-10-25 21:27:39 +0000
+++ tarmac/plugins/tests/test_commitmessage.py	2013-03-13 16:57:22 +0000
@@ -12,6 +12,7 @@
         super(TestCommitMessageTemplateInfo, self).setUp()
         self.proposal = Thing(
             source_branch=Thing(
+                name="name",
                 owner=Thing(display_name="Arthur Author", name="arthur"),
                 linked_bugs=[Thing(id=1234), Thing(id=5678)]),
             commit_message="Awesome",
@@ -83,8 +84,8 @@
             return "{info:%s}" % name
 
 
-class TestCommitMessageTemplate(TarmacTestCase):
-
+class TestCommitMessageTemplate(TestCommitMessageTemplateInfo):
+    
     def test_render(self):
         message_template = CommitMessageTemplate()
         message_info = FakeCommitMessageTemplateInfo()
@@ -99,3 +100,7 @@
         self.assertEqual(
             "{info:author} {info:reviewer}",
             render("%(author)s %(reviewer)s", message_info))
+        self.assertEqual(
+            "{info:author} {info:branch_name} {info:reviewer}",
+            render("<author> <branch_name> <reviewer>", message_info))
+        self.assertEqual("one\ntwo", render("one\\ntwo", message_info))
 
=== modified file 'tarmac/tests/mock.py'
--- tarmac/tests/mock.py	2010-11-19 18:11:56 +0000
+++ tarmac/tests/mock.py	2013-03-13 16:57:22 +0000
@@ -50,6 +50,8 @@
             self.revision_count = 0
         self.bzr_identity = 'lp:%s' % os.path.basename(self.tree_dir)
         self.project = MockLPProject()
+        self.web_link = 'http://code.launchpad.net/+branch/%s' % (
+            os.path.basename(self.tree_dir),)
 
 
 class cmd_mock(TarmacCommand):
 
=== modified file 'tarmac/tests/test_branch.py'
--- tarmac/tests/test_branch.py	2012-06-29 20:03:55 +0000
+++ tarmac/tests/test_branch.py	2013-03-13 16:57:22 +0000
@@ -38,7 +38,25 @@
         a_branch = branch.Branch.create(MockLPBranch(tree_dir), self.config)
         self.assertTrue(isinstance(a_branch, branch.Branch))
         self.assertTrue(a_branch.lp_branch.bzr_identity is not None)
-        self.assertFalse(hasattr(a_branch, 'tree'))
+        self.remove_branch_config(tree_dir)
+
+    def test_create_missing_parent_dir(self):
+        '''Test the creation of a TarmacBranch instance in a path that does
+        not fully exist, with a tree'''
+        branch_name = 'test_branch'
+        parent_dir = os.path.join(self.TEST_ROOT, 'missing')
+        tree_dir = os.path.join(parent_dir, branch_name)
+        self.add_branch_config(tree_dir)
+        # Create the mock somewhere other than where the tarmac branch will be
+        # located.  Keep it right under TEST_ROOT so the
+        # TarmacDirectoryFactory mocking will work.
+        mock = MockLPBranch(os.path.join(self.TEST_ROOT, branch_name))
+        self.assertFalse(os.path.exists(parent_dir))
+        a_branch = branch.Branch.create(mock, self.config, create_tree=True)
+        self.assertTrue(os.path.exists(parent_dir))
+        self.assertTrue(isinstance(a_branch, branch.Branch))
+        self.assertTrue(a_branch.lp_branch.bzr_identity is not None)
+        self.assertTrue(hasattr(a_branch, 'tree'))
         self.remove_branch_config(tree_dir)
 
     def test_create_missing_parent_dir(self):
@@ -66,19 +84,6 @@
         self.assertTrue(self.branch1.lp_branch.bzr_identity is not None)
         self.assertTrue(hasattr(self.branch1, 'tree'))
 
-    def test_merge_raises_exception_with_no_tree(self):
-        '''A merge on a branch with no tree will raise an exception.'''
-        branch3_dir = os.path.join(self.TEST_ROOT, 'branch3')
-        self.add_branch_config(branch3_dir)
-        branch3 = branch.Branch.create(MockLPBranch(
-                branch3_dir, source_branch=self.branch1.lp_branch),
-                                       self.config)
-
-        self.assertRaises(
-            AttributeError, branch3.merge, self.branch2)
-        self.remove_branch_config(branch3_dir)
-        shutil.rmtree(branch3_dir)
-
     def test_merge_no_changes(self):
         '''A merge on a branch with a tree will raise an exception if no
         changes are present.'''
 
=== modified file 'tarmac/tests/test_commands.py'
--- tarmac/tests/test_commands.py	2012-06-29 20:03:55 +0000
+++ tarmac/tests/test_commands.py	2013-03-13 16:57:22 +0000
@@ -1,5 +1,6 @@
 '''Tests for tarmac.bin.commands.py.'''
 from cStringIO import StringIO
+import logging
 import os
 import shutil
 import sys
@@ -105,15 +106,20 @@
                 display_name=self.branch2.lp_branch.bzr_identity,
                 name='source',
                 revision_count=self.branch2.lp_branch.revision_count,
-                landing_candidates=[]),
+                landing_candidates=[],
+                linked_bugs=[],
+                web_link=u'http://launchpad.net/branches/source'),
                          Thing(
                 bzr_identity=self.branch1.lp_branch.bzr_identity,
                 display_name=self.branch1.lp_branch.bzr_identity,
                 name='target',
                 revision_count=self.branch1.lp_branch.revision_count,
-                landing_candidates=None)]
+                landing_candidates=None,
+                linked_bugs=[],
+                web_link=u'http://launchpad.net/branches/target')]
         self.proposals = [Thing(
                 self_link=u'http://api.edge.launchpad.net/devel/proposal0',
+                web_link=u'http://edge.launchpad.net/proposal0',
                 queue_status=u'Needs Review',
                 commit_message=u'Commitable.',
                 source_branch=self.branches[0],
@@ -128,6 +134,7 @@
                         reviewer=Thing(display_name=u'Reviewer'))]),
                           Thing(
                 self_link=u'https://api.launchpad.net/1.0/proposal1',
+                web_link=u'https://launchpad.net/proposal1',
                 queue_status=u'Approved',
                 commit_message=u'Commit this.',
                 source_branch=self.branches[0],
@@ -137,6 +144,7 @@
                 setStatus=self.lp_save,
                 lp_save=self.lp_save,
                 reviewed_revid=None,
+                reviewer=Thing(name=u'reviewer', display_name=u'Reviewer'),
                 votes=[Thing(
                         comment=Thing(vote=u'Approve'),
                         reviewer=Thing(display_name=u'Reviewer')),
@@ -145,7 +153,8 @@
                         reviewer=Thing(display_name=u'Reviewer2'))])]
         self.branches[1].landing_candidates = self.proposals
 
-        self.launchpad = Thing(branches=Thing(getByUrl=self.getBranchByUrl))
+        self.launchpad = Thing(branches=Thing(getByUrl=self.getBranchByUrl),
+                               me=None)
         self.error = None
         registry = CommandRegistry(config=self.config)
         registry.register_command('merge', commands.cmd_merge)
@@ -167,6 +176,20 @@
         except IndexError:
             return None
 
+    def test__get_branch_exists(self):
+        # _get_branch returns the lp_branch if it exists.
+        branch = commands._get_branch(
+            self.branch2.lp_branch.bzr_identity,
+            self.launchpad,
+            logging.getLogger('tarmac'))
+        self.assertIs(self.branches[0], branch)
+
+    def test__get_branch_doesnt_exist(self):
+        # _get_branch returns None if it doesn't exist.
+        branch = commands._get_branch(
+            'doesntexist', self.launchpad, logging.getLogger('tarmac'))
+        self.assertIs(None, branch)
+
     def test_run(self):
         """Test that the merge command merges a branch successfully."""
         self.proposals[1].reviewed_revid = \
@@ -191,10 +214,12 @@
 
     def test_get_reviews(self):
         """Test that the _get_reviews method gives the right lists."""
-        self.assertEqual(self.command._get_reviews(self.proposals[0]),
-                         [u'Reviewer;Needs Fixing'])
-        self.assertEqual(self.command._get_reviews(self.proposals[1]),
-                         [u'Reviewer;Approve', u'Reviewer2;Abstain'])
+        self.assertEqual(
+            commands._get_reviews(self.proposals[0]),
+            [u'Reviewer;Needs Fixing'])
+        self.assertEqual(
+            commands._get_reviews(self.proposals[1]),
+            [u'Reviewer;Approve', u'Reviewer2;Abstain'])
 
     def test_run_merge_url_substitution(self):
         """Test that the merge urls get substituted correctly."""
@@ -212,6 +237,7 @@
         # Make a new commit, approve the propsoal, merge, and verify
         self.branch2.commit('New commit to merge.')
         self.proposals[0].queue_status = u'Approved'
+        self.proposals[0].reviewer = Thing(name=u'reviewer')
         self.proposals[0].reviewed_revid = \
             self.branch2.bzr_branch.last_revision()
         self.command.run(launchpad=self.launchpad)
@@ -242,6 +268,7 @@
         branch3.lp_branch.landing_candidates = []
         b3_proposal = Thing(
             self_link=u'http://api.edge.launchpad.net/devel/proposal3',
+            web_link=u'http://edge.launchpad.net/proposal3',
             queue_status=u'Work in Progress',
             commit_message=u'Commitable.',
             source_branch=branch3.lp_branch,
@@ -264,8 +291,9 @@
         self.command.run(launchpad=self.launchpad)
         shutil.rmtree(branch3_dir)
         self.assertEqual(self.error.comment,
-                         u'The prerequisite lp:branch3 has not yet been '
-                         u'merged into lp:branch1.')
+                         u'The prerequisite '
+                         u'http://code.launchpad.net/+branch/branch3 has not '
+                         u'yet been merged into http://launchpad.net/branches/target.')
 
     def test_run_merge_with_unproposed_prerequisite_fails(self):
         """Test that mereging a branch with an unmerged prerequisite fails."""
@@ -289,6 +317,7 @@
         branch3.lp_branch.landing_candidates = []
         b3_proposal = Thing(
             self_link=u'http://api.edge.launchpad.net/devel/proposal3',
+            web_link=u'http://edge.launchpadnet/proposal3',
             queue_status=u'Work in Progress',
             commit_message=u'Commitable.',
             source_branch=branch3.lp_branch,
@@ -311,8 +340,9 @@
         self.command.run(launchpad=self.launchpad)
         shutil.rmtree(branch3_dir)
         self.assertEqual(self.error.comment,
-                         u'No proposals found for merge of lp:branch3 '
-                         u'into lp:branch1.')
+                         u'No proposals found for merge of '
+                         u'http://code.launchpad.net/+branch/branch3 '
+                         u'into http://launchpad.net/branches/target.')
 
     def test_run_merge_with_prerequisite_with_multiple_proposals_fails(self):
         """Test that mereging a branch with an unmerged prerequisite fails."""
@@ -336,6 +366,7 @@
         branch3.lp_branch.landing_candidates = []
         b3_proposal = Thing(
             self_link=u'http://api.edge.launchpad.net/devel/proposal3',
+            web_link=u'http://edge.launchpadnet/proposal3',
             queue_status=u'Work in Progress',
             commit_message=u'Commitable.',
             source_branch=branch3.lp_branch,
@@ -363,5 +394,6 @@
         shutil.rmtree(branch3_dir)
         self.assertEqual(self.error.comment,
                          u'More than one proposal found for merge of '
-                         u'lp:branch3 into lp:branch1, which is not '
-                         u'Superseded.')
+                         u'http://code.launchpad.net/+branch/branch3 into '
+                         u'http://launchpad.net/branches/target, which '
+                         u'is not Superseded.')