Merge into trunk : revision-support : Precise Pangolin (12.04) : Code : juju-gui package : Juju Charms Collection

Status:	Merged
Merged at revision:	58
Proposed branch:	lp://staging/~frankban/charms/precise/juju-gui/revision-support
Merge into:	lp://staging/~juju-gui/charms/precise/juju-gui/trunk
Diff against target:	117 lines (+38/-8) 4 files modified config.yaml (+2/-1) hooks/utils.py (+26/-5) revision (+1/-1) tests/test_utils.py (+9/-1)
To merge this branch:	bzr merge lp://staging/~frankban/charms/precise/juju-gui/revision-support
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
charmers		2013-05-17	Pending
Review via email: mp+164326@code.staging.launchpad.net

Description of the change

Support juju-gui-source=branch:revno

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

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

Revision history for this message

Francesco Banconi (frankban) wrote on 2013-05-17:

#

Download full text (5.5 KiB)

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

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: nicola.larosa@canonical.com-20130503134613-5g3lt9zlqbbkyvb6
+New revision:  
francesco.banconi@canonical.com-20130517090728-dzwcuvhg7azlm325

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')

Revision history for this message

Brad Crittenden (bac) wrote on 2013-05-17:

#

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/

Revision history for this message

Jeff Pihach (hatch) wrote on 2013-05-17:

#

LGTM - Thanks for this great upgrade

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

Revision history for this message

Francesco Banconi (frankban) wrote on 2013-05-17:

#

*** 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/

Revision history for this message

Francesco Banconi (frankban) wrote on 2013-05-17:

#

Thanks for the reviews Brad and Jeff!

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

Juju Charms Collection
juju-gui package

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

Commit message

Description of the change

Preview Diff

Subscribers

Juju Charms Collectionjuju-gui package

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

Commit message

Description of the change

Preview Diff

Subscribers

Juju Charms Collection
juju-gui package