launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/lpsetup/initrepo into lp:lpsetup.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~bac/lpsetup/initrepo/+merge/113625

* Rename the 'get' subcommand to 'initrepo' as it is more descriptive.

* Move setup_bzr_locations to init_repo.

* Create handle_lp_user_from_login for use when inithost has already been run.
-- 
https://code.launchpad.net/~bac/lpsetup/initrepo/+merge/113625
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/lpsetup/initrepo into lp:lpsetup.

=== modified file 'lpsetup/cli.py'
--- lpsetup/cli.py	2012-07-02 20:43:36 +0000
+++ lpsetup/cli.py	2012-07-05 18:41:20 +0000
@@ -16,9 +16,9 @@
     exceptions,
     )
 from lpsetup.subcommands import (
-    get,
     inithost,
     initlxc,
+    initrepo,
     install,
     lxcinstall,
     version,
@@ -29,7 +29,7 @@
     ('install', install.SubCommand),
     ('inithost', inithost.SubCommand),
     ('initlxc', initlxc.SubCommand),
-    ('get', get.SubCommand),
+    ('initrepo', initrepo.SubCommand),
     ('lxc-install', lxcinstall.SubCommand),
     ('version', version.SubCommand),
     ]

=== modified file 'lpsetup/handlers.py'
--- lpsetup/handlers.py	2012-07-02 20:43:36 +0000
+++ lpsetup/handlers.py	2012-07-05 18:41:20 +0000
@@ -7,7 +7,8 @@
 __metaclass__ = type
 __all__ = [
     'handle_directories',
-    'handle_lpuser',
+    'handle_lpuser_as_username',
+    'handle_lpuser_from_lplogin',
     'handle_ssh_keys',
     'handle_testing',
     'handle_user',
@@ -16,10 +17,12 @@
 
 import os
 import pwd
+import subprocess
 
 from shelltoolbox import (
     bzr_whois,
     get_user_home,
+    run,
     user_exists,
     )
 
@@ -60,7 +63,7 @@
     namespace.home_dir = get_user_home(namespace.user)
 
 
-def handle_lpuser(namespace):
+def handle_lpuser_as_username(namespace):
     """Handle lpuser argument.
 
     If lpuser is not provided by namespace, the user name is used::
@@ -70,7 +73,7 @@
 
         >>> import argparse
         >>> namespace = argparse.Namespace(user=username, lpuser=None)
-        >>> handle_lpuser(namespace)
+        >>> handle_lpuser_as_username(namespace)
         >>> namespace.lpuser == username
         True
     """
@@ -78,6 +81,16 @@
         namespace.lpuser = namespace.user
 
 
+def handle_lpuser_from_lplogin(namespace):
+    """Handle lpuser argument.
+    """
+    if getattr(namespace, 'lpuser', None) is None:
+        try:
+            namespace.lpuser = run('bzr', 'launchpad-login').strip()
+        except subprocess.CalledProcessError:
+            raise ValidationError("No bzr launchpad-login set.")
+
+
 def handle_userdata(namespace, whois=bzr_whois):
     """Handle full_name and email arguments.
 

=== modified file 'lpsetup/subcommands/inithost.py'
--- lpsetup/subcommands/inithost.py	2012-07-03 18:18:24 +0000
+++ lpsetup/subcommands/inithost.py	2012-07-05 18:41:20 +0000
@@ -26,7 +26,7 @@
 
 from lpsetup import argparser
 from lpsetup.handlers import (
-    handle_lpuser,
+    handle_lpuser_as_username,
     handle_ssh_keys,
     handle_user,
     handle_userdata,
@@ -166,7 +166,7 @@
     needs_root = True
     handlers = (
         handle_user,
-        handle_lpuser,
+        handle_lpuser_as_username,
         handle_userdata,
         handle_ssh_keys,
         )

=== renamed file 'lpsetup/subcommands/get.py' => 'lpsetup/subcommands/initrepo.py'
--- lpsetup/subcommands/get.py	2012-07-03 20:05:47 +0000
+++ lpsetup/subcommands/initrepo.py	2012-07-05 18:41:20 +0000
@@ -2,7 +2,7 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""get subcommand: prepare source code destinations and download it."""
+"""initrepo subcommand: prepare source code destinations and download it."""
 
 __metaclass__ = type
 __all__ = [
@@ -18,10 +18,15 @@
 from lpsetup.settings import (
     CHECKOUT_DIR,
     LP_BRANCH_NAME,
+    LP_BZR_LOCATIONS,
     LP_CHECKOUT_NAME,
     LP_SSH_REPO,
     )
-from lpsetup.utils import call
+from lpsetup.utils import (
+    call,
+    ConfigParser,
+    get_file_header,
+    )
 
 
 def fetch(source, repository, branch_name, checkout_name):
@@ -37,16 +42,42 @@
     call('bzr', 'co', '--lightweight', branch_dir, checkout_dir)
 
 
+def setup_bzr_locations(lpuser, repository, template=LP_BZR_LOCATIONS):
+    """Set up bazaar locations."""
+    context = {
+        'checkout_dir': os.path.join(repository, LP_BRANCH_NAME),
+        'repository': repository,
+        'lpuser': lpuser,
+        }
+
+    bazaar_dir = os.path.expanduser('~/.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:
+            f.write(get_file_header() + '\n')
+            parser.write(f)
+
+
 class SubCommand(argparser.StepsBasedSubCommand):
     """Get the Launchpad source"""
 
     steps = (
         (fetch, 'source', 'repository', 'branch_name', 'checkout_name'),
+        (setup_bzr_locations, 'lpuser', 'repository'),
         )
 
     help = __doc__
     handlers = (
         handlers.handle_user,
+        handlers.handle_lpuser_from_lplogin,
         handlers.handle_directories,
         handlers.handle_source,
         )

=== modified file 'lpsetup/subcommands/install.py'
--- lpsetup/subcommands/install.py	2012-07-03 20:05:47 +0000
+++ lpsetup/subcommands/install.py	2012-07-05 18:41:20 +0000
@@ -6,7 +6,7 @@
 
 __metaclass__ = type
 __all__ = [
-    'setup_bzr_locations',
+    'setup_bzr_locations_as_root',
     'setup_launchpad',
     'SubCommand',
     ]
@@ -24,7 +24,10 @@
     )
 
 from lpsetup.handlers import handle_directories
-from lpsetup.subcommands import inithost
+from lpsetup.subcommands import (
+    inithost,
+    initrepo,
+    )
 from lpsetup.settings import (
     CHECKOUT_DIR,
     DEPENDENCIES_DIR,
@@ -32,14 +35,9 @@
     HOSTS_FILE,
     LP_APACHE_MODULES,
     LP_APACHE_ROOTS,
-    LP_BZR_LOCATIONS,
     LP_BRANCH_NAME,
     )
-from lpsetup.utils import (
-    call,
-    ConfigParser,
-    get_file_header,
-    )
+from lpsetup.utils import call
 
 
 def make_launchpad(user, checkout_dir, install=False):
@@ -99,36 +97,15 @@
         file_append(HOSTS_FILE, line)
 
 
-def setup_bzr_locations(user, lpuser, repository, template=LP_BZR_LOCATIONS):
-    """Set up bazaar locations."""
-    context = {
-        'checkout_dir': os.path.join(repository, LP_BRANCH_NAME),
-        'repository': repository,
-        '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:
-            f.write(get_file_header() + '\n')
-            parser.write(f)
+def setup_bzr_locations_as_root(user, lpuser, repository):
+    with su(user):
+        initrepo.setup_bzr_locations(lpuser, repository)
 
 
 class SubCommand(inithost.SubCommand):
     """Install the Launchpad environment."""
 
     # The steps for "install" are a superset of the steps for "inithost".
-    setup_bzr_locations_step = (setup_bzr_locations,
-         'user', 'lpuser', 'repository')
 
     @property
     def steps(self):
@@ -138,7 +115,8 @@
         return (
             inithost.SubCommand.initialize_step,
             lxcinstall.SubCommand.fetch_step,
-            self.setup_bzr_locations_step,
+            (setup_bzr_locations_as_root,
+             'user', 'lpuser', 'repository'),
             inithost.SubCommand.setup_apt_step,
             (setup_launchpad,
             'user', 'dependencies_dir', 'repository', 'valid_ssh_keys'),

=== modified file 'lpsetup/subcommands/lxcinstall.py'
--- lpsetup/subcommands/lxcinstall.py	2012-07-03 20:05:47 +0000
+++ lpsetup/subcommands/lxcinstall.py	2012-07-05 18:41:20 +0000
@@ -37,7 +37,6 @@
 from lpsetup.subcommands import (
     inithost,
     initlxc,
-    install,
     )
 from lpsetup.utils import (
     call,
@@ -86,8 +85,8 @@
     """Set up the Launchpad environment inside an LXC."""
     # Use ssh to call this script from inside the container.
     args = [
-        'install', '-u', user, '-s', 'setup_apt', 'setup_launchpad',
-        '-d', dependencies_dir, '-r', repository
+        'install', '-u', user, '-s', 'setup_apt', 'setup_bzr_locatoins',
+        'setup_launchpad', '-d', dependencies_dir, '-r', repository,
         ]
     cmd = this_command(repository, args)
     ssh(lxc_name, cmd, key=ssh_key_path)
@@ -154,7 +153,6 @@
     steps = (
         inithost.SubCommand.initialize_step,
         fetch_step,
-        install.SubCommand.setup_bzr_locations_step,
         (create_scripts,
          'lxc_name', 'ssh_key_path', 'user'),
         initlxc.SubCommand.create_lxc_step + ('install_subunit',),

=== modified file 'lpsetup/tests/subcommands/test_inithost.py'
--- lpsetup/tests/subcommands/test_inithost.py	2012-06-26 18:20:23 +0000
+++ lpsetup/tests/subcommands/test_inithost.py	2012-07-05 18:41:20 +0000
@@ -40,7 +40,7 @@
     expected_arguments = get_arguments()
     expected_handlers = (
         handlers.handle_user,
-        handlers.handle_lpuser,
+        handlers.handle_lpuser_as_username,
         handlers.handle_userdata,
         handlers.handle_ssh_keys,
         )

=== modified file 'lpsetup/tests/subcommands/test_initlxc.py'
--- lpsetup/tests/subcommands/test_initlxc.py	2012-06-28 17:22:45 +0000
+++ lpsetup/tests/subcommands/test_initlxc.py	2012-07-05 18:41:20 +0000
@@ -46,7 +46,7 @@
     expected_arguments = get_arguments()
     expected_handlers = (
         handlers.handle_user,
-        handlers.handle_lpuser,
+        handlers.handle_lpuser_as_username,
         handlers.handle_userdata,
         handlers.handle_ssh_keys,
         )

=== renamed file 'lpsetup/tests/subcommands/test_get.py' => 'lpsetup/tests/subcommands/test_initrepo.py'
--- lpsetup/tests/subcommands/test_get.py	2012-07-02 19:53:45 +0000
+++ lpsetup/tests/subcommands/test_initrepo.py	2012-07-05 18:41:20 +0000
@@ -2,20 +2,22 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Tests for the get sub command."""
+"""Tests for the initrepo subcommand."""
 
 import unittest
 
 from lpsetup import handlers
-from lpsetup.subcommands import get
+from lpsetup.subcommands import initrepo
 from lpsetup.tests.utils import (
     get_random_string,
     StepsBasedSubCommandTestMixin,
     )
 
 
-fetch_step = (get.fetch,
+fetch_step = (initrepo.fetch,
     ['source', 'repository', 'branch_name', 'checkout_name'])
+setup_bzr_locations_step = (initrepo.setup_bzr_locations,
+                            ['lpuser', 'repository'])
 
 
 def get_arguments():
@@ -26,17 +28,19 @@
         )
 
 
-class GetTest(StepsBasedSubCommandTestMixin, unittest.TestCase):
+class InitrepoTest(StepsBasedSubCommandTestMixin, unittest.TestCase):
 
-    sub_command_name = 'get'
-    sub_command_class = get.SubCommand
+    sub_command_name = 'initrepo'
+    sub_command_class = initrepo.SubCommand
     expected_arguments = get_arguments()
     expected_handlers = (
         handlers.handle_user,
+        handlers.handle_lpuser_from_lplogin,
         handlers.handle_directories,
         handlers.handle_source,
         )
     expected_steps = (
         fetch_step,
+        setup_bzr_locations_step,
         )
     needs_root = False

=== modified file 'lpsetup/tests/subcommands/test_install.py'
--- lpsetup/tests/subcommands/test_install.py	2012-07-03 18:13:48 +0000
+++ lpsetup/tests/subcommands/test_install.py	2012-07-05 18:41:20 +0000
@@ -16,7 +16,7 @@
 
 
 setup_bzr_locations_step = (
-    install.setup_bzr_locations, ['user', 'lpuser', 'repository'])
+    install.setup_bzr_locations_as_root, ['user', 'lpuser', 'repository'])
 setup_launchpad_step = (
     install.setup_launchpad, ['user', 'dependencies_dir', 'repository',
     'valid_ssh_keys'])
@@ -35,7 +35,7 @@
     expected_arguments = get_arguments()
     expected_handlers = (
         handlers.handle_user,
-        handlers.handle_lpuser,
+        handlers.handle_lpuser_as_username,
         handlers.handle_userdata,
         handlers.handle_ssh_keys,
         handlers.handle_directories,

=== modified file 'lpsetup/tests/subcommands/test_lxcinstall.py'
--- lpsetup/tests/subcommands/test_lxcinstall.py	2012-07-02 21:02:57 +0000
+++ lpsetup/tests/subcommands/test_lxcinstall.py	2012-07-05 18:41:20 +0000
@@ -11,7 +11,6 @@
 from lpsetup.tests.subcommands import (
     test_inithost,
     test_initlxc,
-    test_install,
     )
 from lpsetup.tests.utils import (
     get_random_string,
@@ -46,7 +45,7 @@
     expected_arguments = get_arguments()
     expected_handlers = (
         handlers.handle_user,
-        handlers.handle_lpuser,
+        handlers.handle_lpuser_as_username,
         handlers.handle_userdata,
         handlers.handle_ssh_keys,
         handlers.handle_directories,
@@ -55,7 +54,6 @@
     expected_steps = (
         test_inithost.initialize_step,
         fetch_step,
-        test_install.setup_bzr_locations_step,
         create_scripts_step,
         create_lxc_step,
         test_initlxc.start_lxc_step,

=== modified file 'lpsetup/tests/test_handlers.py'
--- lpsetup/tests/test_handlers.py	2012-07-03 18:13:48 +0000
+++ lpsetup/tests/test_handlers.py	2012-07-05 18:41:20 +0000
@@ -13,7 +13,7 @@
 from lpsetup.exceptions import ValidationError
 from lpsetup.handlers import (
     handle_directories,
-    handle_lpuser,
+    handle_lpuser_as_username,
     handle_ssh_keys,
     handle_testing,
     handle_user,
@@ -97,7 +97,7 @@
         # If lpuser is not provided by namespace, the user name is used.
         username = getpass.getuser()
         namespace = argparse.Namespace(user=username, lpuser=None)
-        handle_lpuser(namespace)
+        handle_lpuser_as_username(namespace)
         self.assertEqual(username, namespace.lpuser)