Rev 3782: CommitBuilder handles no-change commits to roots properly with record_iter_changes. in http://people.ubuntu.com/~robertc/baz2.0/commit-iterchanges

Robert Collins robertc at robertcollins.net
Tue Nov 18 02:43:55 GMT 2008


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

------------------------------------------------------------
revno: 3782
revision-id: robertc at robertcollins.net-20081118024351-btz27tj0cohw0qjc
parent: robertc at robertcollins.net-20081118005440-9fdc3t2ga7szsrbo
committer: Robert Collins <robertc at robertcollins.net>
branch nick: commit-iterchanges
timestamp: Tue 2008-11-18 13:43:51 +1100
message:
  CommitBuilder handles no-change commits to roots properly with record_iter_changes.
modified:
  bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
  bzrlib/tests/per_repository/test_commit_builder.py test_commit_builder.py-20060606110838-76e3ra5slucqus81-1
=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2008-11-18 00:32:28 +0000
+++ b/bzrlib/repository.py	2008-11-18 02:43:51 +0000
@@ -123,6 +123,18 @@
         self.__heads = graph.HeadsCache(repository.get_graph()).heads
         self.basis_delta = []
         self._recording_deletes = False
+        # memo'd check for no-op commits.
+        self._any_entries_changed = False
+
+    def any_entries_changed(self):
+        """Return True if any entries were changed.
+        
+        This includes merge-only changes. It is the core for the --unchanged
+        detection in commit.
+
+        :return: True if any changes have occured.
+        """
+        return self._any_entries_changed
 
     def commit(self, message):
         """Make the actual commit.
@@ -170,12 +182,12 @@
             # an inventory delta was accumulated without creating a new
             # inventory.
             try:
-                basis_id = self.parents[0]
+                basis_id = self.parents[0].revision_id
             except IndexError:
                 basis_id = _mod_revision.NULL_REVISION
             self.inv_sha1 = self.repository.add_inventory_delta(
                 basis_id, self.basis_delta, self._new_revision_id,
-                self.parents)
+                [parent.revision_id for parent in self.parents])
         else:
             if self.new_inventory.root is None:
                 raise AssertionError('Root entry should be supplied to'
@@ -230,6 +242,19 @@
         # _new_revision_id
         ie.revision = self._new_revision_id
 
+    def _require_root_change(self):
+        """Enforce an appropriate root object change.
+
+        This is called once when record_iter_changes is called, if and only if
+        the root was not in the delta calculated by record_iter_changes.
+        """
+        # NB: if there are no parents then this method is not called, so no
+        # need to guard on parents having length.
+        entry = entry_factory['directory'](self.parents[0].root.file_id, '',
+            None)
+        entry.revision = self._new_revision_id
+        self.basis_delta.append(('', '', entry.file_id, entry))
+
     def _get_delta(self, ie, basis_inv, path):
         """Get a delta against the basis inventory for ie."""
         if ie.file_id not in basis_inv:
@@ -477,6 +502,7 @@
         else:
             raise NotImplementedError('unknown kind')
         ie.revision = self._new_revision_id
+        self._any_entries_changed = True
         return self._get_delta(ie, basis_inv, path), True, fingerprint
 
     def record_iter_changes(self, basis_revision_id, iter_changes,
@@ -501,17 +527,26 @@
             changes[change[0]] = change
         # inv delta is:
         # old_path, new_path, file_id, new_inventory_entry
+        seen_root = False # Is the root in the basis delta?
         inv_delta = self.basis_delta
         modified_rev = self._new_revision_id
         for change in changes.values():
             if change[3][1]: # versioned in target.
-                entry = entry_factory[change[6][1]](
+                entry = _entry_factory[change[6][1]](
                     change[0], change[5][1], change[4][1])
                 entry.revision = modified_rev
             else:
                 entry = None
-            inv_delta.append((change[1][0], change[1][1], change[0], entry))
+            new_path = change[1][1]
+            inv_delta.append((change[1][0], new_path, change[0], entry))
+            if new_path == '':
+                seen_root = True
         self.new_inventory = None
+        if len(inv_delta):
+            self._any_entries_changed = True
+        if not seen_root:
+            # housekeeping root entry changes do not affect no-change commits.
+            self._require_root_change()
 
     def _add_text_to_weave(self, file_id, new_lines, parents, nostore_sha):
         # Note: as we read the content directly from the tree, we know its not
@@ -541,6 +576,14 @@
         :param tree: The tree that is being committed.
         """
 
+    def _require_root_change(self):
+        """Enforce an appropriate root object change.
+
+        This is called once when record_iter_changes is called, if and only if
+        the root was not in the delta calculated by record_iter_changes.
+        """
+        # versioned roots do not change unless the tree found a change.
+
 
 ######################################################################
 # Repositories

=== modified file 'bzrlib/tests/per_repository/test_commit_builder.py'
--- a/bzrlib/tests/per_repository/test_commit_builder.py	2008-11-18 00:54:40 +0000
+++ b/bzrlib/tests/per_repository/test_commit_builder.py	2008-11-18 02:43:51 +0000
@@ -198,7 +198,7 @@
         self.assertEqual(revision_id,
             tree.branch.repository.get_inventory(revision_id).revision_id)
 
-    def test_commit_without_root_errors(self):
+    def test_commit_without_root_or_record_iter_changes_errors(self):
         tree = self.make_branch_and_tree(".")
         tree.lock_write()
         try:
@@ -227,6 +227,9 @@
             delta, version_recorded, fs_hash = builder.record_entry_contents(
                 ie, [parent_tree.inventory], '', tree,
                 tree.path_content_summary(''))
+            # Regardless of repository root behaviour we should consider this a
+            # pointless commit.
+            self.assertFalse(builder.any_entries_changed())
             self.assertFalse(version_recorded)
             # if the repository format recorded a new root revision, that
             # should be in the delta
@@ -248,6 +251,33 @@
         else:
             tree.unlock()
 
+    def test_commit_unchanged_root_record_iter_changes(self):
+        tree = self.make_branch_and_tree(".")
+        old_revision_id = 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:
+            builder.record_iter_changes(old_revision_id,
+                tree.iter_changes(parent_tree))
+            # Regardless of repository root behaviour we should consider this a
+            # pointless commit.
+            self.assertFalse(builder.any_entries_changed())
+            builder.finish_inventory()
+            new_root = tree.branch.repository.get_inventory(
+                builder._new_revision_id).root
+            if tree.branch.repository.supports_rich_root():
+                # We should not have seen a new root revision
+                self.assertEqual(old_revision_id, new_root.revision)
+            else:
+                # We should see a new root revision
+                self.assertNotEqual(old_revision_id, new_root.revision)
+        finally:
+            builder.abort()
+            tree.unlock()
+
     def test_commit(self):
         tree = self.make_branch_and_tree(".")
         tree.lock_write()




More information about the bazaar-commits mailing list