Merge lp:~jameinel/bzr/2.4-tree-root-dirstate-830947 into lp:bzr/2.4

[John A Meinel]

Fix bug #830947. Dirstate.set_root_id() should update the id_index correctly.

The symptom is that if you have a tree with the file_id TREE_ROOT somewhere other than at '', it could cause 'bzr co' to fail because it tries to lookup the file in the id_index map, but it incorrectly points to a location ('') where the id never existed.

This fixes bug #830947.

Basically, there was a patch from Ian that I wasn't sure about, but we merged it. Turns out it introduced a bug. Specifically, when changing the root id of a newly created tree, it was forcing the old root id into the id_index map. However, that entry doesn't actually exist (because we are forcibly removing it.) So we don't want to add it.

update_minimal should be doing the right thing anyway.

This updates Dirstate._validate to validate the id_index more carefully.

I backported the patch the 2.4 because that is the first release that has the bug.

[Richard Wilbur (richard-wilbur)]

Thanks for the fix and for back-porting it to 2.4. Does this affect 2.5 and trunk?
Seems like this would be a great candidate for merging into all affected, maintained versions.
+1

review: Approve

[John A Meinel (jameinel)]

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

On 2013-05-20 9:15, Richard Wilbur wrote:
> Review: Approve
>
> Thanks for the fix and for back-porting it to 2.4. Does this
> affect 2.5 and trunk? Seems like this would be a great candidate
> for merging into all affected, maintained versions. +1
>

Yes it does. Which is why I targeted 2.4 and intend to merge it up the
stack.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlGZyhcACgkQJdeBCYSNAAO06QCfZRz46AsH5cRhGp5LssIjUCHt
2wsAoLzd1Pu6MVxB2irZYDgN/yVAjulG
=DSG5
-----END PGP SIGNATURE-----

[John A Meinel (jameinel)]

sent to pqm by email

=== modified file 'bzrlib/dirstate.py'
--- bzrlib/dirstate.py	2011-09-28 15:41:38 +0000
+++ bzrlib/dirstate.py	2013-05-23 08:02:28 +0000
@@ -2609,13 +2609,6 @@
         self.update_minimal(('', '', new_id), 'd',
             path_utf8='', packed_stat=entry[1][0][4])
         self._mark_modified()
-        # XXX: This was added by Ian, we need to make sure there
-        #      are tests for it, because it isn't in bzr.dev TRUNK
-        #      It looks like the only place it is called is in setting the root
-        #      id of the tree. So probably we never had an _id_index when we
-        #      don't even have a root yet.
-        if self._id_index is not None:
-            self._add_to_id_index(self._id_index, entry[0])
 
     def set_parent_trees(self, trees, ghosts):
         """Set the parent trees for the dirstate.
@@ -3328,10 +3321,20 @@
         if self._id_index is not None:
             for file_id, entry_keys in self._id_index.iteritems():
                 for entry_key in entry_keys:
+                    # Check that the entry in the map is pointing to the same
+                    # file_id
                     if entry_key[2] != file_id:
                         raise AssertionError(
                             'file_id %r did not match entry key %s'
                             % (file_id, entry_key))
+                    # And that from this entry key, we can look up the original
+                    # record
+                    block_index, present = self._find_block_index_from_key(entry_key)
+                    if not present:
+                        raise AssertionError('missing block for entry key: %r', entry_key)
+                    entry_index, present = self._find_entry_index(entry_key, self._dirblocks[block_index][1])
+                    if not present:
+                        raise AssertionError('missing entry for key: %r', entry_key)
                 if len(entry_keys) != len(set(entry_keys)):
                     raise AssertionError(
                         'id_index contained non-unique data for %s'

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- doc/en/release-notes/bzr-2.4.txt	2012-06-08 06:59:50 +0000
+++ doc/en/release-notes/bzr-2.4.txt	2013-05-23 08:02:28 +0000
@@ -35,6 +35,10 @@
 * Cope with Unix filesystems, such as smbfs, where chmod gives 'permission
   denied'.  (Martin Pool, #606537)
 
+* Fix a traceback when trying to checkout a tree that also has an entry
+  with file-id `TREE_ROOT` somewhere other than at the root directory.
+  (John Arbash Meinel, #830947)
+
 * When the ``limbo`` or ``pending-deletion`` directories exist, typically
   because of an interrupted tree update, but are empty, bzr no longer
   errors out, because there is nothing for the user to clean up.  Also,