Rev 2874: * The CommitBuilder method ``record_entry_contents`` now returns summary in http://people.ubuntu.com/~robertc/baz2.0/commit-builder

Robert Collins robertc at robertcollins.net
Fri Sep 28 08:06:43 BST 2007


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

------------------------------------------------------------
revno: 2874
revision-id: robertc at robertcollins.net-20070928070556-lxsw7332u3kdltwz
parent: robertc at robertcollins.net-20070928012059-sbuwqzv68dlbb9z9
committer: Robert Collins <robertc at robertcollins.net>
branch nick: commit-builder
timestamp: Fri 2007-09-28 17:05:56 +1000
message:
  * The CommitBuilder method ``record_entry_contents`` now returns summary
    information about the effect of the commit on the repository. This tuple
    contains an inventory delta item if the entry changed from the basis, and a
    boolean indicating whether a new file graph node was recorded.
    (Robert Collins)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  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 'NEWS'
--- a/NEWS	2007-09-28 01:20:59 +0000
+++ b/NEWS	2007-09-28 07:05:56 +0000
@@ -163,6 +163,12 @@
    * Knit joining has been enhanced to support plain to annotated conversion
      and annotated to plain conversion. (Ian Clatworthy)
 
+   * The CommitBuilder method ``record_entry_contents`` now returns summary
+     information about the effect of the commit on the repository. This tuple
+     contains an inventory delta item if the entry changed from the basis, and a
+     boolean indicating whether a new file graph node was recorded.
+     (Robert Collins)
+
   TESTING:
 
 

=== modified file 'bzrlib/commit.py'
--- a/bzrlib/commit.py	2007-09-21 04:22:53 +0000
+++ b/bzrlib/commit.py	2007-09-28 07:05:56 +0000
@@ -661,8 +661,9 @@
                 # required after that changes.
                 if len(self.parents) > 1:
                     ie.revision = None
-                if self.builder.record_entry_contents(ie, self.parent_invs, path,
-                    self.basis_tree):
+                delta, version_recorded = self.builder.record_entry_contents(
+                    ie, self.parent_invs, path, self.basis_tree)
+                if version_recorded:
                     self.any_entries_changed = True
 
         # note that deletes have occurred
@@ -764,8 +765,9 @@
         else:
             ie = existing_ie.copy()
             ie.revision = None
-        if self.builder.record_entry_contents(ie, self.parent_invs, 
-            path, self.work_tree):
+        delta, version_recorded = self.builder.record_entry_contents(ie,
+            self.parent_invs, path, self.work_tree)
+        if version_recorded:
             self.any_entries_changed = True
         if report_changes:
             self._report_change(ie, path)

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2007-09-28 01:20:59 +0000
+++ b/bzrlib/repository.py	2007-09-28 07:05:56 +0000
@@ -210,9 +210,12 @@
         :param path: The path the entry is at in the tree.
         :param tree: The tree which contains this entry and should be used to 
         obtain content.
-        :return: True if a new version of the entry has been recorded.
-            (Committing a merge where a file was only changed on the other side
-            will not return True.)
+        :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.
+            version_recorded is True if a new version of the entry has been
+            recorded. For instance, committing a merge where a file was only
+            changed on the other side will return (delta, False).
         """
         if self.new_inventory.root is None:
             if ie.parent_id is not None:
@@ -220,11 +223,38 @@
             self._check_root(ie, parent_invs, tree)
         self.new_inventory.add(ie)
 
+        # TODO: slow, take it out of the inner loop.
+        try:
+            basis_inv = parent_invs[0]
+        except IndexError:
+            basis_inv = Inventory(root_id=None)
+
         # ie.revision is always None if the InventoryEntry is considered
         # for committing. ie.snapshot will record the correct revision 
         # which may be the sole parent if it is untouched.
         if ie.revision is not None:
-            return ie.revision == self._new_revision_id and (path != '' or
+            if self._versioned_root or path != '':
+                # not considered for commit
+                delta = None
+            else:
+                # 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 ie.file_id not in basis_inv:
+                    # add
+                    delta = (None, path, ie.file_id, ie)
+                else:
+                    basis_id = basis_inv[ie.file_id]
+                    if basis_id.name != '':
+                        # not the root
+                        delta = (basis_inv.id2path(ie.file_id), path,
+                            ie.file_id, ie)
+                    else:
+                        # common, unaltered
+                        delta = None
+            # not considered for commit, OR, for non-rich-root 
+            return delta, ie.revision == self._new_revision_id and (path != '' or
                 self._versioned_root)
 
         parent_candiate_entries = ie.parent_candidates(parent_invs)
@@ -236,7 +266,17 @@
         # we are creating a new revision for ie in the history store and
         # inventory.
         ie.snapshot(self._new_revision_id, path, previous_entries, tree, self)
-        return ie.revision == self._new_revision_id and (path != '' or
+        if ie.file_id not in basis_inv:
+            # add
+            delta = (None, path, ie.file_id, ie)
+        elif ie != basis_inv[ie.file_id]:
+            # common but altered
+            delta = (basis_inv.id2path(ie.file_id), path, ie.file_id,
+                ie)
+        else:
+            # common, unaltered
+            delta = None
+        return delta, ie.revision == self._new_revision_id and (path != '' or
             self._versioned_root)
 
     def modified_directory(self, file_id, file_parents):

=== modified file 'bzrlib/tests/repository_implementations/test_commit_builder.py'
--- a/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-09-28 01:20:59 +0000
+++ b/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-09-28 07:05:56 +0000
@@ -16,6 +16,7 @@
 
 """Tests for repository commit builder."""
 
+from copy import copy
 import errno
 import os
 import sys
@@ -145,8 +146,10 @@
         try:
             ie = inventory.make_entry('directory', '', None,
                     tree.inventory.root.file_id)
-            self.assertFalse(builder.record_entry_contents(
-                ie, [parent_tree.inventory], '', tree))
+            delta, version_recorded = builder.record_entry_contents(
+                ie, [parent_tree.inventory], '', tree)
+            self.assertFalse(version_recorded)
+            self.assertEqual(None, delta)
             builder.abort()
         except:
             builder.abort()
@@ -221,7 +224,7 @@
     def _add_commit_check_unchanged(self, tree, name):
         tree.add([name], [name + 'id'])
         rev1 = tree.commit('')
-        rev2 = tree.commit('')
+        rev2 = self.mini_commit(tree, name, name, False, False)
         tree1, tree2 = self._get_revtrees(tree, [rev1, rev2])
         self.assertEqual(rev1, tree1.inventory[name + 'id'].revision)
         self.assertEqual(rev1, tree2.inventory[name + 'id'].revision)
@@ -315,15 +318,37 @@
         tree.add([name], [name + 'id'])
         rev1 = tree.commit('')
         changer()
+        rev2 = self.mini_commit(tree, name, tree.id2path(name + 'id'))
+        tree1, tree2 = self._get_revtrees(tree, [rev1, rev2])
+        self.assertEqual(rev1, tree1.inventory[name + 'id'].revision)
+        self.assertEqual(rev2, tree2.inventory[name + 'id'].revision)
+        self.assertFileAncestry([rev1, rev2], tree, name)
+
+    def mini_commit(self, tree, name, new_name, records_version=True,
+        delta_against_basis=True):
+        """Perform a miniature commit looking for record entry results.
+        
+        :param tree: The tree to commit.
+        :param name: The path in the basis tree of the tree being committed.
+        :param new_name: The path in the tree being committed.
+        :param records_version: True if the commit of new_name is expected to
+            record a new version.
+        :param delta_against_basis: True of the commit of new_name is expected
+            to have a delta against the basis.
+        """
         tree.lock_write()
         try:
             # mini manual commit here so we can check the return of
             # record_entry_contents.
-            builder = tree.branch.get_commit_builder([tree.last_revision()])
+            parent_ids = tree.get_parent_ids()
+            builder = tree.branch.get_commit_builder(parent_ids)
             parent_tree = tree.basis_tree()
             parent_tree.lock_read()
             self.addCleanup(parent_tree.unlock)
             parent_invs = [parent_tree.inventory]
+            for parent_id in parent_ids[1:]:
+                parent_invs.append(tree.branch.repository.revision_tree(
+                    parent_id).inventory)
             # root
             builder.record_entry_contents(
                 inventory.make_entry('directory', '', None,
@@ -335,13 +360,25 @@
                     old_ie.parent_id, file_id)
                 return builder.record_entry_contents(ie, parent_invs, path, tree)
 
-            file_id = name + 'id'
+            file_id = tree.path2id(new_name)
             parent_id = tree.inventory[file_id].parent_id
             if parent_id != tree.inventory.root.file_id:
                 commit_id(parent_id)
             # because a change of some sort is meant to have occurred,
             # recording the entry must return True.
-            self.assertTrue(commit_id(file_id))
+            delta, version_recorded = commit_id(file_id)
+            if records_version:
+                self.assertTrue(version_recorded)
+            else:
+                self.assertFalse(version_recorded)
+            new_entry = builder.new_inventory[file_id]
+            if delta_against_basis:
+                expected_delta = (name, new_name, file_id, new_entry)
+            else:
+                expected_delta = None
+            if expected_delta != delta:
+                import pdb;pdb.set_trace()
+            self.assertEqual(expected_delta, delta)
             builder.finish_inventory()
             rev2 = builder.commit('')
             tree.set_parent_ids([rev2])
@@ -351,10 +388,7 @@
             raise
         else:
             tree.unlock()
-        tree1, tree2 = self._get_revtrees(tree, [rev1, rev2])
-        self.assertEqual(rev1, tree1.inventory[name + 'id'].revision)
-        self.assertEqual(rev2, tree2.inventory[name + 'id'].revision)
-        self.assertFileAncestry([rev1, rev2], tree, name)
+        return rev2
 
     def assertFileAncestry(self, ancestry, tree, name, alt_ancestry=None):
         # all the changes that have occured should be in the ancestry
@@ -403,7 +437,7 @@
         rev2 = self._rename_in_tree(tree1, name)
         rev3 = self._rename_in_tree(tree2, name)
         tree1.merge_from_branch(tree2.branch)
-        rev4 = tree1.commit('')
+        rev4 = self.mini_commit(tree1, 'new_' + name, 'new_' + name)
         tree3, = self._get_revtrees(tree1, [rev4])
         self.assertEqual(rev4, tree3.inventory[name + 'id'].revision)
         # TODO: change this to an assertFileGraph call to check the
@@ -435,7 +469,7 @@
         # change on the other side to merge back
         rev2 = self._rename_in_tree(tree2, name)
         tree1.merge_from_branch(tree2.branch)
-        rev3 = tree1.commit('')
+        rev3 = self.mini_commit(tree1, name, 'new_' + name, False)
         tree3, = self._get_revtrees(tree1, [rev2])
         self.assertEqual(rev2, tree3.inventory[name + 'id'].revision)
         self.assertFileAncestry([rev1, rev2], tree1, name)



More information about the bazaar-commits mailing list