Merge lp:~rockstar/launchpad/upgrade-job-2.0-changes into lp:launchpad

[Paul Hummer]

[Paul Hummer (rockstar)]

Hi there-

  This branch makes some changes to the branch upgrade structure and the way a
branch determines whether or not it needs to be upgraded. Most of the original
code was pre-2.0, and now that bzr 2.0 has been released, the formats are a
little more stable, and we won't need to be concerned too much about changing
formats anymore.

  Instead of saying X format needs to be upgraded to Y format, we can just
upgrade it and it'll go to the 2.0 format regardless of the original format (as
opposed to the potential downgrade of older bzr).

Cheers,
Paul

[Paul Hummer (rockstar)]

Hrm, this diff seems confused. It has this branch's changes as well as the next branch's changes... Here's the real diff:

=== modified file 'lib/lp/code/bzr.py'
--- lib/lp/code/bzr.py 2009-06-30 16:56:07 +0000
+++ lib/lp/code/bzr.py 2010-01-16 07:38:47 +0000
@@ -8,6 +8,8 @@
     'BranchFormat',
     'BRANCH_FORMAT_UPGRADE_PATH',
     'ControlFormat',
+ 'CURRENT_BRANCH_FORMATS',
+ 'CURRENT_REPOSITORY_FORMATS',
     'RepositoryFormat',
     'REPOSITORY_FORMAT_UPGRADE_PATH',
     ]
@@ -222,6 +224,36 @@
     BZR_METADIR_1 = _format_enum(1, BzrDirMetaFormat1)

+# A tuple of branch formats that should not suggest upgrading.
+CURRENT_BRANCH_FORMATS = (
+ BranchFormat.UNRECOGNIZED,
+ BranchFormat.BRANCH_REFERENCE,
+ BranchFormat.BZR_BRANCH_8,
+ BranchFormat.BZR_LOOM_1,
+ BranchFormat.BZR_LOOM_2,
+ BranchFormat.BZR_LOOM_3,
+)
+
+# A tuple of repository formats that should not suggest upgrading.
+CURRENT_REPOSITORY_FORMATS = (
+ RepositoryFormat.UNRECOGNIZED,
+ RepositoryFormat.BZR_KNITPACK_3,
+ RepositoryFormat.BZR_KNITPACK_5,
+ RepositoryFormat.BZR_KNITPACK_5_RR,
+ RepositoryFormat.BZR_KNITPACK_6,
+ RepositoryFormat.BZR_KNITPACK_6_RR,
+ RepositoryFormat.BZR_PACK_DEV_0,
+ RepositoryFormat.BZR_PACK_DEV_0_SUBTREE,
+ RepositoryFormat.BZR_DEV_1,
+ RepositoryFormat.BZR_DEV_1_SUBTREE,
+ RepositoryFormat.BZR_DEV_2,
+ RepositoryFormat.BZR_DEV_2_SUBTREE,
+ RepositoryFormat.BZR_CHK1,
+ RepositoryFormat.BZR_CHK2,
+ RepositoryFormat.BZR_CHK_2A
+
+)
+
 BRANCH_FORMAT_UPGRADE_PATH = {
     BranchFormat.UNRECOGNIZED: None,
     BranchFormat.BRANCH_REFERENCE: None,

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2010-01-06 14:14:35 +0000
+++ lib/lp/code/configure.zcml 2010-01-18 01:19:38 +0000
@@ -487,6 +487,7 @@
                     pending_writes
                     commitsForDays
                     needs_upgrading
+ upgrade_pending
                     requestUpgrade
                     getUpgradeFormat
                     isBranchMergeable

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2010-01-06 12:15:42 +0000
+++ lib/lp/code/interfaces/branch.py 2010-01-18 01:19:39 +0000
@@ -1057,6 +1057,8 @@
         """

     needs_upgrading = Attribute("Whether the branch needs to be upgraded.")
+ upgrade_pending = Attribute(
+ "Whether a branch has had an upgrade requested.")

     def requestUpgrade():
         """Create an IBranchUpgradeJob to upgrade this branch."""

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2009-12-11 00:56:16 +0000
+++ lib/lp/code/model/branch.py 2010-01-19 21:57:57 +0000
@@ -42,7 +42,8 @@
     IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)

 from lp.code.bzr import (
- BranchFormat, BRANCH_FORMAT_UPGRADE_PATH, ControlFormat, RepositoryFormat,
+ BranchFormat, BRANCH_FORMAT_UPGRADE_PATH, ControlFormat,
+ CURRENT_BRANCH_FORMATS, CURRENT_REPOSITORY_FORMATS, RepositoryFormat,
     REPOSITORY_FORMAT_UPGRADE_PATH)
 from lp.code.enums import (
     BranchLifecycleStatus, BranchMergeControlStatus,
@@ -991,10 +992,22 @@
     @property
  ...

[Edwin Grubbs (edwin-grubbs)]

Hi Paul,

This is a nice improvement. Please change the zcml to require launchpad.Edit for requestUpgrade(). I just have one comment below.

merge-conditional

-Edwin

> === modified file 'lib/lp/code/model/tests/test_branch.py'
> --- lib/lp/code/model/tests/test_branch.py 2010-01-07 06:37:14 +0000
> +++ lib/lp/code/model/tests/test_branch.py 2010-01-19 22:00:02 +0000
> @@ -315,24 +315,33 @@
> SourcePackage(branch.sourcepackagename, branch.distroseries),
> branch.sourcepackage)
>
> + def test_needsUpgrading_already_requested(self):
> + # A branch has a needs_upgrading attribute that returns whether or
> not
> + # a branch needs to be upgraded or not. If the format is
> + # unrecognized, we don't try to upgrade it.
> + branch = self.factory.makePersonalBranch(
> + branch_format=BranchFormat.BZR_BRANCH_6,
> + repository_format=RepositoryFormat.BZR_CHK_2A)
> + branch.requestUpgrade()
> +
> + self.assertFalse(branch.needs_upgrading)
> +
> def test_needsUpgrading_branch_format_unrecognized(self):
> # A branch has a needs_upgrading attribute that returns whether or
> not
> # a branch needs to be upgraded or not. If the format is
> # unrecognized, we don't try to upgrade it.
> branch = self.factory.makePersonalBranch(
> - branch_format=BranchFormat.UNRECOGNIZED)
> + branch_format=BranchFormat.UNRECOGNIZED,
> + repository_format=RepositoryFormat.BZR_CHK_2A)
> self.assertFalse(branch.needs_upgrading)
>
> def test_needsUpgrading_branch_format_upgrade_not_needed(self):
> # A branch has a needs_upgrading attribute that returns whether or
> not
> # a branch needs to be upgraded or not. If a branch is up to date,
> it

s/up to date/up-to-date/

review: Approve (code)

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2010-01-17 00:59:01 +0000
+++ lib/lp/code/browser/branch.py	2010-01-20 00:20:29 +0000
@@ -20,6 +20,7 @@
     'BranchEditMenu',
     'BranchInProductView',
     'BranchSparkView',
+    'BranchUpgradeView',
     'BranchURL',
     'BranchView',
     'BranchSubscriptionsView',
@@ -231,7 +232,7 @@
     links = [
         'add_subscriber', 'browse_revisions', 'link_bug',
         'link_blueprint', 'register_merge', 'source', 'subscription',
-        'edit_status']
+        'edit_status', 'upgrade_branch']
 
     @enabled_with_permission('launchpad.Edit')
     def edit_status(self):
@@ -298,6 +299,14 @@
         url = self.context.codebrowse_url('files')
         return Link(url, text, icon='info', enabled=enabled)
 
+    @enabled_with_permission('launchpad.Edit')
+    def upgrade_branch(self):
+        enabled = False
+        if self.context.needs_upgrading:
+            enabled = True
+        return Link(
+            '+upgrade', 'Upgrade this branch', icon='edit', enabled=enabled)
+
 
 class DecoratedBug:
     """Provide some additional attributes to a normal bug."""
@@ -877,6 +886,27 @@
         return canonical_url(self.context)
 
 
+class BranchUpgradeView(LaunchpadFormView):
+    """Used to upgrade a branch."""
+
+    schema = IBranch
+    field_names = []
+
+    @property
+    def page_title(self):
+        return smartquote('Upgrade branch "%s"' % self.context.displayname)
+
+    @property
+    def next_url(self):
+        return canonical_url(self.context)
+
+    cancel_url = next_url
+
+    @action('Upgrade', name='upgrade_branch')
+    def upgrade_branch_action(self, action, data):
+        self.context.requestUpgrade()
+
+
 class BranchEditView(BranchEditFormView, BranchNameValidationMixin):
     """The main branch view for editing the branch attributes."""
 

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2010-01-17 10:15:48 +0000
+++ lib/lp/code/browser/configure.zcml	2010-01-20 00:20:29 +0000
@@ -474,6 +474,13 @@
         permission="launchpad.Edit"
         template="../../app/templates/generic-edit.pt"/>
     <browser:page
+        name="+upgrade"
+        for="lp.code.interfaces.branch.IBranch"
+        class="lp.code.browser.branch.BranchUpgradeView"
+        facet="branches"
+        permission="launchpad.Edit"
+        template="../../app/templates/generic-edit.pt"/>
+    <browser:page
         name="+reviewer"
         for="lp.code.interfaces.branch.IBranch"
         class="lp.code.browser.branch.BranchReviewerEditView"

=== modified file 'lib/lp/code/bzr.py'
--- lib/lp/code/bzr.py	2009-06-30 16:56:07 +0000
+++ lib/lp/code/bzr.py	2010-01-20 00:20:29 +0000
@@ -8,6 +8,8 @@
     'BranchFormat',
     'BRANCH_FORMAT_UPGRADE_PATH',
     'ControlFormat',
+    'CURRENT_BRANCH_FORMATS',
+    'CURRENT_REPOSITORY_FORMATS',
     'RepositoryFormat',
     'REPOSITORY_FORMAT_UPGRADE_PATH',
     ]
@@ -222,6 +224,36 @@
     BZR_METADIR_1 = _format_enum(1, BzrDirMetaFormat1)
 
 
+# A tuple of branch formats that should not suggest upgrading.
+CURRENT_BRANCH_FORMATS = (
+    BranchFormat.UNRECOGNIZED,
+    BranchFormat.BRANCH_REFERENCE,
+    BranchFormat.BZR_BRANCH_8,
+    BranchFormat.BZR_LOOM_1,
+    BranchFormat.BZR_LOOM_2,
+    BranchFormat.BZR_LOOM_3,
+)
+
+# A tuple of repository formats that should not suggest upgrading.
+CURRENT_REPOSITORY_FORMATS = (
+    RepositoryFormat.UNRECOGNIZED,
+    RepositoryFormat.BZR_KNITPACK_3,
+    RepositoryFormat.BZR_KNITPACK_5,
+    RepositoryFormat.BZR_KNITPACK_5_RR,
+    RepositoryFormat.BZR_KNITPACK_6,
+    RepositoryFormat.BZR_KNITPACK_6_RR,
+    RepositoryFormat.BZR_PACK_DEV_0,
+    RepositoryFormat.BZR_PACK_DEV_0_SUBTREE,
+    RepositoryFormat.BZR_DEV_1,
+    RepositoryFormat.BZR_DEV_1_SUBTREE,
+    RepositoryFormat.BZR_DEV_2,
+    RepositoryFormat.BZR_DEV_2_SUBTREE,
+    RepositoryFormat.BZR_CHK1,
+    RepositoryFormat.BZR_CHK2,
+    RepositoryFormat.BZR_CHK_2A
+
+)
+
 BRANCH_FORMAT_UPGRADE_PATH = {
     BranchFormat.UNRECOGNIZED: None,
     BranchFormat.BRANCH_REFERENCE: None,

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-01-16 09:57:33 +0000
+++ lib/lp/code/configure.zcml	2010-01-20 00:20:29 +0000
@@ -487,6 +487,7 @@
                     pending_writes
                     commitsForDays
                     needs_upgrading
+                    upgrade_pending
                     requestUpgrade
                     getUpgradeFormat
                     isBranchMergeable

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2010-01-15 03:22:12 +0000
+++ lib/lp/code/interfaces/branch.py	2010-01-20 00:20:29 +0000
@@ -1068,6 +1068,8 @@
         """
 
     needs_upgrading = Attribute("Whether the branch needs to be upgraded.")
+    upgrade_pending = Attribute(
+        "Whether a branch has had an upgrade requested.")
 
     def requestUpgrade():
         """Create an IBranchUpgradeJob to upgrade this branch."""

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-01-15 03:22:12 +0000
+++ lib/lp/code/model/branch.py	2010-01-20 00:20:29 +0000
@@ -42,7 +42,8 @@
     IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)
 
 from lp.code.bzr import (
-    BranchFormat, BRANCH_FORMAT_UPGRADE_PATH, ControlFormat, RepositoryFormat,
+    BranchFormat, BRANCH_FORMAT_UPGRADE_PATH, ControlFormat,
+    CURRENT_BRANCH_FORMATS, CURRENT_REPOSITORY_FORMATS, RepositoryFormat,
     REPOSITORY_FORMAT_UPGRADE_PATH)
 from lp.code.enums import (
     BranchLifecycleStatus, BranchMergeControlStatus,
@@ -995,10 +996,22 @@
     @property
     def needs_upgrading(self):
         """See `IBranch`."""
-        if (REPOSITORY_FORMAT_UPGRADE_PATH.get(self.repository_format, None)
-            or BRANCH_FORMAT_UPGRADE_PATH.get(self.branch_format, None)):
-            return True
-        return False
+        if self.upgrade_pending:
+            return False
+        return not (
+            self.branch_format in CURRENT_BRANCH_FORMATS and
+            self.repository_format in CURRENT_REPOSITORY_FORMATS)
+
+    @property
+    def upgrade_pending(self):
+        """See `IBranch`."""
+        from lp.code.model.branchjob import BranchJob, BranchJobType
+        store = Store.of(self)
+        jobs = store.find(
+            BranchJob,
+            BranchJob.branch == self,
+            BranchJob.job_type == BranchJobType.UPGRADE_BRANCH)
+        return jobs.count() > 0
 
     def requestUpgrade(self):
         """See `IBranch`."""

=== modified file 'lib/lp/code/model/branchjob.py'
--- lib/lp/code/model/branchjob.py	2010-01-15 03:32:46 +0000
+++ lib/lp/code/model/branchjob.py	2010-01-20 00:20:29 +0000
@@ -264,6 +264,8 @@
         """See `IBranchUpgradeJobSource`."""
         if not branch.needs_upgrading:
             raise AssertionError('Branch does not need upgrading.')
+        if branch.upgrade_pending:
+            raise AssertionError('Branch already has upgrade pending.')
         branch_job = BranchJob(branch, BranchJobType.UPGRADE_BRANCH, {})
         return cls(branch_job)
 

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2010-01-07 06:37:14 +0000
+++ lib/lp/code/model/tests/test_branch.py	2010-01-20 00:20:29 +0000
@@ -315,24 +315,33 @@
             SourcePackage(branch.sourcepackagename, branch.distroseries),
             branch.sourcepackage)
 
+    def test_needsUpgrading_already_requested(self):
+        # A branch has a needs_upgrading attribute that returns whether or not
+        # a branch needs to be upgraded or not.  If the format is
+        # unrecognized, we don't try to upgrade it.
+        branch = self.factory.makePersonalBranch(
+            branch_format=BranchFormat.BZR_BRANCH_6,
+            repository_format=RepositoryFormat.BZR_CHK_2A)
+        branch.requestUpgrade()
+
+        self.assertFalse(branch.needs_upgrading)
+
     def test_needsUpgrading_branch_format_unrecognized(self):
         # A branch has a needs_upgrading attribute that returns whether or not
         # a branch needs to be upgraded or not.  If the format is
         # unrecognized, we don't try to upgrade it.
         branch = self.factory.makePersonalBranch(
-            branch_format=BranchFormat.UNRECOGNIZED)
+            branch_format=BranchFormat.UNRECOGNIZED,
+            repository_format=RepositoryFormat.BZR_CHK_2A)
         self.assertFalse(branch.needs_upgrading)
 
     def test_needsUpgrading_branch_format_upgrade_not_needed(self):
         # A branch has a needs_upgrading attribute that returns whether or not
         # a branch needs to be upgraded or not.  If a branch is up to date, it
         # doesn't need to be upgraded.
-        #
-        # XXX: JonathanLange 2009-06-06: This test needs to be changed every
-        # time Bazaar adds a new branch format. Surely we can think of a
-        # better way of testing this?
         branch = self.factory.makePersonalBranch(
-            branch_format=BranchFormat.BZR_BRANCH_8)
+            branch_format=BranchFormat.BZR_BRANCH_8,
+            repository_format=RepositoryFormat.BZR_CHK_2A)
         self.assertFalse(branch.needs_upgrading)
 
     def test_needsUpgrading_branch_format_upgrade_needed(self):
@@ -340,7 +349,8 @@
         # a branch needs to be upgraded or not.  If a branch doesn't support
         # stacking, it needs to be upgraded.
         branch = self.factory.makePersonalBranch(
-            branch_format=BranchFormat.BZR_BRANCH_6)
+            branch_format=BranchFormat.BZR_BRANCH_6,
+            repository_format=RepositoryFormat.BZR_CHK_2A)
         self.assertTrue(branch.needs_upgrading)
 
     def test_needsUpgrading_repository_format_unrecognized(self):
@@ -348,6 +358,7 @@
         # a branch needs to be upgraded or not.  In the repo format is
         # unrecognized, we don't try to upgrade it.
         branch = self.factory.makePersonalBranch(
+            branch_format=BranchFormat.BZR_BRANCH_8,
             repository_format=RepositoryFormat.UNRECOGNIZED)
         self.assertFalse(branch.needs_upgrading)
 
@@ -356,6 +367,7 @@
         # branch needs to be upgraded or not.  If the repo format is up to
         # date, there's no need to upgrade it.
         branch = self.factory.makePersonalBranch(
+            branch_format=BranchFormat.BZR_BRANCH_8,
             repository_format=RepositoryFormat.BZR_KNITPACK_6)
         self.assertFalse(branch.needs_upgrading)
 
@@ -364,6 +376,7 @@
         # branch needs to be upgraded or not.  If the format doesn't support
         # stacking, it needs to be upgraded.
         branch = self.factory.makePersonalBranch(
+            branch_format=BranchFormat.BZR_BRANCH_8,
             repository_format=RepositoryFormat.BZR_REPOSITORY_4)
         self.assertTrue(branch.needs_upgrading)
 
@@ -378,6 +391,37 @@
             jobs,
             [job,])
 
+    def test_requestUpgrade_no_upgrade_needed(self):
+        # If a branch doesn't need to be upgraded, requestUpgrade raises an
+        # AssertionError.
+        branch = self.factory.makeAnyBranch(
+            branch_format=BranchFormat.BZR_BRANCH_8,
+            repository_format=RepositoryFormat.BZR_CHK_2A)
+        self.assertRaises(AssertionError, branch.requestUpgrade)
+
+    def test_requestUpgrade_upgrade_pending(self):
+        # If there is a pending upgrade already requested, requestUpgrade
+        # raises an AssertionError.
+        branch = self.factory.makeAnyBranch(
+            branch_format=BranchFormat.BZR_BRANCH_6)
+        branch.requestUpgrade()
+
+        self.assertRaises(AssertionError, branch.requestUpgrade)
+
+    def test_upgradePending(self):
+        # If there is a BranchUpgradeJob pending for the branch, return True.
+        branch = self.factory.makeAnyBranch(
+            branch_format=BranchFormat.BZR_BRANCH_6)
+        branch.requestUpgrade()
+
+        self.assertTrue(branch.upgrade_pending)
+
+    def test_upgradePending_no_upgrade_requested(self):
+        # If the branch never had an upgrade requested, return False.
+        branch = self.factory.makeAnyBranch()
+
+        self.assertFalse(branch.upgrade_pending)
+
 
 class TestBzrIdentity(TestCaseWithFactory):
     """Test IBranch.bzr_identity."""

=== added file 'lib/lp/code/stories/branches/xx-upgrading-branches.txt'
--- lib/lp/code/stories/branches/xx-upgrading-branches.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/stories/branches/xx-upgrading-branches.txt	2010-01-20 00:20:29 +0000
@@ -0,0 +1,49 @@
+==================
+Upgrading Branches
+==================
+
+Launchpad can upgrade branches that were in an older format to a more up to
+date format.
+
+
+Creating the branch
+===================
+
+Branches are not available to be upgraded if they are in a more up to date
+format.
+
+    >>> from lp.code.bzr import BranchFormat, RepositoryFormat
+    >>> login('[e-mail]')
+    >>> domino = factory.makePerson(
+    ...     name='domino', email="[e-mail]", password="test")
+    >>> twist = factory.makeAnyBranch(
+    ...     branch_format=BranchFormat.BZR_BRANCH_6,
+    ...     repository_format=RepositoryFormat.BZR_CHK_2A,
+    ...     owner=domino)
+    >>> branch_url = canonical_url(twist)
+    >>> logout()
+
+
+Requesting an upgrade
+=====================
+
+Only those with edit permissions on a branch can request an upgrade.
+
+    >>> nopriv_browser = setupBrowser(
+    ...     auth='Basic [e-mail]:test')
+    >>> nopriv_browser.open(branch_url)
+    >>> link = nopriv_browser.getLink('Upgrade this branch')
+    Traceback (most recent call last):
+    LinkNotFoundError
+
+    >>> domino_browser = setupBrowser(
+    ...     auth='Basic [e-mail]:test')
+    >>> domino_browser.open(branch_url)
+    >>> domino_browser.getLink("Upgrade this branch").click()
+    >>> print domino_browser.url
+    http://code.launchpad.dev/~domino/.../+upgrade
+    >>> domino_browser.getControl('Upgrade').click()
+
+    >>> for msg in get_feedback_messages(domino_browser.contents):
+    ...     print msg
+    An upgrade of this branch is in progress.

=== modified file 'lib/lp/code/templates/branch-messages.pt'
--- lib/lp/code/templates/branch-messages.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/code/templates/branch-messages.pt	2010-01-20 00:20:29 +0000
@@ -115,6 +115,12 @@
          tal:condition="branch/mirror_status_message"
          tal:content="branch/mirror_status_message">
     </div>
+    <div id="upgrade-pending"
+         class="warning message" style="font-weight: normal"
+         tal:condition="branch/upgrade_pending">
+         An upgrade of this branch is in progress.
+    </div>
+
   </tal:hosted-branch>
 
 </div>

=== modified file 'lib/lp/code/templates/branch-metadata.pt'
--- lib/lp/code/templates/branch-metadata.pt	2009-09-08 21:26:03 +0000
+++ lib/lp/code/templates/branch-metadata.pt	2010-01-20 00:20:29 +0000
@@ -1,7 +1,8 @@
 <div
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n">
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  tal:define="context_menu context/menu:context">
 
   <div class="two-column-list">
     <dl id="branch-format" tal:condition="context/branch_format">
@@ -18,5 +19,13 @@
       <dd tal:content="structure context/stacked_on/fmt:bzr-link" />
     </dl>
   </div>
+  <div class="actions">
+    <div
+      tal:define="link context_menu/upgrade_branch"
+      tal:condition="link/enabled"
+      >
+      <a tal:replace="structure link/fmt:link" />
+    </div>
+  </div>
 
 </div>