Rev 4852: (spiv) Fix bug in CHKMap.apply_delta that was generating in file:///home/pqm/archives/thelove/bzr/2.1/

[Canonical.com Patch Queue Manager]

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

------------------------------------------------------------
revno: 4852 [merge]
revision-id: pqm at pqm.ubuntu.com-20100714115733-e9rueysqk5064akm
parent: pqm at pqm.ubuntu.com-20100713140929-it8pi3wmf2z52vog
parent: andrew.bennetts at canonical.com-20100714101145-sg8s9vr60id7ne7j
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.1
timestamp: Wed 2010-07-14 12:57:33 +0100
message:
  (spiv) Fix bug in CHKMap.apply_delta that was generating
  	non-canonical CHK maps when entries are removed. (Andrew
  	Bennetts, #522637)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/chk_map.py              chk_map.py-20081001014447-ue6kkuhofvdecvxa-1
  bzrlib/tests/test_chk_map.py   test_chk_map.py-20081001014447-ue6kkuhofvdecvxa-2
=== modified file 'NEWS'
--- a/NEWS	2010-07-13 13:25:38 +0000
+++ b/NEWS	2010-07-14 10:11:45 +0000
@@ -27,6 +27,11 @@
 * Don't traceback trying to unversion children files of an already
   unversioned directory.  (Vincent Ladeuil, #494221)
 
+* Prevent ``CHKMap.apply_delta`` from generating non-canonical CHK maps,
+  which can result in "missing referenced chk root keys" errors when
+  fetching from repositories with affected revisions.
+  (Andrew Bennetts, #522637)
+
 * Raise ValueError instead of a string exception.
   (John Arbash Meinel, #586926)
 
@@ -512,6 +517,11 @@
 * Don't traceback trying to unversion children files of an already
   unversioned directory.  (Vincent Ladeuil, #494221)
 
+* Prevent ``CHKMap.apply_delta`` from generating non-canonical CHK maps,
+  which can result in "missing referenced chk root keys" errors when
+  fetching from repositories with affected revisions.
+  (Andrew Bennetts, #522637)
+
 * Raise ValueError instead of a string exception.
   (John Arbash Meinel, #586926)
 

=== modified file 'bzrlib/chk_map.py'
--- a/bzrlib/chk_map.py	2010-02-17 17:11:16 +0000
+++ b/bzrlib/chk_map.py	2010-07-14 07:51:25 +0000
@@ -90,8 +90,6 @@
 _INTERESTING_NEW_SIZE = 50
 # If a ChildNode shrinks by more than this amount, we check for a remap
 _INTERESTING_SHRINKAGE_LIMIT = 20
-# If we delete more than this many nodes applying a delta, we check for a remap
-_INTERESTING_DELETES_LIMIT = 5
 
 
 def _search_key_plain(key):
@@ -135,7 +133,7 @@
             into the map; if old_key is not None, then the old mapping
             of old_key is removed.
         """
-        delete_count = 0
+        has_deletes = False
         # Check preconditions first.
         as_st = StaticTuple.from_sequence
         new_items = set([as_st(key) for (old, key, value) in delta
@@ -148,12 +146,11 @@
         for old, new, value in delta:
             if old is not None and old != new:
                 self.unmap(old, check_remap=False)
-                delete_count += 1
+                has_deletes = True
         for old, new, value in delta:
             if new is not None:
                 self.map(new, value)
-        if delete_count > _INTERESTING_DELETES_LIMIT:
-            trace.mutter("checking remap as %d deletions", delete_count)
+        if has_deletes:
             self._check_remap()
         return self._save()
 
@@ -573,7 +570,7 @@
         """Check if nodes can be collapsed."""
         self._ensure_root()
         if type(self._root_node) is InternalNode:
-            self._root_node._check_remap(self._store)
+            self._root_node = self._root_node._check_remap(self._store)
 
     def _save(self):
         """Save the map completely.

=== modified file 'bzrlib/tests/test_chk_map.py'
--- a/bzrlib/tests/test_chk_map.py	2010-02-17 17:11:16 +0000
+++ b/bzrlib/tests/test_chk_map.py	2010-07-14 08:32:49 +0000
@@ -467,6 +467,25 @@
         # updated key.
         self.assertEqual(new_root, chkmap._root_node._key)
 
+    def test_apply_delete_to_internal_node(self):
+        # applying a delta should be convert an internal root node to a leaf
+        # node if the delta shrinks the map enough.
+        store = self.get_chk_bytes()
+        chkmap = CHKMap(store, None)
+        # Add three items: 2 small enough to fit in one node, and one huge to
+        # force multiple nodes.
+        chkmap._root_node.set_maximum_size(100)
+        chkmap.map(('small',), 'value')
+        chkmap.map(('little',), 'value')
+        chkmap.map(('very-big',), 'x' * 100)
+        # (Check that we have constructed the scenario we want to test)
+        self.assertIsInstance(chkmap._root_node, InternalNode)
+        # Delete the huge item so that the map fits in one node again.
+        delta = [(('very-big',), None, None)]
+        chkmap.apply_delta(delta)
+        self.assertCanonicalForm(chkmap)
+        self.assertIsInstance(chkmap._root_node, LeafNode)
+
     def test_apply_new_keys_must_be_new(self):
         # applying a delta (None, "a", "b") to a map with 'a' in it generates
         # an error.