launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #808035 in Launchpad itself: "AttributeError: 'NoneType' object has no attribute 'write' during git import"
  https://bugs.launchpad.net/launchpad/+bug/808035

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

Fix processing of code imports that are in an old bzr format:

 1) Implement show_user_warning in the custom LoggingUIFactory used in the code import
    workers. This fixes the immediate issue that causes the traceback,
    and will prevent issues in the future if parts of bazaar print user warnings.

 2) Reintroduce the automatic upgrading of code imports, and fix the related test.
    Previously the test would compare the formats of the bzr dirs, both of which
    would be "Bazaar-NG meta directory, format 1". The test now compares the
    repository formats, which are different.
-- 
https://code.launchpad.net/~jelmer/launchpad/upgrade-stderr/+merge/68579
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/upgrade-stderr into lp:launchpad.

=== modified file 'lib/lp/codehosting/codeimport/tests/test_uifactory.py'
--- lib/lp/codehosting/codeimport/tests/test_uifactory.py	2010-08-20 20:31:18 +0000
+++ lib/lp/codehosting/codeimport/tests/test_uifactory.py	2011-07-20 17:30:27 +0000
@@ -98,6 +98,16 @@
              'hi 3/10'],
             self.messages)
 
+    def test_user_warning(self):
+        factory = self.makeLoggingUIFactory()
+        factory.show_user_warning('cross_format_fetch',
+            from_format="athing", to_format="anotherthing")
+        message = factory._user_warning_templates['cross_format_fetch'] % {
+            "from_format": "athing",
+            "to_format": "anotherthing",
+            }
+        self.assertEqual([message], self.messages)
+
 
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2011-06-21 05:38:15 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2011-07-20 17:30:27 +0000
@@ -24,12 +24,10 @@
     )
 from bzrlib.errors import (
     NoSuchFile,
-    NotBranchError,
     )
 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,
@@ -210,16 +208,17 @@
         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()
+        tree = create_branch_with_one_revision(target_url, format=knit_format)
+        self.assertNotEquals(tree.bzrdir._format.repository_format.network_name(),
+            default_format.repository_format.network_name())
 
         # 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)
+        self.assertEquals(new_branch.repository._format.network_name(),
+            default_format.repository_format.network_name())
 
     def test_pushUpgradesFormat(self):
         # A branch should always be in the most up-to-date format before a
@@ -228,7 +227,6 @@
         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(

=== modified file 'lib/lp/codehosting/codeimport/uifactory.py'
--- lib/lp/codehosting/codeimport/uifactory.py	2010-08-20 20:31:18 +0000
+++ lib/lp/codehosting/codeimport/uifactory.py	2011-07-20 17:30:27 +0000
@@ -36,9 +36,13 @@
         """
         TextUIFactory.__init__(self)
         self.interval = interval
+        self.writer = writer
         self._progress_view = LoggingTextProgressView(
             time_source, writer, interval)
 
+    def show_user_warning(self, warning_id, **message_args):
+        self.writer(self.format_user_warning(warning_id, message_args))
+
 
 class LoggingTextProgressView(TextProgressView):
     """Support class for `LoggingUIFactory`. """

=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py	2011-06-21 05:38:15 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2011-07-20 17:30:27 +0000
@@ -100,15 +100,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
@@ -121,6 +112,12 @@
         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, clean_up=True)
         if needs_tree:
             local_bzr_dir.create_workingtree()
         return local_bzr_dir.open_branch()