Rev 4839: (andrew) Fix AssertionError from _remember_remote_is_before when in file:///home/pqm/archives/thelove/bzr/2.1/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/2.1/

------------------------------------------------------------
revno: 4839 [merge]
revision-id: pqm at pqm.ubuntu.com-20100430085048-eh08ih9dborzlvk2
parent: pqm at pqm.ubuntu.com-20100430074110-r9ffhuboqeevhid9
parent: andrew.bennetts at canonical.com-20100430081334-zgln2xmnpazh4hl2
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.1
timestamp: Fri 2010-04-30 09:50:48 +0100
message:
  (andrew) Fix AssertionError from _remember_remote_is_before when
  	accessing smart servers running Bazaar < 1.6. (#528041)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/smart/medium.py         medium.py-20061103051856-rgu2huy59fkz902q-1
  bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
=== modified file 'NEWS'
--- a/NEWS	2010-04-30 06:27:15 +0000
+++ b/NEWS	2010-04-30 08:13:34 +0000
@@ -16,12 +16,26 @@
 * ``bzr switch`` does not die if a ConfigurableFileMerger is used.
   (Aaron Bentley, #559436)
 
+* Fixed ``AssertionError`` when accessing smart servers running Bazaar
+  versions before 1.6.
+  (Andrew Bennetts, #528041)
+
 * Reset ``siginterrupt`` flag to False every time we handle a signal
   installed with ``set_signal_handler(..., restart_syscall=True)`` (from
   ``bzrlib.osutils``.  Reduces the likelihood of "Interrupted System Call"
   errors after two window resizes.
   (Andrew Bennetts)
 
+Internals
+*********
+
+* ``_remember_remote_is_before`` no longer raises AssertionError when
+  suboptimal network behaviour is noticed; instead it just mutters to the
+  log file (and warns the user if they have set the ``hpss`` debug flag).
+  This was causing unnecessary aborts for performance bugs that are minor
+  at worst.
+  (Andrew Bennetts, #528041)
+
 
 bzr 2.1.1
 #########

=== modified file 'bzrlib/smart/medium.py'
--- a/bzrlib/smart/medium.py	2010-02-17 17:11:16 +0000
+++ b/bzrlib/smart/medium.py	2010-04-27 06:40:37 +0000
@@ -609,10 +609,16 @@
             # which is newer than a previously supplied older-than version.
             # This indicates that some smart verb call is not guarded
             # appropriately (it should simply not have been tried).
-            raise AssertionError(
+            trace.mutter(
                 "_remember_remote_is_before(%r) called, but "
                 "_remember_remote_is_before(%r) was called previously."
-                % (version_tuple, self._remote_version_is_before))
+                , version_tuple, self._remote_version_is_before)
+            if 'hpss' in debug.debug_flags:
+                ui.ui_factory.show_warning(
+                    "_remember_remote_is_before(%r) called, but "
+                    "_remember_remote_is_before(%r) was called previously."
+                    % (version_tuple, self._remote_version_is_before))
+            return
         self._remote_version_is_before = version_tuple
 
     def protocol_version(self):

=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py	2010-04-23 05:28:35 +0000
+++ b/bzrlib/tests/test_remote.py	2010-04-27 07:41:03 +0000
@@ -417,9 +417,12 @@
         # Calling _remember_remote_is_before again with a lower value works.
         client_medium._remember_remote_is_before((1, 5))
         self.assertTrue(client_medium._is_remote_before((1, 5)))
-        # You cannot call _remember_remote_is_before with a larger value.
-        self.assertRaises(
-            AssertionError, client_medium._remember_remote_is_before, (1, 9))
+        # If you call _remember_remote_is_before with a higher value it logs a
+        # warning, and continues to remember the lower value.
+        self.assertNotContainsRe(self.get_log(), '_remember_remote_is_before')
+        client_medium._remember_remote_is_before((1, 9))
+        self.assertContainsRe(self.get_log(), '_remember_remote_is_before')
+        self.assertTrue(client_medium._is_remote_before((1, 5)))
 
 
 class TestBzrDirCloningMetaDir(TestRemote):
@@ -527,6 +530,28 @@
         self.assertIsInstance(bd, RemoteBzrDir)
         self.assertFinished(client)
 
+    def test_backwards_compat_hpss_v2(self):
+        client, transport = self.make_fake_client_and_transport()
+        # Monkey-patch fake client to simulate real-world behaviour with v2
+        # server: upon first RPC call detect the protocol version, and because
+        # the version is 2 also do _remember_remote_is_before((1, 6)) before
+        # continuing with the RPC.
+        orig_check_call = client._check_call
+        def check_call(method, args):
+            client._medium._protocol_version = 2
+            client._medium._remember_remote_is_before((1, 6))
+            client._check_call = orig_check_call
+            client._check_call(method, args)
+        client._check_call = check_call
+        client.add_expected_call(
+            'BzrDir.open_2.1', ('quack/',), 'unknown', ('BzrDir.open_2.1',))
+        client.add_expected_call(
+            'BzrDir.open', ('quack/',), 'success', ('yes',))
+        bd = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+            _client=client, _force_probe=True)
+        self.assertIsInstance(bd, RemoteBzrDir)
+        self.assertFinished(client)
+
 
 class TestBzrDirOpenBranch(TestRemote):