Rev 4528: Handle deltas with new paths not matching the actual path. in http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta

Robert Collins robertc at robertcollins.net
Mon Jul 13 05:02:38 BST 2009


At http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta

------------------------------------------------------------
revno: 4528
revision-id: robertc at robertcollins.net-20090713040233-u4y2zlbahqwp21f2
parent: robertc at robertcollins.net-20090713011624-s41u6bywgdm7c0of
committer: Robert Collins <robertc at robertcollins.net>
branch nick: apply-inventory-delta
timestamp: Mon 2009-07-13 14:02:33 +1000
message:
  Handle deltas with new paths not matching the actual path.
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2009-07-13 01:16:24 +0000
+++ b/bzrlib/dirstate.py	2009-07-13 04:02:33 +0000
@@ -1297,6 +1297,7 @@
             inventory._check_delta_unique_new_paths(
             inventory._check_delta_ids_match_entry(delta))), reverse=True):
             if (file_id in insertions) or (file_id in removals):
+                self._changes_aborted = True
                 raise errors.InconsistentDelta(old_path or new_path, file_id,
                     "repeated file_id")
             if old_path is not None:
@@ -1336,10 +1337,19 @@
                                                   child_basename)
                     insertions[child[0][2]] = (key, minikind, executable,
                                                fingerprint, new_child_path)
-        self._apply_removals(removals.values())
-        self._apply_insertions(insertions.values())
-        # Validate parents
-        self._after_delta_check_parents(parents, 0)
+        try:
+            self._apply_removals(removals.values())
+            self._apply_insertions(insertions.values())
+            # Validate parents
+            self._after_delta_check_parents(parents, 0)
+        except errors.BzrError, e:
+            if 'integrity error' not in str(e):
+                raise
+            # _get_entry raises BzrError when a request is inconsistent; we
+            # want such errors to be shown as InconsistentDelta - and that 
+            # fits the behaviour we trigger.
+            self._changes_aborted = True
+            raise errors.InconsistentDeltaDelta(delta, "error from _get_entry.")
 
     def _apply_removals(self, removals):
         for path in sorted(removals, reverse=True):

=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py	2009-07-10 05:18:29 +0000
+++ b/bzrlib/inventory.py	2009-07-13 04:02:33 +0000
@@ -1169,6 +1169,9 @@
             except AttributeError:
                 raise errors.InconsistentDelta(new_path, new_entry.file_id,
                     "Parent is not a directory.")
+            if self.id2path(new_entry.file_id) != new_path:
+                raise errors.InconsistentDelta(new_path, new_entry.file_id,
+                    "New path is not consistent with parent path.")
         if len(children):
             # Get the parent id that was deleted
             parent_id, children = children.popitem()
@@ -1254,12 +1257,11 @@
         To add  a file to a branch ready to be committed, use Branch.add,
         which calls this.
 
-        Returns the new entry object.
+        :return: entry
         """
         if entry.file_id in self._byid:
             raise errors.DuplicateFileId(entry.file_id,
                                          self._byid[entry.file_id])
-
         if entry.parent_id is None:
             self.root = entry
         else:
@@ -1590,6 +1592,7 @@
           copied to and updated for the result.
         :return: The new CHKInventory.
         """
+        split = osutils.split
         result = CHKInventory(self._search_key_name)
         if propagate_caches:
             # Just propagate the path-to-fileid cache for now
@@ -1630,7 +1633,8 @@
         inventory_delta = _check_delta_unique_new_paths(inventory_delta)
         # Check for entries that don't match the fileid
         inventory_delta = _check_delta_ids_match_entry(inventory_delta)
-        # All changed entries need to have their parents be directories.
+        # All changed entries need to have their parents be directories and be
+        # at the right path. This set contains (path, id) tuples.
         parents = set()
         for old_path, new_path, file_id, entry in inventory_delta:
             # file id changes
@@ -1652,7 +1656,7 @@
                 # Update caches. It's worth doing this whether
                 # we're propagating the old caches or not.
                 result._path_to_fileid_cache[new_path] = file_id
-                parents.add(entry.parent_id)
+                parents.add((split(new_path)[0], entry.parent_id))
             if old_path is None:
                 old_key = None
             else:
@@ -1678,8 +1682,8 @@
         result.id_to_entry.apply_delta(id_to_entry_delta)
         if parent_id_basename_delta:
             result.parent_id_basename_to_file_id.apply_delta(parent_id_basename_delta)
-        parents.discard(None)
-        for parent in parents:
+        parents.discard(('', None))
+        for parent_path, parent in parents:
             try:
                 if result[parent].kind != 'directory':
                     raise errors.InconsistentDelta(result.id2path(parent), parent,
@@ -1687,6 +1691,9 @@
             except errors.NoSuchId:
                 raise errors.InconsistentDelta("<unknown>", parent,
                     "Parent is not present in resulting inventory.")
+            if result.path2id(parent_path) != parent:
+                raise errors.InconsistentDelta(parent_path, parent,
+                    "Parent has wrong path %r." % result.path2id(parent_path))
         return result
 
     @classmethod

=== modified file 'bzrlib/tests/test_inv.py'
--- a/bzrlib/tests/test_inv.py	2009-07-13 01:16:24 +0000
+++ b/bzrlib/tests/test_inv.py	2009-07-13 04:02:33 +0000
@@ -54,10 +54,12 @@
             'apply_delta':apply_inventory_Repository_add_inventory_by_delta,
             'format':format}))
     for format in workingtree_formats():
-        scenarios.append((str(format.__class__.__name__), {
+        scenarios.append(
+            (str(format.__class__.__name__) + ".update_basis_by_delta", {
             'apply_delta':apply_inventory_WT_basis,
             'format':format}))
-        scenarios.append((str(format.__class__.__name__), {
+        scenarios.append(
+            (str(format.__class__.__name__) + ".apply_inventory_delta", {
             'apply_delta':apply_inventory_WT,
             'format':format}))
     return multiply_tests(to_adapt, scenarios, result)
@@ -360,6 +362,24 @@
         self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
             inv, delta)
 
+    def test_new_parent_path_has_wrong_id(self):
+        inv = self.get_empty_inventory()
+        parent1 = inventory.InventoryDirectory('p-1', 'dir', inv.root.file_id)
+        parent1.revision = 'result'
+        parent2 = inventory.InventoryDirectory('p-2', 'dir2', inv.root.file_id)
+        parent2.revision = 'result'
+        file1 = inventory.InventoryFile('id', 'path', 'p-2')
+        file1.revision = 'result'
+        file1.text_size = 0
+        file1.text_sha1 = ""
+        inv.add(parent1)
+        inv.add(parent2)
+        # This delta claims that file1 is at dir/path, but actually its at
+        # dir2/path if you follow the inventory parent structure.
+        delta = [(None, u'dir/path', 'id', file1)]
+        self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
+            inv, delta)
+
 
 class TestInventoryEntry(TestCase):
 

=== modified file 'doc/developers/inventory.txt'
--- a/doc/developers/inventory.txt	2009-07-02 07:22:27 +0000
+++ b/doc/developers/inventory.txt	2009-07-13 04:02:33 +0000
@@ -554,6 +554,7 @@
    (Wrong path)
  - An entry whose parent is not a directory. (Under non-directory).
  - An entry that is internally inconsistent.
+ - An entry that is already present in the tree (Duplicate id)
 
 Known causes of inconsistency:
  - A 'new' entry which the inventory already has - when this is a directory




More information about the bazaar-commits mailing list