Rev 2901: Avoid internal error tracebacks on failure to lock on readonly transport (#129701) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed Oct 10 02:05:04 BST 2007
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 2901
revision-id: pqm at pqm.ubuntu.com-20071010010501-ejbj03m5w3k9vdsd
parent: pqm at pqm.ubuntu.com-20071009084347-m6boff8rxmzsz28f
parent: mbp at sourcefrog.net-20071010002157-utci0x44m2w47wgd
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2007-10-10 02:05:01 +0100
message:
Avoid internal error tracebacks on failure to lock on readonly transport (#129701)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/errors.py errors.py-20050309040759-20512168c4e14fbd
bzrlib/lock.py lock.py-20050527050856-ec090bb51bc03349
bzrlib/lockdir.py lockdir.py-20060220222025-98258adf27fbdda3
bzrlib/remote.py remote.py-20060720103555-yeeg2x51vn0rbtdp-1
bzrlib/smart/branch.py branch.py-20061124031907-mzh3pla28r83r97f-1
bzrlib/smart/repository.py repository.py-20061128022038-vr5wy5bubyb8xttk-1
bzrlib/smart/request.py request.py-20061108095550-gunadhxmzkdjfeek-1
bzrlib/symbol_versioning.py symbol_versioning.py-20060105104851-9ecf8af605d15a80
bzrlib/tests/blackbox/test_commit.py test_commit.py-20060212094538-ae88fc861d969db0
bzrlib/tests/per_lock/test_lock.py test_lock.py-20070313190612-mfpoa7t8kvrgrhj2-1
bzrlib/tests/test_errors.py test_errors.py-20060210110251-41aba2deddf936a8
bzrlib/tests/test_generate_ids.py test_generate_ids.py-20061102205935-z3do15ipw6m7v26u-2
bzrlib/tests/test_lockdir.py test_lockdir.py-20060220222025-33d4221569a3d600
bzrlib/tests/test_smart.py test_smart.py-20061122024551-ol0l0o0oofsu9b3t-2
------------------------------------------------------------
revno: 2872.3.5
merged: mbp at sourcefrog.net-20071010002157-utci0x44m2w47wgd
parent: mbp at sourcefrog.net-20071005044709-u4eb0xjho6i9d7o9
parent: pqm at pqm.ubuntu.com-20071009084347-m6boff8rxmzsz28f
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 129701-readonly-commit
timestamp: Wed 2007-10-10 10:21:57 +1000
message:
merge news
------------------------------------------------------------
revno: 2872.3.4
merged: mbp at sourcefrog.net-20071005044709-u4eb0xjho6i9d7o9
parent: mbp at sourcefrog.net-20071005044317-od7a5i2bi5fp0o4o
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 129701-readonly-commit
timestamp: Fri 2007-10-05 14:47:09 +1000
message:
Remove unneeded import
------------------------------------------------------------
revno: 2872.3.3
merged: mbp at sourcefrog.net-20071005044317-od7a5i2bi5fp0o4o
parent: mbp at sourcefrog.net-20071004082328-mt15l85kxbz3fc3s
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 129701-readonly-commit
timestamp: Fri 2007-10-05 14:43:17 +1000
message:
Pass back LockFailed from smart server lock methods
------------------------------------------------------------
revno: 2872.3.2
merged: mbp at sourcefrog.net-20071004082328-mt15l85kxbz3fc3s
parent: mbp at sourcefrog.net-20071003080644-oivy0gkg98sex0ed
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 129701-readonly-commit
timestamp: Thu 2007-10-04 18:23:28 +1000
message:
Fix incidental broken message format for SmartServerResponse
------------------------------------------------------------
revno: 2872.3.1
merged: mbp at sourcefrog.net-20071003080644-oivy0gkg98sex0ed
parent: pqm at pqm.ubuntu.com-20070928041435-uls0r9txks111272
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 129701-readonly-commit
timestamp: Wed 2007-10-03 18:06:44 +1000
message:
Avoid internal error tracebacks on failure to lock on readonly transport (#129701).
Add new LockFailed, which doesn't imply that we failed to get it because of
contention. Raise this if we fail to create the pending or lock directories
because of Transport errors.
UnlockableTransport is not an internal error.
ReadOnlyLockError has a message which didn't match its name or usage; it's now
deprecated and callers are updated to use LockFailed which is more appropriate.
Add zero_ninetytwo deprecation symbol.
Unify assertMatchesRe with TestCase.assertContainsRe.
When the constructor is deprecated, just say that the class is deprecated, not
the __init__ method - this works better with applyDeprecated in tests.
=== modified file 'NEWS'
--- a/NEWS 2007-10-09 08:43:47 +0000
+++ b/NEWS 2007-10-10 00:21:57 +0000
@@ -146,6 +146,9 @@
enable_cache() has been called - the caching feature is now exclusively for
reading existing data. (Robert Collins)
+ * ``ReadOnlyLockError`` is deprecated; ``LockFailed`` is usually more
+ appropriate. (Martin Pool)
+
* Removed ``bzrlib.transport.TransportLogger`` - please see the new
``trace+`` transport instead. (Robert Collins)
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py 2007-10-03 06:15:06 +0000
+++ b/bzrlib/errors.py 2007-10-10 00:21:57 +0000
@@ -841,12 +841,25 @@
_fmt = "Cannot acquire write lock on %(fname)s. %(msg)s"
+ @symbol_versioning.deprecated_method(symbol_versioning.zero_ninetytwo)
def __init__(self, fname, msg):
LockError.__init__(self, '')
self.fname = fname
self.msg = msg
+class LockFailed(LockError):
+
+ internal_error = False
+
+ _fmt = "Cannot lock %(lock)s: %(why)s"
+
+ def __init__(self, lock, why):
+ LockError.__init__(self, '')
+ self.lock = lock
+ self.why = why
+
+
class OutSideTransaction(BzrError):
_fmt = ("A transaction related operation was attempted after"
@@ -874,6 +887,8 @@
class UnlockableTransport(LockError):
+ internal_error = False
+
_fmt = "Cannot lock: transport is read only: %(transport)s"
def __init__(self, transport):
@@ -2185,7 +2200,7 @@
class PatchMissing(BzrError):
"""Raise a patch type was specified but no patch supplied"""
- _fmt = "patch_type was %(patch_type)s, but no patch was supplied."
+ _fmt = "Patch_type was %(patch_type)s, but no patch was supplied."
def __init__(self, patch_type):
BzrError.__init__(self)
=== modified file 'bzrlib/lock.py'
--- a/bzrlib/lock.py 2007-06-27 08:06:25 +0000
+++ b/bzrlib/lock.py 2007-10-03 08:06:44 +0000
@@ -57,7 +57,7 @@
return self.f
except IOError, e:
if e.errno in (errno.EACCES, errno.EPERM):
- raise errors.ReadOnlyLockError(self.filename, str(e))
+ raise errors.LockFailed(self.filename, str(e))
if e.errno != errno.ENOENT:
raise
@@ -223,7 +223,7 @@
new_f = open(self.filename, 'rb+')
except IOError, e:
if e.errno in (errno.EACCES, errno.EPERM):
- raise errors.ReadOnlyLockError(self.filename, str(e))
+ raise errors.LockFailed(self.filename, str(e))
raise
try:
# LOCK_NB will cause IOError to be raised if we can't grab a
=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py 2007-07-04 09:21:56 +0000
+++ b/bzrlib/lockdir.py 2007-10-03 08:06:44 +0000
@@ -118,10 +118,12 @@
LockBreakMismatch,
LockBroken,
LockContention,
+ LockFailed,
LockNotHeld,
NoSuchFile,
PathError,
ResourceBusy,
+ TransportError,
UnlockableTransport,
)
from bzrlib.trace import mutter, note
@@ -192,10 +194,12 @@
This is typically only called when the object/directory containing the
directory is first created. The lock is not held when it's created.
"""
- if self.transport.is_readonly():
- raise UnlockableTransport(self.transport)
self._trace("create lock directory")
- self.transport.mkdir(self.path, mode=mode)
+ try:
+ self.transport.mkdir(self.path, mode=mode)
+ except (TransportError, PathError), e:
+ raise LockFailed(self, e)
+
def _attempt_lock(self):
"""Make the pending directory and attempt to rename into place.
@@ -214,10 +218,15 @@
"""
self._trace("lock_write...")
start_time = time.time()
- tmpname = self._create_pending_dir()
+ try:
+ tmpname = self._create_pending_dir()
+ except (errors.TransportError, PathError), e:
+ self._trace("... failed to create pending dir, %s", e)
+ raise LockFailed(self, e)
try:
self.transport.rename(tmpname, self._held_dir)
- except (PathError, DirectoryNotEmpty, FileExists, ResourceBusy), e:
+ except (errors.TransportError, PathError, DirectoryNotEmpty,
+ FileExists, ResourceBusy), e:
self._trace("... contention, %s", e)
self._remove_pending_dir(tmpname)
raise LockContention(self)
@@ -446,8 +455,6 @@
"""
if self._fake_read_lock:
raise LockContention(self)
- if self.transport.is_readonly():
- raise UnlockableTransport(self.transport)
return self._attempt_lock()
def wait_lock(self, timeout=None, poll=None, max_attempts=None):
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py 2007-10-04 01:24:06 +0000
+++ b/bzrlib/remote.py 2007-10-10 00:21:57 +0000
@@ -420,6 +420,8 @@
raise errors.LockContention('(remote lock)')
elif response[0] == 'UnlockableTransport':
raise errors.UnlockableTransport(self.bzrdir.root_transport)
+ elif response[0] == 'LockFailed':
+ raise errors.LockFailed(response[1], response[2])
else:
raise errors.UnexpectedSmartServerResponse(response)
@@ -964,6 +966,8 @@
raise errors.UnlockableTransport(self.bzrdir.root_transport)
elif response[0] == 'ReadOnlyError':
raise errors.ReadOnlyError(self)
+ elif response[0] == 'LockFailed':
+ raise errors.LockFailed(response[1], response[2])
else:
raise errors.UnexpectedSmartServerResponse(response)
=== modified file 'bzrlib/smart/branch.py'
--- a/bzrlib/smart/branch.py 2007-04-24 12:20:09 +0000
+++ b/bzrlib/smart/branch.py 2007-10-05 04:43:17 +0000
@@ -135,6 +135,8 @@
return FailedSmartServerResponse(('TokenMismatch',))
except errors.UnlockableTransport:
return FailedSmartServerResponse(('UnlockableTransport',))
+ except errors.LockFailed, e:
+ return FailedSmartServerResponse(('LockFailed', str(e.lock), str(e.why)))
branch.repository.leave_lock_in_place()
branch.leave_lock_in_place()
branch.unlock()
=== modified file 'bzrlib/smart/repository.py'
--- a/bzrlib/smart/repository.py 2007-07-19 06:34:09 +0000
+++ b/bzrlib/smart/repository.py 2007-10-05 04:43:17 +0000
@@ -163,6 +163,9 @@
return FailedSmartServerResponse(('LockContention',))
except errors.UnlockableTransport:
return FailedSmartServerResponse(('UnlockableTransport',))
+ except errors.LockFailed, e:
+ return FailedSmartServerResponse(('LockFailed',
+ str(e.lock), str(e.why)))
repository.leave_lock_in_place()
repository.unlock()
if token is None:
=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py 2007-04-26 09:07:38 +0000
+++ b/bzrlib/smart/request.py 2007-10-04 08:23:28 +0000
@@ -93,7 +93,7 @@
return other.args == self.args and other.body == self.body
def __repr__(self):
- return "<SmartServerResponse args=%r body=%r>" % (self.is_successful(),
+ return "<SmartServerResponse args=%r body=%r>" % (
self.args, self.body)
=== modified file 'bzrlib/symbol_versioning.py'
--- a/bzrlib/symbol_versioning.py 2007-10-02 06:13:56 +0000
+++ b/bzrlib/symbol_versioning.py 2007-10-10 00:21:57 +0000
@@ -86,6 +86,8 @@
have a single %s operator in it. a_callable will be turned into a nice
python symbol and then substituted into deprecation_version.
"""
+ # We also want to handle old-style classes, in particular exception, and
+ # they don't have an im_class attribute.
if getattr(a_callable, 'im_class', None) is None:
symbol = "%s.%s" % (a_callable.__module__,
a_callable.__name__)
@@ -131,10 +133,15 @@
def decorated_method(self, *args, **kwargs):
"""This is the decorated method."""
- symbol = "%s.%s.%s" % (self.__class__.__module__,
- self.__class__.__name__,
- callable.__name__
- )
+ if callable.__name__ == '__init__':
+ symbol = "%s.%s" % (self.__class__.__module__,
+ self.__class__.__name__,
+ )
+ else:
+ symbol = "%s.%s.%s" % (self.__class__.__module__,
+ self.__class__.__name__,
+ callable.__name__
+ )
warn(deprecation_version % symbol, DeprecationWarning, stacklevel=2)
return callable(self, *args, **kwargs)
_populate_decorated(callable, deprecation_version, "method",
=== modified file 'bzrlib/tests/blackbox/test_commit.py'
--- a/bzrlib/tests/blackbox/test_commit.py 2007-10-04 05:50:44 +0000
+++ b/bzrlib/tests/blackbox/test_commit.py 2007-10-10 00:21:57 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006 Canonical Ltd
+# Copyright (C) 2005, 2006, 2007 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -553,3 +553,16 @@
out, err = self.run_bzr('commit test -m "partial commit"')
self.assertEquals('', out)
self.assertContainsRe(err, r'modified test\nCommitted revision 2.')
+
+ def test_commit_readonly_checkout(self):
+ # https://bugs.edge.launchpad.net/bzr/+bug/129701
+ # "UnlockableTransport error trying to commit in checkout of readonly
+ # branch"
+ self.make_branch('master')
+ master = BzrDir.open_from_transport(
+ self.get_readonly_transport('master')).open_branch()
+ master.create_checkout('checkout')
+ out, err = self.run_bzr(['commit', '--unchanged', '-mfoo', 'checkout'],
+ retcode=3)
+ self.assertContainsRe(err,
+ r'^bzr: ERROR: Cannot lock.*readonly transport')
=== modified file 'bzrlib/tests/per_lock/test_lock.py'
--- a/bzrlib/tests/per_lock/test_lock.py 2007-03-19 22:11:28 +0000
+++ b/bzrlib/tests/per_lock/test_lock.py 2007-10-03 08:06:44 +0000
@@ -68,7 +68,7 @@
a_lock = self.read_lock('a-file')
a_lock.unlock()
- self.assertRaises(errors.ReadOnlyLockError, self.write_lock, 'a-file')
+ self.assertRaises(errors.LockFailed, self.write_lock, 'a-file')
def test_write_lock(self):
"""Smoke test for write locks."""
=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py 2007-09-25 06:29:46 +0000
+++ b/bzrlib/tests/test_errors.py 2007-10-03 08:06:44 +0000
@@ -21,13 +21,11 @@
from bzrlib import (
bzrdir,
errors,
+ symbol_versioning,
)
from bzrlib.tests import TestCase, TestCaseWithTransport
-# TODO: Make sure builtin exception class formats are consistent - e.g. should
-# or shouldn't end with a full stop, etc.
-
class TestErrors(TestCaseWithTransport):
@@ -142,10 +140,17 @@
str(error))
def test_read_only_lock_error(self):
- error = errors.ReadOnlyLockError('filename', 'error message')
+ error = self.applyDeprecated(symbol_versioning.zero_ninetytwo,
+ errors.ReadOnlyLockError, 'filename', 'error message')
self.assertEqualDiff("Cannot acquire write lock on filename."
" error message", str(error))
+ def test_lock_failed(self):
+ error = errors.LockFailed('http://canonical.com/', 'readonly transport')
+ self.assertEqualDiff("Cannot lock http://canonical.com/: readonly transport",
+ str(error))
+ self.assertFalse(error.internal_error)
+
def test_too_many_concurrent_requests(self):
error = errors.TooManyConcurrentRequests("a medium")
self.assertEqualDiff("The medium 'a medium' has reached its concurrent "
=== modified file 'bzrlib/tests/test_generate_ids.py'
--- a/bzrlib/tests/test_generate_ids.py 2007-02-18 00:22:24 +0000
+++ b/bzrlib/tests/test_generate_ids.py 2007-10-03 08:06:44 +0000
@@ -112,15 +112,10 @@
class TestGenRevisionId(tests.TestCase):
"""Test generating revision ids"""
- def assertMatchesRe(self, regex, text):
- """Make sure text is matched by the regex given"""
- if re.match(regex, text) is None:
- self.fail('Pattern %s did not match text %s' % (regex, text))
-
def assertGenRevisionId(self, regex, username, timestamp=None):
"""gen_revision_id should create a revision id matching the regex"""
revision_id = generate_ids.gen_revision_id(username, timestamp)
- self.assertMatchesRe(regex, revision_id)
+ self.assertContainsRe(revision_id, '^'+regex+'$')
# It should be a utf8 revision_id, not a unicode one
self.assertIsInstance(revision_id, str)
# gen_revision_id should always return ascii revision ids.
=== modified file 'bzrlib/tests/test_lockdir.py'
--- a/bzrlib/tests/test_lockdir.py 2007-08-17 08:10:01 +0000
+++ b/bzrlib/tests/test_lockdir.py 2007-10-05 04:47:09 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 Canonical Ltd
+# Copyright (C) 2006, 2007 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -30,10 +30,13 @@
transport,
)
from bzrlib.errors import (
- LockBreakMismatch,
- LockContention, LockError, UnlockableTransport,
- LockNotHeld, LockBroken
- )
+ LockBreakMismatch,
+ LockBroken,
+ LockContention,
+ LockError,
+ LockFailed,
+ LockNotHeld,
+ )
from bzrlib.lockdir import LockDir
from bzrlib.tests import TestCaseWithTransport
from bzrlib.trace import note
@@ -105,14 +108,14 @@
"""Fail to create lock on readonly transport"""
t = self.get_readonly_transport()
lf = LockDir(t, 'test_lock')
- self.assertRaises(UnlockableTransport, lf.create)
+ self.assertRaises(LockFailed, lf.create)
def test_12_lock_readonly_transport(self):
"""Fail to lock on readonly transport"""
lf = LockDir(self.get_transport(), 'test_lock')
lf.create()
lf = LockDir(self.get_readonly_transport(), 'test_lock')
- self.assertRaises(UnlockableTransport, lf.attempt_lock)
+ self.assertRaises(LockFailed, lf.attempt_lock)
def test_20_lock_contested(self):
"""Contention to get a lock"""
@@ -599,7 +602,7 @@
lock_path = ld1.transport.local_abspath('test_lock')
os.mkdir(lock_path)
osutils.make_readonly(lock_path)
- self.assertRaises(errors.PermissionDenied, ld1.attempt_lock)
+ self.assertRaises(errors.LockFailed, ld1.attempt_lock)
def test_lock_by_token(self):
ld1 = self.get_lock()
=== modified file 'bzrlib/tests/test_smart.py'
--- a/bzrlib/tests/test_smart.py 2007-07-17 06:57:01 +0000
+++ b/bzrlib/tests/test_smart.py 2007-10-05 04:43:17 +0000
@@ -441,8 +441,9 @@
request = smart.branch.SmartServerBranchRequestLockWrite(backing)
branch = self.make_branch('.')
response = request.execute('')
- self.assertEqual(
- SmartServerResponse(('UnlockableTransport',)), response)
+ error_name, lock_str, why_str = response.args
+ self.assertFalse(response.is_successful())
+ self.assertEqual('LockFailed', error_name)
class TestSmartServerBranchRequestUnlock(tests.TestCaseWithTransport):
@@ -707,8 +708,8 @@
request = smart.repository.SmartServerRepositoryLockWrite(backing)
repository = self.make_repository('.')
response = request.execute('')
- self.assertEqual(
- SmartServerResponse(('UnlockableTransport',)), response)
+ self.assertFalse(response.is_successful())
+ self.assertEqual('LockFailed', response.args[0])
class TestSmartServerRepositoryUnlock(tests.TestCaseWithTransport):
More information about the bazaar-commits
mailing list