Rev 4811: (andrew) Replace several fragile try/finally blocks in merge.py using in file:///home/pqm/archives/thelove/bzr/2.1/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Feb 11 02:47:35 GMT 2010
At file:///home/pqm/archives/thelove/bzr/2.1/
------------------------------------------------------------
revno: 4811 [merge]
revision-id: pqm at pqm.ubuntu.com-20100211024734-aiture0rgvqnl1dc
parent: pqm at pqm.ubuntu.com-20100211014515-vj7wflpt4ke2qk2v
parent: andrew.bennetts at canonical.com-20100205050643-85ktdw59pu0qnem7
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.1
timestamp: Thu 2010-02-11 02:47:34 +0000
message:
(andrew) Replace several fragile try/finally blocks in merge.py using
bzrlib.cleanup. (#517275)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/cleanup.py cleanup.py-20090922032110-mv6i6y8t04oon9np-1
bzrlib/merge.py merge.py-20050513021216-953b65a438527106
=== modified file 'NEWS'
--- a/NEWS 2010-02-05 07:58:03 +0000
+++ b/NEWS 2010-02-11 02:47:34 +0000
@@ -39,6 +39,15 @@
* The new ``merge_file_content`` should now be ok with tests to avoid
regressions.
(Vincent Ladeuil, #515597)
+
+Internals
+*********
+
+* Use ``bzrlib.cleanup`` rather than less robust ``try``/``finally``
+ blocks in several places in ``bzrlib.merge``. This avoids masking prior
+ errors when errors like ``ImmortalPendingDeletion`` occur during cleanup
+ in ``do_merge``.
+ (Andrew Bennetts, #517275)
API Changes
***********
=== modified file 'bzrlib/cleanup.py'
--- a/bzrlib/cleanup.py 2010-01-13 23:16:20 +0000
+++ b/bzrlib/cleanup.py 2010-02-05 03:37:52 +0000
@@ -31,9 +31,9 @@
If you want to be certain that the first, and only the first, error is raised,
then use::
- operation = OperationWithCleanups(lambda operation: do_something())
+ operation = OperationWithCleanups(do_something)
operation.add_cleanup(cleanup_something)
- operation.run()
+ operation.run_simple()
This is more inconvenient (because you need to make every try block a
function), but will ensure that the first error encountered is the one raised,
=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py 2010-02-01 17:29:44 +0000
+++ b/bzrlib/merge.py 2010-02-05 03:37:52 +0000
@@ -36,6 +36,7 @@
ui,
versionedfile
)
+from bzrlib.cleanup import OperationWithCleanups
from bzrlib.symbol_versioning import (
deprecated_in,
deprecated_method,
@@ -45,11 +46,10 @@
def transform_tree(from_tree, to_tree, interesting_ids=None):
from_tree.lock_tree_write()
- try:
- merge_inner(from_tree.branch, to_tree, from_tree, ignore_zero=True,
- interesting_ids=interesting_ids, this_tree=from_tree)
- finally:
- from_tree.unlock()
+ operation = OperationWithCleanups(merge_inner)
+ operation.add_cleanup(from_tree.unlock)
+ operation.run_simple(from_tree.branch, to_tree, from_tree,
+ ignore_zero=True, interesting_ids=interesting_ids, this_tree=from_tree)
class MergeHooks(hooks.Hooks):
@@ -455,6 +455,7 @@
def _add_parent(self):
new_parents = self.this_tree.get_parent_ids() + [self.other_rev_id]
new_parent_trees = []
+ operation = OperationWithCleanups(self.this_tree.set_parent_trees)
for revision_id in new_parents:
try:
tree = self.revision_tree(revision_id)
@@ -462,14 +463,9 @@
tree = None
else:
tree.lock_read()
+ operation.add_cleanup(tree.unlock)
new_parent_trees.append((revision_id, tree))
- try:
- self.this_tree.set_parent_trees(new_parent_trees,
- allow_leftmost_as_ghost=True)
- finally:
- for _revision_id, tree in new_parent_trees:
- if tree is not None:
- tree.unlock()
+ operation.run_simple(new_parent_trees, allow_leftmost_as_ghost=True)
def set_other(self, other_revision, possible_transports=None):
"""Set the revision and tree to merge from.
@@ -626,7 +622,8 @@
change_reporter=self.change_reporter,
**kwargs)
- def _do_merge_to(self, merge):
+ def _do_merge_to(self):
+ merge = self.make_merger()
if self.other_branch is not None:
self.other_branch.update_references(self.this_branch)
merge.do_merge()
@@ -646,26 +643,19 @@
sub_tree.branch.repository.revision_tree(base_revision)
sub_merge.base_rev_id = base_revision
sub_merge.do_merge()
+ return merge
def do_merge(self):
+ operation = OperationWithCleanups(self._do_merge_to)
self.this_tree.lock_tree_write()
- try:
- if self.base_tree is not None:
- self.base_tree.lock_read()
- try:
- if self.other_tree is not None:
- self.other_tree.lock_read()
- try:
- merge = self.make_merger()
- self._do_merge_to(merge)
- finally:
- if self.other_tree is not None:
- self.other_tree.unlock()
- finally:
- if self.base_tree is not None:
- self.base_tree.unlock()
- finally:
- self.this_tree.unlock()
+ operation.add_cleanup(self.this_tree.unlock)
+ if self.base_tree is not None:
+ self.base_tree.lock_read()
+ operation.add_cleanup(self.base_tree.unlock)
+ if self.other_tree is not None:
+ self.other_tree.lock_read()
+ operation.add_cleanup(self.other_tree.unlock)
+ merge = operation.run_simple()
if len(merge.cooked_conflicts) == 0:
if not self.ignore_zero and not trace.is_quiet():
trace.note("All changes applied successfully.")
@@ -765,41 +755,43 @@
self.do_merge()
def do_merge(self):
+ operation = OperationWithCleanups(self._do_merge)
+ operation.add_cleanup(self.pb.clear)
self.this_tree.lock_tree_write()
+ operation.add_cleanup(self.this_tree.unlock)
self.base_tree.lock_read()
+ operation.add_cleanup(self.base_tree.unlock)
self.other_tree.lock_read()
+ operation.add_cleanup(self.other_tree.unlock)
+ operation.run()
+
+ def _do_merge(self, operation):
+ self.tt = transform.TreeTransform(self.this_tree, self.pb)
+ operation.add_cleanup(self.tt.finalize)
+ self.pp.next_phase()
+ self._compute_transform()
+ self.pp.next_phase()
+ results = self.tt.apply(no_conflicts=True)
+ self.write_modified(results)
try:
- self.tt = transform.TreeTransform(self.this_tree, self.pb)
- try:
- self.pp.next_phase()
- self._compute_transform()
- self.pp.next_phase()
- results = self.tt.apply(no_conflicts=True)
- self.write_modified(results)
- try:
- self.this_tree.add_conflicts(self.cooked_conflicts)
- except errors.UnsupportedOperation:
- pass
- finally:
- self.tt.finalize()
- finally:
- self.other_tree.unlock()
- self.base_tree.unlock()
- self.this_tree.unlock()
- self.pb.clear()
+ self.this_tree.add_conflicts(self.cooked_conflicts)
+ except errors.UnsupportedOperation:
+ pass
def make_preview_transform(self):
+ operation = OperationWithCleanups(self._make_preview_transform)
+ operation.add_cleanup(self.pb.clear)
self.base_tree.lock_read()
+ operation.add_cleanup(self.base_tree.unlock)
self.other_tree.lock_read()
+ operation.add_cleanup(self.other_tree.unlock)
+ return operation.run_simple()
+
+ def _make_preview_transform(self):
self.tt = transform.TransformPreview(self.this_tree)
- try:
- self.pp.next_phase()
- self._compute_transform()
- self.pp.next_phase()
- finally:
- self.other_tree.unlock()
- self.base_tree.unlock()
- self.pb.clear()
+ self.pp.next_phase()
+ self._compute_transform()
+ self.pp.next_phase()
return self.tt
def _compute_transform(self):
More information about the bazaar-commits
mailing list