Rev 5918: (spiv) Removed bzrlib.branch._run_with_write_locked_target. Use in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu May 26 09:43:17 UTC 2011


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

------------------------------------------------------------
revno: 5918 [merge]
revision-id: pqm at pqm.ubuntu.com-20110526094314-4zd3oevd65x49obl
parent: pqm at pqm.ubuntu.com-20110526084744-2bim0fu54g836qd2
parent: andrew.bennetts at canonical.com-20110526085834-ayairsmkocykjxl4
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2011-05-26 09:43:14 +0000
message:
  (spiv) Removed bzrlib.branch._run_with_write_locked_target. Use
   bzrlib.cleanup instead. (Andrew Bennetts)
modified:
  bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
  bzrlib/tests/test_branch.py    test_branch.py-20060116013032-97819aa07b8ab3b5
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2011-05-26 08:05:45 +0000
+++ b/bzrlib/branch.py	2011-05-26 08:58:34 +0000
@@ -24,6 +24,7 @@
 from bzrlib import (
         bzrdir,
         cache_utf8,
+        cleanup,
         config as _mod_config,
         debug,
         errors,
@@ -931,8 +932,9 @@
 
         :seealso: Branch._get_tags_bytes.
         """
-        return _run_with_write_locked_target(self, self._set_tags_bytes_locked,
-                bytes)
+        op = cleanup.OperationWithCleanups(self._set_tags_bytes_locked)
+        op.add_cleanup(self.lock_write().unlock)
+        return op.run_simple(bytes)
 
     def _set_tags_bytes_locked(self, bytes):
         self._tags_bytes = bytes
@@ -3168,42 +3170,6 @@
         branch._transport.put_bytes('format', format.get_format_string())
 
 
-def _run_with_write_locked_target(target, callable, *args, **kwargs):
-    """Run ``callable(*args, **kwargs)``, write-locking target for the
-    duration.
-
-    _run_with_write_locked_target will attempt to release the lock it acquires.
-
-    If an exception is raised by callable, then that exception *will* be
-    propagated, even if the unlock attempt raises its own error.  Thus
-    _run_with_write_locked_target should be preferred to simply doing::
-
-        target.lock_write()
-        try:
-            return callable(*args, **kwargs)
-        finally:
-            target.unlock()
-
-    """
-    # This is very similar to bzrlib.decorators.needs_write_lock.  Perhaps they
-    # should share code?
-    target.lock_write()
-    try:
-        result = callable(*args, **kwargs)
-    except:
-        exc_info = sys.exc_info()
-        try:
-            target.unlock()
-        finally:
-            try:
-                raise exc_info[0], exc_info[1], exc_info[2]
-            finally:
-                del exc_info
-    else:
-        target.unlock()
-        return result
-
-
 class InterBranch(InterObject):
     """This class represents operations taking place between two branches.
 
@@ -3408,14 +3374,12 @@
             raise errors.LossyPushToSameVCS(self.source, self.target)
         # TODO: Public option to disable running hooks - should be trivial but
         # needs tests.
-        self.source.lock_read()
-        try:
-            return _run_with_write_locked_target(
-                self.target, self._push_with_bound_branches, overwrite,
-                stop_revision, 
-                _override_hook_source_branch=_override_hook_source_branch)
-        finally:
-            self.source.unlock()
+
+        op = cleanup.OperationWithCleanups(self._push_with_bound_branches)
+        op.add_cleanup(self.source.lock_read().unlock)
+        op.add_cleanup(self.target.lock_write().unlock)
+        return op.run(overwrite, stop_revision,
+            _override_hook_source_branch=_override_hook_source_branch)
 
     def _basic_push(self, overwrite, stop_revision):
         """Basic implementation of push without bound branches or hooks.
@@ -3439,7 +3403,7 @@
         result.new_revno, result.new_revid = self.target.last_revision_info()
         return result
 
-    def _push_with_bound_branches(self, overwrite, stop_revision,
+    def _push_with_bound_branches(self, operation, overwrite, stop_revision,
             _override_hook_source_branch=None):
         """Push from source into target, and into target's master if any.
         """
@@ -3457,21 +3421,18 @@
             # be bound to itself? -- mbp 20070507
             master_branch = self.target.get_master_branch()
             master_branch.lock_write()
-            try:
-                # push into the master from the source branch.
-                master_inter = InterBranch.get(self.source, master_branch)
-                master_inter._basic_push(overwrite, stop_revision)
-                # and push into the target branch from the source. Note that
-                # we push from the source branch again, because it's considered
-                # the highest bandwidth repository.
-                result = self._basic_push(overwrite, stop_revision)
-                result.master_branch = master_branch
-                result.local_branch = self.target
-                _run_hooks()
-                return result
-            finally:
-                master_branch.unlock()
+            operation.add_cleanup(master_branch.unlock)
+            # push into the master from the source branch.
+            master_inter = InterBranch.get(self.source, master_branch)
+            master_inter._basic_push(overwrite, stop_revision)
+            # and push into the target branch from the source. Note that
+            # we push from the source branch again, because it's considered
+            # the highest bandwidth repository.
+            result = self._basic_push(overwrite, stop_revision)
+            result.master_branch = master_branch
+            result.local_branch = self.target
         else:
+            master_branch = None
             # no master branch
             result = self._basic_push(overwrite, stop_revision)
             # TODO: Why set master_branch and local_branch if there's no
@@ -3479,8 +3440,8 @@
             # 20070504
             result.master_branch = self.target
             result.local_branch = None
-            _run_hooks()
-            return result
+        _run_hooks()
+        return result
 
     def _pull(self, overwrite=False, stop_revision=None,
              possible_transports=None, _hook_master=None, run_hooks=True,

=== modified file 'bzrlib/tests/test_branch.py'
--- a/bzrlib/tests/test_branch.py	2011-05-18 16:42:48 +0000
+++ b/bzrlib/tests/test_branch.py	2011-05-26 08:04:46 +0000
@@ -711,70 +711,3 @@
         r.report(f)
         self.assertEqual("No revisions to pull.\n", f.getvalue())
 
-
-class _StubLockable(object):
-    """Helper for TestRunWithWriteLockedTarget."""
-
-    def __init__(self, calls, unlock_exc=None):
-        self.calls = calls
-        self.unlock_exc = unlock_exc
-
-    def lock_write(self):
-        self.calls.append('lock_write')
-
-    def unlock(self):
-        self.calls.append('unlock')
-        if self.unlock_exc is not None:
-            raise self.unlock_exc
-
-
-class _ErrorFromCallable(Exception):
-    """Helper for TestRunWithWriteLockedTarget."""
-
-
-class _ErrorFromUnlock(Exception):
-    """Helper for TestRunWithWriteLockedTarget."""
-
-
-class TestRunWithWriteLockedTarget(tests.TestCase):
-    """Tests for _run_with_write_locked_target."""
-
-    def setUp(self):
-        tests.TestCase.setUp(self)
-        self._calls = []
-
-    def func_that_returns_ok(self):
-        self._calls.append('func called')
-        return 'ok'
-
-    def func_that_raises(self):
-        self._calls.append('func called')
-        raise _ErrorFromCallable()
-
-    def test_success_unlocks(self):
-        lockable = _StubLockable(self._calls)
-        result = _mod_branch._run_with_write_locked_target(
-            lockable, self.func_that_returns_ok)
-        self.assertEqual('ok', result)
-        self.assertEqual(['lock_write', 'func called', 'unlock'], self._calls)
-
-    def test_exception_unlocks_and_propagates(self):
-        lockable = _StubLockable(self._calls)
-        self.assertRaises(_ErrorFromCallable,
-                          _mod_branch._run_with_write_locked_target,
-                          lockable, self.func_that_raises)
-        self.assertEqual(['lock_write', 'func called', 'unlock'], self._calls)
-
-    def test_callable_succeeds_but_error_during_unlock(self):
-        lockable = _StubLockable(self._calls, unlock_exc=_ErrorFromUnlock())
-        self.assertRaises(_ErrorFromUnlock,
-                          _mod_branch._run_with_write_locked_target,
-                          lockable, self.func_that_returns_ok)
-        self.assertEqual(['lock_write', 'func called', 'unlock'], self._calls)
-
-    def test_error_during_unlock_does_not_mask_original_error(self):
-        lockable = _StubLockable(self._calls, unlock_exc=_ErrorFromUnlock())
-        self.assertRaises(_ErrorFromCallable,
-                          _mod_branch._run_with_write_locked_target,
-                          lockable, self.func_that_raises)
-        self.assertEqual(['lock_write', 'func called', 'unlock'], self._calls)

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-05-26 03:00:40 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-05-26 08:58:34 +0000
@@ -163,6 +163,10 @@
 .. Major internal changes, unlikely to be visible to users or plugin 
    developers, but interesting for bzr developers.
 
+* Removed ``bzrlib.branch._run_with_write_locked_target`` as
+  ``bzrlib.cleanup`` provides the same functionality in a more general
+  way.  (Andrew Bennetts)
+
 Testing
 *******
 




More information about the bazaar-commits mailing list