Rev 4575: Review feedback, including finding a bug with changes at the root. in http://bazaar.launchpad.net/~lifeless/bzr/iter-changes-partial-parents
Robert Collins
robertc at robertcollins.net
Mon Aug 3 04:37:02 BST 2009
At http://bazaar.launchpad.net/~lifeless/bzr/iter-changes-partial-parents
------------------------------------------------------------
revno: 4575
revision-id: robertc at robertcollins.net-20090803033647-vr5uljwpi2f6rdya
parent: robertc at robertcollins.net-20090729061227-l7a8kz6ja0x25nh9
committer: Robert Collins <robertc at robertcollins.net>
branch nick: iter-changes-partial-parents
timestamp: Mon 2009-08-03 13:36:47 +1000
message:
Review feedback, including finding a bug with changes at the root.
=== modified file 'bzrlib/_dirstate_helpers_pyx.pyx'
--- a/bzrlib/_dirstate_helpers_pyx.pyx 2009-07-29 06:12:27 +0000
+++ b/bzrlib/_dirstate_helpers_pyx.pyx 2009-08-03 03:36:47 +0000
@@ -963,7 +963,7 @@
cdef class ProcessEntryC:
- cdef int doing_exact_expansion
+ cdef int doing_consistency_expansion
cdef object old_dirname_to_file_id # dict
cdef object new_dirname_to_file_id # dict
cdef object last_source_parent
@@ -995,7 +995,7 @@
cdef object current_block_list
cdef object current_dir_info
cdef object current_dir_list
- cdef object next_exact_files # list
+ cdef object _pending_consistent_entries # list
cdef int path_index
cdef object root_dir_info
cdef object bisect_left
@@ -1008,7 +1008,7 @@
def __init__(self, include_unchanged, use_filesystem_for_exec,
search_specific_files, state, source_index, target_index,
want_unversioned, tree):
- self.doing_exact_expansion = 0
+ self.doing_consistency_expansion = 0
self.old_dirname_to_file_id = {}
self.new_dirname_to_file_id = {}
# Are we doing a partial iter_changes?
@@ -1058,7 +1058,7 @@
self.current_block_pos = -1
self.current_dir_info = None
self.current_dir_list = None
- self.next_exact_files = []
+ self._pending_consistent_entries = []
self.path_index = 0
self.root_dir_info = None
self.bisect_left = bisect.bisect_left
@@ -1126,12 +1126,12 @@
else:
# add the source to the search path to find any children it
# has. TODO ? : only add if it is a container ?
- if (not self.doing_exact_expansion and
+ if (not self.doing_consistency_expansion and
not osutils.is_inside_any(self.searched_specific_files,
source_details[1])):
self.search_specific_files.add(source_details[1])
- # We don't expand the specific path parents here as we
- # only require valid deltas to the target.
+ # expanding from a user requested path, parent expansion
+ # for delta consistency happens later.
# generate the old path; this is needed for stating later
# as well.
old_path = source_details[1]
@@ -1211,6 +1211,7 @@
self.old_dirname_to_file_id[old_path] = file_id
# parent id is the entry for the path in the target tree
if old_basename and old_dirname == self.last_source_parent[0]:
+ # use a cached hit for non-root source entries.
source_parent_id = self.last_source_parent[1]
else:
try:
@@ -1227,6 +1228,7 @@
self.last_source_parent[1] = source_parent_id
new_dirname = entry[0][0]
if entry[0][1] and new_dirname == self.last_target_parent[0]:
+ # use a cached hit for non-root target entries.
target_parent_id = self.last_target_parent[1]
else:
try:
@@ -1343,7 +1345,7 @@
# a renamed parent. TODO: handle this efficiently. Its not
# common case to rename dirs though, so a correct but slow
# implementation will do.
- if (not self.doing_exact_expansion and
+ if (not self.doing_consistency_expansion and
not osutils.is_inside_any(self.searched_specific_files,
target_details[1])):
self.search_specific_files.add(target_details[1])
@@ -1536,7 +1538,7 @@
# If we reach here, the outer flow continues, which enters into the
# per-root setup logic.
if (self.current_dir_info is None and self.current_block is None and not
- self.doing_exact_expansion):
+ self.doing_consistency_expansion):
# setup iteration of this root:
self.current_dir_list = None
if self.root_dir_info and self.root_dir_info[2] == 'tree-reference':
@@ -1682,11 +1684,11 @@
return self._iter_next()
# Start expanding more conservatively, adding paths the user may not
# have intended but required for consistent deltas.
- self.doing_exact_expansion = 1
- if not self.next_exact_files:
- self.next_exact_files = self._next_exact_files()
- while self.next_exact_files:
- result, changed = self.next_exact_files.pop()
+ self.doing_consistency_expansion = 1
+ if not self._pending_consistent_entries:
+ self._pending_consistent_entries = self._next_consistent_entries()
+ while self._pending_consistent_entries:
+ result, changed = self._pending_consistent_entries.pop()
if changed is not None:
return result
raise StopIteration()
@@ -1847,7 +1849,7 @@
except StopIteration:
self.current_dir_info = None
- cdef object _next_exact_files(self):
+ cdef object _next_consistent_entries(self):
"""Grabs the next specific file parent case to consider.
:return: A list of the results, each of which is as for _process_entry.
@@ -1858,12 +1860,12 @@
# Even in extremely large trees this should be modest, so currently
# no attempt is made to optimise.
path_utf8 = self.search_specific_file_parents.pop()
+ if path_utf8 in self.searched_exact_paths:
+ # We've examined this path.
+ continue
if osutils.is_inside_any(self.searched_specific_files, path_utf8):
# We've examined this path.
continue
- if path_utf8 in self.searched_exact_paths:
- # We've examined this path.
- continue
path_entries = self.state._entries_for_path(path_utf8)
# We need either one or two entries. If the path in
# self.target_index has moved (so the entry in source_index is in
@@ -1936,7 +1938,7 @@
continue
# Path of the entry itself.
self.search_specific_file_parents.add(
- '/'.join(entry[0][:2]))
+ self.pathjoin(*entry[0][:2]))
if changed or self.include_unchanged:
results.append((result, changed))
self.searched_exact_paths.add(path_utf8)
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py 2009-07-29 04:57:50 +0000
+++ b/bzrlib/dirstate.py 2009-08-03 03:36:47 +0000
@@ -3925,8 +3925,9 @@
# included.
continue
# Path of the entry itself.
+
self.search_specific_file_parents.add(
- '/'.join(entry[0][:2]))
+ osutils.pathjoin(*entry[0][:2]))
if changed or self.include_unchanged:
yield result
self.searched_exact_paths.add(path_utf8)
=== modified file 'bzrlib/tests/per_intertree/test_compare.py'
--- a/bzrlib/tests/per_intertree/test_compare.py 2009-07-29 04:57:50 +0000
+++ b/bzrlib/tests/per_intertree/test_compare.py 2009-08-03 03:36:47 +0000
@@ -37,8 +37,6 @@
# -> that is, when the renamed parent is not processed by the function.
# TODO: test items are only emitted once when a specific_files list names a dir
# whose parent is now a child.
-# TODO: test specific_files when the target tree has a file and the source a
-# dir with children, same id and same path.
# TODO: test comparisons between trees with different root ids. mbp 20070301
#
# TODO: More comparisons between trees with subtrees in different states.
@@ -738,6 +736,57 @@
(False, False))],
self.do_iter_changes(tree1, tree2))
+ def test_specific_content_modification_grabs_parents(self):
+ # WHen the only direct change to a specified file is a content change,
+ # and its in a reparented subtree, the parents are grabbed.
+ tree1 = self.make_branch_and_tree('1')
+ tree1.mkdir('changing', 'parent-id')
+ tree1.mkdir('changing/unchanging', 'mid-id')
+ tree1.add(['changing/unchanging/file'], ['file-id'], ['file'])
+ tree1.put_file_bytes_non_atomic('file-id', 'a file')
+ tree2 = self.make_to_branch_and_tree('2')
+ tree2.set_root_id(tree1.get_root_id())
+ tree2.mkdir('changed', 'parent-id')
+ tree2.mkdir('changed/unchanging', 'mid-id')
+ tree2.add(['changed/unchanging/file'], ['file-id'], ['file'])
+ tree2.put_file_bytes_non_atomic('file-id', 'changed content')
+ tree1, tree2 = self.mutable_trees_to_test_trees(self, tree1, tree2)
+ # parent-id has changed, as has file-id
+ root_id = tree1.path2id('')
+ self.assertEqualIterChanges(
+ [self.renamed(tree1, tree2, 'parent-id', False),
+ self.renamed(tree1, tree2, 'file-id', True)],
+ self.do_iter_changes(tree1, tree2,
+ specific_files=['changed/unchanging/file']))
+
+ def test_specific_content_modification_grabs_parents_root_changes(self):
+ # WHen the only direct change to a specified file is a content change,
+ # and its in a reparented subtree, the parents are grabbed, even if
+ # that includes the root.
+ tree1 = self.make_branch_and_tree('1')
+ tree1.set_root_id('old')
+ tree1.mkdir('changed', 'parent-id')
+ tree1.mkdir('changed/unchanging', 'mid-id')
+ tree1.add(['changed/unchanging/file'], ['file-id'], ['file'])
+ tree1.put_file_bytes_non_atomic('file-id', 'a file')
+ tree2 = self.make_to_branch_and_tree('2')
+ tree2.set_root_id('new')
+ tree2.mkdir('changed', 'parent-id')
+ tree2.mkdir('changed/unchanging', 'mid-id')
+ tree2.add(['changed/unchanging/file'], ['file-id'], ['file'])
+ tree2.put_file_bytes_non_atomic('file-id', 'changed content')
+ tree1, tree2 = self.mutable_trees_to_locked_test_trees(tree1, tree2)
+ # old is gone, new is added, parent-id has changed(reparented), as has
+ # file-id(content)
+ root_id = tree1.path2id('')
+ self.assertEqualIterChanges(
+ [self.renamed(tree1, tree2, 'parent-id', False),
+ self.added(tree2, 'new'),
+ self.deleted(tree1, 'old'),
+ self.renamed(tree1, tree2, 'file-id', True)],
+ self.do_iter_changes(tree1, tree2,
+ specific_files=['changed/unchanging/file']))
+
def test_specific_with_rename_under_new_dir_reports_new_dir(self):
tree1 = self.make_branch_and_tree('1')
tree2 = self.make_to_branch_and_tree('2')
=== modified file 'bzrlib/tree.py'
--- a/bzrlib/tree.py 2009-07-29 04:57:50 +0000
+++ b/bzrlib/tree.py 2009-08-03 03:36:47 +0000
@@ -1209,10 +1209,10 @@
old_entry, new_entry)
else:
changes = True
+ # Get this parents parent to examine.
+ new_parent_id = result[4][1]
+ precise_file_ids.add(new_parent_id)
if changes:
- # Get this parents parent.
- new_parent_id = result[4][1]
- precise_file_ids.add(new_parent_id)
if (result[6][0] == 'directory' and
result[6][1] != 'directory'):
# This stopped being a directory, the old children have
More information about the bazaar-commits
mailing list