Merge lp:~jelmer/bzr/sprout-to-bzrdir into lp:bzr
- sprout-to-bzrdir
- Merge into bzr.dev
Proposed by
Jelmer Vernooij
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Jelmer Vernooij | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 5738 | ||||
Proposed branch: | lp:~jelmer/bzr/sprout-to-bzrdir | ||||
Merge into: | lp:bzr | ||||
Diff against target: |
357 lines (+163/-130) 3 files modified
bzrlib/bzrdir.py (+158/-0) bzrlib/controldir.py (+1/-130) doc/en/release-notes/bzr-2.4.txt (+4/-0) |
||||
To merge this branch: | bzr merge lp:~jelmer/bzr/sprout-to-bzrdir | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andrew Bennetts | Approve | ||
Review via email: mp+54561@code.launchpad.net |
Commit message
Move bzr-specific implementation of ControlDir.sprout to Bzrdir.
Description of the change
Move the implementation of ControlDir.sprout() to BzrDir.sprout(). Instead, simply raise NotImplementedE
The current implementation is too BzrDir-specific, so it makes more sense to just have the foreign plugins provide their own implementation.
To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote : | # |
(Don't forget to mention this in release-notes)
Revision history for this message
Jelmer Vernooij (jelmer) wrote : | # |
sent to pqm by email
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'bzrlib/bzrdir.py' | |||
2 | --- bzrlib/bzrdir.py 2011-03-22 12:11:20 +0000 | |||
3 | +++ bzrlib/bzrdir.py 2011-03-24 00:18:30 +0000 | |||
4 | @@ -33,9 +33,11 @@ | |||
5 | 33 | 33 | ||
6 | 34 | import bzrlib | 34 | import bzrlib |
7 | 35 | from bzrlib import ( | 35 | from bzrlib import ( |
8 | 36 | cleanup, | ||
9 | 36 | config, | 37 | config, |
10 | 37 | controldir, | 38 | controldir, |
11 | 38 | errors, | 39 | errors, |
12 | 40 | fetch, | ||
13 | 39 | graph, | 41 | graph, |
14 | 40 | lockable_files, | 42 | lockable_files, |
15 | 41 | lockdir, | 43 | lockdir, |
16 | @@ -60,6 +62,7 @@ | |||
17 | 60 | """) | 62 | """) |
18 | 61 | 63 | ||
19 | 62 | from bzrlib.trace import ( | 64 | from bzrlib.trace import ( |
20 | 65 | mutter, | ||
21 | 63 | note, | 66 | note, |
22 | 64 | ) | 67 | ) |
23 | 65 | 68 | ||
24 | @@ -387,6 +390,161 @@ | |||
25 | 387 | policy = self.determine_repository_policy(force_new_repo) | 390 | policy = self.determine_repository_policy(force_new_repo) |
26 | 388 | return policy.acquire_repository()[0] | 391 | return policy.acquire_repository()[0] |
27 | 389 | 392 | ||
28 | 393 | def _find_source_repo(self, add_cleanup, source_branch): | ||
29 | 394 | """Find the source branch and repo for a sprout operation. | ||
30 | 395 | |||
31 | 396 | This is helper intended for use by _sprout. | ||
32 | 397 | |||
33 | 398 | :returns: (source_branch, source_repository). Either or both may be | ||
34 | 399 | None. If not None, they will be read-locked (and their unlock(s) | ||
35 | 400 | scheduled via the add_cleanup param). | ||
36 | 401 | """ | ||
37 | 402 | if source_branch is not None: | ||
38 | 403 | add_cleanup(source_branch.lock_read().unlock) | ||
39 | 404 | return source_branch, source_branch.repository | ||
40 | 405 | try: | ||
41 | 406 | source_branch = self.open_branch() | ||
42 | 407 | source_repository = source_branch.repository | ||
43 | 408 | except errors.NotBranchError: | ||
44 | 409 | source_branch = None | ||
45 | 410 | try: | ||
46 | 411 | source_repository = self.open_repository() | ||
47 | 412 | except errors.NoRepositoryPresent: | ||
48 | 413 | source_repository = None | ||
49 | 414 | else: | ||
50 | 415 | add_cleanup(source_repository.lock_read().unlock) | ||
51 | 416 | else: | ||
52 | 417 | add_cleanup(source_branch.lock_read().unlock) | ||
53 | 418 | return source_branch, source_repository | ||
54 | 419 | |||
55 | 420 | def sprout(self, url, revision_id=None, force_new_repo=False, | ||
56 | 421 | recurse='down', possible_transports=None, | ||
57 | 422 | accelerator_tree=None, hardlink=False, stacked=False, | ||
58 | 423 | source_branch=None, create_tree_if_local=True): | ||
59 | 424 | """Create a copy of this controldir prepared for use as a new line of | ||
60 | 425 | development. | ||
61 | 426 | |||
62 | 427 | If url's last component does not exist, it will be created. | ||
63 | 428 | |||
64 | 429 | Attributes related to the identity of the source branch like | ||
65 | 430 | branch nickname will be cleaned, a working tree is created | ||
66 | 431 | whether one existed before or not; and a local branch is always | ||
67 | 432 | created. | ||
68 | 433 | |||
69 | 434 | if revision_id is not None, then the clone operation may tune | ||
70 | 435 | itself to download less data. | ||
71 | 436 | :param accelerator_tree: A tree which can be used for retrieving file | ||
72 | 437 | contents more quickly than the revision tree, i.e. a workingtree. | ||
73 | 438 | The revision tree will be used for cases where accelerator_tree's | ||
74 | 439 | content is different. | ||
75 | 440 | :param hardlink: If true, hard-link files from accelerator_tree, | ||
76 | 441 | where possible. | ||
77 | 442 | :param stacked: If true, create a stacked branch referring to the | ||
78 | 443 | location of this control directory. | ||
79 | 444 | :param create_tree_if_local: If true, a working-tree will be created | ||
80 | 445 | when working locally. | ||
81 | 446 | """ | ||
82 | 447 | operation = cleanup.OperationWithCleanups(self._sprout) | ||
83 | 448 | return operation.run(url, revision_id=revision_id, | ||
84 | 449 | force_new_repo=force_new_repo, recurse=recurse, | ||
85 | 450 | possible_transports=possible_transports, | ||
86 | 451 | accelerator_tree=accelerator_tree, hardlink=hardlink, | ||
87 | 452 | stacked=stacked, source_branch=source_branch, | ||
88 | 453 | create_tree_if_local=create_tree_if_local) | ||
89 | 454 | |||
90 | 455 | def _sprout(self, op, url, revision_id=None, force_new_repo=False, | ||
91 | 456 | recurse='down', possible_transports=None, | ||
92 | 457 | accelerator_tree=None, hardlink=False, stacked=False, | ||
93 | 458 | source_branch=None, create_tree_if_local=True): | ||
94 | 459 | add_cleanup = op.add_cleanup | ||
95 | 460 | fetch_spec_factory = fetch.FetchSpecFactory() | ||
96 | 461 | if revision_id is not None: | ||
97 | 462 | fetch_spec_factory.add_revision_ids([revision_id]) | ||
98 | 463 | fetch_spec_factory.source_branch_stop_revision_id = revision_id | ||
99 | 464 | target_transport = _mod_transport.get_transport(url, | ||
100 | 465 | possible_transports) | ||
101 | 466 | target_transport.ensure_base() | ||
102 | 467 | cloning_format = self.cloning_metadir(stacked) | ||
103 | 468 | # Create/update the result branch | ||
104 | 469 | result = cloning_format.initialize_on_transport(target_transport) | ||
105 | 470 | source_branch, source_repository = self._find_source_repo( | ||
106 | 471 | add_cleanup, source_branch) | ||
107 | 472 | fetch_spec_factory.source_branch = source_branch | ||
108 | 473 | # if a stacked branch wasn't requested, we don't create one | ||
109 | 474 | # even if the origin was stacked | ||
110 | 475 | if stacked and source_branch is not None: | ||
111 | 476 | stacked_branch_url = self.root_transport.base | ||
112 | 477 | else: | ||
113 | 478 | stacked_branch_url = None | ||
114 | 479 | repository_policy = result.determine_repository_policy( | ||
115 | 480 | force_new_repo, stacked_branch_url, require_stacking=stacked) | ||
116 | 481 | result_repo, is_new_repo = repository_policy.acquire_repository() | ||
117 | 482 | add_cleanup(result_repo.lock_write().unlock) | ||
118 | 483 | fetch_spec_factory.source_repo = source_repository | ||
119 | 484 | fetch_spec_factory.target_repo = result_repo | ||
120 | 485 | if stacked or (len(result_repo._fallback_repositories) != 0): | ||
121 | 486 | target_repo_kind = fetch.TargetRepoKinds.STACKED | ||
122 | 487 | elif is_new_repo: | ||
123 | 488 | target_repo_kind = fetch.TargetRepoKinds.EMPTY | ||
124 | 489 | else: | ||
125 | 490 | target_repo_kind = fetch.TargetRepoKinds.PREEXISTING | ||
126 | 491 | fetch_spec_factory.target_repo_kind = target_repo_kind | ||
127 | 492 | if source_repository is not None: | ||
128 | 493 | fetch_spec = fetch_spec_factory.make_fetch_spec() | ||
129 | 494 | result_repo.fetch(source_repository, fetch_spec=fetch_spec) | ||
130 | 495 | |||
131 | 496 | if source_branch is None: | ||
132 | 497 | # this is for sprouting a controldir without a branch; is that | ||
133 | 498 | # actually useful? | ||
134 | 499 | # Not especially, but it's part of the contract. | ||
135 | 500 | result_branch = result.create_branch() | ||
136 | 501 | else: | ||
137 | 502 | result_branch = source_branch.sprout(result, | ||
138 | 503 | revision_id=revision_id, repository_policy=repository_policy, | ||
139 | 504 | repository=result_repo) | ||
140 | 505 | mutter("created new branch %r" % (result_branch,)) | ||
141 | 506 | |||
142 | 507 | # Create/update the result working tree | ||
143 | 508 | if (create_tree_if_local and | ||
144 | 509 | isinstance(target_transport, local.LocalTransport) and | ||
145 | 510 | (result_repo is None or result_repo.make_working_trees())): | ||
146 | 511 | wt = result.create_workingtree(accelerator_tree=accelerator_tree, | ||
147 | 512 | hardlink=hardlink, from_branch=result_branch) | ||
148 | 513 | wt.lock_write() | ||
149 | 514 | try: | ||
150 | 515 | if wt.path2id('') is None: | ||
151 | 516 | try: | ||
152 | 517 | wt.set_root_id(self.open_workingtree.get_root_id()) | ||
153 | 518 | except errors.NoWorkingTree: | ||
154 | 519 | pass | ||
155 | 520 | finally: | ||
156 | 521 | wt.unlock() | ||
157 | 522 | else: | ||
158 | 523 | wt = None | ||
159 | 524 | if recurse == 'down': | ||
160 | 525 | basis = None | ||
161 | 526 | if wt is not None: | ||
162 | 527 | basis = wt.basis_tree() | ||
163 | 528 | elif result_branch is not None: | ||
164 | 529 | basis = result_branch.basis_tree() | ||
165 | 530 | elif source_branch is not None: | ||
166 | 531 | basis = source_branch.basis_tree() | ||
167 | 532 | if basis is not None: | ||
168 | 533 | add_cleanup(basis.lock_read().unlock) | ||
169 | 534 | subtrees = basis.iter_references() | ||
170 | 535 | else: | ||
171 | 536 | subtrees = [] | ||
172 | 537 | for path, file_id in subtrees: | ||
173 | 538 | target = urlutils.join(url, urlutils.escape(path)) | ||
174 | 539 | sublocation = source_branch.reference_parent(file_id, path) | ||
175 | 540 | sublocation.bzrdir.sprout(target, | ||
176 | 541 | basis.get_reference_revision(file_id, path), | ||
177 | 542 | force_new_repo=force_new_repo, recurse=recurse, | ||
178 | 543 | stacked=stacked) | ||
179 | 544 | return result | ||
180 | 545 | |||
181 | 546 | |||
182 | 547 | |||
183 | 390 | @staticmethod | 548 | @staticmethod |
184 | 391 | def create_branch_convenience(base, force_new_repo=False, | 549 | def create_branch_convenience(base, force_new_repo=False, |
185 | 392 | force_new_tree=None, format=None, | 550 | force_new_tree=None, format=None, |
186 | 393 | 551 | ||
187 | === modified file 'bzrlib/controldir.py' | |||
188 | --- bzrlib/controldir.py 2011-03-23 14:31:38 +0000 | |||
189 | +++ bzrlib/controldir.py 2011-03-24 00:18:30 +0000 | |||
190 | @@ -27,9 +27,7 @@ | |||
191 | 27 | import textwrap | 27 | import textwrap |
192 | 28 | 28 | ||
193 | 29 | from bzrlib import ( | 29 | from bzrlib import ( |
194 | 30 | cleanup, | ||
195 | 31 | errors, | 30 | errors, |
196 | 32 | fetch, | ||
197 | 33 | revision as _mod_revision, | 31 | revision as _mod_revision, |
198 | 34 | transport as _mod_transport, | 32 | transport as _mod_transport, |
199 | 35 | ui, | 33 | ui, |
200 | @@ -38,9 +36,6 @@ | |||
201 | 38 | from bzrlib.push import ( | 36 | from bzrlib.push import ( |
202 | 39 | PushResult, | 37 | PushResult, |
203 | 40 | ) | 38 | ) |
204 | 41 | from bzrlib.trace import ( | ||
205 | 42 | mutter, | ||
206 | 43 | ) | ||
207 | 44 | from bzrlib.transport import ( | 39 | from bzrlib.transport import ( |
208 | 45 | local, | 40 | local, |
209 | 46 | ) | 41 | ) |
210 | @@ -336,131 +331,7 @@ | |||
211 | 336 | :param create_tree_if_local: If true, a working-tree will be created | 331 | :param create_tree_if_local: If true, a working-tree will be created |
212 | 337 | when working locally. | 332 | when working locally. |
213 | 338 | """ | 333 | """ |
339 | 339 | operation = cleanup.OperationWithCleanups(self._sprout) | 334 | raise NotImplementedError(self.sprout) |
215 | 340 | return operation.run(url, revision_id=revision_id, | ||
216 | 341 | force_new_repo=force_new_repo, recurse=recurse, | ||
217 | 342 | possible_transports=possible_transports, | ||
218 | 343 | accelerator_tree=accelerator_tree, hardlink=hardlink, | ||
219 | 344 | stacked=stacked, source_branch=source_branch, | ||
220 | 345 | create_tree_if_local=create_tree_if_local) | ||
221 | 346 | |||
222 | 347 | def _sprout(self, op, url, revision_id=None, force_new_repo=False, | ||
223 | 348 | recurse='down', possible_transports=None, | ||
224 | 349 | accelerator_tree=None, hardlink=False, stacked=False, | ||
225 | 350 | source_branch=None, create_tree_if_local=True): | ||
226 | 351 | add_cleanup = op.add_cleanup | ||
227 | 352 | fetch_spec_factory = fetch.FetchSpecFactory() | ||
228 | 353 | if revision_id is not None: | ||
229 | 354 | fetch_spec_factory.add_revision_ids([revision_id]) | ||
230 | 355 | fetch_spec_factory.source_branch_stop_revision_id = revision_id | ||
231 | 356 | target_transport = _mod_transport.get_transport(url, | ||
232 | 357 | possible_transports) | ||
233 | 358 | target_transport.ensure_base() | ||
234 | 359 | cloning_format = self.cloning_metadir(stacked) | ||
235 | 360 | # Create/update the result branch | ||
236 | 361 | result = cloning_format.initialize_on_transport(target_transport) | ||
237 | 362 | source_branch, source_repository = self._find_source_repo( | ||
238 | 363 | add_cleanup, source_branch) | ||
239 | 364 | fetch_spec_factory.source_branch = source_branch | ||
240 | 365 | # if a stacked branch wasn't requested, we don't create one | ||
241 | 366 | # even if the origin was stacked | ||
242 | 367 | if stacked and source_branch is not None: | ||
243 | 368 | stacked_branch_url = self.root_transport.base | ||
244 | 369 | else: | ||
245 | 370 | stacked_branch_url = None | ||
246 | 371 | repository_policy = result.determine_repository_policy( | ||
247 | 372 | force_new_repo, stacked_branch_url, require_stacking=stacked) | ||
248 | 373 | result_repo, is_new_repo = repository_policy.acquire_repository() | ||
249 | 374 | add_cleanup(result_repo.lock_write().unlock) | ||
250 | 375 | fetch_spec_factory.source_repo = source_repository | ||
251 | 376 | fetch_spec_factory.target_repo = result_repo | ||
252 | 377 | if stacked or (len(result_repo._fallback_repositories) != 0): | ||
253 | 378 | target_repo_kind = fetch.TargetRepoKinds.STACKED | ||
254 | 379 | elif is_new_repo: | ||
255 | 380 | target_repo_kind = fetch.TargetRepoKinds.EMPTY | ||
256 | 381 | else: | ||
257 | 382 | target_repo_kind = fetch.TargetRepoKinds.PREEXISTING | ||
258 | 383 | fetch_spec_factory.target_repo_kind = target_repo_kind | ||
259 | 384 | if source_repository is not None: | ||
260 | 385 | fetch_spec = fetch_spec_factory.make_fetch_spec() | ||
261 | 386 | result_repo.fetch(source_repository, fetch_spec=fetch_spec) | ||
262 | 387 | |||
263 | 388 | if source_branch is None: | ||
264 | 389 | # this is for sprouting a controldir without a branch; is that | ||
265 | 390 | # actually useful? | ||
266 | 391 | # Not especially, but it's part of the contract. | ||
267 | 392 | result_branch = result.create_branch() | ||
268 | 393 | else: | ||
269 | 394 | result_branch = source_branch.sprout(result, | ||
270 | 395 | revision_id=revision_id, repository_policy=repository_policy, | ||
271 | 396 | repository=result_repo) | ||
272 | 397 | mutter("created new branch %r" % (result_branch,)) | ||
273 | 398 | |||
274 | 399 | # Create/update the result working tree | ||
275 | 400 | if (create_tree_if_local and | ||
276 | 401 | isinstance(target_transport, local.LocalTransport) and | ||
277 | 402 | (result_repo is None or result_repo.make_working_trees())): | ||
278 | 403 | wt = result.create_workingtree(accelerator_tree=accelerator_tree, | ||
279 | 404 | hardlink=hardlink, from_branch=result_branch) | ||
280 | 405 | wt.lock_write() | ||
281 | 406 | try: | ||
282 | 407 | if wt.path2id('') is None: | ||
283 | 408 | try: | ||
284 | 409 | wt.set_root_id(self.open_workingtree.get_root_id()) | ||
285 | 410 | except errors.NoWorkingTree: | ||
286 | 411 | pass | ||
287 | 412 | finally: | ||
288 | 413 | wt.unlock() | ||
289 | 414 | else: | ||
290 | 415 | wt = None | ||
291 | 416 | if recurse == 'down': | ||
292 | 417 | basis = None | ||
293 | 418 | if wt is not None: | ||
294 | 419 | basis = wt.basis_tree() | ||
295 | 420 | elif result_branch is not None: | ||
296 | 421 | basis = result_branch.basis_tree() | ||
297 | 422 | elif source_branch is not None: | ||
298 | 423 | basis = source_branch.basis_tree() | ||
299 | 424 | if basis is not None: | ||
300 | 425 | add_cleanup(basis.lock_read().unlock) | ||
301 | 426 | subtrees = basis.iter_references() | ||
302 | 427 | else: | ||
303 | 428 | subtrees = [] | ||
304 | 429 | for path, file_id in subtrees: | ||
305 | 430 | target = urlutils.join(url, urlutils.escape(path)) | ||
306 | 431 | sublocation = source_branch.reference_parent(file_id, path) | ||
307 | 432 | sublocation.bzrdir.sprout(target, | ||
308 | 433 | basis.get_reference_revision(file_id, path), | ||
309 | 434 | force_new_repo=force_new_repo, recurse=recurse, | ||
310 | 435 | stacked=stacked) | ||
311 | 436 | return result | ||
312 | 437 | |||
313 | 438 | def _find_source_repo(self, add_cleanup, source_branch): | ||
314 | 439 | """Find the source branch and repo for a sprout operation. | ||
315 | 440 | |||
316 | 441 | This is helper intended for use by _sprout. | ||
317 | 442 | |||
318 | 443 | :returns: (source_branch, source_repository). Either or both may be | ||
319 | 444 | None. If not None, they will be read-locked (and their unlock(s) | ||
320 | 445 | scheduled via the add_cleanup param). | ||
321 | 446 | """ | ||
322 | 447 | if source_branch is not None: | ||
323 | 448 | add_cleanup(source_branch.lock_read().unlock) | ||
324 | 449 | return source_branch, source_branch.repository | ||
325 | 450 | try: | ||
326 | 451 | source_branch = self.open_branch() | ||
327 | 452 | source_repository = source_branch.repository | ||
328 | 453 | except errors.NotBranchError: | ||
329 | 454 | source_branch = None | ||
330 | 455 | try: | ||
331 | 456 | source_repository = self.open_repository() | ||
332 | 457 | except errors.NoRepositoryPresent: | ||
333 | 458 | source_repository = None | ||
334 | 459 | else: | ||
335 | 460 | add_cleanup(source_repository.lock_read().unlock) | ||
336 | 461 | else: | ||
337 | 462 | add_cleanup(source_branch.lock_read().unlock) | ||
338 | 463 | return source_branch, source_repository | ||
340 | 464 | 335 | ||
341 | 465 | def push_branch(self, source, revision_id=None, overwrite=False, | 336 | def push_branch(self, source, revision_id=None, overwrite=False, |
342 | 466 | remember=False, create_prefix=False): | 337 | remember=False, create_prefix=False): |
343 | 467 | 338 | ||
344 | === modified file 'doc/en/release-notes/bzr-2.4.txt' | |||
345 | --- doc/en/release-notes/bzr-2.4.txt 2011-03-23 12:56:01 +0000 | |||
346 | +++ doc/en/release-notes/bzr-2.4.txt 2011-03-24 00:18:30 +0000 | |||
347 | @@ -230,6 +230,10 @@ | |||
348 | 230 | ``import_last_revision_info_and_tags`` method instead. | 230 | ``import_last_revision_info_and_tags`` method instead. |
349 | 231 | (Andrew Bennetts) | 231 | (Andrew Bennetts) |
350 | 232 | 232 | ||
351 | 233 | * Because it was too specific to BzrDir implementations, | ||
352 | 234 | ``ControlDir.sprout`` no longer has a default implementation; it now | ||
353 | 235 | raises ``NotImplementedError``. (Jelmer Vernooij, #717937) | ||
354 | 236 | |||
355 | 233 | * ``ControlDirFormat.register_format`` has been removed. Instead, | 237 | * ``ControlDirFormat.register_format`` has been removed. Instead, |
356 | 234 | ``Prober`` implementations should now implement a ``known_formats`` | 238 | ``Prober`` implementations should now implement a ``known_formats`` |
357 | 235 | method. (Jelmer Vernooij) | 239 | method. (Jelmer Vernooij) |
This seems like a reasonable change to me.
It'd be nice to slowly converge on having foreign formats and BzrDir do as much in common as possible, but as a first step acknowledging that sprout is currently quite BzrDir-specific is fair enough.