Rev 2842: faster pointless commit detection (Robert Collins) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Fri Sep 21 03:20:27 BST 2007
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 2842
revision-id: pqm at pqm.ubuntu.com-20070921022023-cgeid5vrxco9o4jo
parent: pqm at pqm.ubuntu.com-20070921005024-anlkzk5nrdtujta4
parent: ian.clatworthy at internode.on.net-20070921002235-u5lbs3wog6na1qxg
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2007-09-21 03:20:23 +0100
message:
faster pointless commit detection (Robert Collins)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/commit.py commit.py-20050511101309-79ec1a0168e0e825
bzrlib/inventory.py inventory.py-20050309040759-6648b84ca2005b37
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/workingtree_implementations/test_commit.py test_commit.py-20060421013633-1610ec2331c8190f
------------------------------------------------------------
revno: 2840.1.1
merged: ian.clatworthy at internode.on.net-20070921002235-u5lbs3wog6na1qxg
parent: pqm at pqm.ubuntu.com-20070920235505-6w61gqyajy9i0ioj
parent: robertc at robertcollins.net-20070920075948-6f32d46hr3oyw4zb
committer: Ian Clatworthy <ian.clatworthy at internode.on.net>
branch nick: ianc-integration
timestamp: Fri 2007-09-21 10:22:35 +1000
message:
faster pointless commit detection (Robert Collins)
------------------------------------------------------------
revno: 2825.5.2
merged: robertc at robertcollins.net-20070920075948-6f32d46hr3oyw4zb
parent: robertc at robertcollins.net-20070920034311-lgjoomiumagdhksn
committer: Robert Collins <robertc at robertcollins.net>
branch nick: record-entry-returns-status
timestamp: Thu 2007-09-20 17:59:48 +1000
message:
Review feedback, and fix pointless commits with nested trees to raise PointlessCommit appropriately.
------------------------------------------------------------
revno: 2825.5.1
merged: 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 file 'NEWS'
--- a/NEWS 2007-09-20 22:44:15 +0000
+++ b/NEWS 2007-09-21 02:20:23 +0000
@@ -32,6 +32,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 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-18 01:29:59 +0000
+++ b/bzrlib/commit.py 2007-09-21 00:22:35 +0000
@@ -241,6 +241,8 @@
" parameter is required for commit().")
self.bound_branch = None
+ self.any_entries_changed = False
+ self.any_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.any_entries_changed
+ or self.any_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.any_entries_changed = True
+ # note that deletes have occurred
+ if set(self.basis_inv._byid.keys()) - set(self.builder.new_inventory._byid.keys()):
+ self.any_entries_deleted = True
# Report what was deleted.
- if self.reporter.is_verbose():
+ 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)
@@ -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)
@@ -788,8 +760,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.any_entries_changed = True
if report_changes:
self._report_change(ie, path)
return ie
=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py 2007-09-04 03:53:07 +0000
+++ b/bzrlib/inventory.py 2007-09-20 07:59:48 +0000
@@ -879,6 +879,13 @@
def _forget_tree_state(self):
self.reference_revision = None
+ def _unchanged(self, previous_ie):
+ """See InventoryEntry._unchanged."""
+ compatible = super(TreeReference, self)._unchanged(previous_ie)
+ if self.reference_revision != previous_ie.reference_revision:
+ compatible = False
+ return compatible
+
class Inventory(object):
"""Inventory of versioned files in a tree.
=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py 2007-09-16 23:48:15 +0000
+++ b/bzrlib/repository.py 2007-09-21 00:22:35 +0000
@@ -70,6 +70,8 @@
# all clients should supply tree roots.
record_root_entry = True
+ # the default CommitBuilder does not manage trees whose root is versioned.
+ _versioned_root = False
def __init__(self, repository, parents, config, timestamp=None,
timezone=None, committer=None, revprops=None,
@@ -216,6 +218,9 @@
: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)
@@ -225,7 +230,8 @@
# 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.
@@ -313,6 +321,9 @@
class RootCommitBuilder(CommitBuilder):
"""This commitbuilder actually records the root id"""
+ # the root entry gets versioned properly by this builder.
+ _versioned_root = True
+
def _check_root(self, ie, parent_invs, tree):
"""Helper for record_entry_contents.
=== modified file 'bzrlib/tests/repository_implementations/test_commit_builder.py'
--- a/bzrlib/tests/repository_implementations/test_commit_builder.py 2007-09-20 09:30:39 +0000
+++ b/bzrlib/tests/repository_implementations/test_commit_builder.py 2007-09-21 00:22:35 +0000
@@ -138,6 +138,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(".")
@@ -299,7 +320,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)
=== modified file 'bzrlib/tests/workingtree_implementations/test_commit.py'
--- a/bzrlib/tests/workingtree_implementations/test_commit.py 2007-08-27 08:38:37 +0000
+++ b/bzrlib/tests/workingtree_implementations/test_commit.py 2007-09-20 07:59:48 +0000
@@ -90,14 +90,14 @@
def test_commit_sets_last_revision(self):
tree = self.make_branch_and_tree('tree')
- committed_id = tree.commit('foo', rev_id='foo', allow_pointless=True)
+ committed_id = tree.commit('foo', rev_id='foo')
self.assertEqual(['foo'], tree.get_parent_ids())
# the commit should have returned the same id we asked for.
self.assertEqual('foo', committed_id)
def test_commit_returns_revision_id(self):
tree = self.make_branch_and_tree('.')
- committed_id = tree.commit('message', allow_pointless=True)
+ committed_id = tree.commit('message')
self.assertTrue(tree.branch.repository.has_revision(committed_id))
self.assertNotEqual(None, committed_id)
@@ -323,6 +323,22 @@
basis.get_reference_revision(basis.path2id('subtree')))
self.assertNotEqual(rev_id, rev_id2)
+ def test_nested_pointless_commits_are_pointless(self):
+ tree = self.make_branch_and_tree('.')
+ if not tree.supports_tree_reference():
+ # inapplicable test.
+ return
+ subtree = self.make_branch_and_tree('subtree')
+ tree.add(['subtree'])
+ # record the reference.
+ rev_id = tree.commit('added reference')
+ child_revid = subtree.last_revision()
+ # now do a no-op commit with allow_pointless=False
+ self.assertRaises(errors.PointlessCommit, tree.commit, '',
+ allow_pointless=False)
+ self.assertEqual(child_revid, subtree.last_revision())
+ self.assertEqual(rev_id, tree.last_revision())
+
class TestCommitProgress(TestCaseWithWorkingTree):
More information about the bazaar-commits
mailing list