Rev 2951: Move basis path generation out to the caller of record_entry_contents in more preparation for obtaining it from dirstate. in http://people.ubuntu.com/~robertc/baz2.0/commit-builder

Robert Collins robertc at robertcollins.net
Sat Oct 27 02:31:52 BST 2007


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

------------------------------------------------------------
revno: 2951
revision-id:robertc at robertcollins.net-20071027013146-qr1z925yj15eozug
parent: robertc at robertcollins.net-20071027010654-x58aavbhxcuzg4w2
committer: Robert Collins <robertc at robertcollins.net>
branch nick: commit-builder
timestamp: Sat 2007-10-27 11:31:46 +1000
message:
  Move basis path generation out to the caller of record_entry_contents in more preparation for obtaining it from dirstate.
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 01:06:54 +0000
+++ b/bzrlib/commit.py	2007-10-27 01:31:46 +0000
@@ -659,11 +659,11 @@
         # recorded in their previous state. For more details, see
         # https://lists.ubuntu.com/archives/bazaar/2007q3/028476.html.
         if specific_files:
-            for path, old_ie in self.basis_tree.iter_entries_by_dir():
+            for old_path, old_ie in self.basis_tree.iter_entries_by_dir():
                 if old_ie.file_id in self.builder.new_inventory:
                     # already added - skip.
                     continue
-                if is_inside_any(specific_files, path):
+                if is_inside_any(specific_files, old_path):
                     # was inside the selected path, if not present it has been
                     # deleted so skip.
                     continue
@@ -678,10 +678,12 @@
                 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, old_ie)
+                    ie, self.parent_invs, old_path, self.basis_tree, None,
+                    old_ie, old_path)
                 if version_recorded:
                     self.any_entries_changed = True
-                if delta: self._basis_delta.append(delta)
+                if delta:
+                    self._basis_delta.append(delta)
 
     def _report_and_accumulate_deletes(self):
         # XXX: Could the list of deleted paths and ids be instead taken from
@@ -803,10 +805,15 @@
             ie.revision = None
         try:
             basis_ie = self.parent_invs[0][ie.file_id]
+            # TODO: avoid this id2path call. e.g. by having a iterator that
+            # returns the old path from the dirstate mapping.
+            basis_path = self.parent_invs[0].id2path(ie.file_id)
         except errors.NoSuchId:
             basis_ie = None
+            basis_path = None
         delta, version_recorded = self.builder.record_entry_contents(ie,
-            self.parent_invs, path, self.work_tree, content_summary, basis_ie)
+            self.parent_invs, path, self.work_tree, content_summary, basis_ie,
+            basis_path)
         if delta:
             self._basis_delta.append(delta)
         if version_recorded:

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2007-10-27 01:06:54 +0000
+++ b/bzrlib/repository.py	2007-10-27 01:31:46 +0000
@@ -198,26 +198,27 @@
         # _new_revision_id
         ie.revision = self._new_revision_id
 
-    def _get_delta(self, ie, basis_inv, path, basis_ie):
+    def _get_delta(self, ie, basis_inv, path, basis_ie, basis_path):
         """Get a delta against the basis inventory for ie."""
-        if basis_ie is None:
-            # add
-            return (None, path, ie.file_id, ie)
-        elif ie != basis_ie:
-            # common but altered
-            # 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)
+        if basis_ie is None or ie != basis_ie:
+            # add/changed
+            return (basis_path, path, ie.file_id, ie)
         else:
             # common, unaltered
             return None
 
     def record_entry_contents(self, ie, parent_invs, path, tree,
-        content_summary, basis_ie):
+        content_summary, basis_ie, basis_path):
         """Record the content of ie from tree into the commit if needed.
 
         Side effect: sets ie.revision when unchanged
 
+        The basis_* parameters are used in the calculation of the inventory
+        delta objects returned to the caller. If the caller does not want a
+        delta, or wants the full tree state returned, passing None for every
+        basis parameter will still result in a correct commit but in a delta
+        that may be less than useful.
+
         :param ie: An inventory entry present in the commit.
         :param parent_invs: The inventories of the parent revisions of the
             commit.
@@ -228,7 +229,10 @@
             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.
+        :param basis_entry: An InventoryEntry for this entry in the basis tree
+            or None if the file id was not present.
+        :param basis_path: The path of the the file id in the basis tree or
+            None if it was not present.
         :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.
@@ -265,13 +269,8 @@
                 # repositories that do not version the root set the root's
                 # 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 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)
+                # so emit a delta.
+                delta = (basis_path, path, ie.file_id, ie)
                 return delta, False
             else:
                 # we don't need to commit this, because the caller already
@@ -329,7 +328,8 @@
                     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, basis_ie), False
+                    return self._get_delta(ie, basis_inv, path, basis_ie,
+                        basis_path), False
                 else:
                     # Either there is only a hash change(no hash cache entry,
                     # or same size content change), or there is no change on
@@ -354,13 +354,15 @@
                 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, basis_ie), False
+                return self._get_delta(ie, basis_inv, path, basis_ie,
+                    basis_path), 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, basis_ie), False
+                return self._get_delta(ie, basis_inv, path, basis_ie,
+                    basis_path), False
             lines = []
             self._add_text_to_weave(ie.file_id, lines, heads, None)
         elif kind == 'symlink':
@@ -374,7 +376,8 @@
                 # unchanged, carry over.
                 ie.revision = parent_entry.revision
                 ie.symlink_target = parent_entry.symlink_target
-                return self._get_delta(ie, basis_inv, path, basis_ie), False
+                return self._get_delta(ie, basis_inv, path, basis_ie,
+                    basis_path), False
             ie.symlink_target = current_link_target
             lines = []
             self._add_text_to_weave(ie.file_id, lines, heads, None)
@@ -386,14 +389,15 @@
                 # unchanged, carry over.
                 ie.reference_revision = parent_entry.reference_revision
                 ie.revision = parent_entry.revision
-                return self._get_delta(ie, basis_inv, path, basis_ie), False
+                return self._get_delta(ie, basis_inv, path, basis_ie,
+                    basis_path), 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, basis_ie), True
+        return self._get_delta(ie, basis_inv, path, basis_ie, basis_path), 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 01:06:54 +0000
+++ b/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-10-27 01:31:46 +0000
@@ -52,9 +52,14 @@
                 tree.unlock()
             parent_tree = tree.branch.repository.revision_tree(None)
             parent_invs = [parent_tree.inventory]
+            parent_entry = parent_invs[0]._byid.get(ie.file_id, None)
+            if parent_entry is None:
+                parent_path = None
+            else:
+                parent_path = parent_tree.id2path(ie.file_id)
             builder.record_entry_contents(ie, parent_invs, '', tree,
                 tree.path_content_summary(''),
-                parent_invs[0]._byid.get(ie.file_id, None))
+                parent_entry, parent_path)
 
     def test_finish_inventory(self):
         tree = self.make_branch_and_tree(".")
@@ -133,7 +138,7 @@
             builder = tree.branch.get_commit_builder([])
             self.assertRaises(errors.RootMissing,
                 builder.record_entry_contents, entry, [], 'foo', tree,
-                    tree.path_content_summary('foo'), None)
+                    tree.path_content_summary('foo'), None, None)
             builder.abort()
         finally:
             tree.unlock()
@@ -151,17 +156,16 @@
                     tree.inventory.root.file_id)
             delta, version_recorded = builder.record_entry_contents(
                 ie, [parent_tree.inventory], '', tree,
-                tree.path_content_summary(''), parent_tree.inventory.root)
+                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
             got_new_revision = ie.revision != old_revision_id
             if got_new_revision:
-                self.assertEqual(
-                    ('', '', ie.file_id, ie),
-                    delta)
+                expected_delta = ('', '', ie.file_id, ie)
             else:
-                self.assertEqual(None, delta)
+                expected_delta = None
+            self.assertEqual(expected_delta, delta)
             builder.abort()
         except:
             builder.abort()
@@ -370,20 +374,29 @@
                 parent_invs.append(tree.branch.repository.revision_tree(
                     parent_id).inventory)
             # root
+            parent_entry = parent_invs[0]._byid.get(tree.inventory.root.file_id, None)
+            if parent_entry is None:
+                parent_path = None
+            else:
+                parent_path = parent_tree.id2path(tree.inventory.root.file_id)
             builder.record_entry_contents(
                 inventory.make_entry('directory', '', None,
                     tree.inventory.root.file_id), parent_invs, '', tree,
                     tree.path_content_summary(''),
-                    parent_invs[0]._byid.get(tree.inventory.root.file_id, None)
-                    )
+                    parent_entry, parent_path)
             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)
+                if basis_ie is None:
+                    basis_path = None
+                else:
+                    basis_path = parent_tree.id2path(file_id)
                 return builder.record_entry_contents(ie, parent_invs, path,
-                    tree, tree.path_content_summary(path), basis_ie)
+                    tree, tree.path_content_summary(path), basis_ie,
+                    basis_path)
 
             file_id = tree.path2id(new_name)
             parent_id = tree.inventory[file_id].parent_id



More information about the bazaar-commits mailing list