Bazaar

Merge lp:~jelmer/bzr/more-colo into lp:bzr

  1. more-colo
  2. Merge into bzr.dev
Proposed by Jelmer Vernooij
Status: Merged
Approved by: Vincent Ladeuil
Approved revision: no longer in the source branch.
Merged at revision: 5218
Proposed branch: lp:~jelmer/bzr/more-colo
Merge into: lp:bzr
Prerequisite: lp:~jelmer/bzr/colo-doc
Diff against target: 292 lines (+64/-34)
7 files modified
bzrlib/branch.py (+11/-7)
bzrlib/bzrdir.py (+20/-10)
bzrlib/remote.py (+8/-5)
bzrlib/switch.py (+1/-1)
bzrlib/tests/per_branch/test_branch.py (+2/-1)
bzrlib/tests/per_bzrdir_colo/test_unsupported.py (+21/-9)
bzrlib/tests/test_remote.py (+1/-1)
To merge this branch: bzr merge lp:~jelmer/bzr/more-colo
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
bzr-core Pending
Review via email: mp+23592@code.launchpad.net

This proposal supersedes a proposal from 2010-04-12.

Description of the change

Pass the colocated branch name along in more places, add extra tests.

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote : Posted in a previous version of this proposal

Heads-up: please, consider *always* using name=name instead of a bare 'name'.

Examples:
57 + return BranchFormat.find_format(self, name)

66 + format = self.find_branch_format(name)

85 + b = self.open_branch(name)

I may have missed some.

This patch and a previous one (nick: use-branch-open ?) are breaking bzr-loom without a way to catch up with your modifications.

And since I didn't track the changes closely I'm not even clear about what loom should do with this parameter and patching blindly sounds like as sure recipe to disaster, help !

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

[15:06] <vila> jelmer: lowering the alert level about using name=name instead of name, it was due to an overly aggressively blind local patch to bzr-loom,
[15:07] <vila> jelmer: the remark still stand though, since you're adding a keyword arg than can be None, better use the name= syntax to avoid breakage
[15:07] <jelmer> vila: I agree it's a good idea to use name= anyway

I've now change the patch to use name= where relevant.

Revision history for this message
Vincent Ladeuil (vila) wrote :

@Jelmer: the conflicts don't help here can you update that branch ?

Also, is there some bits waiting from https://code.edge.launchpad.net/~jelmer/bzr/colo-urls/+merge/20860 and should we wait for Robert here too or can this be reviewed independently ?

review: Needs Information
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Should be fixed now. This branch is a prerequisite for the other one, lp:~jelmer/bzr/colo-urls

Revision history for this message
Vincent Ladeuil (vila) wrote :

Good ! Sorry for the delay.

I'll land it.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py 2010-04-28 09:40:23 +0000
+++ bzrlib/branch.py 2010-04-29 09:01:37 +0000
@@ -1346,6 +1346,8 @@
1346 """1346 """
1347 # XXX: Fix the bzrdir API to allow getting the branch back from the1347 # XXX: Fix the bzrdir API to allow getting the branch back from the
1348 # clone call. Or something. 20090224 RBC/spiv.1348 # clone call. Or something. 20090224 RBC/spiv.
1349 # XXX: Should this perhaps clone colocated branches as well,
1350 # rather than just the default branch? 20100319 JRV
1349 if revision_id is None:1351 if revision_id is None:
1350 revision_id = self.last_revision()1352 revision_id = self.last_revision()
1351 dir_to = self.bzrdir.clone_on_transport(to_transport,1353 dir_to = self.bzrdir.clone_on_transport(to_transport,
@@ -1521,7 +1523,7 @@
1521 """Return the current default format."""1523 """Return the current default format."""
1522 return klass._default_format1524 return klass._default_format
15231525
1524 def get_reference(self, a_bzrdir):1526 def get_reference(self, a_bzrdir, name=None):
1525 """Get the target reference of the branch in a_bzrdir.1527 """Get the target reference of the branch in a_bzrdir.
15261528
1527 format probing must have been completed before calling1529 format probing must have been completed before calling
@@ -1529,12 +1531,13 @@
1529 in a_bzrdir is correct.1531 in a_bzrdir is correct.
15301532
1531 :param a_bzrdir: The bzrdir to get the branch data from.1533 :param a_bzrdir: The bzrdir to get the branch data from.
1534 :param name: Name of the colocated branch to fetch
1532 :return: None if the branch is not a reference branch.1535 :return: None if the branch is not a reference branch.
1533 """1536 """
1534 return None1537 return None
15351538
1536 @classmethod1539 @classmethod
1537 def set_reference(self, a_bzrdir, to_branch):1540 def set_reference(self, a_bzrdir, name, to_branch):
1538 """Set the target reference of the branch in a_bzrdir.1541 """Set the target reference of the branch in a_bzrdir.
15391542
1540 format probing must have been completed before calling1543 format probing must have been completed before calling
@@ -1542,6 +1545,7 @@
1542 in a_bzrdir is correct.1545 in a_bzrdir is correct.
15431546
1544 :param a_bzrdir: The bzrdir to set the branch reference for.1547 :param a_bzrdir: The bzrdir to set the branch reference for.
1548 :param name: Name of colocated branch to set, None for default
1545 :param to_branch: branch that the checkout is to reference1549 :param to_branch: branch that the checkout is to reference
1546 """1550 """
1547 raise NotImplementedError(self.set_reference)1551 raise NotImplementedError(self.set_reference)
@@ -2157,14 +2161,14 @@
2157 """See BranchFormat.get_format_description()."""2161 """See BranchFormat.get_format_description()."""
2158 return "Checkout reference format 1"2162 return "Checkout reference format 1"
21592163
2160 def get_reference(self, a_bzrdir):2164 def get_reference(self, a_bzrdir, name=None):
2161 """See BranchFormat.get_reference()."""2165 """See BranchFormat.get_reference()."""
2162 transport = a_bzrdir.get_branch_transport(None)2166 transport = a_bzrdir.get_branch_transport(None, name=name)
2163 return transport.get_bytes('location')2167 return transport.get_bytes('location')
21642168
2165 def set_reference(self, a_bzrdir, to_branch):2169 def set_reference(self, a_bzrdir, name, to_branch):
2166 """See BranchFormat.set_reference()."""2170 """See BranchFormat.set_reference()."""
2167 transport = a_bzrdir.get_branch_transport(None)2171 transport = a_bzrdir.get_branch_transport(None, name=name)
2168 location = transport.put_bytes('location', to_branch.base)2172 location = transport.put_bytes('location', to_branch.base)
21692173
2170 def initialize(self, a_bzrdir, name=None, target_branch=None):2174 def initialize(self, a_bzrdir, name=None, target_branch=None):
@@ -2221,7 +2225,7 @@
2221 raise AssertionError("wrong format %r found for %r" %2225 raise AssertionError("wrong format %r found for %r" %
2222 (format, self))2226 (format, self))
2223 if location is None:2227 if location is None:
2224 location = self.get_reference(a_bzrdir)2228 location = self.get_reference(a_bzrdir, name)
2225 real_bzrdir = bzrdir.BzrDir.open(2229 real_bzrdir = bzrdir.BzrDir.open(
2226 location, possible_transports=possible_transports)2230 location, possible_transports=possible_transports)
2227 result = real_bzrdir.open_branch(name=name, 2231 result = real_bzrdir.open_branch(name=name,
22282232
=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py 2010-04-28 07:03:38 +0000
+++ bzrlib/bzrdir.py 2010-04-29 09:01:37 +0000
@@ -737,13 +737,18 @@
737 raise errors.NoRepositoryPresent(self)737 raise errors.NoRepositoryPresent(self)
738 return found_repo738 return found_repo
739739
740 def get_branch_reference(self):740 def get_branch_reference(self, name=None):
741 """Return the referenced URL for the branch in this bzrdir.741 """Return the referenced URL for the branch in this bzrdir.
742742
743 :param name: Optional colocated branch name
743 :raises NotBranchError: If there is no Branch.744 :raises NotBranchError: If there is no Branch.
745 :raises NoColocatedBranchSupport: If a branch name was specified
746 but colocated branches are not supported.
744 :return: The URL the branch in this bzrdir references if it is a747 :return: The URL the branch in this bzrdir references if it is a
745 reference branch, or None for regular branches.748 reference branch, or None for regular branches.
746 """749 """
750 if name is not None:
751 raise errors.NoColocatedBranchSupport(self)
747 return None752 return None
748753
749 def get_branch_transport(self, branch_format, name=None):754 def get_branch_transport(self, branch_format, name=None):
@@ -994,9 +999,11 @@
994 raise errors.NotBranchError(path=url)999 raise errors.NotBranchError(path=url)
995 a_transport = new_t1000 a_transport = new_t
9961001
997 def _get_tree_branch(self):1002 def _get_tree_branch(self, name=None):
998 """Return the branch and tree, if any, for this bzrdir.1003 """Return the branch and tree, if any, for this bzrdir.
9991004
1005 :param name: Name of colocated branch to open.
1006
1000 Return None for tree if not present or inaccessible.1007 Return None for tree if not present or inaccessible.
1001 Raise NotBranchError if no branch is present.1008 Raise NotBranchError if no branch is present.
1002 :return: (tree, branch)1009 :return: (tree, branch)
@@ -1005,9 +1012,12 @@
1005 tree = self.open_workingtree()1012 tree = self.open_workingtree()
1006 except (errors.NoWorkingTree, errors.NotLocalUrl):1013 except (errors.NoWorkingTree, errors.NotLocalUrl):
1007 tree = None1014 tree = None
1008 branch = self.open_branch()1015 branch = self.open_branch(name=name)
1009 else:1016 else:
1010 branch = tree.branch1017 if name is not None:
1018 branch = self.open_branch(name=name)
1019 else:
1020 branch = tree.branch
1011 return tree, branch1021 return tree, branch
10121022
1013 @classmethod1023 @classmethod
@@ -1736,13 +1746,13 @@
1736 def destroy_workingtree_metadata(self):1746 def destroy_workingtree_metadata(self):
1737 self.transport.delete_tree('checkout')1747 self.transport.delete_tree('checkout')
17381748
1739 def find_branch_format(self):1749 def find_branch_format(self, name=None):
1740 """Find the branch 'format' for this bzrdir.1750 """Find the branch 'format' for this bzrdir.
17411751
1742 This might be a synthetic object for e.g. RemoteBranch and SVN.1752 This might be a synthetic object for e.g. RemoteBranch and SVN.
1743 """1753 """
1744 from bzrlib.branch import BranchFormat1754 from bzrlib.branch import BranchFormat
1745 return BranchFormat.find_format(self)1755 return BranchFormat.find_format(self, name=name)
17461756
1747 def _get_mkdir_mode(self):1757 def _get_mkdir_mode(self):
1748 """Figure out the mode to use when creating a bzrdir subdir."""1758 """Figure out the mode to use when creating a bzrdir subdir."""
@@ -1750,11 +1760,11 @@
1750 lockable_files.TransportLock)1760 lockable_files.TransportLock)
1751 return temp_control._dir_mode1761 return temp_control._dir_mode
17521762
1753 def get_branch_reference(self):1763 def get_branch_reference(self, name=None):
1754 """See BzrDir.get_branch_reference()."""1764 """See BzrDir.get_branch_reference()."""
1755 from bzrlib.branch import BranchFormat1765 from bzrlib.branch import BranchFormat
1756 format = BranchFormat.find_format(self)1766 format = BranchFormat.find_format(self, name=name)
1757 return format.get_reference(self)1767 return format.get_reference(self, name=name)
17581768
1759 def get_branch_transport(self, branch_format, name=None):1769 def get_branch_transport(self, branch_format, name=None):
1760 """See BzrDir.get_branch_transport()."""1770 """See BzrDir.get_branch_transport()."""
@@ -1854,7 +1864,7 @@
1854 def open_branch(self, name=None, unsupported=False,1864 def open_branch(self, name=None, unsupported=False,
1855 ignore_fallbacks=False):1865 ignore_fallbacks=False):
1856 """See BzrDir.open_branch."""1866 """See BzrDir.open_branch."""
1857 format = self.find_branch_format()1867 format = self.find_branch_format(name=name)
1858 self._check_supported(format, unsupported)1868 self._check_supported(format, unsupported)
1859 return format.open(self, name=name,1869 return format.open(self, name=name,
1860 _found=True, ignore_fallbacks=ignore_fallbacks)1870 _found=True, ignore_fallbacks=ignore_fallbacks)
18611871
=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py 2010-04-28 07:03:38 +0000
+++ bzrlib/remote.py 2010-04-29 09:01:37 +0000
@@ -272,16 +272,19 @@
272 def create_workingtree(self, revision_id=None, from_branch=None):272 def create_workingtree(self, revision_id=None, from_branch=None):
273 raise errors.NotLocalUrl(self.transport.base)273 raise errors.NotLocalUrl(self.transport.base)
274274
275 def find_branch_format(self):275 def find_branch_format(self, name=None):
276 """Find the branch 'format' for this bzrdir.276 """Find the branch 'format' for this bzrdir.
277277
278 This might be a synthetic object for e.g. RemoteBranch and SVN.278 This might be a synthetic object for e.g. RemoteBranch and SVN.
279 """279 """
280 b = self.open_branch()280 b = self.open_branch(name=name)
281 return b._format281 return b._format
282282
283 def get_branch_reference(self):283 def get_branch_reference(self, name=None):
284 """See BzrDir.get_branch_reference()."""284 """See BzrDir.get_branch_reference()."""
285 if name is not None:
286 # XXX JRV20100304: Support opening colocated branches
287 raise errors.NoColocatedBranchSupport(self)
285 response = self._get_branch_reference()288 response = self._get_branch_reference()
286 if response[0] == 'ref':289 if response[0] == 'ref':
287 return response[1]290 return response[1]
@@ -318,9 +321,9 @@
318 raise errors.UnexpectedSmartServerResponse(response)321 raise errors.UnexpectedSmartServerResponse(response)
319 return response322 return response
320323
321 def _get_tree_branch(self):324 def _get_tree_branch(self, name=None):
322 """See BzrDir._get_tree_branch()."""325 """See BzrDir._get_tree_branch()."""
323 return None, self.open_branch()326 return None, self.open_branch(name=name)
324327
325 def open_branch(self, name=None, unsupported=False,328 def open_branch(self, name=None, unsupported=False,
326 ignore_fallbacks=False):329 ignore_fallbacks=False):
327330
=== modified file 'bzrlib/switch.py'
--- bzrlib/switch.py 2010-04-26 23:38:10 +0000
+++ bzrlib/switch.py 2010-04-29 09:01:37 +0000
@@ -78,7 +78,7 @@
78 branch_format = control.find_branch_format()78 branch_format = control.find_branch_format()
79 if branch_format.get_reference(control) is not None:79 if branch_format.get_reference(control) is not None:
80 # Lightweight checkout: update the branch reference80 # Lightweight checkout: update the branch reference
81 branch_format.set_reference(control, to_branch)81 branch_format.set_reference(control, None, to_branch)
82 else:82 else:
83 b = control.open_branch()83 b = control.open_branch()
84 bound_branch = b.get_bound_location()84 bound_branch = b.get_bound_location()
8585
=== modified file 'bzrlib/tests/per_branch/test_branch.py'
--- bzrlib/tests/per_branch/test_branch.py 2010-04-21 04:12:25 +0000
+++ bzrlib/tests/per_branch/test_branch.py 2010-04-29 09:01:37 +0000
@@ -668,7 +668,8 @@
668 this_branch = self.make_branch('this')668 this_branch = self.make_branch('this')
669 other_branch = self.make_branch('other')669 other_branch = self.make_branch('other')
670 try:670 try:
671 this_branch._format.set_reference(this_branch.bzrdir, other_branch)671 this_branch._format.set_reference(this_branch.bzrdir, None,
672 other_branch)
672 except NotImplementedError:673 except NotImplementedError:
673 # that's ok674 # that's ok
674 pass675 pass
675676
=== modified file 'bzrlib/tests/per_bzrdir_colo/test_unsupported.py'
--- bzrlib/tests/per_bzrdir_colo/test_unsupported.py 2010-04-11 19:40:23 +0000
+++ bzrlib/tests/per_bzrdir_colo/test_unsupported.py 2010-04-29 09:01:37 +0000
@@ -35,15 +35,7 @@
3535
36class TestNoColocatedSupport(TestCaseWithBzrDir):36class TestNoColocatedSupport(TestCaseWithBzrDir):
3737
38 def test_destroy_colocated_branch(self):38 def make_bzrdir_with_repo(self):
39 branch = self.make_branch('branch')
40 # Colocated branches should not be supported *or*
41 # destroy_branch should not be supported at all
42 self.assertRaises(
43 (errors.NoColocatedBranchSupport, errors.UnsupportedOperation),
44 branch.bzrdir.destroy_branch, 'colo')
45
46 def test_create_colo_branch(self):
47 # a bzrdir can construct a branch and repository for itself.39 # a bzrdir can construct a branch and repository for itself.
48 if not self.bzrdir_format.is_supported():40 if not self.bzrdir_format.is_supported():
49 # unsupported formats are not loopback testable41 # unsupported formats are not loopback testable
@@ -53,7 +45,27 @@
53 t = get_transport(self.get_url())45 t = get_transport(self.get_url())
54 made_control = self.bzrdir_format.initialize(t.base)46 made_control = self.bzrdir_format.initialize(t.base)
55 made_repo = made_control.create_repository()47 made_repo = made_control.create_repository()
48 return made_control
49
50 def test_destroy_colocated_branch(self):
51 branch = self.make_branch('branch')
52 # Colocated branches should not be supported *or*
53 # destroy_branch should not be supported at all
54 self.assertRaises(
55 (errors.NoColocatedBranchSupport, errors.UnsupportedOperation),
56 branch.bzrdir.destroy_branch, 'colo')
57
58 def test_create_colo_branch(self):
59 made_control = self.make_bzrdir_with_repo()
56 self.assertRaises(errors.NoColocatedBranchSupport, 60 self.assertRaises(errors.NoColocatedBranchSupport,
57 made_control.create_branch, "colo")61 made_control.create_branch, "colo")
5862
63 def test_branch_transport(self):
64 made_control = self.make_bzrdir_with_repo()
65 self.assertRaises(errors.NoColocatedBranchSupport,
66 made_control.get_branch_transport, None, "colo")
5967
68 def test_get_branch_reference(self):
69 made_control = self.make_bzrdir_with_repo()
70 self.assertRaises(errors.NoColocatedBranchSupport,
71 made_control.get_branch_reference, "colo")
6072
=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py 2010-04-20 00:20:00 +0000
+++ bzrlib/tests/test_remote.py 2010-04-29 09:01:37 +0000
@@ -584,7 +584,7 @@
584 # _get_tree_branch is a form of open_branch, but it should only ask for584 # _get_tree_branch is a form of open_branch, but it should only ask for
585 # branch opening, not any other network requests.585 # branch opening, not any other network requests.
586 calls = []586 calls = []
587 def open_branch():587 def open_branch(name=None):
588 calls.append("Called")588 calls.append("Called")
589 return "a-branch"589 return "a-branch"
590 transport = MemoryTransport()590 transport = MemoryTransport()