Rev 4225: (andrew) Fix regression in get_parent_map RPC client when 'null:' is in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Wed Apr 1 03:19:41 BST 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4225
revision-id: pqm at pqm.ubuntu.com-20090401021937-weq1st849rreuonn
parent: pqm at pqm.ubuntu.com-20090401013450-7a9hdobj3p3f033c
parent: andrew.bennetts at canonical.com-20090331232307-akxl4p4or5bm3q8g
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2009-04-01 03:19:37 +0100
message:
  (andrew) Fix regression in get_parent_map RPC client when 'null:' is
  	cached as missing.
modified:
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
    ------------------------------------------------------------
    revno: 4214.2.5
    revision-id: andrew.bennetts at canonical.com-20090331232307-akxl4p4or5bm3q8g
    parent: andrew.bennetts at canonical.com-20090331094238-t3h643mwppc4wn4g
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: get_parent_map_rpc_ghosts
    timestamp: Wed 2009-04-01 10:23:07 +1100
    message:
      Fix the bug.
    modified:
      bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
      bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
    ------------------------------------------------------------
    revno: 4214.2.4
    revision-id: andrew.bennetts at canonical.com-20090331094238-t3h643mwppc4wn4g
    parent: andrew.bennetts at canonical.com-20090331071145-a3tznkxuf46kcqww
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: get_parent_map_rpc_ghosts
    timestamp: Tue 2009-03-31 20:42:38 +1100
    message:
      Further simplify the test to reproduce the bug.
    modified:
      bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
    ------------------------------------------------------------
    revno: 4214.2.3
    revision-id: andrew.bennetts at canonical.com-20090331071145-a3tznkxuf46kcqww
    parent: andrew.bennetts at canonical.com-20090331065007-kc1f0r42b9eed12z
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: get_parent_map_rpc_ghosts
    timestamp: Tue 2009-03-31 18:11:45 +1100
    message:
      Further simplify test case, and add more comments.
    modified:
      bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
    ------------------------------------------------------------
    revno: 4214.2.2
    revision-id: andrew.bennetts at canonical.com-20090331065007-kc1f0r42b9eed12z
    parent: andrew.bennetts at canonical.com-20090331060557-l80f1ab20qjm2xud
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: get_parent_map_rpc_ghosts
    timestamp: Tue 2009-03-31 17:50:07 +1100
    message:
      Slightly more concise test case.
    modified:
      bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
    ------------------------------------------------------------
    revno: 4214.2.1
    revision-id: andrew.bennetts at canonical.com-20090331060557-l80f1ab20qjm2xud
    parent: pqm at pqm.ubuntu.com-20090330004851-70utb2qlpnc4u9um
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: get_parent_map_rpc_ghosts
    timestamp: Tue 2009-03-31 17:05:57 +1100
    message:
      A long but failing test for the get_parent_map RPC bug.
    modified:
      bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2009-03-25 04:20:12 +0000
+++ b/bzrlib/remote.py	2009-03-31 23:23:07 +0000
@@ -1261,9 +1261,17 @@
         # We don't need to send ghosts back to the server as a position to
         # stop either.
         stop_keys.difference_update(self._unstacked_provider.missing_keys)
+        key_count = len(parents_map)
+        if (NULL_REVISION in result_parents
+            and NULL_REVISION in self._unstacked_provider.missing_keys):
+            # If we pruned NULL_REVISION from the stop_keys because it's also
+            # in our cache of "missing" keys we need to increment our key count
+            # by 1, because the reconsitituted SearchResult on the server will
+            # still consider NULL_REVISION to be an included key.
+            key_count += 1
         included_keys = start_set.intersection(result_parents)
         start_set.difference_update(included_keys)
-        recipe = ('manual', start_set, stop_keys, len(parents_map))
+        recipe = ('manual', start_set, stop_keys, key_count)
         body = self._serialise_search_recipe(recipe)
         path = self.bzrdir._path_for_remote_call(self._client)
         for key in keys:

=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py	2009-03-24 23:19:12 +0000
+++ b/bzrlib/tests/test_remote.py	2009-03-31 23:23:07 +0000
@@ -53,6 +53,7 @@
 from bzrlib.revision import NULL_REVISION
 from bzrlib.smart import server, medium
 from bzrlib.smart.client import _SmartClient
+from bzrlib.smart.repository import SmartServerRepositoryGetParentMap
 from bzrlib.tests import (
     condition_isinstance,
     split_suite_by_condition,
@@ -1673,6 +1674,45 @@
                 'more-missing']))
         self.assertLength(1, self.hpss_calls)
 
+    def disableExtraResults(self):
+        old_flag = SmartServerRepositoryGetParentMap.no_extra_results
+        SmartServerRepositoryGetParentMap.no_extra_results = True
+        def reset_values():
+            SmartServerRepositoryGetParentMap.no_extra_results = old_flag
+        self.addCleanup(reset_values)
+
+    def test_null_cached_missing_and_stop_key(self):
+        self.setup_smart_server_with_call_log()
+        # Make a branch with a single revision.
+        builder = self.make_branch_builder('foo')
+        builder.start_series()
+        builder.build_snapshot('first', None, [
+            ('add', ('', 'root-id', 'directory', ''))])
+        builder.finish_series()
+        branch = builder.get_branch()
+        repo = branch.repository
+        self.assertIsInstance(repo, RemoteRepository)
+        # Stop the server from sending extra results.
+        self.disableExtraResults()
+        repo.lock_read()
+        self.addCleanup(repo.unlock)
+        self.reset_smart_call_log()
+        graph = repo.get_graph()
+        # Query for 'first' and 'null:'.  Because 'null:' is a parent of
+        # 'first' it will be a candidate for the stop_keys of subsequent
+        # requests, and because 'null:' was queried but not returned it will be
+        # cached as missing.
+        self.assertEqual({'first': ('null:',)},
+            graph.get_parent_map(['first', 'null:']))
+        # Now query for another key.  This request will pass along a recipe of
+        # start and stop keys describing the already cached results, and this
+        # recipe's revision count must be correct (or else it will trigger an
+        # error from the server).
+        self.assertEqual({}, graph.get_parent_map(['another-key']))
+        # This assertion guards against disableExtraResults silently failing to
+        # work, thus invalidating the test.
+        self.assertLength(2, self.hpss_calls)
+
     def test_get_parent_map_gets_ghosts_from_result(self):
         # asking for a revision should negatively cache close ghosts in its
         # ancestry.




More information about the bazaar-commits mailing list