Code review comment for lp://staging/~frankban/charms/precise/juju-gui/revision-support

Reviewers: mp+164326_code.launchpad.net,

Message:
Please take a look.

Description:
Support juju-gui-source=branch:revno

Added support for creating and deploying
a release from a specific branch revision.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/revision-support/+merge/164326

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/9464043/

Affected files:
   A [revision details]
   M config.yaml
   M hooks/utils.py
   M revision
   M tests/test_utils.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision:
<email address hidden>

Index: config.yaml
=== modified file 'config.yaml'
--- config.yaml 2013-05-02 14:57:04 +0000
+++ config.yaml 2013-05-11 00:18:47 +0000
@@ -10,7 +10,8 @@
          will be deployed;
        - a Bazaar branch (e.g. 'lp:juju-gui'): a release will be created and
          deployed from the specified Bazaar branch. 'http://' prefixed
branches
- work as well.
+ work as well. It is also possible to include the specific branch
+ revision, e.g. 'lp:juju-gui:42' will checkout revno 42.
        - a 'url:' prefixed url (ex: url:http://...) of a specific location
          to pull a release from.
      type: string

Index: revision
=== modified file 'revision'
--- revision 2013-05-02 14:57:04 +0000
+++ revision 2013-05-16 13:54:35 +0000
@@ -1,1 +1,1 @@
-45
+46

Index: hooks/utils.py
=== modified file 'hooks/utils.py'
--- hooks/utils.py 2013-05-03 10:37:06 +0000
+++ hooks/utils.py 2013-05-17 09:07:28 +0000
@@ -44,6 +44,7 @@
  import json
  import os
  import logging
+import re
  import shutil
  from subprocess import CalledProcessError
  import tempfile
@@ -211,6 +212,14 @@
          log("<<< Exiting {}".format(script))

+bzr_url_expression = re.compile(r"""
+ ^ # Beginning of line.
+ ((?:lp:|http:\/\/)[^:]+) # Branch URL (scheme + domain/path).
+ (?::(\d+))? # Optional branch revision.
+ $ # End of line.
+""", re.VERBOSE)
+
+
  def parse_source(source):
      """Parse the ``juju-gui-source`` option.

@@ -220,7 +229,9 @@
         - ('stable', '0.1.0'): stable release v0.1.0;
         - ('trunk', None): latest trunk release;
         - ('trunk', '0.1.0+build.1'): trunk release v0.1.0 bzr revision 1;
- - ('branch', 'lp:juju-gui'): release is made from a branch;
+ - ('branch', ('lp:juju-gui', 42): release is made from a branch -
+ in this case the second element includes the branch URL and
revision;
+ - ('branch', ('lp:juju-gui', None): no revision is specified;
         - ('url', 'http://example.com/gui'): release from a downloaded file.
      """
      if source.startswith('url:'):
@@ -233,8 +244,9 @@
          return 'url', source
      if source in ('stable', 'trunk'):
          return source, None
- if source.startswith('lp:') or source.startswith('http://'):
- return 'branch', source
+ match = bzr_url_expression.match(source)
+ if match is not None:
+ return 'branch', match.groups()
      if 'build' in source:
          return 'trunk', source
      return 'stable', source
@@ -438,14 +450,23 @@
      # Retrieve a Juju GUI release.
      origin, version_or_branch = parse_source(juju_gui_source)
      if origin == 'branch':
+ branch_url, revision = version_or_branch
          # Make sure we have the dependencies necessary for us to actually
make
          # a build.
          _get_build_dependencies()
          # Create a release starting from a branch.
          juju_gui_source_dir = os.path.join(CURRENT_DIR, 'juju-gui-source')
- log('Retrieving Juju GUI source checkout from %s.' %
version_or_branch)
+ if revision is None:
+ checkout_args = []
+ revno = 'latest revno'
+ else:
+ checkout_args = ['--revision', revision]
+ revno = 'revno {}'.format(revision)
+ log('Retrieving Juju GUI source checkout from {} ({}).'.format(
+ branch_url, revno))
          cmd_log(run('rm', '-rf', juju_gui_source_dir))
- cmd_log(bzr_checkout(version_or_branch, juju_gui_source_dir))
+ checkout_args.extend([branch_url, juju_gui_source_dir])
+ cmd_log(bzr_checkout(*checkout_args))
          log('Preparing a Juju GUI release.')
          logdir = os.path.dirname(logpath)
          fd, name = tempfile.mkstemp(prefix='make-distfile-', dir=logdir)

Index: tests/test_utils.py
=== modified file 'tests/test_utils.py'
--- tests/test_utils.py 2013-05-03 10:37:06 +0000
+++ tests/test_utils.py 2013-05-11 00:18:47 +0000
@@ -402,7 +402,15 @@
          # Ensure a Bazaar branch is correctly parsed.
          sources = ('lp:example', 'http://bazaar.launchpad.net/example')
          for source in sources:
- self.assertTupleEqual(('branch', source), parse_source(source))
+ expected = ('branch', (source, None))
+ self.assertEqual(expected, parse_source(source))
+
+ def test_bzr_branch_and_revision(self):
+ # A Bazaar branch is correctly parsed when including revision.
+ sources =
('lp:example:42', 'http://bazaar.launchpad.net/example:1')
+ for source in sources:
+ expected = ('branch', tuple(source.rsplit(':', 1)))
+ self.assertEqual(expected, parse_source(source))

      def test_url(self):
          expected = ('url', 'http://example.com/gui')