Rev 4965: (andrew) Give 'location is a repository' hint for NotBranchError in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Fri Jan 15 05:43:43 GMT 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4965 [merge]
revision-id: pqm at pqm.ubuntu.com-20100115054342-o6ar5y4ch9tcnzyi
parent: pqm at pqm.ubuntu.com-20100115044948-yxz5m3vchxapbq22
parent: andrew.bennetts at canonical.com-20100115051141-urtmica4o7x05kfi
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2010-01-15 05:43:42 +0000
message:
(andrew) Give 'location is a repository' hint for NotBranchError
(#440952).
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/branch.py branch.py-20050309040759-e4baf4e0d046576e
bzrlib/errors.py errors.py-20050309040759-20512168c4e14fbd
bzrlib/remote.py remote.py-20060720103555-yeeg2x51vn0rbtdp-1
bzrlib/smart/bzrdir.py bzrdir.py-20061122024551-ol0l0o0oofsu9b3t-1
bzrlib/smart/request.py request.py-20061108095550-gunadhxmzkdjfeek-1
bzrlib/tests/blackbox/test_shared_repository.py test_shared_repository.py-20060317053531-ed30c0d79325e483
bzrlib/tests/per_branch/test_push.py test_push.py-20070130153159-fhfap8uoifevg30j-1
bzrlib/tests/test_errors.py test_errors.py-20060210110251-41aba2deddf936a8
bzrlib/tests/test_remote.py test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
bzrlib/tests/test_smart.py test_smart.py-20061122024551-ol0l0o0oofsu9b3t-2
=== modified file 'NEWS'
--- a/NEWS 2010-01-15 04:06:45 +0000
+++ b/NEWS 2010-01-15 04:53:03 +0000
@@ -116,6 +116,10 @@
``try``/``finally`` blocks where applicable as it is simpler and more
robust. (Andrew Bennetts)
+* Attempts to open a shared repository as a branch (e.g. ``bzr branch
+ path/to/repo``) will now include "location is a repository" as a hint in
+ the error message. (Brian de Alwis, Andrew Bennetts, #440952)
+
* Push will now inform the user when they are trying to push to a foreign
VCS for which roundtripping is not supported, and will suggest them to
use dpush. (Jelmer Vernooij)
@@ -166,6 +170,10 @@
Internals
*********
+* Added ``BzrDir.open_branchV3`` smart server request, which can receive
+ a string of details (such as "location is a repository") as part of a
+ ``nobranch`` response. (Andrew Bennetts, #440952)
+
* New helper osutils.UnicodeOrBytesToBytesWriter which encodes unicode
objects but passes str objects straight through. This is used for
selftest but may be useful for diff and other operations that generate
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py 2010-01-08 05:28:17 +0000
+++ b/bzrlib/branch.py 2010-01-12 02:48:41 +0000
@@ -1441,7 +1441,7 @@
format_string = transport.get_bytes("format")
return klass._formats[format_string]
except errors.NoSuchFile:
- raise errors.NotBranchError(path=transport.base)
+ raise errors.NotBranchError(path=transport.base, bzrdir=a_bzrdir)
except KeyError:
raise errors.UnknownFormatError(format=format_string, kind='branch')
@@ -1796,7 +1796,7 @@
_repository=a_bzrdir.find_repository(),
ignore_fallbacks=ignore_fallbacks)
except errors.NoSuchFile:
- raise errors.NotBranchError(path=transport.base)
+ raise errors.NotBranchError(path=transport.base, bzrdir=a_bzrdir)
def __init__(self):
super(BranchFormatMetadir, self).__init__()
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py 2010-01-08 06:33:05 +0000
+++ b/bzrlib/errors.py 2010-01-12 02:48:41 +0000
@@ -702,11 +702,32 @@
# TODO: Probably this behavior of should be a common superclass
class NotBranchError(PathError):
- _fmt = 'Not a branch: "%(path)s".'
+ _fmt = 'Not a branch: "%(path)s"%(detail)s.'
- def __init__(self, path):
+ def __init__(self, path, detail=None, bzrdir=None):
import bzrlib.urlutils as urlutils
- self.path = urlutils.unescape_for_display(path, 'ascii')
+ path = urlutils.unescape_for_display(path, 'ascii')
+ if detail is not None:
+ detail = ': ' + detail
+ self.detail = detail
+ self.bzrdir = bzrdir
+ PathError.__init__(self, path=path)
+
+ def _format(self):
+ # XXX: Ideally self.detail would be a property, but Exceptions in
+ # Python 2.4 have to be old-style classes so properties don't work.
+ # Instead we override _format.
+ if self.detail is None:
+ if self.bzrdir is not None:
+ try:
+ self.bzrdir.open_repository()
+ except NoRepositoryPresent:
+ self.detail = ''
+ else:
+ self.detail = ': location is a repository'
+ else:
+ self.detail = ''
+ return PathError._format(self)
class NoSubmitBranch(PathError):
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py 2009-12-15 20:32:34 +0000
+++ b/bzrlib/remote.py 2010-01-11 05:58:22 +0000
@@ -284,21 +284,32 @@
def _get_branch_reference(self):
path = self._path_for_remote_call(self._client)
medium = self._client._medium
- if not medium._is_remote_before((1, 13)):
+ candidate_calls = [
+ ('BzrDir.open_branchV3', (2, 1)),
+ ('BzrDir.open_branchV2', (1, 13)),
+ ('BzrDir.open_branch', None),
+ ]
+ for verb, required_version in candidate_calls:
+ if required_version and medium._is_remote_before(required_version):
+ continue
try:
- response = self._call('BzrDir.open_branchV2', path)
- if response[0] not in ('ref', 'branch'):
- raise errors.UnexpectedSmartServerResponse(response)
- return response
+ response = self._call(verb, path)
except errors.UnknownSmartMethod:
- medium._remember_remote_is_before((1, 13))
- response = self._call('BzrDir.open_branch', path)
- if response[0] != 'ok':
+ if required_version is None:
+ raise
+ medium._remember_remote_is_before(required_version)
+ else:
+ break
+ if verb == 'BzrDir.open_branch':
+ if response[0] != 'ok':
+ raise errors.UnexpectedSmartServerResponse(response)
+ if response[1] != '':
+ return ('ref', response[1])
+ else:
+ return ('branch', '')
+ if response[0] not in ('ref', 'branch'):
raise errors.UnexpectedSmartServerResponse(response)
- if response[1] != '':
- return ('ref', response[1])
- else:
- return ('branch', '')
+ return response
def _get_tree_branch(self):
"""See BzrDir._get_tree_branch()."""
@@ -2823,8 +2834,13 @@
raise NoSuchRevision(find('branch'), err.error_args[0])
elif err.error_verb == 'nosuchrevision':
raise NoSuchRevision(find('repository'), err.error_args[0])
- elif err.error_tuple == ('nobranch',):
- raise errors.NotBranchError(path=find('bzrdir').root_transport.base)
+ elif err.error_verb == 'nobranch':
+ if len(err.error_args) >= 1:
+ extra = err.error_args[0]
+ else:
+ extra = None
+ raise errors.NotBranchError(path=find('bzrdir').root_transport.base,
+ detail=extra)
elif err.error_verb == 'norepository':
raise errors.NoRepositoryPresent(find('bzrdir'))
elif err.error_verb == 'LockContention':
=== modified file 'bzrlib/smart/bzrdir.py'
--- a/bzrlib/smart/bzrdir.py 2009-09-22 00:34:10 +0000
+++ b/bzrlib/smart/bzrdir.py 2010-01-12 01:10:03 +0000
@@ -88,8 +88,8 @@
try:
self._bzrdir = BzrDir.open_from_transport(
self.transport_from_client_path(path))
- except errors.NotBranchError:
- return FailedSmartServerResponse(('nobranch', ))
+ except errors.NotBranchError, e:
+ return FailedSmartServerResponse(('nobranch',))
return self.do_bzrdir_request(*args)
def _boolean_to_yes_no(self, a_boolean):
@@ -465,8 +465,8 @@
return SuccessfulSmartServerResponse(('ok', ''))
else:
return SuccessfulSmartServerResponse(('ok', reference_url))
- except errors.NotBranchError:
- return FailedSmartServerResponse(('nobranch', ))
+ except errors.NotBranchError, e:
+ return FailedSmartServerResponse(('nobranch',))
class SmartServerRequestOpenBranchV2(SmartServerRequestBzrDir):
@@ -481,5 +481,39 @@
return SuccessfulSmartServerResponse(('branch', format))
else:
return SuccessfulSmartServerResponse(('ref', reference_url))
- except errors.NotBranchError:
- return FailedSmartServerResponse(('nobranch', ))
+ except errors.NotBranchError, e:
+ return FailedSmartServerResponse(('nobranch',))
+
+
+class SmartServerRequestOpenBranchV3(SmartServerRequestBzrDir):
+
+ def do_bzrdir_request(self):
+ """Open a branch at path and return the reference or format.
+
+ This version introduced in 2.1.
+
+ Differences to SmartServerRequestOpenBranchV2:
+ * can return 2-element ('nobranch', extra), where 'extra' is a string
+ with an explanation like 'location is a repository'. Previously
+ a 'nobranch' response would never have more than one element.
+ """
+ try:
+ reference_url = self._bzrdir.get_branch_reference()
+ if reference_url is None:
+ br = self._bzrdir.open_branch(ignore_fallbacks=True)
+ format = br._format.network_name()
+ return SuccessfulSmartServerResponse(('branch', format))
+ else:
+ return SuccessfulSmartServerResponse(('ref', reference_url))
+ except errors.NotBranchError, e:
+ # Stringify the exception so that its .detail attribute will be
+ # filled out.
+ str(e)
+ resp = ('nobranch',)
+ detail = e.detail
+ if detail:
+ if detail.startswith(': '):
+ detail = detail[2:]
+ resp += (detail,)
+ return FailedSmartServerResponse(resp)
+
=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py 2009-12-21 17:00:29 +0000
+++ b/bzrlib/smart/request.py 2010-01-11 05:58:22 +0000
@@ -561,6 +561,9 @@
'BzrDir.open_branchV2', 'bzrlib.smart.bzrdir',
'SmartServerRequestOpenBranchV2')
request_handlers.register_lazy(
+ 'BzrDir.open_branchV3', 'bzrlib.smart.bzrdir',
+ 'SmartServerRequestOpenBranchV3')
+request_handlers.register_lazy(
'delete', 'bzrlib.smart.vfs', 'DeleteRequest')
request_handlers.register_lazy(
'get', 'bzrlib.smart.vfs', 'GetRequest')
=== modified file 'bzrlib/tests/blackbox/test_shared_repository.py'
--- a/bzrlib/tests/blackbox/test_shared_repository.py 2009-09-23 23:58:10 +0000
+++ b/bzrlib/tests/blackbox/test_shared_repository.py 2010-01-12 01:10:03 +0000
@@ -18,7 +18,7 @@
import os
-from bzrlib.bzrdir import BzrDir
+from bzrlib.bzrdir import BzrDir, BzrDirMetaFormat1
import bzrlib.errors as errors
from bzrlib.tests import TestCaseInTempDir
@@ -120,3 +120,21 @@
# become necessary for this use case. Please do not adjust this number
# upwards without agreement from bzr's network support maintainers.
self.assertLength(15, self.hpss_calls)
+
+ def test_notification_on_branch_from_repository(self):
+ out, err = self.run_bzr("init-repository -q a")
+ self.assertEqual(out, "")
+ self.assertEqual(err, "")
+ dir = BzrDir.open('a')
+ dir.open_repository() # there is a repository there
+ e = self.assertRaises(errors.NotBranchError, dir.open_branch)
+ self.assertContainsRe(str(e), "location is a repository")
+
+ def test_notification_on_branch_from_nonrepository(self):
+ fmt = BzrDirMetaFormat1()
+ t = self.get_transport()
+ t.mkdir('a')
+ dir = fmt.initialize_on_transport(t.clone('a'))
+ self.assertRaises(errors.NoRepositoryPresent, dir.open_repository)
+ e = self.assertRaises(errors.NotBranchError, dir.open_branch)
+ self.assertNotContainsRe(str(e), "location is a repository")
=== modified file 'bzrlib/tests/per_branch/test_push.py'
--- a/bzrlib/tests/per_branch/test_push.py 2009-09-24 05:31:23 +0000
+++ b/bzrlib/tests/per_branch/test_push.py 2010-01-15 05:11:41 +0000
@@ -414,7 +414,7 @@
self.empty_branch.push(target)
self.assertEqual(
['BzrDir.open_2.1',
- 'BzrDir.open_branchV2',
+ 'BzrDir.open_branchV3',
'BzrDir.find_repositoryV3',
'Branch.get_stacked_on_url',
'Branch.lock_write',
=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py 2010-01-08 06:33:05 +0000
+++ b/bzrlib/tests/test_errors.py 2010-01-12 02:48:41 +0000
@@ -623,6 +623,38 @@
self.assertEqual(
'Repository dummy repo cannot suspend a write group.', str(err))
+ def test_not_branch_no_args(self):
+ err = errors.NotBranchError('path')
+ self.assertEqual('Not a branch: "path".', str(err))
+
+ def test_not_branch_bzrdir_with_repo(self):
+ bzrdir = self.make_repository('repo').bzrdir
+ err = errors.NotBranchError('path', bzrdir=bzrdir)
+ self.assertEqual(
+ 'Not a branch: "path": location is a repository.', str(err))
+
+ def test_not_branch_bzrdir_without_repo(self):
+ bzrdir = self.make_bzrdir('bzrdir')
+ err = errors.NotBranchError('path', bzrdir=bzrdir)
+ self.assertEqual('Not a branch: "path".', str(err))
+
+ def test_not_branch_laziness(self):
+ real_bzrdir = self.make_bzrdir('path')
+ class FakeBzrDir(object):
+ def __init__(self):
+ self.calls = []
+ def open_repository(self):
+ self.calls.append('open_repository')
+ raise errors.NoRepositoryPresent(real_bzrdir)
+ fake_bzrdir = FakeBzrDir()
+ err = errors.NotBranchError('path', bzrdir=fake_bzrdir)
+ self.assertEqual([], fake_bzrdir.calls)
+ str(err)
+ self.assertEqual(['open_repository'], fake_bzrdir.calls)
+ # Stringifying twice doesn't try to open a repository twice.
+ str(err)
+ self.assertEqual(['open_repository'], fake_bzrdir.calls)
+
class PassThroughError(errors.BzrError):
=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py 2009-12-16 22:29:31 +0000
+++ b/bzrlib/tests/test_remote.py 2010-01-11 05:58:22 +0000
@@ -440,7 +440,7 @@
'BzrDir.cloning_metadir', ('quack/', 'False'),
'error', ('BranchReference',)),
client.add_expected_call(
- 'BzrDir.open_branchV2', ('quack/',),
+ 'BzrDir.open_branchV3', ('quack/',),
'success', ('ref', self.get_url('referenced'))),
a_bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
_client=client)
@@ -531,7 +531,7 @@
self.make_branch('.')
a_dir = BzrDir.open(self.get_url('.'))
self.reset_smart_call_log()
- verb = 'BzrDir.open_branchV2'
+ verb = 'BzrDir.open_branchV3'
self.disable_verb(verb)
format = a_dir.open_branch()
call_count = len([call for call in self.hpss_calls if
@@ -547,7 +547,7 @@
transport = transport.clone('quack')
client = FakeClient(transport.base)
client.add_expected_call(
- 'BzrDir.open_branchV2', ('quack/',),
+ 'BzrDir.open_branchV3', ('quack/',),
'success', ('branch', branch_network_name))
client.add_expected_call(
'BzrDir.find_repositoryV3', ('quack/',),
@@ -572,7 +572,7 @@
_client=client)
self.assertRaises(errors.NotBranchError, bzrdir.open_branch)
self.assertEqual(
- [('call', 'BzrDir.open_branchV2', ('quack/',))],
+ [('call', 'BzrDir.open_branchV3', ('quack/',))],
client._calls)
def test__get_tree_branch(self):
@@ -602,7 +602,7 @@
network_name = reference_format.network_name()
branch_network_name = self.get_branch_format().network_name()
client.add_expected_call(
- 'BzrDir.open_branchV2', ('~hello/',),
+ 'BzrDir.open_branchV3', ('~hello/',),
'success', ('branch', branch_network_name))
client.add_expected_call(
'BzrDir.find_repositoryV3', ('~hello/',),
@@ -1190,7 +1190,7 @@
client = FakeClient(self.get_url())
branch_network_name = self.get_branch_format().network_name()
client.add_expected_call(
- 'BzrDir.open_branchV2', ('stacked/',),
+ 'BzrDir.open_branchV3', ('stacked/',),
'success', ('branch', branch_network_name))
client.add_expected_call(
'BzrDir.find_repositoryV3', ('stacked/',),
@@ -1226,7 +1226,7 @@
client = FakeClient(self.get_url())
branch_network_name = self.get_branch_format().network_name()
client.add_expected_call(
- 'BzrDir.open_branchV2', ('stacked/',),
+ 'BzrDir.open_branchV3', ('stacked/',),
'success', ('branch', branch_network_name))
client.add_expected_call(
'BzrDir.find_repositoryV3', ('stacked/',),
@@ -2775,6 +2775,15 @@
expected_error = errors.NotBranchError(path=bzrdir.root_transport.base)
self.assertEqual(expected_error, translated_error)
+ def test_nobranch_one_arg(self):
+ bzrdir = self.make_bzrdir('')
+ translated_error = self.translateTuple(
+ ('nobranch', 'extra detail'), bzrdir=bzrdir)
+ expected_error = errors.NotBranchError(
+ path=bzrdir.root_transport.base,
+ detail='extra detail')
+ self.assertEqual(expected_error, translated_error)
+
def test_LockContention(self):
translated_error = self.translateTuple(('LockContention',))
expected_error = errors.LockContention('(remote lock)')
=== modified file 'bzrlib/tests/test_smart.py'
--- a/bzrlib/tests/test_smart.py 2009-10-29 00:16:50 +0000
+++ b/bzrlib/tests/test_smart.py 2010-01-11 05:58:22 +0000
@@ -524,6 +524,14 @@
self.assertEqual(SmartServerResponse(('ok', reference_url)),
request.execute('reference'))
+ def test_notification_on_branch_from_repository(self):
+ """When there is a repository, the error should return details."""
+ backing = self.get_transport()
+ request = smart.bzrdir.SmartServerRequestOpenBranch(backing)
+ repo = self.make_repository('.')
+ self.assertEqual(SmartServerResponse(('nobranch',)),
+ request.execute(''))
+
class TestSmartServerRequestOpenBranchV2(TestCaseWithChrootedTransport):
@@ -575,6 +583,74 @@
response)
self.assertLength(1, opened_branches)
+ def test_notification_on_branch_from_repository(self):
+ """When there is a repository, the error should return details."""
+ backing = self.get_transport()
+ request = smart.bzrdir.SmartServerRequestOpenBranchV2(backing)
+ repo = self.make_repository('.')
+ self.assertEqual(SmartServerResponse(('nobranch',)),
+ request.execute(''))
+
+
+class TestSmartServerRequestOpenBranchV3(TestCaseWithChrootedTransport):
+
+ def test_no_branch(self):
+ """When there is no branch, ('nobranch', ) is returned."""
+ backing = self.get_transport()
+ self.make_bzrdir('.')
+ request = smart.bzrdir.SmartServerRequestOpenBranchV3(backing)
+ self.assertEqual(SmartServerResponse(('nobranch',)),
+ request.execute(''))
+
+ def test_branch(self):
+ """When there is a branch, 'ok' is returned."""
+ backing = self.get_transport()
+ expected = self.make_branch('.')._format.network_name()
+ request = smart.bzrdir.SmartServerRequestOpenBranchV3(backing)
+ self.assertEqual(SuccessfulSmartServerResponse(('branch', expected)),
+ request.execute(''))
+
+ def test_branch_reference(self):
+ """When there is a branch reference, the reference URL is returned."""
+ self.vfs_transport_factory = local.LocalURLServer
+ backing = self.get_transport()
+ request = smart.bzrdir.SmartServerRequestOpenBranchV3(backing)
+ branch = self.make_branch('branch')
+ checkout = branch.create_checkout('reference',lightweight=True)
+ reference_url = BranchReferenceFormat().get_reference(checkout.bzrdir)
+ self.assertFileEqual(reference_url, 'reference/.bzr/branch/location')
+ self.assertEqual(SuccessfulSmartServerResponse(('ref', reference_url)),
+ request.execute('reference'))
+
+ def test_stacked_branch(self):
+ """Opening a stacked branch does not open the stacked-on branch."""
+ trunk = self.make_branch('trunk')
+ feature = self.make_branch('feature')
+ feature.set_stacked_on_url(trunk.base)
+ opened_branches = []
+ Branch.hooks.install_named_hook('open', opened_branches.append, None)
+ backing = self.get_transport()
+ request = smart.bzrdir.SmartServerRequestOpenBranchV3(backing)
+ request.setup_jail()
+ try:
+ response = request.execute('feature')
+ finally:
+ request.teardown_jail()
+ expected_format = feature._format.network_name()
+ self.assertEqual(
+ SuccessfulSmartServerResponse(('branch', expected_format)),
+ response)
+ self.assertLength(1, opened_branches)
+
+ def test_notification_on_branch_from_repository(self):
+ """When there is a repository, the error should return details."""
+ backing = self.get_transport()
+ request = smart.bzrdir.SmartServerRequestOpenBranchV3(backing)
+ repo = self.make_repository('.')
+ self.assertEqual(
+ SmartServerResponse(('nobranch', 'location is a repository')),
+ request.execute(''))
+
class TestSmartServerRequestRevisionHistory(tests.TestCaseWithMemoryTransport):
@@ -1771,6 +1847,8 @@
smart.bzrdir.SmartServerRequestOpenBranch)
self.assertHandlerEqual('BzrDir.open_branchV2',
smart.bzrdir.SmartServerRequestOpenBranchV2)
+ self.assertHandlerEqual('BzrDir.open_branchV3',
+ smart.bzrdir.SmartServerRequestOpenBranchV3)
self.assertHandlerEqual('PackRepository.autopack',
smart.packrepository.SmartServerPackRepositoryAutopack)
self.assertHandlerEqual('Repository.gather_stats',
More information about the bazaar-commits
mailing list