launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #798560 in Launchpad itself: "Code import upgrades create directories that the puller doesn't accept"
  https://bugs.launchpad.net/launchpad/+bug/798560

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

This fixes a regression that I introduced when we started upgrading code import branches again.

The scanner will delete any branches that contains directories it doesn't approve of; it doesn't just delete the directories it doesn't approve of but the entire branch. This meant that any branch that was upgraded would automatically be removed because we retired the existing bzr directory by renaming it in a way that the scanner didn't know about.

This change fixes the upgrade process to only use names approved by the scanner, and removes the backup directory after the process is complete.
-- 
https://code.launchpad.net/~jelmer/launchpad/auto-upgrade-fix/+merge/65196
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/auto-upgrade-fix into lp:launchpad.

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2011-06-17 09:56:57 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2011-06-20 13:51:22 +0000
@@ -204,6 +204,7 @@
             self.arbitrary_branch_id, self.temp_dir, default_format, True)
         self.assertTrue(new_branch.bzrdir.has_workingtree())
 
+<<<<<<< TREE
     # XXX Tim Penhey 2009-09-18 bug 432217 Automatic upgrade of import
     # branches disabled.  Need an orderly upgrade process.
     def disabled_test_pullUpgradesFormat(self):
@@ -249,6 +250,62 @@
         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)
+        # Make sure backup.bzr is removed, as it interferes with CSCVS.
+        self.assertEquals(os.listdir(self.temp_dir), [".bzr"])
+        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)
+        # Only .bzr is left behind. The scanner removes branches
+        # in which invalid directories (such as .bzr.retire.
+        # exist). (bug #798560)
+        self.assertEquals(
+            target_branch.user_transport.list_dir("."),
+            [".bzr"])
+        self.assertEqual(
+            default_format.get_branch_format(),
+            target_branch._format)
+        self.assertEquals(
+            target_branch.last_revision_info(),
+            new_branch.last_revision_info())
+>>>>>>> MERGE-SOURCE
 
     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-17 09:56:57 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2011-06-20 13:51:22 +0000
@@ -133,11 +133,43 @@
         except NotBranchError:
             remote_branch = BzrDir.create_branch_and_repo(
                 target_url, format=required_format)
+<<<<<<< TREE
+=======
+            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, "backup.bzr")
+                try:
+                    remote_branch.bzrdir.root_transport.delete_tree(
+                        'backup.bzr')
+                except NoSuchFile:
+                    pass
+                remote_branch = BzrDir.create_branch_and_repo(
+                    upgrade_url, format=required_format)
+            else:
+                old_branch = None
+>>>>>>> MERGE-SOURCE
         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)
+<<<<<<< TREE
+=======
+        if old_branch is not None:
+            # The format has changed; move the new format
+            # branch in place.
+            base_transport = old_branch.bzrdir.root_transport
+            base_transport.delete_tree('.bzr')
+            base_transport.rename("backup.bzr/.bzr", ".bzr")
+            base_transport.rmdir("backup.bzr")
+>>>>>>> MERGE-SOURCE
         return pull_result.old_revid != pull_result.new_revid