Rev 2950: Do not look up the basis inventory entry in record_entry_contents, rather pass it in as a step towards removing the basis lookup and using dirstate to answer basis details. in http://people.ubuntu.com/~robertc/baz2.0/commit-builder

Robert Collins robertc at robertcollins.net
Sat Oct 27 02:07:01 BST 2007


At http://people.ubuntu.com/~robertc/baz2.0/commit-builder

------------------------------------------------------------
revno: 2950
revision-id:robertc at robertcollins.net-20071027010654-x58aavbhxcuzg4w2
parent: robertc at robertcollins.net-20071027002911-rzmmo59xmh7gaiw8
committer: Robert Collins <robertc at robertcollins.net>
branch nick: commit-builder
timestamp: Sat 2007-10-27 11:06:54 +1000
message:
  Do not look up the basis inventory entry in record_entry_contents, rather pass it in as a step towards removing the basis lookup and using dirstate to answer basis details.
modified:
  bzrlib/commit.py               commit.py-20050511101309-79ec1a0168e0e825
  bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
  bzrlib/tests/repository_implementations/test_commit_builder.py test_commit_builder.py-20060606110838-76e3ra5slucqus81-1
=== modified file 'bzrlib/commit.py'
--- a/bzrlib/commit.py	2007-10-27 00:01:47 +0000
+++ b/bzrlib/commit.py	2007-10-27 01:06:54 +0000
@@ -678,7 +678,7 @@
                 if len(self.parents) > 1:
                     ie.revision = None
                 delta, version_recorded = self.builder.record_entry_contents(
-                    ie, self.parent_invs, path, self.basis_tree, None)
+                    ie, self.parent_invs, path, self.basis_tree, None, old_ie)
                 if version_recorded:
                     self.any_entries_changed = True
                 if delta: self._basis_delta.append(delta)
@@ -801,8 +801,12 @@
         else:
             ie = existing_ie.copy()
             ie.revision = None
+        try:
+            basis_ie = self.parent_invs[0][ie.file_id]
+        except errors.NoSuchId:
+            basis_ie = None
         delta, version_recorded = self.builder.record_entry_contents(ie,
-            self.parent_invs, path, self.work_tree, content_summary)
+            self.parent_invs, path, self.work_tree, content_summary, basis_ie)
         if delta:
             self._basis_delta.append(delta)
         if version_recorded:

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2007-10-27 00:29:11 +0000
+++ b/bzrlib/repository.py	2007-10-27 01:06:54 +0000
@@ -198,21 +198,22 @@
         # _new_revision_id
         ie.revision = self._new_revision_id
 
-    def _get_delta(self, ie, basis_inv, path):
+    def _get_delta(self, ie, basis_inv, path, basis_ie):
         """Get a delta against the basis inventory for ie."""
-        if ie.file_id not in basis_inv:
+        if basis_ie is None:
             # add
             return (None, path, ie.file_id, ie)
-        elif ie != basis_inv[ie.file_id]:
+        elif ie != basis_ie:
             # common but altered
-            # TODO: avoid tis id2path call.
+            # TODO: avoid this id2path call. e.g. by having a iterator that
+            # returns the old path from the dirstate mapping.
             return (basis_inv.id2path(ie.file_id), path, ie.file_id, ie)
         else:
             # common, unaltered
             return None
 
     def record_entry_contents(self, ie, parent_invs, path, tree,
-        content_summary):
+        content_summary, basis_ie):
         """Record the content of ie from tree into the commit if needed.
 
         Side effect: sets ie.revision when unchanged
@@ -227,6 +228,7 @@
             content - stat, length, exec, sha/link target. This is only
             accessed when the entry has a revision of None - that is when it is
             a candidate to commit.
+        :param basis_ie: An InventoryEntry for this entry in the basis tree.
         :return: A tuple (change_delta, version_recorded). change_delta is 
             an inventory_delta change for this entry against the basis tree of
             the commit, or None if no change occured against the basis tree.
@@ -264,12 +266,12 @@
                 # revision to the new commit even when no change occurs, and
                 # this masks when a change may have occurred against the basis,
                 # so calculate if one happened.
-                if ie.file_id in basis_inv:
+                if basis_ie is None:
+                    # add
+                    delta = (None, path, ie.file_id, ie)
+                else:
                     delta = (basis_inv.id2path(ie.file_id), path,
                         ie.file_id, ie)
-                else:
-                    # add
-                    delta = (None, path, ie.file_id, ie)
                 return delta, False
             else:
                 # we don't need to commit this, because the caller already
@@ -327,7 +329,7 @@
                     ie.text_size = parent_entry.text_size
                     ie.text_sha1 = parent_entry.text_sha1
                     ie.executable = parent_entry.executable
-                    return self._get_delta(ie, basis_inv, path), False
+                    return self._get_delta(ie, basis_inv, path, basis_ie), False
                 else:
                     # Either there is only a hash change(no hash cache entry,
                     # or same size content change), or there is no change on
@@ -352,13 +354,13 @@
                 ie.text_size = parent_entry.text_size
                 ie.text_sha1 = parent_entry.text_sha1
                 ie.executable = parent_entry.executable
-                return self._get_delta(ie, basis_inv, path), False
+                return self._get_delta(ie, basis_inv, path, basis_ie), False
         elif kind == 'directory':
             if not store:
                 # all data is meta here, nothing specific to directory, so
                 # carry over:
                 ie.revision = parent_entry.revision
-                return self._get_delta(ie, basis_inv, path), False
+                return self._get_delta(ie, basis_inv, path, basis_ie), False
             lines = []
             self._add_text_to_weave(ie.file_id, lines, heads, None)
         elif kind == 'symlink':
@@ -372,7 +374,7 @@
                 # unchanged, carry over.
                 ie.revision = parent_entry.revision
                 ie.symlink_target = parent_entry.symlink_target
-                return self._get_delta(ie, basis_inv, path), False
+                return self._get_delta(ie, basis_inv, path, basis_ie), False
             ie.symlink_target = current_link_target
             lines = []
             self._add_text_to_weave(ie.file_id, lines, heads, None)
@@ -384,14 +386,14 @@
                 # unchanged, carry over.
                 ie.reference_revision = parent_entry.reference_revision
                 ie.revision = parent_entry.revision
-                return self._get_delta(ie, basis_inv, path), False
+                return self._get_delta(ie, basis_inv, path, basis_ie), False
             ie.reference_revision = content_summary[3]
             lines = []
             self._add_text_to_weave(ie.file_id, lines, heads, None)
         else:
             raise NotImplementedError('unknown kind')
         ie.revision = self._new_revision_id
-        return self._get_delta(ie, basis_inv, path), True
+        return self._get_delta(ie, basis_inv, path, basis_ie), True
 
     def _add_text_to_weave(self, file_id, new_lines, parents, nostore_sha):
         versionedfile = self.repository.weave_store.get_weave_or_empty(

=== modified file 'bzrlib/tests/repository_implementations/test_commit_builder.py'
--- a/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-10-27 00:29:11 +0000
+++ b/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-10-27 01:06:54 +0000
@@ -51,9 +51,10 @@
             finally:
                 tree.unlock()
             parent_tree = tree.branch.repository.revision_tree(None)
-            parent_invs = [parent_tree]
+            parent_invs = [parent_tree.inventory]
             builder.record_entry_contents(ie, parent_invs, '', tree,
-                tree.path_content_summary(''))
+                tree.path_content_summary(''),
+                parent_invs[0]._byid.get(ie.file_id, None))
 
     def test_finish_inventory(self):
         tree = self.make_branch_and_tree(".")
@@ -132,7 +133,7 @@
             builder = tree.branch.get_commit_builder([])
             self.assertRaises(errors.RootMissing,
                 builder.record_entry_contents, entry, [], 'foo', tree,
-                    tree.path_content_summary('foo'))
+                    tree.path_content_summary('foo'), None)
             builder.abort()
         finally:
             tree.unlock()
@@ -150,7 +151,7 @@
                     tree.inventory.root.file_id)
             delta, version_recorded = builder.record_entry_contents(
                 ie, [parent_tree.inventory], '', tree,
-                tree.path_content_summary(''))
+                tree.path_content_summary(''), parent_tree.inventory.root)
             self.assertFalse(version_recorded)
             # if the repository format recorded a new root revision, that
             # should be in the delta
@@ -194,7 +195,11 @@
             self.record_root(builder, tree)
             builder.finish_inventory()
             rev_id = builder.commit('foo bar')
-        finally:
+        except:
+            builder.abort()
+            tree.unlock()
+            raise
+        else:
             tree.unlock()
         rev_tree = builder.revision_tree()
         # Just a couple simple tests to ensure that it actually follows
@@ -368,14 +373,17 @@
             builder.record_entry_contents(
                 inventory.make_entry('directory', '', None,
                     tree.inventory.root.file_id), parent_invs, '', tree,
-                    tree.path_content_summary(''))
+                    tree.path_content_summary(''),
+                    parent_invs[0]._byid.get(tree.inventory.root.file_id, None)
+                    )
             def commit_id(file_id):
                 old_ie = tree.inventory[file_id]
                 path = tree.id2path(file_id)
                 ie = inventory.make_entry(tree.kind(file_id), old_ie.name,
                     old_ie.parent_id, file_id)
+                basis_ie = parent_tree.inventory._byid.get(file_id, None)
                 return builder.record_entry_contents(ie, parent_invs, path,
-                    tree, tree.path_content_summary(path))
+                    tree, tree.path_content_summary(path), basis_ie)
 
             file_id = tree.path2id(new_name)
             parent_id = tree.inventory[file_id].parent_id



More information about the bazaar-commits mailing list