Merge lp:~jml/launchpad/auto-land into lp:launchpad/db-devel

[Jonathan Lange]

[Jonathan Lange (jml)]

Hello Michael,

This branch adds a new ec2 command 'land' that will land a branch given only the merge proposal URL. My thinking is that 'ec2 test' is pretty complicated, and I can never remember the PQM rules, and I accidentally land branches on devel when I mean db-devel and all the information is on the merge proposal anyway.

This should also make it a lot easier to land branches for community contributors.

It's a bit experimental, and there are a few XXXs, but I think it needs a review to get unstuck.

Thanks,
jml

[Michael Hudson-Doyle (mwhudson)]

Well, I think this branch is an awesome idea. I can see that it's not
totally finished yet, but I think it's the sort of thing that even
half finished is useful. So if you can fix this so it actually does
something I think we should land it as soon as the tree opens. But it
should be at least do something first, I think :)

Apologies for the slight shallowness of this review, rollout related
brain frying in effect.

> === added file 'lib/devscripts/autoland.py'
> --- lib/devscripts/autoland.py 1970-01-01 00:00:00 +0000
> +++ lib/devscripts/autoland.py 2009-09-24 05:03:23 +0000
> @@ -0,0 +1,215 @@
> +"""Land an approved merge proposal."""
> +
> +import os
> +
> +from launchpadlib.launchpad import (
> + Launchpad, EDGE_SERVICE_ROOT, STAGING_SERVICE_ROOT)
> +from lazr.uri import URI
> +
> +DEV_SERVICE_ROOT = 'https://api.launchpad.dev/beta/'
> +LPNET_SERVICE_ROOT = 'https://api.launchpad.net/beta/'
> +
> +# Given the merge proposal URL, get:
> +# - [DONE] the reviewer
> +# - [DONE] the UI reviewer
> +# - [DONE] the DB reviewer
> +# - the release-critical reviewer
> +# - [DONE] the commit message
> +# - [DONE] the branch
> +# - [DONE] the target branch
> +# - whether or not it has been approved
> +# - [DONE] the branch owner

I think the two not-DONE things are worth doing...

> +# If the review has not been approved, warn.
> +
> +# XXX: How do you do TDD of a launchpadlib program?

I certainly don't know.

> +# XXX: What's the right way to write docstrings for launchpadlib programs?
> +
> +
> +class LaunchpadBranchLander:
> +
> + name = 'launchpad-branch-lander'
> + cache_dir = '~/.launchpadlib/cache'
> +
> + def __init__(self, launchpad):
> + self._launchpad = launchpad
> +
> + @classmethod
> + def load(cls, service_root=DEV_SERVICE_ROOT):
> + # XXX: No unit tests.
> + cache_dir = os.path.expanduser(cls.cache_dir)
> + # XXX: If cached data invalid, hard to delete & try again.
> + launchpad = Launchpad.login_with(cls.name, service_root, cache_dir)
> + return cls(launchpad)
> +
> + def load_merge_proposal(self, mp_url):
> + """Get the merge proposal object for the 'mp_url'."""
> + # XXX: No unit tests.
> + web_mp_uri = URI(mp_url)
> + api_mp_uri = self._launchpad._root_uri.append(
> + web_mp_uri.path.lstrip('/'))
> + return MergeProposal(self._launchpad.load(str(api_mp_uri)))
> +
> +
> +class MergeProposal:
> + """Wrapper around launchpadlib `IBranchMergeProposal` for landing."""
> +
> + def __init__(self, mp):
> + """Construct a merge proposal.
> +
> + :param mp: A launchpadlib `IBranchMergeProposal`.
> + """
> + self._mp = mp
> + self._launchpad = mp._root
> +
> + @property
> + def source_branch(self):
> + """The push URL of the source branch."""
> + return str(self._get_push_url(self._mp.source_branch))
> +
> + @property
> + def target_branch(self):
> + """The push URL of the target branch."""
> + return str(self._get_push_url(self._mp.source_branch))

These two implementations are identical, which seems unlikely to be
right.

> + def get_stakeholder_...

review: Approve

[Michael Hudson-Doyle (mwhudson)]

I meant to say this originally...

review: Needs Fixing

[Jonathan Lange (jml)]

On Thu, Sep 24, 2009 at 6:57 AM, Michael Hudson
<email address hidden> wrote:
> Review: Approve
> Well, I think this branch is an awesome idea.  I can see that it's not
> totally finished yet, but I think it's the sort of thing that even
> half finished is useful.  So if you can fix this so it actually does
> something I think we should land it as soon as the tree opens.  But it
> should be at least do something first, I think :)
>

Thanks :)

> Apologies for the slight shallowness of this review, rollout related
> brain frying in effect.
>

No worries.

>> === added file 'lib/devscripts/autoland.py'
>> --- lib/devscripts/autoland.py        1970-01-01 00:00:00 +0000
>> +++ lib/devscripts/autoland.py        2009-09-24 05:03:23 +0000
>> @@ -0,0 +1,215 @@
>> +"""Land an approved merge proposal."""
>> +
>> +import os
>> +
>> +from launchpadlib.launchpad import (
>> +    Launchpad, EDGE_SERVICE_ROOT, STAGING_SERVICE_ROOT)
>> +from lazr.uri import URI
>> +
>> +DEV_SERVICE_ROOT = 'https://api.launchpad.dev/beta/'
>> +LPNET_SERVICE_ROOT = 'https://api.launchpad.net/beta/'
>> +

Added a comment here explaining that these two variables are in recent
versions of launchpadlib.

>> +# Given the merge proposal URL, get:
>> +# - [DONE] the reviewer
>> +# - [DONE] the UI reviewer
>> +# - [DONE] the DB reviewer
>> +# - the release-critical reviewer
>> +# - [DONE] the commit message
>> +# - [DONE] the branch
>> +# - [DONE] the target branch
>> +# - whether or not it has been approved
>> +# - [DONE] the branch owner
>
> I think the two not-DONE things are worth doing...
>

I've removed all of these comments, added release-critical support and
approval checking.

>> +# If the review has not been approved, warn.
>> +
>> +# XXX: How do you do TDD of a launchpadlib program?
>
> I certainly don't know.
>

I've sent an email to launchpad-users about this and removed the comment.

>> +# XXX: What's the right way to write docstrings for launchpadlib programs?
>> +
>> +

Removed this comment.

>> +class LaunchpadBranchLander:
>> +
>> +    name = 'launchpad-branch-lander'
>> +    cache_dir = '~/.launchpadlib/cache'
>> +
>> +    def __init__(self, launchpad):
>> +        self._launchpad = launchpad
>> +
>> +    @classmethod
>> +    def load(cls, service_root=DEV_SERVICE_ROOT):
>> +        # XXX: No unit tests.

Made XXX compliant.

>> +        cache_dir = os.path.expanduser(cls.cache_dir)
>> +        # XXX: If cached data invalid, hard to delete & try again.

Made compliant, and filed bug 435813 about it.

>> +        launchpad = Launchpad.login_with(cls.name, service_root, cache_dir)
>> +        return cls(launchpad)
>> +
>> +    def load_merge_proposal(self, mp_url):
>> +        """Get the merge proposal object for the 'mp_url'."""
>> +        # XXX: No unit tests.

Made compliant.

>> +        web_mp_uri = URI(mp_url)
>> +        api_mp_uri = self._launchpad._root_uri.append(
>> +            web_mp_uri.path.lstrip('/'))
>> +        return MergeProposal(self._launchpad.load(str(api_mp_uri)))
>> +
>> +
>> +class MergeProposal:
>> +    """Wrapper around launchpadlib `IBranchMergeProposal` for landing."""
>> +
>> +    def __init__(self, mp):
>> +        """Construc...

[Jonathan Lange (jml)]

=== modified file 'lib/devscripts/autoland.py'
--- lib/devscripts/autoland.py 2009-09-24 10:43:40 +0000
+++ lib/devscripts/autoland.py 2009-09-24 11:23:10 +0000
@@ -12,10 +12,9 @@
 DEV_SERVICE_ROOT = 'https://api.launchpad.dev/beta/'
 LPNET_SERVICE_ROOT = 'https://api.launchpad.net/beta/'

-# Given the merge proposal URL, get:
-# - the release-critical reviewer
-# - whether or not it has been approved
-# If the review has not been approved, warn.
+
+class MissingReviewError(Exception):
+ """Raised when we try to get a review message without enough reviewers."""

 class LaunchpadBranchLander:
@@ -27,16 +26,17 @@
         self._launchpad = launchpad

     @classmethod
- def load(cls, service_root=DEV_SERVICE_ROOT):
- # XXX: No unit tests.
+ def load(cls, service_root=EDGE_SERVICE_ROOT):
+ # XXX: JonathanLange 2009-09-24: No unit tests.
         cache_dir = os.path.expanduser(cls.cache_dir)
- # XXX: If cached data invalid, hard to delete & try again.
+ # XXX: JonathanLange 2009-09-24 bug=435813: If cached data invalid,
+ # there's no easy way to delete it and try again.
         launchpad = Launchpad.login_with(cls.name, service_root, cache_dir)
         return cls(launchpad)

     def load_merge_proposal(self, mp_url):
         """Get the merge proposal object for the 'mp_url'."""
- # XXX: No unit tests.
+ # XXX: JonathanLange 2009-09-24: No unit tests.
         web_mp_uri = URI(mp_url)
         api_mp_uri = self._launchpad._root_uri.append(
             web_mp_uri.path.lstrip('/'))
@@ -64,6 +64,16 @@
         """The push URL of the target branch."""
         return str(self._get_push_url(self._mp.target_branch))

+ @property
+ def commit_message(self):
+ """The commit message specified on the merge proposal."""
+ return self._mp.commit_message
+
+ @property
+ def is_approved(self):
+ """Is this merge proposal approved for landing."""
+ return self._mp.queue_status == 'Approved'
+
     def get_stakeholder_emails(self):
         """Return a collection of people who should know about branch landing.

@@ -71,7 +81,7 @@

         :return: A set of `IPerson`s.
         """
- # XXX: No unit tests.
+ # XXX: JonathanLange 2009-09-24: No unit tests.
         return map(
             get_email,
             set([self._mp.source_branch.owner, self._launchpad.me]))
@@ -106,7 +116,7 @@

         :param branch: A launchpadlib `IBranch`.
         """
- # XXX: No unit tests.
+ # XXX: JonathanLange 2009-09-24: No unit tests.
         host = get_bazaar_host(str(self._launchpad._root_uri))
         # XXX: JonathanLange 2009-09-24 bug=435790: lazr.uri allows a path
         # without a leading '/' and then doesn't insert a '/' in the final
@@ -167,6 +177,11 @@
     return reviewer.name

+def _comma_separated_names(things):
+ """Return a string of comma-separated names of 'things'."""
+ return ','.join(thing.name for thing in things)
+
+
 def get_reviewer_clause(reviewers):
     """Get the reviewer section of a commit message, given the reviewers.

@@ -177,14 +192,21 @@
     code_reviewers = reviewers.get(None, [])
     c...

[Michael Hudson-Doyle (mwhudson)]

One more time around the mulberry bush!

I'm just marginally on the side of not needing to see this again, but
feel free to send me another diff if any of the following is unclear
in what I'm asking for.

> === added file 'lib/devscripts/autoland.py'
> --- lib/devscripts/autoland.py 1970-01-01 00:00:00 +0000
> +++ lib/devscripts/autoland.py 2009-09-25 04:11:41 +0000
> @@ -0,0 +1,226 @@
> +"""Land an approved merge proposal."""
> +
> +import os
> +
> +from launchpadlib.launchpad import (
> + Launchpad, EDGE_SERVICE_ROOT, STAGING_SERVICE_ROOT)
> +from lazr.uri import URI
> +
> +# XXX: JonathanLange 2009-09-24: Both of these are available in more recent
> +# versions of launchpadlib. When we start using such versions, we should
> +# instead import these from launchpadlib.
> +DEV_SERVICE_ROOT = 'https://api.launchpad.dev/beta/'
> +LPNET_SERVICE_ROOT = 'https://api.launchpad.net/beta/'

For tedious and boring reasons, when called from ec2 test this will
run with the system python's path, not the bin/py path. Does that
affect this comment? The bin/py path will be in effect when running
the tests though.

(In other news, Launchpad on 2.5 pls)

> +class MergeProposal:
> + """Wrapper around launchpadlib `IBranchMergeProposal` for landing."""
> +
> + def __init__(self, mp):
> + """Construct a merge proposal.
> +
> + :param mp: A launchpadlib `IBranchMergeProposal`.
> + """
> + self._mp = mp
> + self._launchpad = mp._root
> +
> + @property
> + def source_branch(self):
> + """The push URL of the source branch."""
> + return str(self._get_push_url(self._mp.source_branch))
> +
> + @property
> + def target_branch(self):
> + """The push URL of the target branch."""
> + return self._mp.target_branch.unique_name.split('/')[-1]

Um, the implementation still doesn't match my expectation of the
docstring -- isn't that going to just return the name of the target
name? (by name i mean ~foo/bar/baz --> baz).

> === modified file 'lib/devscripts/ec2test/builtins.py'
> --- lib/devscripts/ec2test/builtins.py 2009-09-18 05:57:43 +0000
> +++ lib/devscripts/ec2test/builtins.py 2009-09-25 04:11:41 +0000
> @@ -6,7 +6,9 @@
> __metaclass__ = type
> __all__ = []
>
> +import os
> import pdb
> +import subprocess
>
> from bzrlib.commands import Command
> from bzrlib.errors import BzrCommandError
> @@ -15,6 +17,9 @@
>
> import socket
>
> +from devscripts import get_launchpad_root
> +from devscripts.autoland import LaunchpadBranchLander, MissingReviewError
> +
> from devscripts.ec2test.credentials import EC2Credentials
> from devscripts.ec2test.instance import (
> AVAILABLE_INSTANCE_TYPES, DEFAULT_INSTANCE_TYPE, EC2Instance)
> @@ -263,6 +268,87 @@
> instance.set_up_and_run(postmortem, not headless, runner.run_tests)
>
>
> +class cmd_land(EC2Command):
> + """Land a merge proposal on Launchpad."""
> +
> + takes_options = [
> + machine_id_option,
> + instance_type_option,
> + postmortem_option,
> + debug_option,
> + Option('dry-run', help="Just print the equivalent ec2 test command."),
> + Option('print-commit', help="Print the ful...

review: Approve

[Jonathan Lange (jml)]

> One more time around the mulberry bush!
>
> I'm just marginally on the side of not needing to see this again, but
> feel free to send me another diff if any of the following is unclear
> in what I'm asking for.
>

Thanks.

> > === added file 'lib/devscripts/autoland.py'
> > --- lib/devscripts/autoland.py 1970-01-01 00:00:00 +0000
> > +++ lib/devscripts/autoland.py 2009-09-25 04:11:41 +0000
> > @@ -0,0 +1,226 @@
> > +"""Land an approved merge proposal."""
> > +
> > +import os
> > +
> > +from launchpadlib.launchpad import (
> > + Launchpad, EDGE_SERVICE_ROOT, STAGING_SERVICE_ROOT)
> > +from lazr.uri import URI
> > +
> > +# XXX: JonathanLange 2009-09-24: Both of these are available in more recent
> > +# versions of launchpadlib. When we start using such versions, we should
> > +# instead import these from launchpadlib.
> > +DEV_SERVICE_ROOT = 'https://api.launchpad.dev/beta/'
> > +LPNET_SERVICE_ROOT = 'https://api.launchpad.net/beta/'
>
> For tedious and boring reasons, when called from ec2 test this will
> run with the system python's path, not the bin/py path. Does that
> affect this comment? The bin/py path will be in effect when running
> the tests though.
>

No, it's not affected.

> (In other news, Launchpad on 2.5 pls)
>

Thanks.

> > +class MergeProposal:
> > + """Wrapper around launchpadlib `IBranchMergeProposal` for landing."""
> > +
> > + def __init__(self, mp):
> > + """Construct a merge proposal.
> > +
> > + :param mp: A launchpadlib `IBranchMergeProposal`.
> > + """
> > + self._mp = mp
> > + self._launchpad = mp._root
> > +
> > + @property
> > + def source_branch(self):
> > + """The push URL of the source branch."""
> > + return str(self._get_push_url(self._mp.source_branch))
> > +
> > + @property
> > + def target_branch(self):
> > + """The push URL of the target branch."""
> > + return self._mp.target_branch.unique_name.split('/')[-1]
>
> Um, the implementation still doesn't match my expectation of the
> docstring -- isn't that going to just return the name of the target
> name? (by name i mean ~foo/bar/baz --> baz).
>

I've changed this to return the full URL. The call-site then does the split and [-1]. I've added comments along these lines.

> > === modified file 'lib/devscripts/ec2test/builtins.py'
> > --- lib/devscripts/ec2test/builtins.py 2009-09-18 05:57:43 +0000
> > +++ lib/devscripts/ec2test/builtins.py 2009-09-25 04:11:41 +0000
> > @@ -6,7 +6,9 @@
> > __metaclass__ = type
> > __all__ = []
> >
> > +import os
> > import pdb
> > +import subprocess
> >
> > from bzrlib.commands import Command
> > from bzrlib.errors import BzrCommandError
> > @@ -15,6 +17,9 @@
> >
> > import socket
> >
> > +from devscripts import get_launchpad_root
> > +from devscripts.autoland import LaunchpadBranchLander, MissingReviewError
> > +
> > from devscripts.ec2test.credentials import EC2Credentials
> > from devscripts.ec2test.instance import (
> > AVAILABLE_INSTANCE_TYPES, DEFAULT_INSTANCE_TYPE, EC2Instance)
> > @@ -263,6 +268,87 @@
> > instance.set_up_and_run(postmortem, not headless, runner.run_tests)
> >
...

=== modified file 'lib/devscripts/__init__.py'
--- lib/devscripts/__init__.py	2009-09-09 03:27:47 +0000
+++ lib/devscripts/__init__.py	2009-09-30 13:37:18 +0000
@@ -2,3 +2,9 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Scripts that are used in developing Launchpad."""
+
+import os
+
+
+def get_launchpad_root():
+    return os.path.dirname(os.path.dirname(os.path.dirname(__file__)))

=== added file 'lib/devscripts/autoland.py'
--- lib/devscripts/autoland.py	1970-01-01 00:00:00 +0000
+++ lib/devscripts/autoland.py	2009-09-30 13:37:18 +0000
@@ -0,0 +1,226 @@
+"""Land an approved merge proposal."""
+
+import os
+
+from launchpadlib.launchpad import (
+    Launchpad, EDGE_SERVICE_ROOT, STAGING_SERVICE_ROOT)
+from lazr.uri import URI
+
+# XXX: JonathanLange 2009-09-24: Both of these are available in more recent
+# versions of launchpadlib. When we start using such versions, we should
+# instead import these from launchpadlib.
+DEV_SERVICE_ROOT = 'https://api.launchpad.dev/beta/'
+LPNET_SERVICE_ROOT = 'https://api.launchpad.net/beta/'
+
+
+class MissingReviewError(Exception):
+    """Raised when we try to get a review message without enough reviewers."""
+
+
+class LaunchpadBranchLander:
+
+    name = 'launchpad-branch-lander'
+    cache_dir = '~/.launchpadlib/cache'
+
+    def __init__(self, launchpad):
+        self._launchpad = launchpad
+
+    @classmethod
+    def load(cls, service_root=EDGE_SERVICE_ROOT):
+        # XXX: JonathanLange 2009-09-24: No unit tests.
+        cache_dir = os.path.expanduser(cls.cache_dir)
+        # XXX: JonathanLange 2009-09-24 bug=435813: If cached data invalid,
+        # there's no easy way to delete it and try again.
+        launchpad = Launchpad.login_with(cls.name, service_root, cache_dir)
+        return cls(launchpad)
+
+    def load_merge_proposal(self, mp_url):
+        """Get the merge proposal object for the 'mp_url'."""
+        # XXX: JonathanLange 2009-09-24: No unit tests.
+        web_mp_uri = URI(mp_url)
+        api_mp_uri = self._launchpad._root_uri.append(
+            web_mp_uri.path.lstrip('/'))
+        return MergeProposal(self._launchpad.load(str(api_mp_uri)))
+
+
+class MergeProposal:
+    """Wrapper around launchpadlib `IBranchMergeProposal` for landing."""
+
+    def __init__(self, mp):
+        """Construct a merge proposal.
+
+        :param mp: A launchpadlib `IBranchMergeProposal`.
+        """
+        self._mp = mp
+        self._launchpad = mp._root
+
+    @property
+    def source_branch(self):
+        """The push URL of the source branch."""
+        return str(self._get_push_url(self._mp.source_branch))
+
+    @property
+    def target_branch(self):
+        """The push URL of the target branch."""
+        return str(self._get_push_url(self._mp.target_branch))
+
+    @property
+    def commit_message(self):
+        """The commit message specified on the merge proposal."""
+        return self._mp.commit_message
+
+    @property
+    def is_approved(self):
+        """Is this merge proposal approved for landing."""
+        return self._mp.queue_status == 'Approved'
+
+    def get_stakeholder_emails(self):
+        """Return a collection of people who should know about branch landing.
+
+        Used to determine who to email with the ec2 test results.
+
+        :return: A set of `IPerson`s.
+        """
+        # XXX: JonathanLange 2009-09-24: No unit tests.
+        return set(
+            map(get_email, [self._mp.source_branch.owner, self._launchpad.me]))
+
+    def get_reviews(self):
+        """Return a dictionary of all Approved reviews.
+
+        Used to determine who has actually approved a branch for landing. The
+        key of the dictionary is the type of review, and the value is the list
+        of people who have voted Approve with that type.
+
+        Common types include 'code', 'db', 'ui' and of course `None`.
+        """
+        reviews = {}
+        for vote in self._mp.votes:
+            comment = vote.comment
+            if comment is None or comment.vote != "Approve":
+                continue
+            reviewers = reviews.setdefault(vote.review_type, [])
+            reviewers.append(vote.reviewer)
+        return reviews
+
+    def get_bugs(self):
+        """Return a collection of bugs linked to the source branch."""
+        return self._mp.source_branch.linked_bugs
+
+    def _get_push_url(self, branch):
+        """Return the push URL for 'branch'.
+
+        This function is a work-around for Launchpad's lack of exposing the
+        branch's push URL.
+
+        :param branch: A launchpadlib `IBranch`.
+        """
+        # XXX: JonathanLange 2009-09-24: No unit tests.
+        host = get_bazaar_host(str(self._launchpad._root_uri))
+        # XXX: JonathanLange 2009-09-24 bug=435790: lazr.uri allows a path
+        # without a leading '/' and then doesn't insert a '/' in the final
+        # URL. Do it ourselves.
+        return URI(scheme='bzr+ssh', host=host, path='/' + branch.unique_name)
+
+    def get_commit_message(self, commit_text, testfix=False):
+        """Get the Launchpad-style commit message for a merge proposal."""
+        reviews = self.get_reviews()
+        bugs = self.get_bugs()
+        if testfix:
+            testfix = '[testfix]'
+        else:
+            testfix = ''
+        return '%s%s%s %s' % (
+            testfix,
+            get_reviewer_clause(reviews),
+            get_bugs_clause(bugs),
+            commit_text)
+
+
+def get_email(person):
+    """Get the preferred email address for 'person'."""
+    email_object = person.preferred_email_address
+    # XXX: JonathanLange 2009-09-24 bug=319432: This raises a very obscure
+    # error when the email address isn't set. e.g. with name12 in the sample
+    # data. e.g. "httplib2.RelativeURIError: Only absolute URIs are allowed.
+    # uri = tag:launchpad.net:2008:redacted".
+    return email_object.email
+
+
+def get_bugs_clause(bugs):
+    """Return the bugs clause of a commit message.
+
+    :param bugs: A collection of `IBug` objects.
+    :return: A string of the form "[bug=A,B,C]".
+    """
+    if not bugs:
+        return ''
+    return '[bug=%s]' % ','.join(str(bug.id) for bug in bugs)
+
+
+def get_reviewer_handle(reviewer):
+    """Get the handle for 'reviewer'.
+
+    The handles of reviewers are included in the commit message for Launchpad
+    changes. Historically, these handles have been the IRC nicks. Thus, if
+    'reviewer' has an IRC nickname for Freenode, we use that. Otherwise we use
+    their Launchpad username.
+
+    :param reviewer: A launchpadlib `IPerson` object.
+    :return: unicode text.
+    """
+    irc_handles = reviewer.irc_nicknames
+    for handle in irc_handles:
+        if handle.network == 'irc.freenode.net':
+            return handle.nickname
+    return reviewer.name
+
+
+def _comma_separated_names(things):
+    """Return a string of comma-separated names of 'things'."""
+    return ','.join(thing.name for thing in things)
+
+
+def get_reviewer_clause(reviewers):
+    """Get the reviewer section of a commit message, given the reviewers.
+
+    :param reviewers: A dict mapping review types to lists of reviewers, as
+        returned by 'get_reviews'.
+    :return: A string like u'[r=foo,bar][ui=plop]'.
+    """
+    code_reviewers = reviewers.get(None, [])
+    code_reviewers.extend(reviewers.get('code', []))
+    code_reviewers.extend(reviewers.get('db', []))
+    if not code_reviewers:
+        raise MissingReviewError("Need approved votes in order to land.")
+    ui_reviewers = reviewers.get('ui', [])
+    if ui_reviewers:
+        ui_clause = _comma_separated_names(ui_reviewers)
+    else:
+        ui_clause = 'none'
+    rc_reviewers = reviewers.get('release-critical', [])
+    if rc_reviewers:
+        rc_clause = (
+            '[release-critical=%s]' % _comma_separated_names(rc_reviewers))
+    else:
+        rc_clause = ''
+    return '%s[r=%s][ui=%s]' % (
+        rc_clause, _comma_separated_names(code_reviewers), ui_clause)
+
+
+def get_bazaar_host(api_root):
+    """Get the Bazaar service for the given API root."""
+    # XXX: JonathanLange 2009-09-24 bug=435803: This is only needed because
+    # Launchpad doesn't expose the push URL for branches.
+    if api_root == EDGE_SERVICE_ROOT:
+        return 'bazaar.launchpad.net'
+    elif api_root == DEV_SERVICE_ROOT:
+        return 'bazaar.launchpad.dev'
+    elif api_root == STAGING_SERVICE_ROOT:
+        return 'bazaar.staging.launchpad.net'
+    elif api_root == LPNET_SERVICE_ROOT:
+        return 'bazaar.launchpad.net'
+    else:
+        raise ValueError(
+            'Cannot determine Bazaar host. "%s" not a recognized Launchpad '
+            'API root.' % (api_root,))

=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py	2009-09-27 20:36:18 +0000
+++ lib/devscripts/ec2test/builtins.py	2009-09-30 13:37:18 +0000
@@ -6,7 +6,9 @@
 __metaclass__ = type
 __all__ = []
 
+import os
 import pdb
+import subprocess
 
 from bzrlib.commands import Command
 from bzrlib.errors import BzrCommandError
@@ -15,6 +17,9 @@
 
 import socket
 
+from devscripts import get_launchpad_root
+from devscripts.autoland import LaunchpadBranchLander, MissingReviewError
+
 from devscripts.ec2test.credentials import EC2Credentials
 from devscripts.ec2test.instance import (
     AVAILABLE_INSTANCE_TYPES, DEFAULT_INSTANCE_TYPE, EC2Instance)
@@ -264,6 +269,89 @@
         instance.set_up_and_run(postmortem, not headless, runner.run_tests)
 
 
+class cmd_land(EC2Command):
+    """Land a merge proposal on Launchpad."""
+
+    takes_options = [
+        debug_option,
+        Option('dry-run', help="Just print the equivalent ec2 test command."),
+        Option('print-commit', help="Print the full commit message."),
+        Option(
+            'testfix',
+            help="Include the [testfix] prefix in the commit message."),
+        Option(
+            'commit-text', short_name='s', type=str,
+            help=(
+                'A description of the landing, not including reviewer '
+                'metadata etc.')),
+        Option(
+            'force',
+            help="Land the branch even if the proposal is not approved."),
+        ]
+
+    takes_args = ['merge_proposal']
+
+    def _get_landing_command(self, source_url, target_url, commit_message,
+                             emails):
+        """Return the command that would need to be run to submit with ec2."""
+        ec2_path = os.path.join(get_launchpad_root(), 'utilities', 'ec2')
+        command = [ec2_path, 'test', '--headless']
+        command.extend(['--email=%s' % email for email in emails])
+        # 'ec2 test' has a bug where you cannot pass full URLs to branches to
+        # the -b option. It has special logic for 'launchpad' branches, so we
+        # piggy back on this to get 'devel' or 'db-devel'.
+        target_branch_name = target_url.split('/')[-1]
+        command.extend(
+            ['-b', 'launchpad=%s' % (target_branch_name), '-s',
+             commit_message, str(source_url)])
+        return command
+
+    def run(self, merge_proposal, machine=None,
+            instance_type=DEFAULT_INSTANCE_TYPE, postmortem=False,
+            debug=False, commit_text=None, dry_run=False, testfix=False,
+            print_commit=False, force=False):
+        if debug:
+            pdb.set_trace()
+        if print_commit and dry_run:
+            raise BzrCommandError(
+                "Cannot specify --print-commit and --dry-run.")
+        lander = LaunchpadBranchLander.load()
+        mp = lander.load_merge_proposal(merge_proposal)
+        if not mp.is_approved:
+            if force:
+                print "Merge proposal is not approved, landing anyway."
+            else:
+                raise BzrCommandError(
+                    "Merge proposal is not approved. Get it approved, or use "
+                    "--force to land it without approval.")
+        if commit_text is None:
+            commit_text = mp.commit_message
+        if commit_text is None:
+            raise BzrCommandError(
+                "Commit text not specified. Use --commit-text, or specify a "
+                "message on the merge proposal.")
+        try:
+            commit_message = mp.get_commit_message(commit_text, testfix)
+        except MissingReviewError:
+            raise BzrCommandError(
+                "Cannot land branches that haven't got approved code "
+                "reviews. Get an 'Approved' vote so we can fill in the "
+                "[r=REVIEWER] section.")
+        if print_commit:
+            print commit_message
+            return
+
+        landing_command = self._get_landing_command(
+            mp.source_branch, mp.target_branch, commit_message,
+            mp.get_stakeholder_emails())
+        if dry_run:
+            print landing_command
+        else:
+            # XXX: JonathanLange 2009-09-24 bug=439348: Call EC2 APIs
+            # directly, rather than spawning a subprocess.
+            return subprocess.call(landing_command)
+
+
 class cmd_demo(EC2Command):
     """Start a demo instance of Launchpad.
 

=== modified file 'lib/devscripts/sourcecode.py'
--- lib/devscripts/sourcecode.py	2009-09-25 00:07:53 +0000
+++ lib/devscripts/sourcecode.py	2009-09-30 13:37:18 +0000
@@ -23,6 +23,8 @@
 from bzrlib.transport import get_transport
 from bzrlib.workingtree import WorkingTree
 
+from devscripts import get_launchpad_root
+
 
 def parse_config_file(file_handle):
     """Parse the source code config file 'file_handle'.
@@ -182,10 +184,6 @@
         remove_branches(sourcecode_directory, removed)
 
 
-def get_launchpad_root():
-    return os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
-
-
 # XXX: JonathanLange 2009-09-11: By default, the script will operate on the
 # current checkout. Most people only have symlinks to sourcecode in their
 # checkouts. This is fine for updating, but breaks for removing (you can't

=== added file 'lib/devscripts/tests/test_autoland.py'
--- lib/devscripts/tests/test_autoland.py	1970-01-01 00:00:00 +0000
+++ lib/devscripts/tests/test_autoland.py	2009-09-30 13:37:18 +0000
@@ -0,0 +1,190 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for automatic landing thing."""
+
+__metaclass__ = type
+
+import unittest
+
+from launchpadlib.launchpad import EDGE_SERVICE_ROOT, STAGING_SERVICE_ROOT
+
+from devscripts.autoland import (
+    get_bazaar_host, get_bugs_clause, get_reviewer_clause,
+    get_reviewer_handle, MissingReviewError)
+
+
+class FakeBug:
+    """Fake launchpadlib Bug object.
+
+    Only used for the purposes of testing.
+    """
+
+    def __init__(self, id):
+        self.id = id
+
+
+class FakePerson:
+    """Fake launchpadlib Person object.
+
+    Only used for the purposes of testing.
+    """
+
+    def __init__(self, name, irc_handles):
+        self.name = name
+        self.irc_nicknames = list(irc_handles)
+
+
+class FakeIRC:
+    """Fake IRC handle.
+
+    Only used for the purposes of testing.
+    """
+
+    def __init__(self, nickname, network):
+        self.nickname = nickname
+        self.network = network
+
+
+class TestBugsClaused(unittest.TestCase):
+    """Tests for `get_bugs_clause`."""
+
+    def test_no_bugs(self):
+        # If there are no bugs, then there is no bugs clause.
+        bugs_clause = get_bugs_clause([])
+        self.assertEqual('', bugs_clause)
+
+    def test_one_bug(self):
+        # If there's a bug, then the bugs clause is [bug=$ID].
+        bug = FakeBug(45)
+        bugs_clause = get_bugs_clause([bug])
+        self.assertEqual('[bug=45]', bugs_clause)
+
+    def test_two_bugs(self):
+        # If there are two bugs, then the bugs clause is [bug=$ID,$ID].
+        bug1 = FakeBug(20)
+        bug2 = FakeBug(45)
+        bugs_clause = get_bugs_clause([bug1, bug2])
+        self.assertEqual('[bug=20,45]', bugs_clause)
+
+
+class TestGetReviewerHandle(unittest.TestCase):
+    """Tests for `get_reviewer_handle`."""
+
+    def makePerson(self, name, irc_handles):
+        return FakePerson(name, irc_handles)
+
+    def test_no_irc_nicknames(self):
+        # If the person has no IRC nicknames, their reviewer handle is their
+        # Launchpad user name.
+        person = self.makePerson(name='foo', irc_handles=[])
+        self.assertEqual('foo', get_reviewer_handle(person))
+
+    def test_freenode_irc_nick_preferred(self):
+        # If the person has a Freenode IRC nickname, then that is preferred as
+        # their user handle.
+        person = self.makePerson(
+            name='foo', irc_handles=[FakeIRC('bar', 'irc.freenode.net')])
+        self.assertEqual('bar', get_reviewer_handle(person))
+
+    def test_non_freenode_nicks_ignored(self):
+        # If the person has IRC nicks that aren't freenode, we ignore them.
+        person = self.makePerson(
+            name='foo', irc_handles=[FakeIRC('bar', 'irc.efnet.net')])
+        self.assertEqual('foo', get_reviewer_handle(person))
+
+
+class TestGetReviewerClause(unittest.TestCase):
+    """Tests for `get_reviewer_clause`."""
+
+    def makePerson(self, name):
+        return FakePerson(name, [])
+
+    def get_reviewer_clause(self, reviewers):
+        return get_reviewer_clause(reviewers)
+
+    def test_one_reviewer_no_type(self):
+        # It's very common for a merge proposal to be reviewed by one person
+        # with no specified type of review. It such cases the review clause is
+        # '[r=<person>][ui=none]'.
+        clause = self.get_reviewer_clause({None: [self.makePerson('foo')]})
+        self.assertEqual('[r=foo][ui=none]', clause)
+
+    def test_two_reviewers_no_type(self):
+        # Branches can have more than one reviewer.
+        clause = self.get_reviewer_clause(
+            {None: [self.makePerson('foo'), self.makePerson('bar')]})
+        self.assertEqual('[r=foo,bar][ui=none]', clause)
+
+    def test_code_reviewer_counts(self):
+        # Some people explicitly specify the 'code' type when they do code
+        # reviews, these are treated in the same way as reviewers without any
+        # given type.
+        clause = self.get_reviewer_clause({'code': [self.makePerson('foo')]})
+        self.assertEqual('[r=foo][ui=none]', clause)
+
+    def test_release_critical(self):
+        # Reviews that are marked as release-critical are included in a
+        # separate clause.
+        clause = self.get_reviewer_clause(
+            {'code': [self.makePerson('foo')],
+             'release-critical': [self.makePerson('bar')]})
+        self.assertEqual('[release-critical=bar][r=foo][ui=none]', clause)
+
+    def test_db_reviewer_counts(self):
+        # There's no special way of annotating database reviews in Launchpad
+        # commit messages, so they are included with the code reviews.
+        clause = self.get_reviewer_clause({'db': [self.makePerson('foo')]})
+        self.assertEqual('[r=foo][ui=none]', clause)
+
+    def test_ui_reviewers(self):
+        # If someone has done a UI review, then that appears in the clause
+        # separately from the code reviews.
+        clause = self.get_reviewer_clause(
+            {'code': [self.makePerson('foo')],
+             'ui': [self.makePerson('bar')],
+             })
+        self.assertEqual('[r=foo][ui=bar]', clause)
+
+    def test_no_reviewers(self):
+        # If the merge proposal hasn't been approved by anyone, we cannot
+        # generate a valid clause.
+        self.assertRaises(MissingReviewError, self.get_reviewer_clause, {})
+
+
+class TestGetBazaarHost(unittest.TestCase):
+    """Tests for `get_bazaar_host`."""
+
+    def test_dev_service(self):
+        # The Bazaar host for the dev service is bazaar.launchpad.dev.
+        self.assertEqual(
+            'bazaar.launchpad.dev',
+            get_bazaar_host('https://api.launchpad.dev/beta/'))
+
+    def test_edge_service(self):
+        # The Bazaar host for the edge service is bazaar.launchpad.net, since
+        # there's no edge codehosting service.
+        self.assertEqual(
+            'bazaar.launchpad.net', get_bazaar_host(EDGE_SERVICE_ROOT))
+
+    def test_production_service(self):
+        # The Bazaar host for the production service is bazaar.launchpad.net.
+        self.assertEqual(
+            'bazaar.launchpad.net',
+            get_bazaar_host('https://api.launchpad.net/beta/'))
+
+    def test_staging_service(self):
+        # The Bazaar host for the staging service is
+        # bazaar.staging.launchpad.net.
+        self.assertEqual(
+            'bazaar.staging.launchpad.net',
+            get_bazaar_host(STAGING_SERVICE_ROOT))
+
+    def test_unrecognized_service(self):
+        # Any unrecognized URL will raise a ValueError.
+        self.assertRaises(
+            ValueError, get_bazaar_host, 'https://api.lunchpad.net')
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/devscripts/tests/test_sourcecode.py'
--- lib/devscripts/tests/test_sourcecode.py	2009-09-25 00:07:53 +0000
+++ lib/devscripts/tests/test_sourcecode.py	2009-09-30 13:37:18 +0000
@@ -15,9 +15,9 @@
 from bzrlib.tests import TestCase
 from bzrlib.transport import get_transport
 
+from devscripts import get_launchpad_root
 from devscripts.sourcecode import (
-    find_branches, get_launchpad_root, interpret_config, parse_config_file,
-    plan_update)
+    find_branches, interpret_config, parse_config_file, plan_update)
 
 
 class TestParseConfigFile(unittest.TestCase):