Code review comment for lp:~jameinel/bzr/2.1-static-tuple-chk-map

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

Matt Nordhoff wrote:
> I just tried this branch (well, + r4762 of bzr.dev), on my client and
> server. Pushing to the server gave a traceback:

Thanks for the heads up.

...

> line 653, in _check_new_inventories
> for record in _filter_text_keys(chk_diff, text_keys, bytes_to_info):
> File
> "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/repofmt/groupcompress_repo.py",
> line 1189, in _filter_text_keys
> for record, items in interesting_nodes_iterable:
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 1642, in
> process
> for record in self._read_all_roots():
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 1566, in
> _read_all_roots
> self._read_nodes_from_store(new_keys):
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 1500, in
> _read_nodes_from_store
> search_key_func=self._search_key_func)
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 1438, in
> _deserialise
> search_key_func=search_key_func)
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 1023, in
> deserialise
> ' StaticTuple not %s' % (type(key),))
> AssertionError: deserialise should be called with a StaticTuple not
> <type 'tuple'>

I'll try to track down where the plain 'tuple' object came into play,
and also why I didn't catch it with the test suite. Admittedly I only
ran a subset, but I thought I ran "selftest -s bt.per_repo' which should
have covered this.

...

> for result in self.target.inventory.iter_changes(self.source.inventory):
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/inventory.py", line 2085,
> in iter_changes
> self.id_to_entry.iter_changes(basis.id_to_entry):
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 294, in
> iter_changes
> self._ensure_root()
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 140, in
> _ensure_root
> self._root_node = self._get_node(self._root_node)
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 155, in
> _get_node
> search_key_func=self._search_key_func)
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 1421, in
> _deserialise
> search_key_func=search_key_func)
> File "/usr/local/co/bzr/bzr/bzr.dev/bzrlib/chk_map.py", line 1008, in
> deserialise
> search_key_func=search_key_func)
> File "_chk_map_pyx.pyx", line 368, in
> _chk_map_pyx._deserialise_internal_node
> TypeError: key ('sha1:e12a0f03e145bbabf18cc7b933cce82edfc005dd',) is not
> a StaticTuple
>
> Turning off plugins did not help the latter one; I didn't try it with
> the first one.

^- This is pretty surprising, I'll certainly give it a look. Namely, it
looks like the root_key in 'basis.id_to_entry' is not a StaticTuple,
which is surprising given that the code that sets the root key has:
if type(node) is tuple:
    node = StaticTuple.from_sequence(node)

Thanks for looking closely.

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

iEYEARECAAYFAkrgd48ACgkQJdeBCYSNAAOyVgCeIVTtO7/TDyp9nEv9CBUX4oq+
SM0An09eTMf4fXhfMSWtHcj48HGxKTQl
=oe2X
-----END PGP SIGNATURE-----