Rev 2513: (Ian Clatworthy) Improve 'bzr commit' time by removing some lock and dirstate._validate() calls in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed Jun 6 15:42:03 BST 2007
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 2513
revision-id: pqm at pqm.ubuntu.com-20070606144200-rmsd3gyelimh8kal
parent: pqm at pqm.ubuntu.com-20070606083714-rt2za45t9gt5nqqh
parent: ian.clatworthy at internode.on.net-20070606061745-e5zl32uwku409wca
committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2007-06-06 15:42:00 +0100
message:
(Ian Clatworthy) Improve 'bzr commit' time by removing some lock and dirstate._validate() calls
added:
doc/developers/performance-commit.txt performancecommit.tx-20070606061633-4y4rawskx5ejb99w-1
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/commit.py commit.py-20050511101309-79ec1a0168e0e825
bzrlib/dirstate.py dirstate.py-20060728012006-d6mvoihjb3je9peu-1
bzrlib/workingtree_4.py workingtree_4.py-20070208044105-5fgpc5j3ljlh5q6c-1
------------------------------------------------------------
revno: 2511.1.4
merged: ian.clatworthy at internode.on.net-20070606061745-e5zl32uwku409wca
parent: ian.clatworthy at internode.on.net-20070606061158-zbwatmjd2m1p474g
committer: Ian Clatworthy <ian.clatworthy at internode.on.net>
branch nick: bzr.fast-commit
timestamp: Wed 2007-06-06 16:17:45 +1000
message:
updated NEWS and added commit performance notes to doc/developers
------------------------------------------------------------
revno: 2511.1.3
merged: ian.clatworthy at internode.on.net-20070606061158-zbwatmjd2m1p474g
parent: ian.clatworthy at internode.on.net-20070606060745-xchfmk7d9vckgy3r
committer: Ian Clatworthy <ian.clatworthy at internode.on.net>
branch nick: bzr.fast-commit
timestamp: Wed 2007-06-06 16:11:58 +1000
message:
remove implicit read locks for kind() and is_executable() in wt4
------------------------------------------------------------
revno: 2511.1.2
merged: ian.clatworthy at internode.on.net-20070606060745-xchfmk7d9vckgy3r
parent: ian.clatworthy at internode.on.net-20070606055603-monl116zotkbqn2y
committer: Ian Clatworthy <ian.clatworthy at internode.on.net>
branch nick: bzr.fast-commit
timestamp: Wed 2007-06-06 16:07:45 +1000
message:
remove calls to dirstate._validate() that shouldn't be on production code
------------------------------------------------------------
revno: 2511.1.1
merged: ian.clatworthy at internode.on.net-20070606055603-monl116zotkbqn2y
parent: pqm at pqm.ubuntu.com-20070606050006-o4yiw7jnwytgf561
committer: Ian Clatworthy <ian.clatworthy at internode.on.net>
branch nick: bzr.fast-commit
timestamp: Wed 2007-06-06 15:56:03 +1000
message:
commit.py clean-up including logging just to stderr, not bzr.log
=== added file 'doc/developers/performance-commit.txt'
--- a/doc/developers/performance-commit.txt 1970-01-01 00:00:00 +0000
+++ b/doc/developers/performance-commit.txt 2007-06-06 06:17:45 +0000
@@ -0,0 +1,262 @@
+Commit Performance Notes
+========================
+
+.. contents::
+
+Commit: The Minimum Work Required
+---------------------------------
+
+This is Martin Pool's email to the mailing list on the minimum work that
+commit needs to do. Be sure to check the mailing list archives
+(https://lists.ubuntu.com/archives/bazaar/2007q2/025791.html)
+for follow-up comments from Robert and others ...
+
+Here is a description of the minimum work that commit must do. We
+want to make sure that our design doesn't cost too much more than this
+minimum. I am trying to do this without making too many assumptions
+about the underlying storage, but am assuming that the ui and basic
+architecture (wt, branch, repo) stays about the same.
+
+The basic purpose of commit is to:
+
+1. create and store a new revision based on the contents of the working tree
+2. make this the new basis revision for the working tree
+
+We can do a selected commit of only some files or subtrees.
+
+The best performance we could hope for is:
+- stat each versioned selected working file once
+- read from the workingtree and write into the repository any new file texts
+- in general, do work proportional to the size of the shape (eg
+inventory) of the old and new selected trees, and to the total size of
+the modified files
+
+In more detail:
+
+1.0 - Store new file texts: if a versioned file contains a new text
+there is no avoiding storing it. To determine which ones have changed
+we must go over the workingtree and at least stat each file. If the
+file is modified since it was last hashed, it must be read in.
+Ideally we would read it only once, and either notice that it has not
+changed, or store it at that point.
+
+On the other hand we want new code to be able to handle files that are
+larger than will fit in memory. We may then need to read each file up
+to two times: once to determine if there is a new text and calculate
+its hash, and again to store it.
+
+1.1 - Store a tree-shape description (ie inventory or similar.) This
+describes the non-file objects, and provides a reference from the
+Revision to the texts within it.
+
+1.2 - Generate and store a new revision object.
+
+1.3 - Do delta-compression on the stored objects. (git notably does
+not do this at commit time, deferring this entirely until later.)
+This requires finding the appropriate basis for each modified file: in
+the current scheme we get the file id, last-revision from the
+dirstate, look into the knit for that text, extract that text in
+total, generate a delta, then store that into the knit. Most delta
+operations are O(n2) to O(n3) in the size of the modified files.
+
+1.4 - Cache annotation information for the changes: at the moment this
+is done as part of the delta storage. There are some flaws in that
+approach, such as that it is not updated when ghosts are filled, and
+the annotation can't be re-run with new diff parameters.
+
+2.1 - Make the new revision the basis for the tree, and clear the list
+of parents. Strictly this is all that's logically necessary, unless
+the working tree format requires more work.
+
+The dirstate format does require more work, because it caches the
+parent tree data for each file within the working tree data. In
+practice this means that every commit rewrites the entire dirstate
+file - we could try to avoid rewriting the whole file but this may be
+difficult because variable-length data (the last-changed revision id)
+is inserted into many rows.
+
+The current dirstate design then seems to mean that any commit of a
+single file imposes a cost proportional to the size of the current
+workingtree. Maybe there are other benefits that outweigh this.
+Alternatively if it was fast enough for operations to always look at
+the original storage of the parent trees we could do without the
+cache.
+
+2.2 - Record the observed file hashes into the workingtree control
+files. For the files that we just committed, we have the information
+to store a valid hash cache entry: we know their stat information and
+the sha1 of the file contents. This is not strictly necessary to the
+speed of commit, but it will be useful later in avoiding reading those
+files, and the only cost of doing it now is writing it out.
+
+In fact there are some user interface niceties that complicate this:
+
+3 - Before starting the commit proper, we prompt for a commit message
+and in that commit message editor we show a list of the files that
+will be committed: basically the output of bzr status. This is
+basically the same as the list of changes we detect while storing the
+commit, but because the user will sometimes change the tree after
+opening the commit editor and expect the final state to be committed I
+think we do have to look for changes twice. Since it takes the user a
+while to enter a message this is not a big problem as long as both the
+status summary and the commit are individually fast.
+
+4 - As the commit proceeds (or after?) we show another status-like
+summary. Just printing the names of modified files as they're stored
+would be easy. Recording deleted and renamed files or directories is
+more work: this can only be done by reference to the primary parent
+tree and requires it be read in. Worse, reporting renames requires
+searching by id across the entire parent tree. Possibly full
+reporting should be a default-off verbose option because it does
+require more work beyond the commit itself.
+
+5 - Bazaar currently allows for missing files to be automatically
+marked as removed at the time of commit. Leaving aside the ui
+consequences, this means that we have to update the working inventory
+to mark these files as removed. Since as discussed above we always
+have to rewrite the dirstate on commit this is not substantial, though
+we should make sure we do this in one pass, not two. I have
+previously proposed to make this behaviour a non-default option.
+
+We may need to run hooks or generate signatures during commit, but
+they don't seem to have substantial performance consequences.
+
+If one wanted to optimize solely for the speed of commit I think
+hash-addressed file-per-text storage like in git (or bzr 0.1) is very
+good. Remarkably, it does not need to read the inventory for the
+previous revision. For each versioned file, we just need to get its
+hash, either by reading the file or validating its stat data. If that
+hash is not already in the repository, the file is just copied in and
+compressed. As directories are traversed, they're turned into texts
+and stored as well, and then finally the revision is too. This does
+depend on later doing some delta compression of these texts.
+
+Variations on this are possible. Rather than writing a single file
+into the repository for each text, we could fold them into a single
+collation or pack file. That would create a smaller number of files
+in the repository, but looking up a single text would require looking
+into their indexes rather than just asking the filesystem.
+
+Rather than using hashes we can use file-id/rev-id pairs as at
+present, which has several consequences pro and con.
+
+
+Commit vs Status
+----------------
+
+At first glance, commit simply stores the changes status reports. In fact,
+this isn't technically correct: commit considers some files modified that
+status does not. The notes below were put together by John Arbash Meinel
+and Aaron Bentley in May 2007 to explain the finer details of commit to
+Ian Clatworthy. They are recorded here as they are likely to be useful to
+others new to Bazaar ...
+
+1) **Unknown files have a different effect.** With --no-strict (the default)
+ they have no effect and can be completely ignored. With --strict they
+ should cause the commit to abort (so you don't forget to add the two new
+ test files that you just created).
+
+2) **Multiple parents.** 'status' always compares 2 trees, typically the
+ last-committed tree and the current working tree. 'commit' will compare
+ more trees if there has been a merge.
+
+ a) The "last modified" property for files.
+ A file may be marked as changed since the last commit, but that
+ change may have come in from the merge, and the change could have
+ happened several commits back. There are several edge cases to be
+ handled here, like if both branches modified the same file, or if
+ just one branch modified it.
+
+ b) The trickier case is when a file appears unmodified since last
+ commit, but it was modified versus one of the merged branches. I
+ believe there are a few ways this can happen, like if a merged
+ branch changes a file and then reverts it back (you still update
+ the 'last modified' field).
+ In general, if both sides disagree on the 'last-modified' flag,
+ then you need to generate a new entry pointing 'last-modified' at
+ this revision (because you are resolving the differences between
+ the 2 parents).
+
+3) **Automatic deletion of 'missing' files.** This is a point that we go
+ back and forth on. I think the basic idea is that 'bzr commit' by
+ default should abort if it finds a 'missing' file (in case that file was
+ renamed rather than deleted), but 'bzr commit --auto' can add unknown
+ files and remove missing files automatically.
+
+4) **sha1 for newly added files.** status doesn't really need this: it should
+ only care that the file is not present in base, but is present now. In
+ some ways commit doesn't care either, since it needs to read and sha the
+ file itself anyway.
+
+5) **Nested trees.** status doesn't recurse into nested trees, but commit does.
+ This is just because not all of the nested-trees work has been merged yet.
+
+ A tree-reference is considered modified if the subtree has been
+ committed since the last containing-tree commit. But commit needs to
+ recurse into every subtree, to ensure that a commit is done if the
+ subtree has changed since its last commit. _iter_changes only reports
+ on tree-references that are modified, so it can't be used for doing
+ subtree commits.
+
+
+Avoiding Work: Smarter Change Detection
+---------------------------------------
+
+Commit currently walks through every file building an inventory. Here is
+Aaron's brain dump on a better way ...
+
+_iter_changes won't tell us about tree references that haven't changed,
+even if those subtrees have changed. (Unless we ask for unchanged
+files, which we don't want to do, of course.)
+
+There is an iter_references method, but using it looks just as expensive
+as calling kind().
+
+I did some work on updating commit to use iter_changes, but found for
+multi-parent trees, I had to fall back to the slow inventory comparison
+approach.
+
+Really, I think we need a call akin to iter_changes that handles
+multiple parents, and knows to emit entries when InventoryEntry.revision
+is all that's changed.
+
+
+Avoiding Work: Better Layering
+------------------------------
+
+For each file, commit is currently doing more work than it should. Here is
+John's take on a better way ...
+
+Note that "_iter_changes" *does* have to touch every path on disk, but
+it just can do it in a more efficient manner. (It doesn't have to create
+an InventoryEntry for all the ones that haven't changed).
+
+I agree with Aaron that we need something a little different than
+_iter_changes. Both because of handling multiple parents, as well as we
+don't want it to actually read the files if we have a stat-cache miss.
+
+Specifically, the commit code *has* to read the files because it is
+going to add the text to the repository, and we want it to compute the
+sha1 at *that* time, so we are guaranteed to have the valid sha (rather
+than just whatever the last cached one was). So we want the code to
+return 'None' if it doesn't have an up-to-date sha1, rather than reading
+the file and computing it, just before it returns it to the parent.
+
+The commit code (0.16) should really be restructured. It's layering is
+pretty wrong.
+
+Specifically, calling "kind()" requires a stat of the file. But we have
+to do a stat to get the size/whether the record is up-to-date, etc. So
+we really need to have a "create_an_up_to_date_inventory()" function.
+But because we are accessing every object on disk, we want to be working
+in tuples rather than Inventory objects. And because DirState already
+has the parent records next to the current working inventory, it can do
+all the work to do really fast comparison and throw-away of unimportant
+records.
+
+The way I made "bzr status" fast is by moving the 'ignore this record'
+ability as deep into the stack as I could get. Status has the property
+that you don't care about most of the records, just like commit. So the
+sooner you can stop evaluating the 99% that you don't care about, the
+less work you do.
+
=== modified file 'NEWS'
--- a/NEWS 2007-06-06 01:07:25 +0000
+++ b/NEWS 2007-06-06 06:17:45 +0000
@@ -1,5 +1,21 @@
IN DEVELOPMENT
+ NOTES WHEN UPGRADING:
+
+ * The kind() and is_executable() APIs on the WorkingTree interface no
+ longer implicitly (read) locks and unlocks the tree. This *might*
+ impact some plug-ins and tools using this part of the API. If you find
+ an issue that may be caused by this change, please let us know,
+ particularly the plug-in/tool maintainer. If encountered, the API
+ fix is to surround kind() and is_executable() calls with lock_read()
+ and unlock() like so::
+
+ work_tree.lock_read()
+ try:
+ kind = work_tree.kind(...)
+ finally:
+ work_tree.unlock()
+
INTERNALS:
* Rework of LogFormatter API to provide beginning/end of log hooks and to
encapsulate the details of the revision to be logged in a LogRevision
@@ -33,6 +49,11 @@
improving speed. 3.25x speedups have been observed for construction of
kernel-sized-trees, and checkouts are 1.28x faster. (Aaron Bentley)
+ * Commit on large trees is now faster. In my environment, a commit of
+ a small change to the Mozilla tree (55k files) has dropped from
+ 66 seconds to 32 seconds. For a small tree of 600 files, commit of a
+ small change is 33% faster. (Ian Clatworthy)
+
BUGFIXES:
* ``bzr push`` should only connect to the remote location one time.
=== modified file 'bzrlib/commit.py'
--- a/bzrlib/commit.py 2007-05-21 14:14:36 +0000
+++ b/bzrlib/commit.py 2007-06-06 05:56:03 +0000
@@ -106,32 +106,40 @@
class ReportCommitToLog(NullCommitReporter):
- # this may be more useful if 'note' was replaced by an overridable
- # method on self, which would allow more trivial subclassing.
- # alternative, a callable could be passed in, allowing really trivial
- # reuse for some uis. RBC 20060511
+ def _note(self, format, *args):
+ """Output a message.
+
+ Messages are output by writing directly to stderr instead of
+ using bzrlib.trace.note(). The latter constantly updates the
+ log file as we go causing an unnecessary performance hit.
+
+ Subclasses may choose to override this method but need to be aware
+ of its potential impact on performance.
+ """
+ bzrlib.ui.ui_factory.clear_term()
+ sys.stderr.write((format + "\n") % args)
def snapshot_change(self, change, path):
if change == 'unchanged':
return
if change == 'added' and path == '':
return
- note("%s %s", change, path)
+ self._note("%s %s", change, path)
def completed(self, revno, rev_id):
- note('Committed revision %d.', revno)
+ self._note('Committed revision %d.', revno)
def deleted(self, file_id):
- note('deleted %s', file_id)
+ self._note('deleted %s', file_id)
def escaped(self, escape_count, message):
- note("replaced %d control characters in message", escape_count)
+ self._note("replaced %d control characters in message", escape_count)
def missing(self, path):
- note('missing %s', path)
+ self._note('missing %s', path)
def renamed(self, change, old_path, new_path):
- note('%s %s => %s', change, old_path, new_path)
+ self._note('%s %s => %s', change, old_path, new_path)
class Commit(object):
@@ -153,10 +161,7 @@
self.reporter = reporter
else:
self.reporter = NullCommitReporter()
- if config is not None:
- self.config = config
- else:
- self.config = None
+ self.config = config
def commit(self,
message=None,
@@ -177,26 +182,26 @@
recursive='down'):
"""Commit working copy as a new revision.
- message -- the commit message (it or message_callback is required)
-
- timestamp -- if not None, seconds-since-epoch for a
- postdated/predated commit.
-
- specific_files -- If true, commit only those files.
-
- rev_id -- If set, use this as the new revision id.
+ :param message: the commit message (it or message_callback is required)
+
+ :param timestamp: if not None, seconds-since-epoch for a
+ postdated/predated commit.
+
+ :param specific_files: If true, commit only those files.
+
+ :param rev_id: If set, use this as the new revision id.
Useful for test or import commands that need to tightly
control what revisions are assigned. If you duplicate
a revision id that exists elsewhere it is your own fault.
If null (default), a time/random revision id is generated.
- allow_pointless -- If true (default), commit even if nothing
+ :param allow_pointless: If true (default), commit even if nothing
has changed and no merges are recorded.
- strict -- If true, don't allow a commit if the working tree
+ :param strict: If true, don't allow a commit if the working tree
contains unknown files.
- revprops -- Properties for new revision
+ :param revprops: Properties for new revision
:param local: Perform a local only commit.
:param recursive: If set to 'down', commit in any subtrees that have
pending changes of any sort during this commit.
@@ -235,7 +240,6 @@
self.committer = committer
self.strict = strict
self.verbose = verbose
- self.local = local
if reporter is None and self.reporter is None:
self.reporter = NullCommitReporter()
@@ -248,30 +252,15 @@
self.basis_tree.lock_read()
try:
# Cannot commit with conflicts present.
- if len(self.work_tree.conflicts())>0:
+ if len(self.work_tree.conflicts()) > 0:
raise ConflictsInTree
- # setup the bound branch variables as needed.
+ # Setup the bound branch variables as needed.
self._check_bound_branch()
- # check for out of date working trees
- try:
- first_tree_parent = self.work_tree.get_parent_ids()[0]
- except IndexError:
- # if there are no parents, treat our parent as 'None'
- # this is so that we still consier the master branch
- # - in a checkout scenario the tree may have no
- # parents but the branch may do.
- first_tree_parent = bzrlib.revision.NULL_REVISION
- old_revno, master_last = self.master_branch.last_revision_info()
- if master_last != first_tree_parent:
- if master_last != bzrlib.revision.NULL_REVISION:
- raise errors.OutOfDateTree(self.work_tree)
- if self.branch.repository.has_revision(first_tree_parent):
- new_revno = old_revno + 1
- else:
- # ghost parents never appear in revision history.
- new_revno = 1
+ # Check that the working tree is up to date
+ old_revno,new_revno = self._check_out_of_date_tree()
+
if strict:
# raise an exception as soon as we find a single unknown.
for unknown in self.work_tree.unknowns():
@@ -289,6 +278,8 @@
# cheaply?
tree.find_ids_across_trees(specific_files,
[self.basis_tree, self.work_tree])
+
+ # Setup the progress bar ...
# one to finish, one for rev and inventory, and one for each
# inventory entry, and the same for the new inventory.
# note that this estimate is too long when we do a partial tree
@@ -301,16 +292,15 @@
if len(self.parents) > 1 and self.specific_files:
raise errors.CannotCommitSelectedFileMerge(self.specific_files)
+ # Build the new inventory
self.builder = self.branch.get_commit_builder(self.parents,
self.config, timestamp, timezone, committer, revprops, rev_id)
-
self._remove_deleted()
self._populate_new_inv()
self._report_deletes()
-
self._check_pointless()
+ self._emit_progress_update()
- self._emit_progress_update()
# TODO: Now the new inventory is known, check for conflicts and
# prompt the user for a commit message.
# ADHB 2006-08-08: If this is done, populate_new_inv should not add
@@ -323,9 +313,9 @@
self.message = message
self._escape_commit_message()
+ # Add revision data to the local branch
self.rev_id = self.builder.commit(self.message)
self._emit_progress_update()
- # revision data is in the local branch now.
# upload revision data to the master.
# this will propagate merged revisions too if needed.
@@ -333,46 +323,21 @@
self.master_branch.repository.fetch(self.branch.repository,
revision_id=self.rev_id)
# now the master has the revision data
- # 'commit' to the master first so a timeout here causes the local
- # branch to be out of date
+ # 'commit' to the master first so a timeout here causes the
+ # local branch to be out of date
self.master_branch.set_last_revision_info(new_revno,
self.rev_id)
# and now do the commit locally.
self.branch.set_last_revision_info(new_revno, self.rev_id)
+ # Make the working tree up to date with the branch
rev_tree = self.builder.revision_tree()
self.work_tree.set_parent_trees([(self.rev_id, rev_tree)])
- # now the work tree is up to date with the branch
-
self.reporter.completed(new_revno, self.rev_id)
- # old style commit hooks - should be deprecated ? (obsoleted in
- # 0.15)
- if self.config.post_commit() is not None:
- hooks = self.config.post_commit().split(' ')
- # this would be nicer with twisted.python.reflect.namedAny
- for hook in hooks:
- result = eval(hook + '(branch, rev_id)',
- {'branch':self.branch,
- 'bzrlib':bzrlib,
- 'rev_id':self.rev_id})
- # new style commit hooks:
- if not self.bound_branch:
- hook_master = self.branch
- hook_local = None
- else:
- hook_master = self.master_branch
- hook_local = self.branch
- # With bound branches, when the master is behind the local branch,
- # the 'old_revno' and old_revid values here are incorrect.
- # XXX: FIXME ^. RBC 20060206
- if self.parents:
- old_revid = self.parents[0]
- else:
- old_revid = bzrlib.revision.NULL_REVISION
- for hook in Branch.hooks['post_commit']:
- hook(hook_local, hook_master, old_revno, old_revid, new_revno,
- self.rev_id)
+
+ # Process the post commit hooks, if any
+ self._process_hooks(old_revno, new_revno)
self._emit_progress_update()
finally:
self._cleanup()
@@ -475,6 +440,60 @@
self.master_branch.lock_write()
self.master_locked = True
+ def _check_out_of_date_tree(self):
+ """Check that the working tree is up to date.
+
+ :return: old_revision_number,new_revision_number tuple
+ """
+ try:
+ first_tree_parent = self.work_tree.get_parent_ids()[0]
+ except IndexError:
+ # if there are no parents, treat our parent as 'None'
+ # this is so that we still consider the master branch
+ # - in a checkout scenario the tree may have no
+ # parents but the branch may do.
+ first_tree_parent = bzrlib.revision.NULL_REVISION
+ old_revno, master_last = self.master_branch.last_revision_info()
+ if master_last != first_tree_parent:
+ if master_last != bzrlib.revision.NULL_REVISION:
+ raise errors.OutOfDateTree(self.work_tree)
+ if self.branch.repository.has_revision(first_tree_parent):
+ new_revno = old_revno + 1
+ else:
+ # ghost parents never appear in revision history.
+ new_revno = 1
+ return old_revno,new_revno
+
+ def _process_hooks(self, old_revno, new_revno):
+ """Process any registered commit hooks."""
+ # old style commit hooks - should be deprecated ? (obsoleted in
+ # 0.15)
+ if self.config.post_commit() is not None:
+ hooks = self.config.post_commit().split(' ')
+ # this would be nicer with twisted.python.reflect.namedAny
+ for hook in hooks:
+ result = eval(hook + '(branch, rev_id)',
+ {'branch':self.branch,
+ 'bzrlib':bzrlib,
+ 'rev_id':self.rev_id})
+ # new style commit hooks:
+ if not self.bound_branch:
+ hook_master = self.branch
+ hook_local = None
+ else:
+ hook_master = self.master_branch
+ hook_local = self.branch
+ # With bound branches, when the master is behind the local branch,
+ # the 'old_revno' and old_revid values here are incorrect.
+ # XXX: FIXME ^. RBC 20060206
+ if self.parents:
+ old_revid = self.parents[0]
+ else:
+ old_revid = bzrlib.revision.NULL_REVISION
+ for hook in Branch.hooks['post_commit']:
+ hook(hook_local, hook_master, old_revno, old_revid, new_revno,
+ self.rev_id)
+
def _cleanup(self):
"""Cleanup any open locks, progress bars etc."""
cleanups = [self._cleanup_bound_branch,
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py 2007-05-31 20:29:04 +0000
+++ b/bzrlib/dirstate.py 2007-06-06 06:07:45 +0000
@@ -1792,7 +1792,6 @@
:param ghosts: A list of the revision_ids that are ghosts at the time
of setting.
"""
- self._validate()
# TODO: generate a list of parent indexes to preserve to save
# processing specific parent trees. In the common case one tree will
# be preserved - the left most parent.
@@ -1923,7 +1922,6 @@
self._header_state = DirState.IN_MEMORY_MODIFIED
self._dirblock_state = DirState.IN_MEMORY_MODIFIED
self._id_index = id_index
- self._validate()
def _sort_entries(self, entry_list):
"""Given a list of entries, sort them into the right order.
=== modified file 'bzrlib/workingtree_4.py'
--- a/bzrlib/workingtree_4.py 2007-06-01 13:42:02 +0000
+++ b/bzrlib/workingtree_4.py 2007-06-06 06:11:58 +0000
@@ -472,6 +472,7 @@
@needs_read_lock
def id2path(self, file_id):
+ "Convert a file-id to a path."
file_id = osutils.safe_file_id(file_id)
state = self.current_dirstate()
entry = self._get_entry(file_id=file_id)
@@ -481,16 +482,22 @@
return path_utf8.decode('utf8')
if not osutils.supports_executable():
- @needs_read_lock
def is_executable(self, file_id, path=None):
+ """Test if a file is executable or not.
+
+ Note: The caller is expected to take a read-lock before calling this.
+ """
file_id = osutils.safe_file_id(file_id)
entry = self._get_entry(file_id=file_id, path=path)
if entry == (None, None):
return False
return entry[1][0][3]
else:
- @needs_read_lock
def is_executable(self, file_id, path=None):
+ """Test if a file is executable or not.
+
+ Note: The caller is expected to take a read-lock before calling this.
+ """
if not path:
file_id = osutils.safe_file_id(file_id)
path = self.id2path(file_id)
@@ -530,12 +537,13 @@
# path is missing on disk.
continue
- @needs_read_lock
def kind(self, file_id):
"""Return the kind of a file.
This is always the actual kind that's on disk, regardless of what it
was added as.
+
+ Note: The caller is expected to take a read-lock before calling this.
"""
relpath = self.id2path(file_id)
assert relpath != None, \
@@ -1060,10 +1068,8 @@
except (errors.NoSuchRevision, errors.RevisionNotPresent):
revtree = None
trees.append((revision_id, revtree))
- self.current_dirstate()._validate()
self.set_parent_trees(trees,
allow_leftmost_as_ghost=allow_leftmost_as_ghost)
- self.current_dirstate()._validate()
@needs_tree_write_lock
def set_parent_trees(self, parents_list, allow_leftmost_as_ghost=False):
@@ -1074,7 +1080,6 @@
parent tree - i.e. a ghost.
"""
dirstate = self.current_dirstate()
- dirstate._validate()
if len(parents_list) > 0:
if not allow_leftmost_as_ghost and parents_list[0][1] is None:
raise errors.GhostRevisionUnusableHere(parents_list[0][0])
@@ -1090,11 +1095,8 @@
real_trees.append((rev_id,
self.branch.repository.revision_tree(None)))
ghosts.append(rev_id)
- dirstate._validate()
dirstate.set_parent_trees(real_trees, ghosts=ghosts)
- dirstate._validate()
self._make_dirty(reset_inventory=False)
- dirstate._validate()
def _set_root_id(self, file_id):
"""See WorkingTree.set_root_id."""
@@ -1283,7 +1285,6 @@
_control_files=control_files)
wt._new_tree()
wt.lock_tree_write()
- wt.current_dirstate()._validate()
try:
if revision_id in (None, NULL_REVISION):
if branch.repository.supports_rich_root():
@@ -1291,7 +1292,6 @@
else:
wt._set_root_id(ROOT_ID)
wt.flush()
- wt.current_dirstate()._validate()
wt.set_last_revision(revision_id)
wt.flush()
basis = wt.basis_tree()
More information about the bazaar-commits
mailing list