Rev 2372: Update DirState._validate() to detect rename errors. in http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/move_children_correctly

John Arbash Meinel john at arbash-meinel.com
Thu Mar 22 23:47:53 GMT 2007


At http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/move_children_correctly

------------------------------------------------------------
revno: 2372
revision-id: john at arbash-meinel.com-20070322234720-re8l7jtva8msgbsv
parent: pqm at pqm.ubuntu.com-20070322152522-228285cac46c0dbc
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: move_children_correctly
timestamp: Thu 2007-03-22 18:47:20 -0500
message:
  Update DirState._validate() to detect rename errors.
  WorkingTree.move() would incorrectly update children of a renamed directory.
  (Some of the references would point to the directory instead of the child)
  This also adds WorkingTree._validate() which is a no-op for most trees,
  and just calls self._dirstate._validate() for WT4 trees.
  It then updates test_move() to have the WT validate itself after all tests.
modified:
  bzrlib/dirstate.py             dirstate.py-20060728012006-d6mvoihjb3je9peu-1
  bzrlib/tests/workingtree_implementations/test_move.py test_move.py-20070225171927-mohn2vqj5fx7edc6-1
  bzrlib/workingtree.py          workingtree.py-20050511021032-29b6ec0a681e02e3
  bzrlib/workingtree_4.py        workingtree_4.py-20070208044105-5fgpc5j3ljlh5q6c-1
-------------- next part --------------
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2007-03-21 04:14:35 +0000
+++ b/bzrlib/dirstate.py	2007-03-22 23:47:20 +0000
@@ -2193,6 +2193,34 @@
                 "dirblock for %r is not sorted:\n%s" % \
                 (dirblock[0], pformat(dirblock))
 
+        # Make sure that all renamed entries point to the correct location.
+        for entry in self._iter_entries():
+            for tree_index, tree_state in enumerate(entry[1]):
+                if tree_state[0] == 'r': # Renamed entry
+                    target_location = tree_state[1]
+                    other_entry = self._get_entry(tree_index,
+                                                  path_utf8=target_location)
+                    this_path = osutils.pathjoin(entry[0][0], entry[0][1])
+                    other_path = osutils.pathjoin(other_entry[0][0],
+                                                  other_entry[0][1])
+                    assert entry[0][2] == other_entry[0][2], \
+                        ('A rename entry points to a record with a different'
+                         ' file id. %s => %s'
+                         % (pformat(entry), pformat(other_entry)))
+                    if len(entry[1]) == 2: # Check the rename is symmetric
+                        if tree_index == 0:
+                            other_index = 1
+                        else:
+                            other_index = 0
+                        assert other_entry[1][other_index][0] == 'r', \
+                            ('a rename points to a record which doesn\'t'
+                             ' point back. %s => %s'
+                             % (pformat(entry), pformat(other_entry)))
+                        assert other_entry[1][other_index][1] == this_path, \
+                            ('a rename points to a record which points to a'
+                             ' different location. %s => %s'
+                             % (pformat(entry), pformat(other_entry)))
+
     def _wipe_state(self):
         """Forget all state information about the dirstate."""
         self._header_state = DirState.NOT_IN_MEMORY

=== modified file 'bzrlib/tests/workingtree_implementations/test_move.py'
--- a/bzrlib/tests/workingtree_implementations/test_move.py	2007-03-07 03:09:14 +0000
+++ b/bzrlib/tests/workingtree_implementations/test_move.py	2007-03-22 23:47:20 +0000
@@ -54,6 +54,7 @@
         tree.commit('initial commit')
         self.assertEqual([('a1', 'sub1/a1')],
             tree.move(['a1'], to_dir='sub1', after=False))
+        tree._validate()
 
     def test_move_correct_call_unnamed(self):
         """tree.move has the deprecated parameter 'to_name'.
@@ -66,6 +67,7 @@
         tree.commit('initial commit')
         self.assertEqual([('a1', 'sub1/a1')],
             tree.move(['a1'], 'sub1', after=False))
+        tree._validate()
 
     def test_move_deprecated_wrong_call(self):
         """tree.move has the deprecated parameter 'to_name'.
@@ -79,6 +81,7 @@
         self.assertRaises(TypeError, tree.move, ['a1'],
                           to_this_parameter_does_not_exist='sub1',
                           after=False)
+        tree._validate()
 
     def test_move_deprecated_call(self):
         """tree.move has the deprecated parameter 'to_name'.
@@ -100,6 +103,7 @@
             # since it was deprecated before the class was introduced.
             if not isinstance(self.workingtree_format, WorkingTreeFormat4):
                 raise
+        tree._validate()
 
     def test_move_target_not_dir(self):
         tree = self.make_branch_and_tree('.')
@@ -109,6 +113,7 @@
 
         self.assertRaises(errors.BzrMoveFailedError,
                           tree.move, ['a'], 'not-a-dir')
+        tree._validate()
 
     def test_move_non_existent(self):
         tree = self.make_branch_and_tree('.')
@@ -119,6 +124,7 @@
                           tree.move, ['not-a-file'], 'a')
         self.assertRaises(errors.BzrMoveFailedError,
                           tree.move, ['not-a-file'], '')
+        tree._validate()
 
     def test_move_target_not_versioned(self):
         tree = self.make_branch_and_tree('.')
@@ -127,6 +133,7 @@
         tree.commit('initial', rev_id='rev-1')
         self.assertRaises(errors.BzrMoveFailedError,
                           tree.move, ['b'], 'a')
+        tree._validate()
 
     def test_move_unversioned(self):
         tree = self.make_branch_and_tree('.')
@@ -135,6 +142,7 @@
         tree.commit('initial', rev_id='rev-1')
         self.assertRaises(errors.BzrMoveFailedError,
                           tree.move, ['b'], 'a')
+        tree._validate()
 
     def test_move_multi_unversioned(self):
         tree = self.make_branch_and_tree('.')
@@ -157,6 +165,7 @@
                                    ('d', 'd-id')], tree)
         self.assertTreeLayout([('', root_id), ('a', 'a-id'), ('c', 'c-id'),
                                ('d', 'd-id')], tree.basis_tree())
+        tree._validate()
 
     def test_move_subdir(self):
         tree = self.make_branch_and_tree('.')
@@ -177,6 +186,7 @@
                                ('b/c', 'c-id')], tree.basis_tree())
         self.failIfExists('a')
         self.assertFileEqual(a_contents, 'b/a')
+        tree._validate()
 
     def test_move_parent_dir(self):
         tree = self.make_branch_and_tree('.')
@@ -193,6 +203,7 @@
                                ('b/c', 'c-id')], tree.basis_tree())
         self.failIfExists('b/c')
         self.assertFileEqual(c_contents, 'c')
+        tree._validate()
 
     def test_move_fail_consistent(self):
         tree = self.make_branch_and_tree('.')
@@ -214,6 +225,7 @@
                                    ('b/c', 'c-id')], tree)
         self.assertTreeLayout([('', root_id), ('a', 'a-id'), ('b', 'b-id'),
                                ('c', 'c-id')], tree.basis_tree())
+        tree._validate()
 
     def test_move_onto_self(self):
         tree = self.make_branch_and_tree('.')
@@ -223,6 +235,7 @@
 
         self.assertRaises(errors.BzrMoveFailedError,
                           tree.move, ['b/a'], 'b')
+        tree._validate()
 
     def test_move_onto_self_root(self):
         tree = self.make_branch_and_tree('.')
@@ -232,6 +245,7 @@
 
         self.assertRaises(errors.BzrMoveFailedError,
                           tree.move, ['a'], 'a')
+        tree._validate()
 
     def test_move_after(self):
         tree = self.make_branch_and_tree('.')
@@ -251,6 +265,7 @@
                               tree)
         self.assertTreeLayout([('', root_id), ('a', 'a-id'), ('b', 'b-id')],
                               tree.basis_tree())
+        tree._validate()
 
     def test_move_after_with_after(self):
         tree = self.make_branch_and_tree('.')
@@ -269,6 +284,7 @@
                               tree)
         self.assertTreeLayout([('', root_id), ('a', 'a-id'), ('b', 'b-id')],
                               tree.basis_tree())
+        tree._validate()
 
     def test_move_after_no_target(self):
         tree = self.make_branch_and_tree('.')
@@ -282,6 +298,7 @@
                           tree.move, ['a'], 'b', after=True)
         self.assertTreeLayout([('', root_id), ('a', 'a-id'), ('b', 'b-id')],
                               tree.basis_tree())
+        tree._validate()
 
     def test_move_after_source_and_dest(self):
         tree = self.make_branch_and_tree('.')
@@ -321,6 +338,7 @@
         # But it shouldn't actually move anything
         self.assertFileEqual(a_text, 'a')
         self.assertFileEqual(ba_text, 'b/a')
+        tree._validate()
 
     def test_move_directory(self):
         tree = self.make_branch_and_tree('.')
@@ -338,6 +356,7 @@
         self.assertTreeLayout([('', root_id), ('a', 'a-id'), ('e', 'e-id'),
                                ('a/b', 'b-id'), ('a/c', 'c-id'),
                                ('a/c/d', 'd-id')], tree.basis_tree())
+        tree._validate()
 
     def test_move_moved(self):
         """Moving a moved entry works as expected."""
@@ -360,3 +379,4 @@
                                ('c', 'c-id')], tree)
         self.assertTreeLayout([('', root_id), ('a', 'a-id'), ('c', 'c-id'),
                                ('a/b', 'b-id')], tree.basis_tree())
+        tree._validate()

=== modified file 'bzrlib/workingtree.py'
--- a/bzrlib/workingtree.py	2007-03-10 20:35:58 +0000
+++ b/bzrlib/workingtree.py	2007-03-22 23:47:20 +0000
@@ -2287,6 +2287,17 @@
         self.set_conflicts(un_resolved)
         return un_resolved, resolved
 
+    def _validate(self):
+        """Validate internal structures.
+
+        This is meant mostly for the test suite. To give it a chance to detect
+        corruption after actions have occurred. The default implementation is a
+        just a no-op.
+
+        :return: None. An exception should be raised if there is an error.
+        """
+        return
+
 
 class WorkingTree2(WorkingTree):
     """This is the Format 2 working tree.

=== modified file 'bzrlib/workingtree_4.py'
--- a/bzrlib/workingtree_4.py	2007-03-21 04:14:35 +0000
+++ b/bzrlib/workingtree_4.py	2007-03-22 23:47:20 +0000
@@ -809,7 +809,7 @@
                                      size=cur_details[2],
                                      to_block=to_block,
                                      to_key=to_key,
-                                     to_path_utf8=to_rel_utf8)
+                                     to_path_utf8=to_path_utf8)
                             if minikind == 'd':
                                 # We need to move all the children of this
                                 # entry
@@ -1202,6 +1202,10 @@
             for file_id in file_ids:
                 self._inventory.remove_recursive_id(file_id)
 
+    @needs_read_lock
+    def _validate(self):
+        self._dirstate._validate()
+
     @needs_tree_write_lock
     def _write_inventory(self, inv):
         """Write inventory as the current inventory."""
@@ -1256,6 +1260,7 @@
         # write out new dirstate (must exist when we create the tree)
         state = dirstate.DirState.initialize(local_path)
         state.unlock()
+        del state
         wt = WorkingTree4(a_bzrdir.root_transport.local_abspath('.'),
                          branch,
                          _format=self,
@@ -1263,7 +1268,7 @@
                          _control_files=control_files)
         wt._new_tree()
         wt.lock_tree_write()
-        state._validate()
+        wt.current_dirstate()._validate()
         try:
             if revision_id in (None, NULL_REVISION):
                 wt._set_root_id(generate_ids.gen_root_id())



More information about the bazaar-commits mailing list