launchpad-reviewers team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/lpsetup/bug-1016645-add-command-line-options into lp:lpsetup.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1016645 in lpsetup: "lpsetup: get command"
  https://bugs.launchpad.net/lpsetup/+bug/1016645

For more details, see:
https://code.launchpad.net/~benji/lpsetup/bug-1016645-add-command-line-options/+merge/112588

This branch (mostly, see below) finishes the work on bug 1016645 by
adding command-line options to the "get" subcommand.

Two things from that bug were not done: the first was adding a
"repository" optional argument because --directory already does that.

The second undone thing is the adding of a "branch" argument to specify
one other than "devel".  I couldn't find a non-super-invasive way of
implementing that functionality and since it is listed as optional, I
decided not to further overrun my personal timebox.  If it is in fact
non-optional, then I can do that next in a different branch.

Tests: no new tests were needed but the existing tests spotted at least
one bug, so that was nice.

Lint: the pocketlint report is clean

-- 
https://code.launchpad.net/~benji/lpsetup/bug-1016645-add-command-line-options/+merge/112588
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/lpsetup/bug-1016645-add-command-line-options into lp:lpsetup.

=== modified file 'lpsetup/handlers.py'
--- lpsetup/handlers.py	2012-06-27 08:44:32 +0000
+++ lpsetup/handlers.py	2012-06-28 15:25:21 +0000
@@ -25,7 +25,11 @@
     )
 
 from lpsetup.exceptions import ValidationError
-from lpsetup.settings import LP_CHECKOUT
+from lpsetup.settings import (
+    LP_CHECKOUT,
+    LP_HTTP_REPO,
+    LP_SSH_REPO,
+    )
 
 
 def handle_user(namespace):
@@ -259,6 +263,17 @@
         namespace.feed_random = True
 
 
+def handle_source(namespace):
+    """Handle the --source and --use-http options."""
+    # If a source has been provided, then there is nothing else to do.
+    if namespace.source:
+        return
+    if namespace.use_http:
+        namespace.source = LP_HTTP_REPO
+    else:
+        namespace.source = LP_SSH_REPO
+
+
 def handle_branch_creation(namespace):
     """Handle directories involved in branch creation.
 

=== modified file 'lpsetup/settings.py'
--- lpsetup/settings.py	2012-05-25 13:37:09 +0000
+++ lpsetup/settings.py	2012-06-28 15:25:21 +0000
@@ -51,6 +51,7 @@
         }
     }
 LP_CHECKOUT = 'devel'
+LP_SANDBOX = 'sandbox'
 LP_PACKAGES = [
     # "launchpad-database-dependencies-9.1" can be removed once 8.x is
     # no longer an option in "launchpad-developer-dependencies.
@@ -58,10 +59,8 @@
     'launchpad-developer-dependencies', 'apache2',
     'apache2-mpm-worker', 'libapache2-mod-wsgi'
     ]
-LP_REPOS = (
-    'http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel',
-    'lp:launchpad',
-    )
+LP_HTTP_REPO = 'http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel'
+LP_SSH_REPO = 'lp:launchpad'
 LP_SOURCE_DEPS = (
     'http://bazaar.launchpad.net/~launchpad/lp-source-dependencies/trunk')
 LPSETUP_PPA = 'ppa:yellow/ppa'

=== modified file 'lpsetup/subcommands/get.py'
--- lpsetup/subcommands/get.py	2012-06-27 09:10:56 +0000
+++ lpsetup/subcommands/get.py	2012-06-28 15:25:21 +0000
@@ -25,6 +25,7 @@
 from lpsetup.handlers import (
     handle_directories,
     handle_lpuser,
+    handle_source,
     handle_ssh_keys,
     handle_user,
     handle_userdata,
@@ -33,41 +34,50 @@
     CHECKOUT_DIR,
     DEPENDENCIES_DIR,
     LP_CHECKOUT,
-    LP_REPOS,
+    LP_HTTP_REPO,
+    LP_SANDBOX,
     LP_SOURCE_DEPS,
+    LP_SSH_REPO,
     SSH_KEY_NAME,
     )
 from lpsetup.utils import call
 
 
-def setup_codebase(user, checkout_dir, dependencies_dir, valid_ssh_keys=True):
+def find_build_dir(sandbox_dir, checkout_dir):
+    if os.path.exists(sandbox_dir):
+        return sandbox_dir
+    elif os.path.exists(checkout_dir):
+        return checkout_dir
+
+
+def setup_codebase(user, sandbox_dir, checkout_dir,
+        dependencies_dir, valid_ssh_keys=True, lightweight=True):
     """Set up Launchpad repository and source dependencies.
 
     Return True if new changes are pulled from bzr repository.
     """
-    # TODO If doing no trees, use --lightweight.
+    pull_target = find_build_dir(sandbox_dir, checkout_dir)
     # Using real su because bzr uses uid.
-    if os.path.exists(checkout_dir):
+    if pull_target is not None:
         # Pull the repository.
-        revno_args = ('bzr', 'revno', checkout_dir)
+        revno_args = ('bzr', 'revno', pull_target)
         revno = run(*revno_args)
-        call(*get_su_command(user, ['bzr', 'pull', '-d', checkout_dir]))
+        call(*get_su_command(user, ['bzr', 'pull', '-d', pull_target]))
         changed = revno != run(*revno_args)
     else:
         # Branch the repository.
         # If we have valid SSH keys we should use the "lp:" style URL, "http"
         # if not.
         if valid_ssh_keys:
-            repo = LP_REPOS[1]
+            repo = LP_SSH_REPO
         else:
-            repo = LP_REPOS[0]
+            repo = LP_HTTP_REPO
         cmd = ('bzr', 'branch', repo, checkout_dir)
         call(*get_su_command(user, cmd))
+        if lightweight:
+            cmd = ('bzr', 'co', '--lightweight', checkout_dir, sandbox_dir)
+            call(*get_su_command(user, cmd))
         changed = True
-    # Check repository integrity.
-    if subprocess.call(['bzr', 'status', '-q', checkout_dir]):
-        raise subprocess.CalledProcessError(
-            'Repository {0} is corrupted.'.format(checkout_dir))
     # Set up source dependencies.
     with su(user):
         for subdir in ('eggs', 'yui', 'sourcecode'):
@@ -76,31 +86,36 @@
         if os.path.exists(download_cache):
             call('bzr', 'up', download_cache)
         else:
-            call('bzr', 'co', '--lightweight', LP_SOURCE_DEPS, download_cache)
+            call('bzr', 'co', LP_SOURCE_DEPS, download_cache)
     return changed
 
 
-def fetch(user, directory, dependencies_dir, valid_ssh_keys):
+def fetch(user, directory, dependencies_dir, valid_ssh_keys, lightweight):
     """Create a repo for the Launchpad code and retrieve it."""
-    # TODO Add --no-trees handling.
+    args = ('--2a', directory)
+    if lightweight:
+        args += ('--no-trees',)
     with su(user):
         # Set up the repository.
         mkdirs(directory)
-        call('bzr', 'init-repo', '--2a', directory)
+        call('bzr', 'init-repo', *args)
     # Set up the codebase.
     checkout_dir = os.path.join(directory, LP_CHECKOUT)
-    setup_codebase(user, checkout_dir, dependencies_dir, valid_ssh_keys)
+    sandbox_dir = os.path.join(directory, LP_SANDBOX)
+    setup_codebase(user, sandbox_dir, checkout_dir, dependencies_dir,
+        valid_ssh_keys, lightweight)
 
 
 def setup_external_sourcecode(
-    user, checkout_dir, dependencies_dir, valid_ssh_keys=True):
+    user, sandbox_dir, checkout_dir, dependencies_dir, valid_ssh_keys=True):
     """Update and link external sourcecode."""
     cmd = (
         'utilities/update-sourcecode',
         None if valid_ssh_keys else '--use-http',
         os.path.join(dependencies_dir, 'sourcecode'),
         )
-    with cd(checkout_dir):
+    build_dir = find_build_dir(sandbox_dir, checkout_dir)
+    with cd(build_dir):
         # Using real su because update-sourcecode uses uid.
         call(*get_su_command(user, cmd))
         with su(user):
@@ -121,9 +136,10 @@
         call('apt-get', 'update')
         call('apt-get', 'upgrade')
     checkout_dir = os.path.join(directory, LP_CHECKOUT)
+    sandbox_dir = os.path.join(directory, LP_SANDBOX)
     # Update the Launchpad codebase.
-    changed = setup_codebase(user, checkout_dir, dependencies_dir)
-    setup_external_sourcecode(user, checkout_dir, dependencies_dir)
+    changed = setup_codebase(user, sandbox_dir, checkout_dir, dependencies_dir)
+    setup_external_sourcecode(user, sandbox_dir, checkout_dir, dependencies_dir)
     if changed:
         make_launchpad(user, checkout_dir, install=False)
         if make_schema:
@@ -134,12 +150,15 @@
 def link_sourcecode_in_branches(user, dependencies_dir, directory):
     """Link external sourcecode for all branches in the project."""
     checkout_dir = os.path.join(directory, LP_CHECKOUT)
-    cmd = os.path.join(checkout_dir, 'utilities', 'link-external-sourcecode')
+    sandbox_dir = os.path.join(directory, LP_SANDBOX)
+    build_dir = find_build_dir(sandbox_dir, checkout_dir)
+    cmd = os.path.join(build_dir, 'utilities', 'link-external-sourcecode')
     with su(user):
         for dirname in os.listdir(directory):
             branch = os.path.join(directory, dirname)
             sourcecode_dir = os.path.join(branch, 'sourcecode')
             if (branch != checkout_dir and
+                branch != sandbox_dir and
                 os.path.exists(sourcecode_dir) and
                 os.path.isdir(sourcecode_dir)):
                 call(cmd, '--parent', dependencies_dir, '--target', branch)
@@ -149,7 +168,8 @@
     """Prepare a host machine to run Launchpad."""
 
     fetch_step = (fetch,
-         'user', 'directory', 'dependencies_dir', 'valid_ssh_keys')
+         'user', 'directory', 'dependencies_dir', 'valid_ssh_keys',
+         'lightweight')
     update_launchpad_step = (update_launchpad,
          'user', 'dependencies_dir', 'directory', 'make_schema', 'apt')
     link_sourcecode_in_branches_step = (link_sourcecode_in_branches,
@@ -168,6 +188,7 @@
         handle_userdata,
         handle_ssh_keys,
         handle_directories,
+        handle_source,
         )
 
     def get_needs_root(self, namespace):
@@ -241,4 +262,16 @@
             '--feed-random', action='store_true',
             help='Use haveged to maintain a pool of random bytes used to '
                  'fill /dev/random and avoid entropy exhaustion.')
+        parser.add_argument(
+            '--source', default=LP_SSH_REPO,
+            help='Location from which to retrieve Launchpad source.  Default '
+                'is lp:launchpad (if --use-http is false), otherwise '
+                'http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel.')
+        parser.add_argument(
+            '--use-http', action='store_true', dest='use_http',
+            default=False, help='Use http in place of bzr+ssh, ignored if '
+            '--source is provided.  Defaults to off.')
+        parser.add_argument(
+            '-t', '--with-trees', action='store_false', dest='lightweight',
+            default=True, help='Do not use a lightweight checkout.')
         self.add_update_arguments(parser)

=== modified file 'lpsetup/subcommands/install.py'
--- lpsetup/subcommands/install.py	2012-06-27 09:10:56 +0000
+++ lpsetup/subcommands/install.py	2012-06-28 15:25:21 +0000
@@ -129,3 +129,6 @@
                  'The directory must reside under the home directory of the '
                  'given user (see -u argument). '
                  '[DEFAULT={0}]'.format(CHECKOUT_DIR))
+        parser.add_argument(
+            '-t', '--with-trees', action='store_false', dest='lightweight',
+            default=True, help='Do not use a lightweight checkout.')

=== modified file 'lpsetup/subcommands/update.py'
--- lpsetup/subcommands/update.py	2012-06-27 09:10:56 +0000
+++ lpsetup/subcommands/update.py	2012-06-28 15:25:21 +0000
@@ -21,7 +21,6 @@
     """Update the Launchpad environment to latest version."""
 
     steps = (
-        get.SubCommand.fetch_step,
         get.SubCommand.update_launchpad_step,
         get.SubCommand.link_sourcecode_in_branches_step,
         )

=== modified file 'lpsetup/tests/subcommands/test_get.py'
--- lpsetup/tests/subcommands/test_get.py	2012-06-26 10:17:27 +0000
+++ lpsetup/tests/subcommands/test_get.py	2012-06-28 15:25:21 +0000
@@ -15,7 +15,8 @@
 
 
 fetch_step = (
-    get.fetch, ['user', 'directory', 'dependencies_dir', 'valid_ssh_keys'])
+    get.fetch, ['user', 'directory', 'dependencies_dir', 'valid_ssh_keys',
+    'lightweight'])
 update_launchpad_step = (
     get.update_launchpad, ['user', 'dependencies_dir', 'directory',
     'make_schema', 'apt'])
@@ -57,6 +58,7 @@
         handlers.handle_userdata,
         handlers.handle_ssh_keys,
         handlers.handle_directories,
+        handlers.handle_source,
         )
     expected_steps = (
         fetch_step,

=== modified file 'lpsetup/tests/subcommands/test_install.py'
--- lpsetup/tests/subcommands/test_install.py	2012-06-26 18:20:23 +0000
+++ lpsetup/tests/subcommands/test_install.py	2012-06-28 15:25:21 +0000
@@ -29,6 +29,7 @@
     inithost_arguments = test_inithost.get_arguments()
     dependencies_dir = '~/' + get_random_string()
     directory = '~/' + get_random_string()
+    source = get_random_string()
     return inithost_arguments + ('-d', dependencies_dir, '-c', directory)