Rev 4510: Parents used in a delta must be directories. in http://people.ubuntu.com/~robertc/baz2.0/pending/apply-inventory-delta

Robert Collins robertc at robertcollins.net
Thu Jul 9 06:32:53 BST 2009


At http://people.ubuntu.com/~robertc/baz2.0/pending/apply-inventory-delta

------------------------------------------------------------
revno: 4510
revision-id: robertc at robertcollins.net-20090709053225-xzyadu36hda1w0az
parent: robertc at robertcollins.net-20090708022539-lse8fiw0zbmdhgu0
committer: Robert Collins <robertc at robertcollins.net>
branch nick: apply-inventory-delta
timestamp: Thu 2009-07-09 15:32:25 +1000
message:
  Parents used in a delta must be directories.
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2009-07-08 02:25:39 +0000
+++ b/bzrlib/dirstate.py	2009-07-09 05:32:25 +0000
@@ -1285,7 +1285,8 @@
         removals = {}
         for old_path, new_path, file_id, inv_entry in sorted(delta, reverse=True):
             if (file_id in insertions) or (file_id in removals):
-                raise AssertionError("repeated file id in delta %r" % (file_id,))
+                raise errors.InconsistentDelta(old_path or new_path, file_id,
+                    "repeated file_id")
             if old_path is not None:
                 old_path = old_path.encode('utf-8')
                 removals[file_id] = old_path
@@ -1399,6 +1400,9 @@
         # At the same time, to reduce interface friction we convert the input
         # inventory entries to dirstate.
         root_only = ('', '')
+        # Accumulate parent references (path and id), to check for parentless
+        # items or items placed under files/links/tree-references.
+        parents = 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,
@@ -1406,6 +1410,9 @@
             if old_path is None:
                 adds.append((None, encode(new_path), file_id,
                     inv_to_entry(inv_entry), True))
+                # note the parent for validation
+                dirname, basename = osutils.split(new_path)
+                parents.add((dirname, inv_entry.parent_id))
             elif new_path is None:
                 deletes.append((encode(old_path), None, file_id, None, True))
             elif (old_path, new_path) != root_only:
@@ -1450,6 +1457,9 @@
                         (source_path, target_path, entry[0][2], None, False))
                 deletes.append(
                     (encode(old_path), new_path, file_id, None, False))
+                # note the parent for validation
+                dirname, basename = osutils.split(new_path)
+                parents.add((dirname, inv_entry.parent_id))
             else:
                 # changes to just the root should not require remove/insertion
                 # of everything.
@@ -1463,12 +1473,17 @@
             self._update_basis_apply_adds(adds)
             # Apply in-situ changes.
             self._update_basis_apply_changes(changes)
-        except errors.BzrError:
+            # Validate parents
+            self._update_basis_check_parents(parents)
+        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. Partof this is driven by dirstate
             # only supporting deltas that turn the basis into a closer fit to
             # the active tree.
+            self._changes_aborted = True
             raise errors.InconsistentDeltaDelta(delta, "error from _get_entry.")
 
         self._dirblock_state = DirState.IN_MEMORY_MODIFIED
@@ -1584,6 +1599,18 @@
                     # it is being resurrected here, so blank it out temporarily.
                     self._dirblocks[block_index][1][entry_index][1][1] = null
 
+    def _update_basis_check_parents(self, parents):
+        """Check that parents required by the delta are all intact."""
+        for dirname, file_id in parents:
+            # Get the entry - the ensures that file_id, dirname exists and has
+            # the right file id.
+            entry = self._get_entry(1, file_id, dirname)
+            # Parents of things must be directories
+            if entry[1][1][0] != 'd':
+                self._changes_aborted = True
+                raise errors.InconsistentDelta(dirname, file_id,
+                    "This parent is not a directory.")
+
     def _observed_sha1(self, entry, sha1, stat_value,
         _stat_to_minikind=_stat_to_minikind, _pack_stat=pack_stat):
         """Note the sha1 of a file.

=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py	2009-07-08 02:25:39 +0000
+++ b/bzrlib/inventory.py	2009-07-09 05:32:25 +0000
@@ -1166,7 +1166,11 @@
                 replacement.revision = new_entry.revision
                 replacement.children = children.pop(replacement.file_id, {})
                 new_entry = replacement
-            self.add(new_entry)
+            try:
+                self.add(new_entry)
+            except AttributeError:
+                raise errors.InconsistentDelta(new_path, new_entry.file_id,
+                    "Parent is not a directory.")
         if len(children):
             # Get the parent id that was deleted
             parent_id, children = children.popitem()
@@ -1629,6 +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.
+        parents = set()
         for old_path, new_path, file_id, entry in inventory_delta:
             # file id changes
             if new_path == '':
@@ -1649,6 +1655,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)
             if old_path is None:
                 old_key = None
             else:
@@ -1674,6 +1681,10 @@
         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)
+        for parent in parents:
+            if result[parent].kind != 'directory':
+                raise errors.InconsistentDelta(result.id2path(parent), parent,
+                    'Not a directory, but given children')
         return result
 
     @classmethod

=== modified file 'bzrlib/tests/test_inv.py'
--- a/bzrlib/tests/test_inv.py	2009-07-08 02:25:39 +0000
+++ b/bzrlib/tests/test_inv.py	2009-07-09 05:32:25 +0000
@@ -110,10 +110,11 @@
             raise
         else:
             repo.commit_write_group()
+        # Set the basis state as the trees current state
+        tree._write_inventory(basis)
         # This reads basis from the repo and puts it into the tree's local
         # cache, if it has one.
         tree.set_parent_ids(['basis'])
-        inv = tree.inventory
         paths = {}
         parents = set()
         for old, new, id, entry in delta:
@@ -123,9 +124,10 @@
             parents.add(osutils.dirname(new))
         parents = osutils.minimum_path_selection(parents)
         parents.discard('')
-        if parents:
-            # Put place holders in the tree to permit adding the other entries.
-            import pdb;pdb.set_trace()
+        # Put place holders in the tree to permit adding the other entries.
+        for parent in parents:
+            if not tree.path2id(parent):
+                import pdb;pdb.set_trace()
         if paths:
             # Many deltas may cause this mini-apply to fail, but we want to see what
             # the delta application code says, not the prep that we do to deal with 
@@ -292,6 +294,21 @@
         self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
             inv, delta)
 
+    def test_parent_is_not_directory(self):
+        inv = self.get_empty_inventory()
+        file1 = inventory.InventoryFile('id1', 'path', inv.root.file_id)
+        file1.revision = 'result'
+        file1.text_size = 0
+        file1.text_sha1 = ""
+        file2 = inventory.InventoryFile('id2', 'path2', 'id1')
+        file2.revision = 'result'
+        file2.text_size = 0
+        file2.text_sha1 = ""
+        inv.add(file1)
+        delta = [(None, 'path/path2', 'id2', file2)]
+        self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
+            inv, delta)
+
 
 class TestInventoryEntry(TestCase):
 




More information about the bazaar-commits mailing list