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
1=== modified file 'bzrlib/branch.py'
2--- bzrlib/branch.py 2010-04-28 09:40:23 +0000
3+++ bzrlib/branch.py 2010-04-29 09:01:37 +0000
4@@ -1346,6 +1346,8 @@
5 """
6 # XXX: Fix the bzrdir API to allow getting the branch back from the
7 # clone call. Or something. 20090224 RBC/spiv.
8+ # XXX: Should this perhaps clone colocated branches as well,
9+ # rather than just the default branch? 20100319 JRV
10 if revision_id is None:
11 revision_id = self.last_revision()
12 dir_to = self.bzrdir.clone_on_transport(to_transport,
13@@ -1521,7 +1523,7 @@
14 """Return the current default format."""
15 return klass._default_format
16
17- def get_reference(self, a_bzrdir):
18+ def get_reference(self, a_bzrdir, name=None):
19 """Get the target reference of the branch in a_bzrdir.
20
21 format probing must have been completed before calling
22@@ -1529,12 +1531,13 @@
23 in a_bzrdir is correct.
24
25 :param a_bzrdir: The bzrdir to get the branch data from.
26+ :param name: Name of the colocated branch to fetch
27 :return: None if the branch is not a reference branch.
28 """
29 return None
30
31 @classmethod
32- def set_reference(self, a_bzrdir, to_branch):
33+ def set_reference(self, a_bzrdir, name, to_branch):
34 """Set the target reference of the branch in a_bzrdir.
35
36 format probing must have been completed before calling
37@@ -1542,6 +1545,7 @@
38 in a_bzrdir is correct.
39
40 :param a_bzrdir: The bzrdir to set the branch reference for.
41+ :param name: Name of colocated branch to set, None for default
42 :param to_branch: branch that the checkout is to reference
43 """
44 raise NotImplementedError(self.set_reference)
45@@ -2157,14 +2161,14 @@
46 """See BranchFormat.get_format_description()."""
47 return "Checkout reference format 1"
48
49- def get_reference(self, a_bzrdir):
50+ def get_reference(self, a_bzrdir, name=None):
51 """See BranchFormat.get_reference()."""
52- transport = a_bzrdir.get_branch_transport(None)
53+ transport = a_bzrdir.get_branch_transport(None, name=name)
54 return transport.get_bytes('location')
55
56- def set_reference(self, a_bzrdir, to_branch):
57+ def set_reference(self, a_bzrdir, name, to_branch):
58 """See BranchFormat.set_reference()."""
59- transport = a_bzrdir.get_branch_transport(None)
60+ transport = a_bzrdir.get_branch_transport(None, name=name)
61 location = transport.put_bytes('location', to_branch.base)
62
63 def initialize(self, a_bzrdir, name=None, target_branch=None):
64@@ -2221,7 +2225,7 @@
65 raise AssertionError("wrong format %r found for %r" %
66 (format, self))
67 if location is None:
68- location = self.get_reference(a_bzrdir)
69+ location = self.get_reference(a_bzrdir, name)
70 real_bzrdir = bzrdir.BzrDir.open(
71 location, possible_transports=possible_transports)
72 result = real_bzrdir.open_branch(name=name,
73
74=== modified file 'bzrlib/bzrdir.py'
75--- bzrlib/bzrdir.py 2010-04-28 07:03:38 +0000
76+++ bzrlib/bzrdir.py 2010-04-29 09:01:37 +0000
77@@ -737,13 +737,18 @@
78 raise errors.NoRepositoryPresent(self)
79 return found_repo
80
81- def get_branch_reference(self):
82+ def get_branch_reference(self, name=None):
83 """Return the referenced URL for the branch in this bzrdir.
84
85+ :param name: Optional colocated branch name
86 :raises NotBranchError: If there is no Branch.
87+ :raises NoColocatedBranchSupport: If a branch name was specified
88+ but colocated branches are not supported.
89 :return: The URL the branch in this bzrdir references if it is a
90 reference branch, or None for regular branches.
91 """
92+ if name is not None:
93+ raise errors.NoColocatedBranchSupport(self)
94 return None
95
96 def get_branch_transport(self, branch_format, name=None):
97@@ -994,9 +999,11 @@
98 raise errors.NotBranchError(path=url)
99 a_transport = new_t
100
101- def _get_tree_branch(self):
102+ def _get_tree_branch(self, name=None):
103 """Return the branch and tree, if any, for this bzrdir.
104
105+ :param name: Name of colocated branch to open.
106+
107 Return None for tree if not present or inaccessible.
108 Raise NotBranchError if no branch is present.
109 :return: (tree, branch)
110@@ -1005,9 +1012,12 @@
111 tree = self.open_workingtree()
112 except (errors.NoWorkingTree, errors.NotLocalUrl):
113 tree = None
114- branch = self.open_branch()
115+ branch = self.open_branch(name=name)
116 else:
117- branch = tree.branch
118+ if name is not None:
119+ branch = self.open_branch(name=name)
120+ else:
121+ branch = tree.branch
122 return tree, branch
123
124 @classmethod
125@@ -1736,13 +1746,13 @@
126 def destroy_workingtree_metadata(self):
127 self.transport.delete_tree('checkout')
128
129- def find_branch_format(self):
130+ def find_branch_format(self, name=None):
131 """Find the branch 'format' for this bzrdir.
132
133 This might be a synthetic object for e.g. RemoteBranch and SVN.
134 """
135 from bzrlib.branch import BranchFormat
136- return BranchFormat.find_format(self)
137+ return BranchFormat.find_format(self, name=name)
138
139 def _get_mkdir_mode(self):
140 """Figure out the mode to use when creating a bzrdir subdir."""
141@@ -1750,11 +1760,11 @@
142 lockable_files.TransportLock)
143 return temp_control._dir_mode
144
145- def get_branch_reference(self):
146+ def get_branch_reference(self, name=None):
147 """See BzrDir.get_branch_reference()."""
148 from bzrlib.branch import BranchFormat
149- format = BranchFormat.find_format(self)
150- return format.get_reference(self)
151+ format = BranchFormat.find_format(self, name=name)
152+ return format.get_reference(self, name=name)
153
154 def get_branch_transport(self, branch_format, name=None):
155 """See BzrDir.get_branch_transport()."""
156@@ -1854,7 +1864,7 @@
157 def open_branch(self, name=None, unsupported=False,
158 ignore_fallbacks=False):
159 """See BzrDir.open_branch."""
160- format = self.find_branch_format()
161+ format = self.find_branch_format(name=name)
162 self._check_supported(format, unsupported)
163 return format.open(self, name=name,
164 _found=True, ignore_fallbacks=ignore_fallbacks)
165
166=== modified file 'bzrlib/remote.py'
167--- bzrlib/remote.py 2010-04-28 07:03:38 +0000
168+++ bzrlib/remote.py 2010-04-29 09:01:37 +0000
169@@ -272,16 +272,19 @@
170 def create_workingtree(self, revision_id=None, from_branch=None):
171 raise errors.NotLocalUrl(self.transport.base)
172
173- def find_branch_format(self):
174+ def find_branch_format(self, name=None):
175 """Find the branch 'format' for this bzrdir.
176
177 This might be a synthetic object for e.g. RemoteBranch and SVN.
178 """
179- b = self.open_branch()
180+ b = self.open_branch(name=name)
181 return b._format
182
183- def get_branch_reference(self):
184+ def get_branch_reference(self, name=None):
185 """See BzrDir.get_branch_reference()."""
186+ if name is not None:
187+ # XXX JRV20100304: Support opening colocated branches
188+ raise errors.NoColocatedBranchSupport(self)
189 response = self._get_branch_reference()
190 if response[0] == 'ref':
191 return response[1]
192@@ -318,9 +321,9 @@
193 raise errors.UnexpectedSmartServerResponse(response)
194 return response
195
196- def _get_tree_branch(self):
197+ def _get_tree_branch(self, name=None):
198 """See BzrDir._get_tree_branch()."""
199- return None, self.open_branch()
200+ return None, self.open_branch(name=name)
201
202 def open_branch(self, name=None, unsupported=False,
203 ignore_fallbacks=False):
204
205=== modified file 'bzrlib/switch.py'
206--- bzrlib/switch.py 2010-04-26 23:38:10 +0000
207+++ bzrlib/switch.py 2010-04-29 09:01:37 +0000
208@@ -78,7 +78,7 @@
209 branch_format = control.find_branch_format()
210 if branch_format.get_reference(control) is not None:
211 # Lightweight checkout: update the branch reference
212- branch_format.set_reference(control, to_branch)
213+ branch_format.set_reference(control, None, to_branch)
214 else:
215 b = control.open_branch()
216 bound_branch = b.get_bound_location()
217
218=== modified file 'bzrlib/tests/per_branch/test_branch.py'
219--- bzrlib/tests/per_branch/test_branch.py 2010-04-21 04:12:25 +0000
220+++ bzrlib/tests/per_branch/test_branch.py 2010-04-29 09:01:37 +0000
221@@ -668,7 +668,8 @@
222 this_branch = self.make_branch('this')
223 other_branch = self.make_branch('other')
224 try:
225- this_branch._format.set_reference(this_branch.bzrdir, other_branch)
226+ this_branch._format.set_reference(this_branch.bzrdir, None,
227+ other_branch)
228 except NotImplementedError:
229 # that's ok
230 pass
231
232=== modified file 'bzrlib/tests/per_bzrdir_colo/test_unsupported.py'
233--- bzrlib/tests/per_bzrdir_colo/test_unsupported.py 2010-04-11 19:40:23 +0000
234+++ bzrlib/tests/per_bzrdir_colo/test_unsupported.py 2010-04-29 09:01:37 +0000
235@@ -35,15 +35,7 @@
236
237 class TestNoColocatedSupport(TestCaseWithBzrDir):
238
239- def test_destroy_colocated_branch(self):
240- branch = self.make_branch('branch')
241- # Colocated branches should not be supported *or*
242- # destroy_branch should not be supported at all
243- self.assertRaises(
244- (errors.NoColocatedBranchSupport, errors.UnsupportedOperation),
245- branch.bzrdir.destroy_branch, 'colo')
246-
247- def test_create_colo_branch(self):
248+ def make_bzrdir_with_repo(self):
249 # a bzrdir can construct a branch and repository for itself.
250 if not self.bzrdir_format.is_supported():
251 # unsupported formats are not loopback testable
252@@ -53,7 +45,27 @@
253 t = get_transport(self.get_url())
254 made_control = self.bzrdir_format.initialize(t.base)
255 made_repo = made_control.create_repository()
256+ return made_control
257+
258+ def test_destroy_colocated_branch(self):
259+ branch = self.make_branch('branch')
260+ # Colocated branches should not be supported *or*
261+ # destroy_branch should not be supported at all
262+ self.assertRaises(
263+ (errors.NoColocatedBranchSupport, errors.UnsupportedOperation),
264+ branch.bzrdir.destroy_branch, 'colo')
265+
266+ def test_create_colo_branch(self):
267+ made_control = self.make_bzrdir_with_repo()
268 self.assertRaises(errors.NoColocatedBranchSupport,
269 made_control.create_branch, "colo")
270
271+ def test_branch_transport(self):
272+ made_control = self.make_bzrdir_with_repo()
273+ self.assertRaises(errors.NoColocatedBranchSupport,
274+ made_control.get_branch_transport, None, "colo")
275
276+ def test_get_branch_reference(self):
277+ made_control = self.make_bzrdir_with_repo()
278+ self.assertRaises(errors.NoColocatedBranchSupport,
279+ made_control.get_branch_reference, "colo")
280
281=== modified file 'bzrlib/tests/test_remote.py'
282--- bzrlib/tests/test_remote.py 2010-04-20 00:20:00 +0000
283+++ bzrlib/tests/test_remote.py 2010-04-29 09:01:37 +0000
284@@ -584,7 +584,7 @@
285 # _get_tree_branch is a form of open_branch, but it should only ask for
286 # branch opening, not any other network requests.
287 calls = []
288- def open_branch():
289+ def open_branch(name=None):
290 calls.append("Called")
291 return "a-branch"
292 transport = MemoryTransport()