Rev 2913: commit produces (but does not yet use) a basis delta and avoids one iter_entries (mbp) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Wed Oct 17 06:59:15 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2913
revision-id: pqm at pqm.ubuntu.com-20071017055911-jots6fwy20740n0i
parent: pqm at pqm.ubuntu.com-20071017052031-xesht1ei7y0nn2ea
parent: mbp at sourcefrog.net-20071015050205-p2c73neus72bqmub
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2007-10-17 06:59:11 +0100
message:
  commit produces (but does not yet use) a basis delta and avoids one iter_entries (mbp)
modified:
  bzrlib/commit.py               commit.py-20050511101309-79ec1a0168e0e825
  bzrlib/mutabletree.py          mutabletree.py-20060906023413-4wlkalbdpsxi2r4y-2
  bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
  bzrlib/tests/repository_implementations/test_commit_builder.py test_commit_builder.py-20060606110838-76e3ra5slucqus81-1
  bzrlib/tests/test_version_info.py test_version_info.py-20051228204928-2c364e30b702b41b
  bzrlib/tests/workingtree_implementations/test_parents.py test_set_parents.py-20060807231740-yicmnlci1mj8smu1-1
    ------------------------------------------------------------
    revno: 2903.2.11
    merged: mbp at sourcefrog.net-20071015050205-p2c73neus72bqmub
    parent: mbp at sourcefrog.net-20071015045406-051qpludajj5vwk0
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Mon 2007-10-15 15:02:05 +1000
    message:
      Temporarily disable using update_basis_by_delta as it's slower
    ------------------------------------------------------------
    revno: 2903.2.10
    merged: mbp at sourcefrog.net-20071015045406-051qpludajj5vwk0
    parent: mbp at sourcefrog.net-20071015045034-390357g4hrq386qp
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Mon 2007-10-15 14:54:06 +1000
    message:
      make Commit.basis_delta private
    ------------------------------------------------------------
    revno: 2903.2.9
    merged: mbp at sourcefrog.net-20071015045034-390357g4hrq386qp
    parent: mbp at sourcefrog.net-20071015042629-83b1oaswtisv1adf
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Mon 2007-10-15 14:50:34 +1000
    message:
      Review cleanups, mostly documentation
    ------------------------------------------------------------
    revno: 2903.2.8
    merged: mbp at sourcefrog.net-20071015042629-83b1oaswtisv1adf
    parent: mbp at sourcefrog.net-20071012080007-vf80woayyom8s8e1
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Mon 2007-10-15 14:26:29 +1000
    message:
      More efficient reporting of deletions from a large tree during commit
    ------------------------------------------------------------
    revno: 2903.2.7
    merged: mbp at sourcefrog.net-20071012080007-vf80woayyom8s8e1
    parent: mbp at sourcefrog.net-20071012065423-81g0dgg2prhkfp47
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Fri 2007-10-12 18:00:07 +1000
    message:
      Rename update_to_one_parent_via_delta to more wieldy update_basis_by_delta
    ------------------------------------------------------------
    revno: 2903.2.6
    merged: mbp at sourcefrog.net-20071012065423-81g0dgg2prhkfp47
    parent: mbp at sourcefrog.net-20071012064749-7t00zaab23qo5qjw
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Fri 2007-10-12 16:54:23 +1000
    message:
      Remove unnecessary double handling of deletions
    ------------------------------------------------------------
    revno: 2903.2.5
    merged: mbp at sourcefrog.net-20071012064749-7t00zaab23qo5qjw
    parent: mbp at sourcefrog.net-20071012063831-40o7w602gk4w5fgr
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Fri 2007-10-12 16:47:49 +1000
    message:
      record_entry_contents should give back deltas for changed roots; clean it up a bit
    ------------------------------------------------------------
    revno: 2903.2.4
    merged: mbp at sourcefrog.net-20071012063831-40o7w602gk4w5fgr
    parent: mbp at sourcefrog.net-20071012063701-sa6225u0bbffx37h
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Fri 2007-10-12 16:38:31 +1000
    message:
      Remove old deprecated support for CommitBuilders that don't accept a root; capture deltas for selected-file commit
    ------------------------------------------------------------
    revno: 2903.2.3
    merged: mbp at sourcefrog.net-20071012063701-sa6225u0bbffx37h
    parent: mbp at sourcefrog.net-20071012054231-5tvevguvxp5lox3v
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Fri 2007-10-12 16:37:01 +1000
    message:
      CommitBuilder tests should expect the root to be in the delta iff it's changed in the commit
    ------------------------------------------------------------
    revno: 2903.2.2
    merged: mbp at sourcefrog.net-20071012054231-5tvevguvxp5lox3v
    parent: mbp at sourcefrog.net-20071012025039-0i3zvyrxvqdr6dw4
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Fri 2007-10-12 15:42:31 +1000
    message:
      doc
    ------------------------------------------------------------
    revno: 2903.2.1
    merged: mbp at sourcefrog.net-20071012025039-0i3zvyrxvqdr6dw4
    parent: pqm at pqm.ubuntu.com-20071010023136-u18s3z51ux7pwsme
    committer: Martin Pool <mbp at sourcefrog.net>
    branch nick: commit
    timestamp: Fri 2007-10-12 12:50:39 +1000
    message:
      Commit now tells the working tree about the new basis by passing the an inventory delta from the previous basis
=== modified file 'bzrlib/commit.py'
--- a/bzrlib/commit.py	2007-10-03 23:24:50 +0000
+++ b/bzrlib/commit.py	2007-10-15 05:02:05 +0000
@@ -263,6 +263,9 @@
         self.committer = committer
         self.strict = strict
         self.verbose = verbose
+        # accumulates an inventory delta to the basis entry, so we can make
+        # just the necessary updates to the workingtree's cached basis.
+        self._basis_delta = []
 
         self.work_tree.lock_write()
         self.pb = bzrlib.ui.ui_factory.nested_progress_bar()
@@ -338,6 +341,7 @@
                 self.reporter.started(new_revno, self.rev_id, master_location)
 
                 self._update_builder_with_changes()
+                self._report_and_accumulate_deletes()
                 self._check_pointless()
 
                 # TODO: Now the new inventory is known, check for conflicts.
@@ -380,6 +384,15 @@
             # Make the working tree up to date with the branch
             self._set_progress_stage("Updating the working tree")
             rev_tree = self.builder.revision_tree()
+            # XXX: This will need to be changed if we support doing a
+            # selective commit while a merge is still pending - then we'd
+            # still have multiple parents after the commit.
+            #
+            # XXX: update_basis_by_delta is slower at present because it works
+            # on inventories, so this is not active until there's a native
+            # dirstate implementation.
+            ## self.work_tree.update_basis_by_delta(self.rev_id,
+            ##      self._basis_delta)
             self.work_tree.set_parent_trees([(self.rev_id, rev_tree)])
             self.reporter.completed(new_revno, self.rev_id)
             self._process_post_hooks(old_revno, new_revno)
@@ -399,8 +412,10 @@
         # A merge with no effect on files
         if len(self.parents) > 1:
             return
-        # work around the fact that a newly-initted tree does differ from its
-        # basis
+        # TODO: we could simplify this by using self._basis_delta.
+
+        # The inital commit adds a root directory, but this in itself is not
+        # a worthwhile commit.  
         if len(self.basis_inv) == 0 and len(self.builder.new_inventory) == 1:
             raise PointlessCommit()
         # Shortcut, if the number of entries changes, then we obviously have
@@ -633,13 +648,6 @@
         specific_files = self.specific_files
         mutter("Selecting files for commit with filter %s", specific_files)
 
-        # Check and warn about old CommitBuilders
-        if not self.builder.record_root_entry:
-            symbol_versioning.warn('CommitBuilders should support recording'
-                ' the root entry as of bzr 0.10.', DeprecationWarning, 
-                stacklevel=1)
-            self.builder.new_inventory.add(self.basis_inv.root.copy())
-
         # Build the new inventory
         self._populate_from_inventory(specific_files)
 
@@ -669,15 +677,22 @@
                     ie, self.parent_invs, path, self.basis_tree, None)
                 if version_recorded:
                     self.any_entries_changed = True
+                if delta: self._basis_delta.append(delta)
 
-        # note that deletes have occurred
-        if set(self.basis_inv._byid.keys()) - set(self.builder.new_inventory._byid.keys()):
+    def _report_and_accumulate_deletes(self):
+        # XXX: Could the list of deleted paths and ids be instead taken from
+        # _populate_from_inventory?
+        deleted_ids = set(self.basis_inv._byid.keys()) - \
+            set(self.builder.new_inventory._byid.keys())
+        if deleted_ids:
             self.any_entries_deleted = True
-        # Report what was deleted.
-        if self.any_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)
+            deleted = [(self.basis_inv.id2path(file_id), file_id)
+                for file_id in deleted_ids]
+            deleted.sort()
+            # XXX: this is not quite directory-order sorting
+            for path, file_id in deleted:
+                self._basis_delta.append((path, None, file_id, None))
+                self.reporter.deleted(path)
 
     def _populate_from_inventory(self, specific_files):
         """Populate the CommitBuilder by walking the working tree inventory."""
@@ -694,8 +709,6 @@
         # XXX: Note that entries may have the wrong kind.
         entries = work_inv.iter_entries_by_dir(
             specific_file_ids=self.specific_file_ids, yield_parents=True)
-        if not self.builder.record_root_entry:
-            entries.next()
         for path, existing_ie in entries:
             file_id = existing_ie.file_id
             name = existing_ie.name
@@ -787,6 +800,8 @@
             ie.revision = None
         delta, version_recorded = self.builder.record_entry_contents(ie,
             self.parent_invs, path, self.work_tree, content_summary)
+        if delta:
+            self._basis_delta.append(delta)
         if version_recorded:
             self.any_entries_changed = True
         if report_changes:

=== modified file 'bzrlib/mutabletree.py'
--- a/bzrlib/mutabletree.py	2007-10-08 07:29:57 +0000
+++ b/bzrlib/mutabletree.py	2007-10-12 08:00:07 +0000
@@ -429,11 +429,11 @@
                 self.read_working_inventory()
         return added, ignored
 
-    def update_to_one_parent_via_delta(self, new_revid, delta):
+    def update_basis_by_delta(self, new_revid, delta):
         """Update the parents of this tree after a commit.
 
         This gives the tree one parent, with revision id new_revid. The
-        inventory delta is applied ot the current basis tree to generate the
+        inventory delta is applied to the current basis tree to generate the
         inventory for the parent new_revid, and all other parent trees are
         discarded.
 

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2007-10-16 02:42:33 +0000
+++ b/bzrlib/repository.py	2007-10-17 05:59:11 +0000
@@ -157,7 +157,7 @@
     def finish_inventory(self):
         """Tell the builder that the inventory is finished."""
         if self.new_inventory.root is None:
-            symbol_versioning.warn('Root entry should be supplied to'
+            raise AssertionError('Root entry should be supplied to'
                 ' record_entry_contents, as of bzr 0.10.',
                  DeprecationWarning, stacklevel=2)
             self.new_inventory.add(InventoryDirectory(ROOT_ID, '', None))
@@ -253,8 +253,6 @@
             # mismatch between commit builder logic and repository:
             # this needs the entry creation pushed down into the builder.
             raise NotImplementedError('Missing repository subtree support.')
-        # transitional assert only, will remove before release.
-        assert ie.kind == kind
         self.new_inventory.add(ie)
 
         # TODO: slow, take it out of the inner loop.
@@ -267,30 +265,23 @@
         # for committing. We may record the previous parents revision if the
         # content is actually unchanged against a sole head.
         if ie.revision is not None:
-            if self._versioned_root or path != '':
-                # not considered for commit
-                delta = None
-            else:
+            if not self._versioned_root and path == '':
                 # 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:
+                if ie.file_id in basis_inv:
+                    delta = (basis_inv.id2path(ie.file_id), path,
+                        ie.file_id, ie)
+                else:
                     # 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)
-
+                return delta, False
+            else:
+                # we don't need to commit this, because the caller already
+                # determined that an existing revision of this file is
+                # appropriate.
+                return None, (ie.revision == self._new_revision_id)
         # XXX: Friction: parent_candidates should return a list not a dict
         #      so that we don't have to walk the inventories again.
         parent_candiate_entries = ie.parent_candidates(parent_invs)

=== modified file 'bzrlib/tests/repository_implementations/test_commit_builder.py'
--- a/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-10-04 05:50:44 +0000
+++ b/bzrlib/tests/repository_implementations/test_commit_builder.py	2007-10-12 06:37:01 +0000
@@ -139,7 +139,7 @@
     
     def test_commit_unchanged_root(self):
         tree = self.make_branch_and_tree(".")
-        tree.commit('')
+        old_revision_id = tree.commit('')
         tree.lock_write()
         parent_tree = tree.basis_tree()
         parent_tree.lock_read()
@@ -152,7 +152,15 @@
                 ie, [parent_tree.inventory], '', tree,
                 tree.path_content_summary(''))
             self.assertFalse(version_recorded)
-            self.assertEqual(None, delta)
+            # 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)
+            else:
+                self.assertEqual(None, delta)
             builder.abort()
         except:
             builder.abort()

=== modified file 'bzrlib/tests/test_version_info.py'
--- a/bzrlib/tests/test_version_info.py	2006-09-27 02:28:44 +0000
+++ b/bzrlib/tests/test_version_info.py	2007-10-15 04:50:34 +0000
@@ -111,6 +111,9 @@
         stanza = regen(check_for_clean=True, include_file_revisions=True)
         self.assertEqual(['False'], stanza.get_all('clean'))
 
+        # XXX: This assumes it's being run against a repository that updates
+        # the root revision on every commit.  Newer ones that use
+        # RootCommitBuilder won't update it on each commit.
         file_rev_stanza = get_one_stanza(stanza, 'file-revisions')
         self.assertEqual(['', 'a', 'b', 'c'], file_rev_stanza.get_all('path'))
         self.assertEqual(['r3', 'r3', 'r2', 'unversioned'],

=== modified file 'bzrlib/tests/workingtree_implementations/test_parents.py'
--- a/bzrlib/tests/workingtree_implementations/test_parents.py	2007-10-05 02:41:37 +0000
+++ b/bzrlib/tests/workingtree_implementations/test_parents.py	2007-10-12 08:00:07 +0000
@@ -250,7 +250,7 @@
 
 
 class UpdateToOneParentViaDeltaTests(TestParents):
-    """Tests for the update_to_one_parent_via_delta call.
+    """Tests for the update_basis_by_delta call.
     
     This is intuitively defined as 'apply an inventory delta to the basis and
     discard other parents', but for trees that have an inventory that is not
@@ -260,7 +260,7 @@
 
     def assertDeltaApplicationResultsInExpectedBasis(self, tree, revid, delta,
         expected_inventory):
-        tree.update_to_one_parent_via_delta(revid, delta)
+        tree.update_basis_by_delta(revid, delta)
         # check the last revision was adjusted to rev_id
         self.assertEqual(revid, tree.last_revision())
         # check the parents are what we expect




More information about the bazaar-commits mailing list