Merge lp:~frankban/charms/precise/juju-gui/revision-support into lp:~juju-gui/charms/precise/juju-gui/trunk

[Francesco Banconi]

Support juju-gui-source=branch:revno

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

https://codereview.appspot.com/9464043/

[Francesco Banconi (frankban)]

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(sourc...

[Brad Crittenden (bac)]

LGTM where G=great. Thanks Frankban.

https://codereview.appspot.com/9464043/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/9464043/diff/1/hooks/utils.py#newcode220
hooks/utils.py:220: """, re.VERBOSE)
If only all regex-es were this well documented. Thanks Francesco!

https://codereview.appspot.com/9464043/

[Jeff Pihach (hatch)]

LGTM - Thanks for this great upgrade

https://codereview.appspot.com/9464043/

[Francesco Banconi (frankban)]

*** Submitted:

Support juju-gui-source=branch:revno

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

R=bac, jeff.pihach
CC=
https://codereview.appspot.com/9464043

https://codereview.appspot.com/9464043/

[Francesco Banconi (frankban)]

Thanks for the reviews Brad and Jeff!

https://codereview.appspot.com/9464043/

=== modified file 'config.yaml'
--- config.yaml	2013-05-02 14:57:04 +0000
+++ config.yaml	2013-05-17 09:13:43 +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

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2013-05-03 10:37:06 +0000
+++ hooks/utils.py	2013-05-17 09:13:43 +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)

=== modified file 'revision'
--- revision	2013-05-02 14:57:04 +0000
+++ revision	2013-05-17 09:13:43 +0000
@@ -1,1 +1,1 @@
-45
+46

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2013-05-03 10:37:06 +0000
+++ tests/test_utils.py	2013-05-17 09:13:43 +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')