launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/auto-upgrade into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #432217 in Launchpad itself: "Import branches have auto-format upgrade disabled"
  https://bugs.launchpad.net/launchpad/+bug/432217

For more details, see:
https://code.launchpad.net/~jelmer/launchpad/auto-upgrade/+merge/64398

Reintroduce automatic upgrades of code imports.

Automatic upgrades of code imports were disabled earlier because they
were much too slow. Upgrades were disabled in September 2009 because
the upgrade from knits to 2a was very slow and memory hogging.

Bazaar is now much faster at doing upgrades. The upgrade is also
no longer done on the remote branch. Instead, the worker upgrades
its local branch and then replaces the remote branch with that.
-- 
https://code.launchpad.net/~jelmer/launchpad/auto-upgrade/+merge/64398
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/auto-upgrade into lp:launchpad.

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2011-05-27 21:12:25 +0000
+++ lib/lp/code/browser/branch.py	2011-06-13 12:40:06 +0000
@@ -118,7 +118,6 @@
 from lp.bugs.interfaces.bug import IBugSet
 from lp.bugs.interfaces.bugbranch import IBugBranch
 from lp.bugs.interfaces.bugtask import UNRESOLVED_BUGTASK_STATUSES
-from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSet
 from lp.code.browser.branchmergeproposal import (
     latest_proposals_for_each_branch,
     )
@@ -126,7 +125,6 @@
 from lp.code.browser.decorations import DecoratedBranch
 from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
 from lp.code.enums import (
-    BranchLifecycleStatus,
     BranchType,
     CodeImportResultStatus,
     CodeImportReviewStatus,

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2011-06-01 15:52:13 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2011-06-13 12:40:06 +0000
@@ -29,7 +29,6 @@
 from bzrlib.tests import TestCaseWithTransport
 from bzrlib import trace
 from bzrlib.transport import get_transport
-from bzrlib.upgrade import upgrade
 from bzrlib.urlutils import (
     join as urljoin,
     local_path_from_url,
@@ -204,51 +203,52 @@
             self.arbitrary_branch_id, self.temp_dir, default_format, True)
         self.assertTrue(new_branch.bzrdir.has_workingtree())
 
-    # XXX Tim Penhey 2009-09-18 bug 432217 Automatic upgrade of import
-    # branches disabled.  Need an orderly upgrade process.
-    def disabled_test_pullUpgradesFormat(self):
-        # A branch should always be in the most up-to-date format before a
-        # pull is performed.
-        store = self.makeBranchStore()
-        target_url = store._getMirrorURL(self.arbitrary_branch_id)
-        knit_format = format_registry.get('knit')()
-        create_branch_with_one_revision(target_url, format=knit_format)
-        default_format = BzrDirFormat.get_default_format()
-
-        # The fetched branch is in the default format.
-        new_tree = store.pull(
-            self.arbitrary_branch_id, self.temp_dir, default_format)
-        self.assertEqual(
-            default_format, new_tree.branch.bzrdir._format)
-
-        # In addition. the remote branch has been upgraded as well.
-        new_branch = Branch.open(target_url)
-        self.assertEqual(
-            default_format.get_branch_format(), new_branch._format)
-
-    # XXX Tim Penhey 2009-09-18 bug 432217 Automatic upgrade of import
-    # branches disabled.  Need an orderly upgrade process.
-    def disabled_test_pullUpgradesFormatWithBackupDirPresent(self):
-        # pull can upgrade the remote branch even if there is a backup.bzr
-        # directory from a previous upgrade.
-        store = self.makeBranchStore()
-        target_url = store._getMirrorURL(self.arbitrary_branch_id)
-        knit_format = format_registry.get('knit')()
-        create_branch_with_one_revision(target_url, format=knit_format)
-        upgrade(target_url, format_registry.get('dirstate-tags')())
-        self.failUnless(get_transport(target_url).has('backup.bzr'))
-        default_format = BzrDirFormat.get_default_format()
-
-        # The fetched branch is in the default format.
-        new_tree = store.pull(
-            self.arbitrary_branch_id, self.temp_dir, default_format)
-        self.assertEqual(
-            default_format, new_tree.branch.bzrdir._format)
-
-        # In addition. the remote branch has been upgraded as well.
-        new_branch = Branch.open(target_url)
-        self.assertEqual(
-            default_format.get_branch_format(), new_branch._format)
+    def test_pullUpgradesFormat(self):
+        # A branch should always be in the most up-to-date format before a
+        # pull is performed.
+        store = self.makeBranchStore()
+        target_url = store._getMirrorURL(self.arbitrary_branch_id)
+        knit_format = format_registry.get('knit')()
+        create_branch_with_one_revision(target_url, format=knit_format)
+        default_format = BzrDirFormat.get_default_format()
+
+        # The fetched branch is in the default format.
+        new_branch = store.pull(
+            self.arbitrary_branch_id, self.temp_dir, default_format)
+        self.assertEqual(
+            default_format, new_branch.bzrdir._format)
+
+    def test_pushUpgradesFormat(self):
+        # A branch should always be in the most up-to-date format before a
+        # pull is performed.
+        store = self.makeBranchStore()
+        target_url = store._getMirrorURL(self.arbitrary_branch_id)
+        knit_format = format_registry.get('knit')()
+        create_branch_with_one_revision(target_url, format=knit_format)
+        default_format = BzrDirFormat.get_default_format()
+
+        # The fetched branch is in the default format.
+        new_branch = store.pull(
+            self.arbitrary_branch_id, self.temp_dir, default_format)
+        self.assertEqual(
+            default_format, new_branch.bzrdir._format)
+
+        # The remote branch is still in the old format at this point.
+        target_branch = Branch.open(target_url)
+        self.assertEqual(
+            knit_format.get_branch_format(),
+            target_branch._format)
+
+        store.push(self.arbitrary_branch_id, new_branch, default_format)
+
+        # The remote branch is now in the new format.
+        target_branch = Branch.open(target_url)
+        self.assertEqual(
+            default_format.get_branch_format(),
+            target_branch._format)
+        self.assertEquals(
+            target_branch.last_revision_info(),
+            new_branch.last_revision_info())
 
     def test_pushTwiceThenPull(self):
         # We can push up a branch to the store twice and then pull it from the

=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py	2011-06-01 15:59:45 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2011-06-13 12:40:06 +0000
@@ -95,15 +95,6 @@
             if needs_tree:
                 local_branch.bzrdir.create_workingtree()
             return local_branch
-        # XXX Tim Penhey 2009-09-18 bug 432217 Automatic upgrade of import
-        # branches disabled.  Need an orderly upgrade process.
-        if False and remote_bzr_dir.needs_format_conversion(
-            format=required_format):
-            try:
-                remote_bzr_dir.root_transport.delete_tree('backup.bzr')
-            except NoSuchFile:
-                pass
-            upgrade(remote_url, required_format)
         # The proper thing to do here would be to call
         # "remote_bzr_dir.sprout()".  But 2a fetch slowly checks which
         # revisions are in the ancestry of the tip of the remote branch, which
@@ -116,6 +107,13 @@
         target_control.create_prefix()
         remote_bzr_dir.transport.copy_tree_to_transport(target_control)
         local_bzr_dir = BzrDir.open_from_transport(target)
+        if local_bzr_dir.needs_format_conversion(
+            format=required_format):
+            try:
+                local_bzr_dir.root_transport.delete_tree('backup.bzr')
+            except NoSuchFile:
+                pass
+            upgrade(target_path, required_format)
         if needs_tree:
             local_bzr_dir.create_workingtree()
         return local_bzr_dir.open_branch()
@@ -133,11 +131,36 @@
         except NotBranchError:
             remote_branch = BzrDir.create_branch_and_repo(
                 target_url, format=required_format)
+            old_branch = None
+        else:
+            if remote_branch.bzrdir.needs_format_conversion(
+                    required_format):
+                # For upgrades, push to a new branch in
+                # the new format. When done pushing,
+                # retire the old .bzr directory and rename
+                # the new one in place.
+                old_branch = remote_branch
+                upgrade_url = urljoin(target_url, "upgrade.bzr")
+                try:
+                    remote_branch.bzrdir.root_transport.delete_tree(
+                        'upgrade.bzr')
+                except NoSuchFile:
+                    pass
+                remote_branch = BzrDir.create_branch_and_repo(
+                    upgrade_url, format=required_format)
+            else:
+                old_branch = None
         pull_result = remote_branch.pull(bzr_branch, overwrite=True)
         # Because of the way we do incremental imports, there may be revisions
         # in the branch's repo that are not in the ancestry of the branch tip.
         # We need to transfer them too.
         remote_branch.repository.fetch(bzr_branch.repository)
+        if old_branch is not None:
+            # The format has changed; move the new format
+            # branch in place.
+            base_transport = old_branch.bzrdir.root_transport
+            old_branch.bzrdir.retire_bzrdir()
+            base_transport.rename("upgrade.bzr/.bzr", ".bzr")
         return pull_result.old_revid != pull_result.new_revid