Rev 2826: * Committing a change which is not a merge and does not change the number of in http://people.ubuntu.com/~robertc/baz2.0/record-entry-returns-status

Robert Collins robertc at robertcollins.net
Thu Sep 20 04:44:22 BST 2007


At http://people.ubuntu.com/~robertc/baz2.0/record-entry-returns-status

------------------------------------------------------------
revno: 2826
revision-id: robertc at robertcollins.net-20070920034311-lgjoomiumagdhksn
parent: pqm at pqm.ubuntu.com-20070917005035-cshdkpzbj63id1uw
committer: Robert Collins <robertc at robertcollins.net>
branch nick: record-entry-returns-status
timestamp: Thu 2007-09-20 13:43:11 +1000
message:
  * Committing a change which is not a merge and does not change the number of
    files in the tree is faster by utilising the data about whether files are
    changed to determine if a the tree is unchanged rather than recalculating
    it at the end of the commit process. (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-16 19:29:00 +0000
+++ b/NEWS	2007-09-20 03:43:11 +0000
@@ -16,6 +16,11 @@
    * Commit in quiet mode is now slightly faster as the information to
      output is no longer calculated. (Ian Clatworthy)
 
+   * Committing a change which is not a merge and does not change the number of
+     files in the tree is faster by utilising the data about whether files are
+     changed to determine if a the tree is unchanged rather than recalculating
+     it at the end of the commit process. (Robert Collins)
+
    * Inventory serialisation no longer double-sha's the content.
      (Robert Collins)
 

=== modified file 'bzrlib/commit.py'
--- a/bzrlib/commit.py	2007-09-14 00:13:04 +0000
+++ b/bzrlib/commit.py	2007-09-20 03:43:11 +0000
@@ -241,6 +241,8 @@
                                " parameter is required for commit().")
 
         self.bound_branch = None
+        self.entries_changed = False
+        self.entries_deleted = False
         self.local = local
         self.master_branch = None
         self.master_locked = False
@@ -383,41 +385,6 @@
             return NullCommitReporter()
         return ReportCommitToLog()
 
-    def _any_real_changes(self):
-        """Are there real changes between new_inventory and basis?
-
-        For trees without rich roots, inv.root.revision changes every commit.
-        But if that is the only change, we want to treat it as though there
-        are *no* changes.
-        """
-        new_entries = self.builder.new_inventory.iter_entries()
-        basis_entries = self.basis_inv.iter_entries()
-        new_path, new_root_ie = new_entries.next()
-        basis_path, basis_root_ie = basis_entries.next()
-
-        # This is a copy of InventoryEntry.__eq__ only leaving out .revision
-        def ie_equal_no_revision(this, other):
-            return ((this.file_id == other.file_id)
-                    and (this.name == other.name)
-                    and (this.symlink_target == other.symlink_target)
-                    and (this.text_sha1 == other.text_sha1)
-                    and (this.text_size == other.text_size)
-                    and (this.text_id == other.text_id)
-                    and (this.parent_id == other.parent_id)
-                    and (this.kind == other.kind)
-                    and (this.executable == other.executable)
-                    and (this.reference_revision == other.reference_revision)
-                    )
-        if not ie_equal_no_revision(new_root_ie, basis_root_ie):
-            return True
-
-        for new_ie, basis_ie in zip(new_entries, basis_entries):
-            if new_ie != basis_ie:
-                return True
-
-        # No actual changes present
-        return False
-
     def _check_pointless(self):
         if self.allow_pointless:
             return
@@ -434,7 +401,8 @@
             return
         # If length == 1, then we only have the root entry. Which means
         # that there is no real difference (only the root could be different)
-        if (len(self.builder.new_inventory) != 1 and self._any_real_changes()):
+        if len(self.builder.new_inventory) != 1 and (self.entries_changed or
+            self.entries_deleted):
             return
         raise PointlessCommit()
 
@@ -678,11 +646,15 @@
                     continue
                 ie = new_ie.copy()
                 ie.revision = None
-                self.builder.record_entry_contents(ie, self.parent_invs, path,
-                                                   self.basis_tree)
+                if self.builder.record_entry_contents(ie, self.parent_invs, path,
+                    self.basis_tree):
+                    self.entries_changed = True
 
+        # note that deletes have occured
+        if set(self.basis_inv._byid.keys()) - set(self.builder.new_inventory._byid.keys()):
+            self.entries_deleted = True
         # Report what was deleted.
-        if self.reporter.is_verbose():
+        if self.entries_deleted and self.reporter.is_verbose():
             for path, ie in self.basis_inv.iter_entries():
                 if ie.file_id not in self.builder.new_inventory:
                     self.reporter.deleted(path)
@@ -735,7 +707,7 @@
             # Note: I don't particularly want to have the existing_ie
             # parameter but the test suite currently (28-Jun-07) breaks
             # without it thanks to a unicode normalisation issue. :-(
-            definitely_changed = kind != existing_ie.kind 
+            definitely_changed = kind != existing_ie.kind
             self._record_entry(path, file_id, specific_files, kind, name,
                 parent_id, definitely_changed, existing_ie, report_changes)
 
@@ -766,6 +738,8 @@
                 local=self.local, reporter=self.reporter)
         except errors.PointlessCommit:
             pass
+        else:
+            self.entries_changed = True
 
     def _record_entry(self, path, file_id, specific_files, kind, name,
             parent_id, definitely_changed, existing_ie=None,
@@ -788,8 +762,9 @@
                 # this entry is new and not being committed
                 ie = None
         if ie is not None:
-            self.builder.record_entry_contents(ie, self.parent_invs, 
-                path, self.work_tree)
+            if self.builder.record_entry_contents(ie, self.parent_invs,
+                path, self.work_tree):
+                self.entries_changed = True
             if report_changes:
                 self._report_change(ie, path)
         return ie

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2007-09-16 23:48:15 +0000
+++ b/bzrlib/repository.py	2007-09-20 03:43:11 +0000
@@ -191,6 +191,7 @@
         :param parent_invs: The inventories of the parent revisions of the
             commit.
         :param tree: The tree that is being committed.
+        :return: True if the root entry is versioned properly.
         """
         if ie.parent_id is not None:
             # if ie is not root, add a root automatically.
@@ -204,6 +205,7 @@
             # serializing out to disk and back in root.revision is always
             # _new_revision_id
             ie.revision = self._new_revision_id
+        return False
 
     def record_entry_contents(self, ie, parent_invs, path, tree):
         """Record the content of ie from tree into the commit if needed.
@@ -216,16 +218,20 @@
         :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.)
         """
         if self.new_inventory.root is None:
-            self._check_root(ie, parent_invs, tree)
+            self._versioned_root = self._check_root(ie, parent_invs, tree)
         self.new_inventory.add(ie)
 
         # 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
+            return ie.revision == self._new_revision_id and (path != '' or
+                self._versioned_root)
 
         parent_candiate_entries = ie.parent_candidates(parent_invs)
         heads = self.repository.get_graph().heads(parent_candiate_entries.keys())
@@ -236,6 +242,8 @@
         # 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
+            self._versioned_root)
 
     def modified_directory(self, file_id, file_parents):
         """Record the presence of a symbolic link.
@@ -320,9 +328,11 @@
         :param parent_invs: The inventories of the parent revisions of the
             commit.
         :param tree: The tree that is being committed.
+        :return: True if the root entry is versioned properly.
         """
         # ie must be root for this builder
         assert ie.parent_id is None
+        return True
 
 
 ######################################################################

=== modified file 'bzrlib/tests/repository_implementations/test_commit_builder.py'
--- a/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-09-10 01:27:10 +0000
+++ b/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-09-20 03:43:11 +0000
@@ -134,6 +134,27 @@
             rev_id = builder.commit('foo bar')
         finally:
             tree.unlock()
+    
+    def test_commit_unchanged_root(self):
+        tree = self.make_branch_and_tree(".")
+        tree.commit('')
+        tree.lock_write()
+        parent_tree = tree.basis_tree()
+        parent_tree.lock_read()
+        self.addCleanup(parent_tree.unlock)
+        builder = tree.branch.get_commit_builder([parent_tree.inventory])
+        try:
+            ie = inventory.make_entry('directory', '', None,
+                    tree.inventory.root.file_id)
+            self.assertFalse(builder.record_entry_contents(
+                ie, [parent_tree.inventory], '', tree))
+            builder.abort()
+        except:
+            builder.abort()
+            tree.unlock()
+            raise
+        else:
+            tree.unlock()
 
     def test_commit(self):
         tree = self.make_branch_and_tree(".")
@@ -295,7 +316,42 @@
         tree.add([name], [name + 'id'])
         rev1 = tree.commit('')
         changer()
-        rev2 = tree.commit('')
+        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_tree = tree.basis_tree()
+            parent_tree.lock_read()
+            self.addCleanup(parent_tree.unlock)
+            parent_invs = [parent_tree.inventory]
+            # root
+            builder.record_entry_contents(
+                inventory.make_entry('directory', '', None,
+                    tree.inventory.root.file_id), parent_invs, '', tree)
+            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)
+                return builder.record_entry_contents(ie, parent_invs, path, tree)
+
+            file_id = name + 'id'
+            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))
+            builder.finish_inventory()
+            rev2 = builder.commit('')
+            tree.set_parent_ids([rev2])
+        except:
+            builder.abort()
+            tree.unlock()
+            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)



More information about the bazaar-commits mailing list