[brisbane-core/MERGE] create_by_apply_delta() fixes & CHKMap improvements

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

ian.clatworthy at internode.on.net wrote:
> The attached patch fixes a number of bugs in CHKInventory.create_by_apply_delta():
> 
> * the root_node key needs to be passed to CHKMap(), not the root node itself
>   (the two aren't the same if _ensure_root has ever been called)
> 
> * the maximum size and key width need to be propagated
> 
> It also adds a better __repr__ method for CHK LeafNodes(), improves a
> few docstrings, makes CHKInventory.has_id() more efficient and fixes
> a bug in CHKInventory._get_mutable_inventory().
> 
> Hopefully, this gets a warmer reception than my last patch. :-)
> 
> Once again, I'd apprciate it if the reviewer can land this if it's acceptable.
> 
> Thanks,
> Ian C.

+    def __repr__(self):
+        items_str = sorted(self._items)
+        if len(items_str) > 20:
+            items_str = items_str[16] + '...]'
+        return \
+            '%s(key:%s len:%s size:%s max:%s prefix:%s keywidth:%s
items:%s)' \
+            % (self.__class__.__name__, self._key, self._len,
self._raw_size,
+            self._maximum_size, self._search_prefix, self._key_width,
items_str)
+

I think you are looking for a items_str = str(sorted(self._items))
I'm not positive, but I'm pretty sure the above change will count the
number of items in the list, and do weird things during the 'add'.
(namely, I think it will do ['foo', 'bar', '.', '.', '.', ']']).

But I think I know what you are going for.

...

+    * id_to_entry - a map from (file_id,) => InventoryEntry as bytes
+    * parent_id_basename_to_file_id - a map from (parent_id, basename_utf8)
+        => file_id as bytes
+
+    The second map is optional and not present in early CHkRepository's.

^- this is a true statement, though all current formats that we support
have both maps. As such, I'm thinking to remove all the 'optional'
parts. It should clean up the code a bit to avoid all of those 'if
parent_id_basename is not None' checks.

I'm also thinking to re-evaluate the serialized form in light of the
latest compression results. For now, though, your changes are accurate.


...
+            result.parent_id_basename_to_file_id._root_node._key_width = 2

I think if we are setting the _key_width, we shoul dbe setting it from
the source _key_width, just to make sure if it ever changes, it is
quickly propagated.

Otherwise it looks pretty good. I'll mostly merge it as is, and we can
update the docs later as we prune things. Mostly I wanted to convey some
ideas that I've reached (so far).

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmv4icACgkQJdeBCYSNAANaogCcCugvJyLF3chEntlYIZrNrod7
7KwAoItp0Ow7a9V3mTNO7/q0caEV4vWq
=tQBI
-----END PGP SIGNATURE-----