Merge lp:~frankban/lpsetup/branch-subcommand into lp:lpsetup

[Francesco Banconi]

== Summary ==

This branch adds the *branch* subcommand to *lp-setup*.
The branch subcommand can be used to create and build a new Launchpad branch,
e.g.::

    lp-setup branch bug-666

It is similar (and intended to replace) *rocketfuel-get*,
but adds some functionalities:

- it can be called by root (specifying the actual user with -u argument)
- it can be called specifying different launchpad repository and dependencies
  paths (i.e. when you have multiple launchpad checkouts)
- `make schema` can be triggered after the branch is created (-S argument).

== Changes ==

Added a branch sub command.

Added a validator for branch parent and target.

Bumped version up: branch sub command.

Other minor fixes:

the *install* subcommand now changes the owner of `/srv/launchpad.dev/`.

Apparmor workaround has been fixed to not fail if the hack was already applied.

Better handling of bzr locations: now it is a separate step, and it takes
advantage of a customized config parser that parses bzr location files.

Updated *lxc-install* sub command to reflect the new step added in
the *install* one.

== Tests ==

$ nosetests
............................................................
Name Stmts Miss Cover Missing
---------------------------------------------------
lpsetup 6 1 83% 16
lpsetup.argparser 125 6 95% 113, 221, 278-279, 298, 307
lpsetup.exceptions 6 0 100%
lpsetup.handlers 65 1 98% 57
lpsetup.settings 28 0 100%
lpsetup.subcommands 0 0 100%
lpsetup.utils 133 32 76% 96, 130-140, 155, 206, 216, 237-239, 257-263, 278-279, 291-297
---------------------------------------------------
TOTAL 363 40 89%
----------------------------------------------------------------------
Ran 60 tests in 0.521s

OK

[Graham Binns (gmb)]

review: Approve (code)

=== modified file 'lpsetup/__init__.py'
--- lpsetup/__init__.py	2012-05-08 10:52:06 +0000
+++ lpsetup/__init__.py	2012-05-15 17:00:40 +0000
@@ -9,7 +9,7 @@
     'get_version',
     ]
 
-VERSION = (0, 2, 0)
+VERSION = (0, 2, 1)
 
 
 def get_version():

=== modified file 'lpsetup/cli.py'
--- lpsetup/cli.py	2012-05-02 16:32:06 +0000
+++ lpsetup/cli.py	2012-05-15 17:00:40 +0000
@@ -15,6 +15,7 @@
     exceptions,
     )
 from lpsetup.subcommands import (
+    branch,
     install,
     lxcinstall,
     update,
@@ -26,6 +27,7 @@
     parser.register_subcommand('install', install.SubCommand)
     parser.register_subcommand('update', update.SubCommand)
     parser.register_subcommand('lxc-install', lxcinstall.SubCommand)
+    parser.register_subcommand('branch', branch.SubCommand)
     args = parser.parse_args()
     try:
         return args.main(args)

=== modified file 'lpsetup/handlers.py'
--- lpsetup/handlers.py	2012-04-20 14:56:44 +0000
+++ lpsetup/handlers.py	2012-05-15 17:00:40 +0000
@@ -6,6 +6,7 @@
 
 __metaclass__ = type
 __all__ = [
+    'handle_branch_creation',
     'handle_directories',
     'handle_lpuser',
     'handle_ssh_keys',
@@ -24,6 +25,7 @@
     )
 
 from lpsetup.exceptions import ValidationError
+from lpsetup.settings import LP_CHECKOUT
 
 
 def handle_user(namespace):
@@ -255,3 +257,22 @@
         namespace.create_scripts = True
         namespace.install_subunit = True
         namespace.use_urandom = True
+
+
+def handle_branch_creation(namespace):
+    """Handle directories involved in branch creation.
+
+    The validation fails if the branch parent does not exist, or if a branch
+    with the provided name already exists.
+
+    After this handler is called, `parent` and `branch` names
+    are present as attributes of the namespace.
+    """
+    base_dir = namespace.directory
+    parent = os.path.join(base_dir, LP_CHECKOUT)
+    branch = os.path.join(base_dir, namespace.name)
+    if not os.path.isdir(parent):
+        raise ValidationError('parent branch {0} not found.'.format(parent))
+    if os.path.exists(branch):
+        raise ValidationError('{0} already exists.'.format(branch))
+    namespace.parent, namespace.branch = parent, branch

=== modified file 'lpsetup/settings.py'
--- lpsetup/settings.py	2012-05-08 08:06:52 +0000
+++ lpsetup/settings.py	2012-05-15 17:00:40 +0000
@@ -35,15 +35,21 @@
     '/var/tmp/archive',
     '/var/tmp/ppa',
     )
-LP_BZR_LOCATIONS = (
-    ('submit_branch', '{checkout_dir}'),
-    ('public_branch', 'bzr+ssh://bazaar.launchpad.net/~{lpuser}/launchpad'),
-    ('public_branch:policy', 'appendpath'),
-    ('push_location', 'lp:~{lpuser}/launchpad'),
-    ('push_location:policy', 'appendpath'),
-    ('merge_target', '{checkout_dir}'),
-    ('submit_to', '[e-mail]'),
-    )
+LP_BZR_LOCATIONS = {
+    '{directory}': {
+        'submit_branch': '{checkout_dir}',
+        'public_branch': 'bzr+ssh://bazaar.launchpad.net/~{lpuser}/launchpad',
+        'public_branch:policy': 'appendpath',
+        'push_location': 'lp:~{lpuser}/launchpad',
+        'push_location:policy': 'appendpath',
+        'merge_target': '{checkout_dir}',
+        'submit_to': '[e-mail]',
+        },
+    '{checkout_dir}': {
+        'public_branch':
+            'bzr+ssh://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel',
+        }
+    }
 LP_CHECKOUT = 'devel'
 LP_PACKAGES = [
     # "launchpad-database-dependencies-9.1" can be removed once 8.x is

=== added file 'lpsetup/subcommands/branch.py'
--- lpsetup/subcommands/branch.py	1970-01-01 00:00:00 +0000
+++ lpsetup/subcommands/branch.py	2012-05-15 17:00:40 +0000
@@ -0,0 +1,88 @@
+#!/usr/bin/env python
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Branch subcommand: create and build a new Launchpad branch."""
+
+__metaclass__ = type
+__all__ = [
+    'create_branch',
+    'setup_and_make',
+    'SubCommand',
+    ]
+
+from shelltoolbox import (
+    cd,
+    get_su_command,
+    su,
+    )
+
+from lpsetup import (
+    argparser,
+    handlers,
+    )
+from lpsetup.settings import (
+    CHECKOUT_DIR,
+    DEPENDENCIES_DIR,
+    )
+from lpsetup.subcommands import install
+from lpsetup.utils import call
+
+
+def create_branch(user, parent, branch):
+    """Create a branch of `devel`."""
+    cmd = ('bzr', 'branch', parent, branch)
+    # Using real su because bzr uses uid.
+    call(*get_su_command(user, cmd), stderr=None)
+
+
+def setup_and_make(user, dependencies_dir, branch, make_schema):
+    """Set up external source code for the new branch and run `make`."""
+    with cd(branch):
+        with su(user):
+            call('utilities/link-external-sourcecode', dependencies_dir)
+    install.make_launchpad(user, branch, install=False)
+    if make_schema:
+        with su(user):
+            call('make', '-C', branch, 'schema')
+
+
+class SubCommand(argparser.StepsBasedSubCommand):
+    """Create and build a new Launchpad branch."""
+
+    steps = (
+        (create_branch,
+         'user', 'parent', 'branch'),
+        (setup_and_make,
+         'user', 'dependencies_dir', 'branch', 'make_schema'),
+        )
+    help = __doc__
+    validators = (
+        handlers.handle_user,
+        handlers.handle_directories,
+        handlers.handle_branch_creation,
+        )
+
+    def add_arguments(self, parser):
+        super(SubCommand, self).add_arguments(parser)
+        parser.add_argument(
+            '-u', '--user',
+            help='The name of the system user that will own the branch. '
+                 'The current user is used if this script is not run as '
+                 'root and this argument is omitted.')
+        parser.add_argument(
+            '-d', '--dependencies-dir', default=DEPENDENCIES_DIR,
+            help='The directory of the Launchpad dependencies to be linked. '
+                 'The directory must reside under the home directory of the '
+                 'given user (see -u argument). '
+                 '[DEFAULT={0}]'.format(DEPENDENCIES_DIR))
+        parser.add_argument(
+            '-c', '--directory', default=CHECKOUT_DIR,
+            help='The directory of the existing Launchpad repository. '
+                 'The directory must reside under the home directory of the '
+                 'given user (see -u argument). '
+                 '[DEFAULT={0}]'.format(CHECKOUT_DIR))
+        parser.add_argument(
+            '-S', '--make-schema', action='store_true',
+            help='Run `make schema` in the newly created branch.')
+        parser.add_argument('name', help='The name of the branch to create.')

=== modified file 'lpsetup/subcommands/install.py'
--- lpsetup/subcommands/install.py	2012-05-08 13:43:08 +0000
+++ lpsetup/subcommands/install.py	2012-05-15 17:00:40 +0000
@@ -52,7 +52,10 @@
     LP_SOURCE_DEPS,
     SSH_KEY_NAME,
     )
-from lpsetup.utils import call
+from lpsetup.utils import (
+    call,
+    ConfigParser,
+    )
 
 
 def setup_codebase(user, valid_ssh_keys, checkout_dir, dependencies_dir):
@@ -157,14 +160,6 @@
     # Set up the codebase.
     checkout_dir = os.path.join(directory, LP_CHECKOUT)
     setup_codebase(user, valid_ssh_keys, checkout_dir, dependencies_dir)
-    # Set up bzr locations
-    with su(user) as env:
-        bzr_locations = os.path.join(env.home, '.bazaar', 'locations.conf')
-        file_append(bzr_locations, '[{0}]\n'.format(directory))
-        lines = ['{0} = {1}\n'.format(k, v) for k, v in LP_BZR_LOCATIONS]
-        for line in lines:
-            location = line.format(checkout_dir=checkout_dir, lpuser=lpuser)
-            file_append(bzr_locations, location)
     # rng-tools is used to set /dev/urandom as random data source, avoiding
     # entropy exhaustion during automated parallel tests.
     if use_urandom:
@@ -173,6 +168,29 @@
         call('/etc/init.d/rng-tools', 'start')
 
 
+def setup_bzr_locations(user, lpuser, directory, template=LP_BZR_LOCATIONS):
+    """Set up bazaar locations."""
+    context = {
+        'checkout_dir': os.path.join(directory, LP_CHECKOUT),
+        'directory': directory,
+        'lpuser': lpuser,
+        }
+    with su(user) as env:
+        bazaar_dir = os.path.join(env.home, '.bazaar')
+        mkdirs(bazaar_dir)
+        path = os.path.join(bazaar_dir, 'locations.conf')
+        parser = ConfigParser()
+        parser.read(path)
+        for section_template, options in template.items():
+            section = section_template.format(**context)
+            if not parser.has_section(section):
+                parser.add_section(section)
+            for option, value in options.items():
+                parser.set(section, option, value.format(**context))
+        with open(path, 'w') as f:
+            parser.write(f)
+
+
 def setup_apt(no_repositories=True):
     """Setup, update and upgrade deb packages."""
     if not no_repositories:
@@ -191,8 +209,8 @@
     """Set up the Launchpad environment."""
     # User configuration.
     subprocess.call(['adduser', user, 'sudo'])
-    gid = pwd.getpwnam(user).pw_gid
-    subprocess.call(['addgroup', '--gid', str(gid), user])
+    pwd_database = pwd.getpwnam(user)
+    subprocess.call(['addgroup', '--gid', str(pwd_database.pw_gid), user])
     # Set up Launchpad dependencies.
     checkout_dir = os.path.join(directory, LP_CHECKOUT)
     setup_external_sourcecode(
@@ -208,6 +226,8 @@
         call('utilities/launchpad-database-setup', user)
     # Make and install launchpad.
     make_launchpad(user, checkout_dir, install=True)
+    # Change owner of /srv/launchpad.dev/.
+    os.chown('/srv/launchpad.dev/', pwd_database.pw_uid, pwd_database.pw_gid)
     # Set up container hosts file.
     lines = ['{0}\t{1}\n'.format(ip, names) for ip, names in HOSTS_CONTENT]
     for line in lines:
@@ -222,6 +242,8 @@
          'user', 'full_name', 'email', 'lpuser',
          'private_key', 'public_key', 'valid_ssh_keys', 'ssh_key_path',
          'use_urandom', 'dependencies_dir', 'directory'),
+        (setup_bzr_locations,
+         'user', 'lpuser', 'directory'),
         (setup_apt,
          'no_repositories'),
         (setup_launchpad,

=== modified file 'lpsetup/subcommands/lxcinstall.py'
--- lpsetup/subcommands/lxcinstall.py	2012-05-08 13:43:08 +0000
+++ lpsetup/subcommands/lxcinstall.py	2012-05-15 17:00:40 +0000
@@ -103,9 +103,11 @@
     #     much a Good Thing.
     # Disable the apparmor profiles for lxc so that we don't have
     # problems installing postgres.
-    call('ln', '-s',
-         '/etc/apparmor.d/usr.bin.lxc-start', '/etc/apparmor.d/disable/')
-    call('apparmor_parser', '-R', '/etc/apparmor.d/usr.bin.lxc-start')
+    subprocess.call([
+        'ln', '-s',
+        '/etc/apparmor.d/usr.bin.lxc-start', '/etc/apparmor.d/disable/'])
+    subprocess.call([
+        'apparmor_parser', '-R', '/etc/apparmor.d/usr.bin.lxc-start'])
     # Container configuration template.
     lxc_gateway = get_lxc_gateway()
     if lxc_gateway is None:
@@ -204,6 +206,8 @@
          'user', 'full_name', 'email', 'lpuser',
          'private_key', 'public_key', 'valid_ssh_keys', 'ssh_key_path',
          'use_urandom', 'dependencies_dir', 'directory'),
+        (install.setup_bzr_locations,
+         'user', 'lpuser', 'directory'),
         (create_scripts,
          'user', 'lxc_name', 'ssh_key_path'),
         (create_lxc,

=== modified file 'lpsetup/tests/test_handlers.py'
--- lpsetup/tests/test_handlers.py	2012-04-23 17:13:36 +0000
+++ lpsetup/tests/test_handlers.py	2012-05-15 17:00:40 +0000
@@ -7,11 +7,15 @@
 import argparse
 from contextlib import contextmanager
 import getpass
+import os
 import pwd
+import shutil
+import tempfile
 import unittest
 
 from lpsetup.exceptions import ValidationError
 from lpsetup.handlers import (
+    handle_branch_creation,
     handle_directories,
     handle_lpuser,
     handle_ssh_keys,
@@ -19,6 +23,7 @@
     handle_user,
     handle_userdata,
     )
+from lpsetup.settings import LP_CHECKOUT
 
 
 class HandlersTestMixin(object):
@@ -61,6 +66,42 @@
                 raise TypeError
 
 
+class HandleBranchCreationTest(HandlersTestMixin, unittest.TestCase):
+
+    def setUp(self):
+        directory = tempfile.mkdtemp()
+        self.namespace = argparse.Namespace(
+            directory=directory, name='new-branch')
+        self.parent = os.path.join(directory, LP_CHECKOUT)
+        os.mkdir(self.parent)
+
+    def get_branch(self):
+        return os.path.join(self.namespace.directory, self.namespace.name)
+
+    def tearDown(self):
+        shutil.rmtree(self.namespace.directory)
+
+    def test_correct_paths(self):
+        # Ensure the namespace contains the correct `parent` and `branch`
+        # attributes if the validation passes.
+        handle_branch_creation(self.namespace)
+        self.assertEqual(self.parent, self.namespace.parent)
+        self.assertEqual(self.get_branch(), self.namespace.branch)
+
+    def test_invalid_parent(self):
+        # The validation fails if the parent branch is not found.
+        os.rmdir(self.parent)
+        with self.assertNotValid(self.parent):
+            handle_branch_creation(self.namespace)
+
+    def test_invalid_branch_name(self):
+        # The validation fails if the branch already exists.
+        branch = self.get_branch()
+        os.mkdir(branch)
+        with self.assertNotValid(branch):
+            handle_branch_creation(self.namespace)
+
+
 class HandleDirectoriesTest(HandlersTestMixin, unittest.TestCase):
 
     home_dir = '/home/foo'

=== modified file 'lpsetup/tests/test_utils.py'
--- lpsetup/tests/test_utils.py	2012-04-30 16:53:35 +0000
+++ lpsetup/tests/test_utils.py	2012-05-15 17:00:40 +0000
@@ -8,6 +8,7 @@
 import os
 import shutil
 import sys
+import StringIO
 import tempfile
 import unittest
 
@@ -17,6 +18,7 @@
     LXC_NAME,
     )
 from lpsetup.utils import (
+    ConfigParser,
     get_container_path,
     get_lxc_gateway,
     get_network_interfaces,
@@ -28,6 +30,21 @@
     )
 
 
+class ConfigParserTest(unittest.TestCase):
+
+    def get_parser(self, config):
+        parser = ConfigParser()
+        parser.readfp(StringIO.StringIO(config))
+        return parser
+
+    def test_parser(self):
+        # Ensure the parser correctly parses options containing colons.
+        config = '[section]\noption1 = value1\noption2:colon = value2\n'
+        items = dict(self.get_parser(config).items('section'))
+        self.assertEqual('value1', items['option1'])
+        self.assertEqual('value2', items['option2:colon'])
+
+
 class GetContainerPathTest(unittest.TestCase):
 
     def test_root_path(self):

=== modified file 'lpsetup/utils.py'
--- lpsetup/utils.py	2012-05-02 16:32:06 +0000
+++ lpsetup/utils.py	2012-05-15 17:00:40 +0000
@@ -21,6 +21,7 @@
     'this_command',
     ]
 
+from ConfigParser import RawConfigParser
 from functools import (
     partial,
     wraps,
@@ -55,6 +56,20 @@
 call = partial(run, stdout=None)
 
 
+class ConfigParser(RawConfigParser):
+    """A customized configuration parser.
+
+    The base `RawConfigParser` separates options from values using a colon
+    or a equal sign. This parser forces the separator to be an equal sign,
+    allowing the colon to be part of an option name.
+    """
+    OPTCRE = re.compile(
+        r'(?P<option>[^=\s][^=]*)'  # very permissive!
+        r'\s*(?P<vi>=)\s*'          # space/tab, separator, space/tab
+        r'(?P<value>.*)$'           # everything up to eol
+        )
+
+
 def get_container_path(lxc_name, path='', base_path=LXC_PATH):
     """Return the path of LXC container called `lxc_name`.