Rev 2209: Change Branch.lock_token to only accept and receive the branch lock token (rather than the branch and repo lock tokens). in sftp://bazaar.launchpad.net/%7Ebzr/bzr/hpss/
Andrew Bennetts
andrew.bennetts at canonical.com
Thu Apr 12 08:09:54 BST 2007
At sftp://bazaar.launchpad.net/%7Ebzr/bzr/hpss/
------------------------------------------------------------
revno: 2209
revision-id: andrew.bennetts at canonical.com-20070412070833-5fio6r0fkgnf10u1
parent: andrew.bennetts at canonical.com-20070411055816-oy64yjhzvtnrmfb9
committer: Andrew Bennetts <andrew.bennetts at canonical.com>
branch nick: hpss
timestamp: Thu 2007-04-12 17:08:33 +1000
message:
Change Branch.lock_token to only accept and receive the branch lock token (rather than the branch and repo lock tokens).
modified:
bzrlib/branch.py branch.py-20050309040759-e4baf4e0d046576e
bzrlib/remote.py remote.py-20060720103555-yeeg2x51vn0rbtdp-1
bzrlib/smart/branch.py branch.py-20061124031907-mzh3pla28r83r97f-1
bzrlib/tests/branch_implementations/test_locking.py test_locking.py-20060707151933-tav3o2hpibwi53u4-4
bzrlib/tests/test_smart.py test_smart.py-20061122024551-ol0l0o0oofsu9b3t-2
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py 2007-04-10 15:54:15 +0000
+++ b/bzrlib/branch.py 2007-04-12 07:08:33 +0000
@@ -1264,26 +1264,14 @@
def is_locked(self):
return self.control_files.is_locked()
- def lock_write(self, tokens=None):
- if tokens is not None:
- branch_token, repo_token = tokens
- else:
- branch_token = repo_token = None
- repo_token = self.repository.lock_write(token=repo_token)
+ def lock_write(self, token=None):
+ repo_token = self.repository.lock_write()
try:
- branch_token = self.control_files.lock_write(token=branch_token)
+ token = self.control_files.lock_write(token=token)
except:
self.repository.unlock()
raise
- else:
- tokens = (branch_token, repo_token)
- assert tokens == (None, None) or None not in tokens, (
- 'Both branch and repository locks must return tokens, or else '
- 'neither must return tokens. Got %r.' % (tokens,))
- if tokens == (None, None):
- return None
- else:
- return tokens
+ return token
def lock_read(self):
self.repository.lock_read()
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py 2007-04-05 09:35:26 +0000
+++ b/bzrlib/remote.py 2007-04-12 07:08:33 +0000
@@ -740,11 +740,13 @@
else:
self._lock_count += 1
- def _remote_lock_write(self, tokens):
- if tokens is None:
+ def _remote_lock_write(self, token):
+ if token is None:
branch_token = repo_token = ''
else:
- branch_token, repo_token = tokens
+ branch_token = token
+ repo_token = self.repository.lock_write()
+ self.repository.unlock()
path = self.bzrdir._path_for_remote_call(self._client)
response = self._client.call('Branch.lock_write', path, branch_token,
repo_token)
@@ -754,7 +756,7 @@
elif response[0] == 'LockContention':
raise errors.LockContention('(remote lock)')
elif response[0] == 'TokenMismatch':
- raise errors.TokenMismatch(tokens, '(remote tokens)')
+ raise errors.TokenMismatch(token, '(remote token)')
elif response[0] == 'UnlockableTransport':
raise errors.UnlockableTransport(self.bzrdir.root_transport)
elif response[0] == 'ReadOnlyError':
@@ -762,9 +764,9 @@
else:
assert False, 'unexpected response code %r' % (response,)
- def lock_write(self, tokens=None):
+ def lock_write(self, token=None):
if not self._lock_mode:
- remote_tokens = self._remote_lock_write(tokens)
+ remote_tokens = self._remote_lock_write(token)
self._lock_token, self._repo_lock_token = remote_tokens
assert self._lock_token, 'Remote server did not return a token!'
# TODO: We really, really, really don't want to call _ensure_real
@@ -776,25 +778,29 @@
# -- Andrew Bennetts, 2007-02-22.
self._ensure_real()
if self._real_branch is not None:
- self._real_branch.lock_write(tokens=remote_tokens)
- if tokens is not None:
+ self._real_branch.repository.lock_write(
+ token=self._repo_lock_token)
+ try:
+ self._real_branch.lock_write(token=self._lock_token)
+ finally:
+ self._real_branch.repository.unlock()
+ if token is not None:
self._leave_lock = True
else:
- # XXX: this case seems to be unreachable; tokens cannot be None.
+ # XXX: this case seems to be unreachable; token cannot be None.
self._leave_lock = False
self._lock_mode = 'w'
self._lock_count = 1
elif self._lock_mode == 'r':
raise errors.ReadOnlyTransaction
else:
- if tokens is not None:
- # Tokens were given to lock_write, and we're relocking, so check
- # that the given tokens actually match the ones we already have.
- held_tokens = (self._lock_token, self._repo_lock_token)
- if tokens != held_tokens:
- raise errors.TokenMismatch(str(tokens), str(held_tokens))
+ if token is not None:
+ # A token was given to lock_write, and we're relocking, so check
+ # that the given token actually matches the one we already have.
+ if token != self._lock_token:
+ raise errors.TokenMismatch(token, self._lock_token)
self._lock_count += 1
- return self._lock_token, self._repo_lock_token
+ return self._lock_token
def _unlock(self, branch_token, repo_token):
path = self.bzrdir._path_for_remote_call(self._client)
=== modified file 'bzrlib/smart/branch.py'
--- a/bzrlib/smart/branch.py 2007-03-13 05:52:01 +0000
+++ b/bzrlib/smart/branch.py 2007-04-12 07:08:33 +0000
@@ -52,11 +52,15 @@
processed. The physical lock state won't be changed.
"""
# XXX: write a test for LockContention
- branch.lock_write(tokens=(branch_token, repo_token))
+ branch.repository.lock_write(token=repo_token)
try:
- return self.do_with_locked_branch(branch, *args)
+ branch.lock_write(token=branch_token)
+ try:
+ return self.do_with_locked_branch(branch, *args)
+ finally:
+ branch.unlock()
finally:
- branch.unlock()
+ branch.repository.unlock()
class SmartServerBranchGetConfigFile(SmartServerBranchRequest):
@@ -118,11 +122,12 @@
branch_token = None
if repo_token == '':
repo_token = None
- tokens = (branch_token, repo_token)
- if tokens == ('', ''):
- tokens = None
try:
- branch_token, repo_token = branch.lock_write(tokens=tokens)
+ repo_token = branch.repository.lock_write(token=repo_token)
+ try:
+ branch_token = branch.lock_write(token=branch_token)
+ finally:
+ branch.repository.unlock()
except errors.LockContention:
return SmartServerResponse(('LockContention',))
except errors.TokenMismatch:
@@ -138,9 +143,12 @@
class SmartServerBranchRequestUnlock(SmartServerBranchRequest):
def do_with_branch(self, branch, branch_token, repo_token):
- tokens = branch_token, repo_token
try:
- tokens = branch.lock_write(tokens=tokens)
+ branch.repository.lock_write(token=repo_token)
+ try:
+ branch.lock_write(token=branch_token)
+ finally:
+ branch.repository.unlock()
except errors.TokenMismatch:
return SmartServerResponse(('TokenMismatch',))
branch.repository.dont_leave_lock_in_place()
=== modified file 'bzrlib/tests/branch_implementations/test_locking.py'
--- a/bzrlib/tests/branch_implementations/test_locking.py 2007-03-28 07:48:37 +0000
+++ b/bzrlib/tests/branch_implementations/test_locking.py 2007-04-12 07:08:33 +0000
@@ -231,53 +231,45 @@
('rc', 'ul', True),
], self.locks)
- def test_lock_write_returns_None_refuses_tokens(self):
+ def test_lock_write_returns_None_refuses_token(self):
branch = self.make_branch('b')
- tokens = branch.lock_write()
+ token = branch.lock_write()
try:
- if tokens is not None:
+ if token is not None:
# This test does not apply, because this lockable supports
# tokens.
return
self.assertRaises(errors.TokenLockingNotSupported,
- branch.lock_write, tokens=('token','token'))
+ branch.lock_write, token='token')
finally:
branch.unlock()
def test_reentering_lock_write_raises_on_token_mismatch(self):
branch = self.make_branch('b')
- tokens = branch.lock_write()
+ token = branch.lock_write()
try:
- if tokens is None:
+ if token is None:
# This test does not apply, because this lockable refuses
# tokens.
return
- branch_token, repo_token = tokens
- different_branch_token = branch_token + 'xxx'
- different_repo_token = repo_token + 'xxx'
+ different_branch_token = token + 'xxx'
# Re-using the same lockable instance with a different branch token
# will raise TokenMismatch.
self.assertRaises(errors.TokenMismatch,
branch.lock_write,
- tokens=(different_branch_token, repo_token))
- # Similarly for a different repository token.
- self.assertRaises(errors.TokenMismatch,
- branch.lock_write,
- tokens=(branch_token, different_repo_token))
+ token=different_branch_token)
finally:
branch.unlock()
def test_lock_write_with_nonmatching_token(self):
branch = self.make_branch('b')
- tokens = branch.lock_write()
+ token = branch.lock_write()
try:
- if tokens is None:
+ if token is None:
# This test does not apply, because this branch refuses
# tokens.
return
- branch_token, repo_token = tokens
- different_branch_token = branch_token + 'xxx'
- different_repo_token = repo_token + 'xxx'
+ different_branch_token = token + 'xxx'
new_branch = branch.bzrdir.open_branch()
# We only want to test the relocking abilities of branch, so use the
@@ -285,10 +277,7 @@
new_branch.repository = branch.repository
self.assertRaises(errors.TokenMismatch,
new_branch.lock_write,
- tokens=(different_branch_token, repo_token))
- self.assertRaises(errors.TokenMismatch,
- new_branch.lock_write,
- tokens=(branch_token, different_repo_token))
+ token=different_branch_token)
finally:
branch.unlock()
@@ -297,14 +286,14 @@
"""Test that a branch can be locked with a token, if it is already
locked by that token."""
branch = self.make_branch('b')
- tokens = branch.lock_write()
+ token = branch.lock_write()
try:
- if tokens is None:
+ if token is None:
# This test does not apply, because this branch refuses tokens.
return
# The same instance will accept a second lock_write if the specified
# token matches.
- branch.lock_write(tokens=tokens)
+ branch.lock_write(token=token)
branch.unlock()
# Calling lock_write on a new instance for the same lockable will
# also succeed.
@@ -312,18 +301,18 @@
# We only want to test the relocking abilities of branch, so use the
# existing repository object which is already locked.
new_branch.repository = branch.repository
- new_branch.lock_write(tokens=tokens)
+ new_branch.lock_write(token=token)
new_branch.unlock()
finally:
branch.unlock()
- def test_unlock_after_lock_write_with_tokens(self):
+ def test_unlock_after_lock_write_with_token(self):
# If lock_write did not physically acquire the lock (because it was
# passed some tokens), then unlock should not physically release it.
branch = self.make_branch('b')
- tokens = branch.lock_write()
+ token = branch.lock_write()
try:
- if tokens is None:
+ if token is None:
# This test does not apply, because this lockable refuses
# tokens.
return
@@ -331,38 +320,38 @@
# We only want to test the relocking abilities of branch, so use the
# existing repository object which is already locked.
new_branch.repository = branch.repository
- new_branch.lock_write(tokens=tokens)
+ new_branch.lock_write(token=token)
new_branch.unlock()
self.assertTrue(branch.get_physical_lock_status()) #XXX
finally:
branch.unlock()
- def test_lock_write_with_tokens_fails_when_unlocked(self):
- # Lock and unlock to get superficially valid tokens. This mimics a
- # likely programming error, where a caller accidentally tries to lock
- # with tokens that are no longer valid (because the original lock was
- # released).
+ def test_lock_write_with_token_fails_when_unlocked(self):
+ # First, lock and then unlock to get superficially valid tokens. This
+ # mimics a likely programming error, where a caller accidentally tries
+ # to lock with a token that is no longer valid (because the original
+ # lock was released).
branch = self.make_branch('b')
- tokens = branch.lock_write()
+ token = branch.lock_write()
branch.unlock()
- if tokens is None:
+ if token is None:
# This test does not apply, because this lockable refuses
# tokens.
return
self.assertRaises(errors.TokenMismatch,
- branch.lock_write, tokens=tokens)
+ branch.lock_write, token=token)
- def test_lock_write_reenter_with_tokens(self):
+ def test_lock_write_reenter_with_token(self):
branch = self.make_branch('b')
- tokens = branch.lock_write()
+ token = branch.lock_write()
try:
- if tokens is None:
+ if token is None:
# This test does not apply, because this lockable refuses
# tokens.
return
# Relock with a token.
- branch.lock_write(tokens=tokens)
+ branch.lock_write(token=token)
branch.unlock()
finally:
branch.unlock()
@@ -377,12 +366,13 @@
branch = self.make_branch('b')
# Lock the branch, then use leave_lock_in_place so that when we
# unlock the branch the lock is still held on disk.
- tokens = branch.lock_write()
+ token = branch.lock_write()
try:
- if tokens is None:
+ if token is None:
# This test does not apply, because this repository refuses lock
# tokens.
- self.assertRaises(NotImplementedError, branch.leave_lock_in_place)
+ self.assertRaises(NotImplementedError,
+ branch.leave_lock_in_place)
return
branch.leave_lock_in_place()
finally:
@@ -393,9 +383,9 @@
def test_dont_leave_lock_in_place(self):
branch = self.make_branch('b')
# Create a lock on disk.
- tokens = branch.lock_write()
+ token = branch.lock_write()
try:
- if tokens is None:
+ if token is None:
# This test does not apply, because this branch refuses lock
# tokens.
self.assertRaises(NotImplementedError,
@@ -407,12 +397,19 @@
# This branch doesn't support this API.
return
branch.repository.leave_lock_in_place()
+ repo_token = branch.repository.lock_write()
+ branch.repository.unlock()
finally:
branch.unlock()
# Reacquire the lock (with a different branch object) by using the
# tokens.
new_branch = branch.bzrdir.open_branch()
- new_branch.lock_write(tokens=tokens)
+ # We have to explicitly lock the repository first.
+ new_branch.repository.lock_write(token=repo_token)
+ new_branch.lock_write(token=token)
+ # Now we don't need our own repository lock anymore (the branch is
+ # holding it for us).
+ new_branch.repository.unlock()
# Call dont_leave_lock_in_place, so that the lock will be released by
# this instance, even though the lock wasn't originally acquired by it.
new_branch.dont_leave_lock_in_place()
=== modified file 'bzrlib/tests/test_smart.py'
--- a/bzrlib/tests/test_smart.py 2007-04-05 09:35:26 +0000
+++ b/bzrlib/tests/test_smart.py 2007-04-12 07:08:33 +0000
@@ -389,7 +389,9 @@
backing = self.get_transport()
request = smart.branch.SmartServerBranchRequestLockWrite(backing)
branch = self.make_branch('.')
- branch_token, repo_token = branch.lock_write()
+ branch_token = branch.lock_write()
+ repo_token = branch.repository.lock_write()
+ branch.repository.unlock()
branch.leave_lock_in_place()
branch.repository.leave_lock_in_place()
branch.unlock()
@@ -402,7 +404,9 @@
backing = self.get_transport()
request = smart.branch.SmartServerBranchRequestLockWrite(backing)
branch = self.make_branch('.')
- branch_token, repo_token = branch.lock_write()
+ branch_token = branch.lock_write()
+ repo_token = branch.repository.lock_write()
+ branch.repository.unlock()
branch.leave_lock_in_place()
branch.repository.leave_lock_in_place()
branch.unlock()
@@ -442,7 +446,9 @@
request = smart.branch.SmartServerBranchRequestUnlock(backing)
branch = self.make_branch('.')
# Lock the branch
- branch_token, repo_token = branch.lock_write()
+ branch_token = branch.lock_write()
+ repo_token = branch.repository.lock_write()
+ branch.repository.unlock()
# Unlock the branch (and repo) object, leaving the physical locks
# in place.
branch.leave_lock_in_place()
More information about the bazaar-commits
mailing list