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