Rev 2440: (John Arbash Meinel) Fix bug #105479: properly handle moving a directory with children that have been added, renamed, removed in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Sat Apr 21 16:11:42 BST 2007
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 2440
revision-id: pqm at pqm.ubuntu.com-20070421151139-5wau2ukbpx5z1hv2
parent: pqm at pqm.ubuntu.com-20070421144713-wrfv38pfywoeg408
parent: john at arbash-meinel.com-20070421144319-c0lnova7qdlvvr70
committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Sat 2007-04-21 16:11:39 +0100
message:
(John Arbash Meinel) Fix bug #105479: properly handle moving a directory with children that have been added, renamed, removed
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/dirstate.py dirstate.py-20060728012006-d6mvoihjb3je9peu-1
bzrlib/tests/workingtree_implementations/test_move.py test_move.py-20070225171927-mohn2vqj5fx7edc6-1
bzrlib/workingtree_4.py workingtree_4.py-20070208044105-5fgpc5j3ljlh5q6c-1
------------------------------------------------------------
revno: 2438.1.16
merged: john at arbash-meinel.com-20070421144319-c0lnova7qdlvvr70
parent: john at arbash-meinel.com-20070421143950-glyo2gbmjwopp2w7
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Sat 2007-04-21 09:43:19 -0500
message:
NEWS for fixing bug #105479
------------------------------------------------------------
revno: 2438.1.15
merged: john at arbash-meinel.com-20070421143950-glyo2gbmjwopp2w7
parent: john at arbash-meinel.com-20070420203953-qljfem7wrpujktl9
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Sat 2007-04-21 09:39:50 -0500
message:
Remove a superfluous 'else' (recommended by Martin)
------------------------------------------------------------
revno: 2438.1.14
merged: john at arbash-meinel.com-20070420203953-qljfem7wrpujktl9
parent: john at arbash-meinel.com-20070420203630-tmc7pwdid0mhkg77
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 15:39:53 -0500
message:
Clean up assert helper for missing parent.
------------------------------------------------------------
revno: 2438.1.13
merged: john at arbash-meinel.com-20070420203630-tmc7pwdid0mhkg77
parent: john at arbash-meinel.com-20070420202856-mnb25dnlfwgkkqmh
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 15:36:30 -0500
message:
Add a test for moving a directory where a child has been moved into a subdir.
------------------------------------------------------------
revno: 2438.1.12
merged: john at arbash-meinel.com-20070420202856-mnb25dnlfwgkkqmh
parent: john at arbash-meinel.com-20070420202552-mz87cu22t6kqnyg2
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 15:28:56 -0500
message:
Add a test with children that have been swapped
------------------------------------------------------------
revno: 2438.1.11
merged: john at arbash-meinel.com-20070420202552-mz87cu22t6kqnyg2
parent: john at arbash-meinel.com-20070420195824-ec0j48hfl8w5rii4
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 15:25:52 -0500
message:
Add a test for moving a directory with a renamed child.
------------------------------------------------------------
revno: 2438.1.10
merged: john at arbash-meinel.com-20070420195824-ec0j48hfl8w5rii4
parent: john at arbash-meinel.com-20070420195649-hx0feab8y15wdxdj
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 14:58:24 -0500
message:
Make WT4.move() properly handle moving a directory with a renamed child.
------------------------------------------------------------
revno: 2438.1.9
merged: john at arbash-meinel.com-20070420195649-hx0feab8y15wdxdj
parent: john at arbash-meinel.com-20070420194857-g37q43yd3rsqqk85
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 14:56:49 -0500
message:
Add another (failing) test when we move an entry which has a renamed child.
------------------------------------------------------------
revno: 2438.1.8
merged: john at arbash-meinel.com-20070420194857-g37q43yd3rsqqk85
parent: john at arbash-meinel.com-20070420194239-ee3ze9cwyoealgl6
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 14:48:57 -0500
message:
Fix the case when renaming a directory with deleted children.
------------------------------------------------------------
revno: 2438.1.7
merged: john at arbash-meinel.com-20070420194239-ee3ze9cwyoealgl6
parent: john at arbash-meinel.com-20070420193709-m0f5ga3ixhlugbnn
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 14:42:39 -0500
message:
While in this area, add a test for renaming a directory with removed children.
Which actually currently fails.
------------------------------------------------------------
revno: 2438.1.6
merged: john at arbash-meinel.com-20070420193709-m0f5ga3ixhlugbnn
parent: john at arbash-meinel.com-20070420193042-d2v07ewsqzemihcp
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 14:37:09 -0500
message:
Simplify the test since all we require is renaming a newly added entry's parent dir.
------------------------------------------------------------
revno: 2438.1.5
merged: john at arbash-meinel.com-20070420193042-d2v07ewsqzemihcp
parent: john at arbash-meinel.com-20070420182016-yalhqlbsw2fmfl3w
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 14:30:42 -0500
message:
Fix the bug by not iterating over the same list we are modifying.
If a record only exists in one tree (current) move_one() will remove it from
the current block. Which means that the block we are
iterating over may change underneath us. So grab a copy before iterating.
Do some other small doc updates.
------------------------------------------------------------
revno: 2438.1.4
merged: john at arbash-meinel.com-20070420182016-yalhqlbsw2fmfl3w
parent: john at arbash-meinel.com-20070420181339-dzr68u9iywq2elxs
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 13:20:16 -0500
message:
Extend the test slightly.
Oddly enough, both newly added records are moved correctly,
but the existing directory 'c/b/d' is not moved.
------------------------------------------------------------
revno: 2438.1.3
merged: john at arbash-meinel.com-20070420181339-dzr68u9iywq2elxs
parent: john at arbash-meinel.com-20070420175713-64x3r196vbqad3y2
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 13:13:39 -0500
message:
Add 2 new WT.move() tests, one of which exposes bug #105479
------------------------------------------------------------
revno: 2438.1.2
merged: john at arbash-meinel.com-20070420175713-64x3r196vbqad3y2
parent: john at arbash-meinel.com-20070420164821-ml7cw68e09gcr54d
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 12:57:13 -0500
message:
Add a check that all entries have a valid parent entry.
If we have a non-absent entry, it must have a valid directory entry as a parent.
(You can't have a present entry in an absent directory, or in a missing dir)
------------------------------------------------------------
revno: 2438.1.1
merged: john at arbash-meinel.com-20070420164821-ml7cw68e09gcr54d
parent: pqm at pqm.ubuntu.com-20070420154033-kkrk7tn575z1o491
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: rename_directory_105479
timestamp: Fri 2007-04-20 11:48:21 -0500
message:
Create an assertion to help track down what the bug is.
=== modified file 'NEWS'
--- a/NEWS 2007-04-21 14:15:08 +0000
+++ b/NEWS 2007-04-21 15:11:39 +0000
@@ -124,6 +124,9 @@
* "dirstate" and "dirstate-tags" formats now produce branches compatible
with old versions of bzr. (Aaron Bentley, #107168))
+ * Handle moving a directory when children have been added, removed,
+ and renamed. (John Arbash Meinel, #105479)
+
TESTING:
* Added ``bzrlib.strace.strace`` which will strace a single callable and
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py 2007-04-20 08:05:24 +0000
+++ b/bzrlib/dirstate.py 2007-04-21 14:39:50 +0000
@@ -2212,6 +2212,28 @@
"dirblock for %r is not sorted:\n%s" % \
(dirblock[0], pformat(dirblock)))
+
+ def check_valid_parent():
+ """Check that the current entry has a valid parent.
+
+ This makes sure that the parent has a record,
+ and that the parent isn't marked as "absent" in the
+ current tree. (It is invalid to have a non-absent file in an absent
+ directory.)
+ """
+ if entry[0][0:2] == ('', ''):
+ # There should be no parent for the root row
+ return
+ parent_entry = self._get_entry(tree_index, path_utf8=entry[0][0])
+ if parent_entry == (None, None):
+ raise AssertionError(
+ "no parent entry for: %s in tree %s"
+ % (this_path, tree_index))
+ if parent_entry[1][tree_index][0] != 'd':
+ raise AssertionError(
+ "Parent entry for %s is not marked as a valid"
+ " directory. %s" % (this_path, parent_entry,))
+
# For each file id, for each tree: either
# the file id is not present at all; all rows with that id in the
# key have it marked as 'absent'
@@ -2258,6 +2280,7 @@
raise AssertionError(
"entry %r inconsistent with previous path %r" % \
(entry, previous_path))
+ check_valid_parent()
else:
if minikind == 'a':
# absent; should not occur anywhere else
@@ -2267,6 +2290,7 @@
this_tree_map[file_id] = tree_state[1]
else:
this_tree_map[file_id] = this_path
+ check_valid_parent()
def _wipe_state(self):
"""Forget all state information about the dirstate."""
=== modified file 'bzrlib/tests/workingtree_implementations/test_move.py'
--- a/bzrlib/tests/workingtree_implementations/test_move.py 2007-03-22 23:47:20 +0000
+++ b/bzrlib/tests/workingtree_implementations/test_move.py 2007-04-20 20:36:30 +0000
@@ -358,6 +358,180 @@
('a/c/d', 'd-id')], tree.basis_tree())
tree._validate()
+ def test_move_directory_into_parent(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['c/', 'c/b/', 'c/b/d/'])
+ tree.add(['c', 'c/b', 'c/b/d'],
+ ['c-id', 'b-id', 'd-id'])
+ tree.commit('initial', rev_id='rev-1')
+ root_id = tree.get_root_id()
+
+ self.assertEqual([('c/b', 'b')],
+ tree.move(['c/b'], ''))
+ self.assertTreeLayout([('', root_id),
+ ('b', 'b-id'),
+ ('c', 'c-id'),
+ ('b/d', 'd-id'),
+ ], tree)
+ tree._validate()
+
+ def test_move_directory_with_children_in_subdir(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/b', 'a/c/', 'd/'])
+ tree.add(['a', 'a/b', 'a/c', 'd'],
+ ['a-id', 'b-id', 'c-id', 'd-id'])
+ tree.commit('initial', rev_id='rev-1')
+ root_id = tree.get_root_id()
+
+
+ tree.rename_one('a/b', 'a/c/b')
+ self.assertTreeLayout([('', root_id),
+ ('a', 'a-id'),
+ ('d', 'd-id'),
+ ('a/c', 'c-id'),
+ ('a/c/b', 'b-id'),
+ ], tree)
+ self.assertEqual([('a', 'd/a')],
+ tree.move(['a'], 'd'))
+ self.assertTreeLayout([('', root_id),
+ ('d', 'd-id'),
+ ('d/a', 'a-id'),
+ ('d/a/c', 'c-id'),
+ ('d/a/c/b', 'b-id'),
+ ], tree)
+ tree._validate()
+
+ def test_move_directory_with_deleted_children(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/b', 'a/c', 'a/d', 'b/'])
+ tree.add(['a', 'b', 'a/b', 'a/c', 'a/d'],
+ ['a-id', 'b-id', 'ab-id', 'ac-id', 'ad-id'])
+ tree.commit('initial', rev_id='rev-1')
+ root_id = tree.get_root_id()
+
+ tree.remove(['a/b', 'a/d'])
+
+ self.assertEqual([('a', 'b/a')],
+ tree.move(['a'], 'b'))
+ self.assertTreeLayout([('', root_id),
+ ('b', 'b-id'),
+ ('b/a', 'a-id'),
+ ('b/a/c', 'ac-id'),
+ ], tree)
+ tree._validate()
+
+ def test_move_directory_with_new_children(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/c', 'b/'])
+ tree.add(['a', 'b', 'a/c'], ['a-id', 'b-id', 'ac-id'])
+ tree.commit('initial', rev_id='rev-1')
+ root_id = tree.get_root_id()
+
+ self.build_tree(['a/b', 'a/d'])
+ tree.add(['a/b', 'a/d'], ['ab-id', 'ad-id'])
+
+ self.assertEqual([('a', 'b/a')],
+ tree.move(['a'], 'b'))
+ self.assertTreeLayout([('', root_id),
+ ('b', 'b-id'),
+ ('b/a', 'a-id'),
+ ('b/a/b', 'ab-id'),
+ ('b/a/c', 'ac-id'),
+ ('b/a/d', 'ad-id'),
+ ], tree)
+ tree._validate()
+
+ def test_move_directory_with_moved_children(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/b', 'a/c', 'd', 'e/'])
+ tree.add(['a', 'a/b', 'a/c', 'd', 'e'],
+ ['a-id', 'b-id', 'c-id', 'd-id', 'e-id'])
+ tree.commit('initial', rev_id='rev-1')
+ root_id = tree.get_root_id()
+
+ self.assertEqual([('a/b', 'b')],
+ tree.move(['a/b'], ''))
+ self.assertTreeLayout([('', root_id),
+ ('a', 'a-id'),
+ ('b', 'b-id'),
+ ('d', 'd-id'),
+ ('e', 'e-id'),
+ ('a/c', 'c-id'),
+ ], tree)
+ self.assertEqual([('d', 'a/d')],
+ tree.move(['d'], 'a'))
+ self.assertTreeLayout([('', root_id),
+ ('a', 'a-id'),
+ ('b', 'b-id'),
+ ('e', 'e-id'),
+ ('a/c', 'c-id'),
+ ('a/d', 'd-id'),
+ ], tree)
+ self.assertEqual([('a', 'e/a')],
+ tree.move(['a'], 'e'))
+ self.assertTreeLayout([('', root_id),
+ ('b', 'b-id'),
+ ('e', 'e-id'),
+ ('e/a', 'a-id'),
+ ('e/a/c', 'c-id'),
+ ('e/a/d', 'd-id'),
+ ], tree)
+ tree._validate()
+
+ def test_move_directory_with_renamed_child(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/b', 'a/c', 'd/'])
+ tree.add(['a', 'a/b', 'a/c', 'd'],
+ ['a-id', 'b-id', 'c-id', 'd-id'])
+ tree.commit('initial', rev_id='rev-1')
+ root_id = tree.get_root_id()
+
+ tree.rename_one('a/b', 'a/d')
+ self.assertTreeLayout([('', root_id),
+ ('a', 'a-id'),
+ ('d', 'd-id'),
+ ('a/c', 'c-id'),
+ ('a/d', 'b-id'),
+ ], tree)
+ self.assertEqual([('a', 'd/a')],
+ tree.move(['a'], 'd'))
+ self.assertTreeLayout([('', root_id),
+ ('d', 'd-id'),
+ ('d/a', 'a-id'),
+ ('d/a/c', 'c-id'),
+ ('d/a/d', 'b-id'),
+ ], tree)
+ tree._validate()
+
+ def test_move_directory_with_swapped_children(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/b', 'a/c', 'a/d', 'e/'])
+ tree.add(['a', 'a/b', 'a/c', 'a/d', 'e'],
+ ['a-id', 'b-id', 'c-id', 'd-id', 'e-id'])
+ tree.commit('initial', rev_id='rev-1')
+ root_id = tree.get_root_id()
+
+ tree.rename_one('a/b', 'a/bb')
+ tree.rename_one('a/d', 'a/b')
+ tree.rename_one('a/bb', 'a/d')
+ self.assertTreeLayout([('', root_id),
+ ('a', 'a-id'),
+ ('e', 'e-id'),
+ ('a/b', 'd-id'),
+ ('a/c', 'c-id'),
+ ('a/d', 'b-id'),
+ ], tree)
+ self.assertEqual([('a', 'e/a')],
+ tree.move(['a'], 'e'))
+ self.assertTreeLayout([('', root_id),
+ ('e', 'e-id'),
+ ('e/a', 'a-id'),
+ ('e/a/b', 'd-id'),
+ ('e/a/c', 'c-id'),
+ ('e/a/d', 'b-id'),
+ ], tree)
+ tree._validate()
+
def test_move_moved(self):
"""Moving a moved entry works as expected."""
tree = self.make_branch_and_tree('.')
=== modified file 'bzrlib/workingtree_4.py'
--- a/bzrlib/workingtree_4.py 2007-04-20 15:40:33 +0000
+++ b/bzrlib/workingtree_4.py 2007-04-20 20:39:53 +0000
@@ -670,7 +670,6 @@
new_entry = to_block[1][added_entry_index]
rollbacks.append(lambda:state._make_absent(new_entry))
- # create rename entries and tuples
for from_rel in from_paths:
# from_rel is 'pathinroot/foo/bar'
from_rel_utf8 = from_rel.encode('utf8')
@@ -774,11 +773,7 @@
if minikind == 'd':
def update_dirblock(from_dir, to_key, to_dir_utf8):
- """all entries in this block need updating.
-
- TODO: This is pretty ugly, and doesn't support
- reverting, but it works.
- """
+ """Recursively update all entries in this dirblock."""
assert from_dir != '', "renaming root not supported"
from_key = (from_dir, '')
from_block_idx, present = \
@@ -795,13 +790,21 @@
to_block_index = state._ensure_block(
to_block_index, to_entry_index, to_dir_utf8)
to_block = state._dirblocks[to_block_index]
- for entry in from_block[1]:
+
+ # Grab a copy since move_one may update the list.
+ for entry in from_block[1][:]:
assert entry[0][0] == from_dir
cur_details = entry[1][0]
to_key = (to_dir_utf8, entry[0][1], entry[0][2])
from_path_utf8 = osutils.pathjoin(entry[0][0], entry[0][1])
to_path_utf8 = osutils.pathjoin(to_dir_utf8, entry[0][1])
minikind = cur_details[0]
+ if minikind in 'ar':
+ # Deleted children of a renamed directory
+ # Do not need to be updated.
+ # Children that have been renamed out of this
+ # directory should also not be updated
+ continue
move_one(entry, from_path_utf8=from_path_utf8,
minikind=minikind,
executable=cur_details[3],
@@ -1920,6 +1923,9 @@
# parent entry will be the same as the source entry.
target_parent_entry = state._get_entry(target_index,
path_utf8=new_dirname)
+ assert target_parent_entry != (None, None), (
+ "Could not find target parent in wt: %s\nparent of: %s"
+ % (new_dirname, entry))
target_parent_id = target_parent_entry[0][2]
if target_parent_id == entry[0][2]:
# This is the root, so the parent is None
More information about the bazaar-commits
mailing list