Rev 5869: Cleanup dirstate a bit. in http://bazaar.launchpad.net/~jameinel/bzr/2.4-set-parent-trees-delta-282941
John Arbash Meinel
john at arbash-meinel.com
Thu May 19 10:24:18 UTC 2011
At http://bazaar.launchpad.net/~jameinel/bzr/2.4-set-parent-trees-delta-282941
------------------------------------------------------------
revno: 5869
revision-id: john at arbash-meinel.com-20110519102409-m3fmucz042cvc52q
parent: john at arbash-meinel.com-20110519100207-6xc2q4sz3301n2yc
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.4-set-parent-trees-delta-282941
timestamp: Thu 2011-05-19 12:24:09 +0200
message:
Cleanup dirstate a bit.
Lots of places were fumbling around trying to raise InconsistentDelta,
not all of them were properly setting _changes_aborted=True.
So force them all through a helper. Also gives a little bit more room
per line, though from what I can see, the reasons are always long
enough to wrap to the next line anyway.
-------------- next part --------------
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py 2011-05-19 10:02:07 +0000
+++ b/bzrlib/dirstate.py 2011-05-19 10:24:09 +0000
@@ -1360,7 +1360,7 @@
delta = sorted(self._check_delta_is_valid(delta), reverse=True)
for old_path, new_path, file_id, inv_entry in delta:
if (file_id in insertions) or (file_id in removals):
- raise errors.InconsistentDelta(old_path or new_path, file_id,
+ self._raise_invalid(old_path or new_path, file_id,
"repeated file_id")
if old_path is not None:
old_path = old_path.encode('utf-8')
@@ -1369,7 +1369,7 @@
new_ids.add(file_id)
if new_path is not None:
if inv_entry is None:
- raise errors.InconsistentDelta(new_path, file_id,
+ self._raise_invalid(new_path, file_id,
"new_path with no entry")
new_path = new_path.encode('utf-8')
dirname_utf8, basename = osutils.split(new_path)
@@ -1428,16 +1428,13 @@
try:
entry = self._dirblocks[block_i][1][entry_i]
except IndexError:
- self._changes_aborted = True
- raise errors.InconsistentDelta(path, file_id,
+ self._raise_invalid(path, file_id,
"Wrong path for old path.")
if not f_present or entry[1][0][0] in 'ar':
- self._changes_aborted = True
- raise errors.InconsistentDelta(path, file_id,
+ self._raise_invalid(path, file_id,
"Wrong path for old path.")
if file_id != entry[0][2]:
- self._changes_aborted = True
- raise errors.InconsistentDelta(path, file_id,
+ self._raise_invalid(path, file_id,
"Attempt to remove path has wrong id - found %r."
% entry[0][2])
self._make_absent(entry)
@@ -1453,8 +1450,7 @@
# be due to it being in a parent tree, or a corrupt delta.
for child_entry in self._dirblocks[block_i][1]:
if child_entry[1][0][0] not in ('r', 'a'):
- self._changes_aborted = True
- raise errors.InconsistentDelta(path, entry[0][2],
+ self._raise_invalid(path, entry[0][2],
"The file id was deleted but its children were "
"not deleted.")
@@ -1464,8 +1460,7 @@
self.update_minimal(key, minikind, executable, fingerprint,
path_utf8=path_utf8)
except errors.NotVersionedError:
- self._changes_aborted = True
- raise errors.InconsistentDelta(path_utf8.decode('utf8'), key[2],
+ self._raise_invalid(path_utf8.decode('utf8'), key[2],
"Missing parent")
def update_basis_by_delta(self, delta, new_revid):
@@ -1527,13 +1522,13 @@
new_ids = set()
for old_path, new_path, file_id, inv_entry in delta:
if inv_entry is not None and file_id != inv_entry.file_id:
- raise errors.InconsistentDelta(new_path, file_id,
+ self._raise_invalid(new_path, file_id,
"mismatched entry file_id %r" % inv_entry)
if new_path is None:
new_path_utf8 = None
else:
if inv_entry is None:
- raise errors.InconsistentDelta(new_path, file_id,
+ self._raise_invalid(new_path, file_id,
"new_path with no entry")
new_path_utf8 = encode(new_path)
# note the parent for validation
@@ -1646,6 +1641,10 @@
"This file_id is new in the delta but already present in "
"the target")
+ def _raise_invalid(self, path, file_id, reason):
+ self._changes_aborted = True
+ raise errors.InconsistentDelta(path, file_id, reason)
+
def _update_basis_apply_adds(self, adds, rename_targets):
"""Apply a sequence of adds to tree 1 during update_basis_by_delta.
@@ -1675,7 +1674,7 @@
entry_index, present = self._find_entry_index(entry_key, block)
if real_add:
if old_path is not None:
- raise errors.InconsistentDelta(new_path, file_id,
+ self._raise_invalid(new_path, file_id,
'considered a real add but still had old_path at %s'
% (old_path,))
if present:
@@ -1686,8 +1685,7 @@
elif basis_kind == 'r':
raise NotImplementedError()
else:
- self._changes_aborted = True
- raise errors.InconsistentDelta(new_path, file_id,
+ self._raise_invalid(new_path, file_id,
"An entry was marked as a new add"
" but the basis target already existed")
else:
@@ -1722,7 +1720,7 @@
# We found a record, which was not *this* record,
# which matches the file_id, but is not actually
# present. Something seems *really* wrong.
- raise errors.InconsistentDelta(new_path, file_id,
+ self._raise_invalid(new_path, file_id,
"We found a tree0 entry that doesnt make sense")
# Now, we've found a tree0 entry which matches the file_id
# but is at a different location. So update them to be
@@ -1748,13 +1746,11 @@
# the entry for this file_id must be in tree 0.
entry = self._get_entry(0, file_id, new_path)
if entry[0] is None or entry[0][2] != file_id:
- self._changes_aborted = True
- raise errors.InconsistentDelta(new_path, file_id,
+ self._raise_invalid(new_path, file_id,
'working tree does not contain new entry')
if (entry[1][0][0] in absent or
entry[1][1][0] in absent):
- self._changes_aborted = True
- raise errors.InconsistentDelta(new_path, file_id,
+ self._raise_invalid(new_path, file_id,
'changed considered absent')
entry[1][1] = new_details
@@ -1778,22 +1774,19 @@
null = DirState.NULL_PARENT_DETAILS
for old_path, new_path, file_id, _, real_delete in deletes:
if real_delete != (new_path is None):
- self._changes_aborted = True
- raise errors.InconsistentDelta("bad delete delta")
+ self._raise_invalid(old_path, file_id, "bad delete delta")
# the entry for this file_id must be in tree 1.
dirname, basename = osutils.split(old_path)
block_index, entry_index, dir_present, file_present = \
self._get_block_entry_index(dirname, basename, 1)
if not file_present:
- self._changes_aborted = True
- raise errors.InconsistentDelta(old_path, file_id,
+ self._raise_invalid(old_path, file_id,
'basis tree does not contain removed entry')
entry = self._dirblocks[block_index][1][entry_index]
# The state of the entry in the 'active' WT
active_kind = entry[1][0][0]
if entry[0][2] != file_id:
- self._changes_aborted = True
- raise errors.InconsistentDelta(old_path, file_id,
+ self._raise_invalid(old_path, file_id,
'mismatched file_id in tree 1')
if real_delete:
# This entry is being completely removed from the basis tree.
@@ -1811,10 +1804,10 @@
# tree0 has a rename record, tree1 should have the reverse
# rename record.
if active_entry[1][1][0] != 'r':
- raise errors.InconsistentDelta(old_path, file_id,
+ self._raise_invalid(old_path, file_id,
"Dirstate did not have matching rename entries")
elif active_entry[1][0][0] in 'ar':
- raise errors.InconsistentDelta(old_path, file_id,
+ self._raise_invalid(old_path, file_id,
"Dirstate had a rename pointing at an inactive"
" tree0")
active_entry[1][1] = null
@@ -1831,8 +1824,7 @@
# be due to it being in a parent tree, or a corrupt delta.
for child_entry in self._dirblocks[block_i][1]:
if child_entry[1][1][0] not in ('r', 'a'):
- self._changes_aborted = True
- raise errors.InconsistentDelta(old_path, entry[0][2],
+ self._raise_invalid(old_path, entry[0][2],
"The file id was deleted but its children were "
"not deleted.")
else:
@@ -1870,13 +1862,11 @@
# has the right file id.
entry = self._get_entry(index, file_id, dirname_utf8)
if entry[1] is None:
- self._changes_aborted = True
- raise errors.InconsistentDelta(dirname_utf8.decode('utf8'),
+ self._raise_invalid(dirname_utf8.decode('utf8'),
file_id, "This parent is not present.")
# Parents of things must be directories
if entry[1][index][0] != 'd':
- self._changes_aborted = True
- raise errors.InconsistentDelta(dirname_utf8.decode('utf8'),
+ self._raise_invalid(dirname_utf8.decode('utf8'),
file_id, "This parent is not a directory.")
def _observed_sha1(self, entry, sha1, stat_value,
@@ -3014,7 +3004,7 @@
# This entry has the same path (but a different id) as
# the new entry we're adding, and is present in ths
# tree.
- raise errors.InconsistentDelta(
+ self._raise_invalid(
("%s/%s" % key[0:2]).decode('utf8'), key[2],
"Attempt to add item at path already occupied by "
"id %r" % entry[0][2])
=== modified file 'bzrlib/tests/test_dirstate.py'
--- a/bzrlib/tests/test_dirstate.py 2011-05-19 10:02:07 +0000
+++ b/bzrlib/tests/test_dirstate.py 2011-05-19 10:24:09 +0000
@@ -2444,33 +2444,31 @@
class TestUpdateBasisByDelta(tests.TestCase):
+ def path_to_ie(self, path, file_id, rev_id, dir_ids):
+ if path.endswith('/'):
+ is_dir = True
+ path = path[:1]
+ else:
+ is_dir = False
+ dirname, basename = osutils.split(path)
+ dir_id = dir_ids[dirname]
+ if is_dir:
+ ie = inventory.InventoryDirectory(file_id, basename, dir_id)
+ dir_ids[path] = dir_id
+ else:
+ ie = inventory.InventoryFile(file_id, basename, dir_id)
+ ie.text_size = 0
+ ie.text_sha1 = ''
+ ie.revision = rev_id
+ return ie
+
def create_tree_from_shape(self, rev_id, shape):
dir_ids = {'': 'root-id'}
inv = inventory.Inventory('root-id', rev_id)
for path, file_id in shape:
- if path.endswith('/'):
- is_dir = True
- path = path[:1]
- else:
- is_dir = False
- dirname, basename = osutils.split(path)
- dir_id = dir_ids[dirname]
- if is_dir:
- ie = inventory.InventoryDirectory(file_id, basename, dir_id)
- else:
- ie = inventory.InventoryFile(file_id, basename, dir_id)
- ie.text_size = 0
- ie.text_sha1 = ''
- ie.revision = rev_id
- inv.add(ie)
+ inv.add(self.path_to_ie(path, file_id, rev_id, dir_ids))
return revisiontree.InventoryRevisionTree(_Repo(), inv, rev_id)
- def create_revs(self, active_shape, basis_shape, target_shape):
- rt_active = self.create_tree_from_shape('active', active_shape)
- rt_basis = self.create_tree_from_shape('basis', basis_shape)
- rt_target = self.create_tree_from_shape('target', target_shape)
- return rt_active, rt_basis, rt_target
-
def create_empty_dirstate(self):
fd, path = tempfile.mkstemp(prefix='bzr-dirstate')
self.addCleanup(os.remove, path)
@@ -2479,6 +2477,22 @@
self.addCleanup(state.unlock)
return state
+ def create_inv_delta(self, delta, rev_id):
+ """Translate a 'delta shape' into an actual InventoryDelta"""
+ dir_ids = {'': 'root-id'}
+ inv_delta = []
+ for old_path, new_path, file_id in delta:
+ if old_path is not None and old_path.endswith('/'):
+ # Don't have to actually do anything for this, because only
+ # new_path creates InventoryEntries
+ old_path = old_path[:1]
+ if new_path is None: # Delete
+ inv_delta.append((old_path, None, file_id, None))
+ continue
+ ie = self.path_to_ie(new_path, file_id, rev_id, dir_ids)
+ inv_delta.append((old_path, new_path, file_id, ie))
+ return inv_delta
+
def assertUpdate(self, active, basis, target):
"""Assert that update_basis_by_delta works how we want.
@@ -2487,8 +2501,9 @@
and assert that the DirState is still valid, and that its stored
content matches the target_shape.
"""
- (active_tree, basis_tree,
- target_tree) = self.create_revs(active, basis, target)
+ active_tree = self.create_tree_from_shape('active', active)
+ basis_tree = self.create_tree_from_shape('basis', basis)
+ target_tree = self.create_tree_from_shape('target', target)
state = self.create_empty_dirstate()
state.set_state_from_scratch(active_tree.inventory,
[('basis', basis_tree)], [])
@@ -2508,6 +2523,28 @@
self.assertEqual(state2._dirblocks, state._dirblocks)
return state
+ def assertBadDelta(self, active, basis, delta):
+ """Test that we raise InconsistentDelta when appropriate.
+
+ :param active: The active tree shape
+ :param basis: The basis tree shape
+ :param delta: A description of the delta to apply. Similar to the form
+ for regular inventory deltas, but omitting the InventoryEntry.
+ So adding a file is: (None, 'path', 'file-id')
+ Adding a directory is: (None, 'path/', 'dir-id')
+ Renaming a dir is: ('old/', 'new/', 'dir-id')
+ etc.
+ """
+ active_tree = self.create_tree_from_shape('active', active)
+ basis_tree = self.create_tree_from_shape('basis', basis)
+ inv_delta = self.create_inv_delta(delta, 'target')
+ state = self.create_empty_dirstate()
+ state.set_state_from_scratch(active_tree.inventory,
+ [('basis', basis_tree)], [])
+ self.assertRaises(errors.InconsistentDelta,
+ state.update_basis_by_delta, delta, 'target')
+ self.assertTrue(state._changes_aborted)
+
def test_remove_file_matching_active_state(self):
state = self.assertUpdate(
active=[],
=== modified file 'bzrlib/tests/test_inv.py'
--- a/bzrlib/tests/test_inv.py 2011-05-18 19:28:37 +0000
+++ b/bzrlib/tests/test_inv.py 2011-05-19 10:24:09 +0000
@@ -622,6 +622,7 @@
parent2.revision = 'result'
inv.add(parent1)
delta = [(None, u'dir1', 'p-2', parent2)]
+ self.apply_delta(self, inv, delta)
self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
inv, delta)
More information about the bazaar-commits
mailing list