| Status: | Rejected |
|---|---|
| Rejected by: | Jelmer Vernooij |
| Proposed branch: | lp:~jelmer/bzr/colo-urls |
| Merge into: | lp:bzr |
| Prerequisite: | lp:~jelmer/bzr/use-branch-open |
| Diff against target: |
778 lines (+247/-89) 13 files modified
bzrlib/branch.py (+45/-11) bzrlib/builtins.py (+38/-35) bzrlib/bzrdir.py (+29/-15) bzrlib/errors.py (+7/-0) bzrlib/reconcile.py (+8/-11) 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_branch.py (+37/-0) bzrlib/tests/test_remote.py (+1/-1) bzrlib/tests/test_urlutils.py (+24/-0) bzrlib/urlutils.py (+26/-0) |
| To merge this branch: | bzr merge lp:~jelmer/bzr/colo-urls |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Robert Collins (community) | Needs Fixing | ||
| Andrew Bennetts | Needs Information | ||
|
Review via email:
|
|||
Commit message
Description of the change
Add support for addressing colocated branches using comma's.
To post a comment you must log in.
Revision history for this message
| Jelmer Vernooij (jelmer) wrote : | # |
Revision history for this message
| Andrew Bennetts (spiv) wrote : | # |
My understanding of STD 66 is that commas should bind to path segments, rather than delimit paths entirely, but I'm happy to be corrected if I'm wrong. Also, I don't see any problem with having commas or even slashes in a branch name, that's what percent-encoding is for.
So, if the plan is that ",foo" in a URL indicates a branch name of "foo", then I'd expect URLs to be parsed like:
* http://
* branch location: http://
* branch name: branchname
* http://
* branch location: http://
* branch name: branch,name
* http://
* branch location: http://
* branch name: branchname
* path inside branch: README.txt [e.g. I want to be able to use this with bzr log, bzr cat, and bzr annotate]
I wasn't at Strasbourg. Did we decide against a key/value syntax (like ",branch=name"), or is it still intended that we might support e.g. ",revid=AAAA..." at some point?
(What happened to the promised update to doc/developers/
review:
Needs Information
Revision history for this message
| Martin Pool (mbp) wrote : | # |
I'm still a bit concerned that this is passing the branch name around so many different places, rather than currying that into an object. I guess the problem here is that the places you're updating here are "places a branch could be" not necessarily an existing open branch. Aside from that it looks pretty good, and perhaps it's easier to bring it in and then change it later.
- this_branch.
+ this_branch.
+ other_branch)
Inserting parameters into the middle is a somewhat noxious way to change the api in Python. It's ok to have an api bump but perhaps we should add to the end?
Revision history for this message
| Robert Collins (lifeless) wrote : | # |
actually this is clearly bogus: other_branch *is a branch object*, there is
no need for the branch name to be there - its known already by the branch
object.
I need to go through this with a fine comb I think, this makes me worry
other changes that aren't needed are being done.
Revision history for this message
| Vincent Ladeuil (vila) wrote : | # |
@Robert: Did you unpack your comb ? :-)
Revision history for this message
| Jelmer Vernooij (jelmer) wrote : | # |
Rob, we're setting a branch reference in a local bzrdir to another branch. The name of the local colocated branch is specified by the new parameter.
Revision history for this message
| Robert Collins (lifeless) wrote : | # |
Am looking at this, iz not forgotten.
Revision history for this message
| Robert Collins (lifeless) wrote : | # |
Oh, and @spiv - yes, , should be per segment in the path, not whole-string - thats the point of this ;)
Revision history for this message
| Robert Collins (lifeless) wrote : | # |
Ok, some notes:
- if we have to edit the url before passing to transport, transport isn't supporting segment parameters properly - need some tests on that layer
e.g. opening foo and foo,name=bar should get two equal transports. (except for parameters we decide transports should support - e.g. passive ftp might be one (though we have a syntax for that already)
So - further discussion:
- Transport is our 'access a url' abstraction; it should take care of getting parameters parsed, exposing the parsed parameters to BzrDir.
- BzrDir should get the default branch from the transport parameters.
- other code shouldn't need to change for this part of the conversion
- we should end up being able to open '/foo/bar,name=gam' as a result ! (without changing all the higher layers).
review:
Needs Fixing
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-10 18:03:54 +0000 |
| 3 | +++ bzrlib/branch.py 2010-04-17 17:41:17 +0000 |
| 4 | @@ -63,6 +63,34 @@ |
| 5 | BZR_BRANCH_FORMAT_6 = "Bazaar Branch Format 6 (bzr 0.15)\n" |
| 6 | |
| 7 | |
| 8 | +def split_colocated_name(url): |
| 9 | + """Extract the colocated branch name from a URL. |
| 10 | + |
| 11 | + :param url: The original URL |
| 12 | + :return: Tuple with new URL and colocated branch name (None if not specified) |
| 13 | + """ |
| 14 | + if url is None: |
| 15 | + return (None, None) |
| 16 | + (base, subsegments) = urlutils.split_subsegments(url) |
| 17 | + if len(subsegments) == 0: |
| 18 | + return (url, None) |
| 19 | + if len(subsegments) > 1: |
| 20 | + raise errors.InvalidURL(url) |
| 21 | + return (base, subsegments[0]) |
| 22 | + |
| 23 | + |
| 24 | +def join_colocated_name(base, branch_name): |
| 25 | + """Combine a location and a colocated branch name URL. |
| 26 | + |
| 27 | + :param base: Base location |
| 28 | + :param branch_name: Co-located branch name |
| 29 | + :return: A URL that addresses a colocated branch |
| 30 | + """ |
| 31 | + if branch_name is None: |
| 32 | + return base |
| 33 | + return urlutils.join_subsegments(base, branch_name) |
| 34 | + |
| 35 | + |
| 36 | # TODO: Maybe include checks for common corruption of newlines, etc? |
| 37 | |
| 38 | # TODO: Some operations like log might retrieve the same revisions |
| 39 | @@ -159,15 +187,16 @@ |
| 40 | return [('revision-existence', revid), ('lefthand-distance', revid)] |
| 41 | |
| 42 | @staticmethod |
| 43 | - def open(base, _unsupported=False, possible_transports=None): |
| 44 | + def open(url, _unsupported=False, possible_transports=None): |
| 45 | """Open the branch rooted at base. |
| 46 | |
| 47 | For instance, if the branch is at URL/.bzr/branch, |
| 48 | Branch.open(URL) -> a Branch instance. |
| 49 | """ |
| 50 | + (base, branch_name) = split_colocated_name(url) |
| 51 | control = bzrdir.BzrDir.open(base, _unsupported, |
| 52 | possible_transports=possible_transports) |
| 53 | - return control.open_branch(unsupported=_unsupported) |
| 54 | + return control.open_branch(name=branch_name, unsupported=_unsupported) |
| 55 | |
| 56 | @staticmethod |
| 57 | def open_from_transport(transport, name=None, _unsupported=False): |
| 58 | @@ -187,9 +216,10 @@ |
| 59 | format, UnknownFormatError or UnsupportedFormatError are raised. |
| 60 | If there is one, it is returned, along with the unused portion of url. |
| 61 | """ |
| 62 | - control, relpath = bzrdir.BzrDir.open_containing(url, |
| 63 | + (base, branch_name) = split_colocated_name(url) |
| 64 | + control, relpath = bzrdir.BzrDir.open_containing(base, |
| 65 | possible_transports) |
| 66 | - return control.open_branch(), relpath |
| 67 | + return control.open_branch(name=branch_name), relpath |
| 68 | |
| 69 | def _push_should_merge_tags(self): |
| 70 | """Should _basic_push merge this branch's tags into the target? |
| 71 | @@ -1335,6 +1365,8 @@ |
| 72 | """ |
| 73 | # XXX: Fix the bzrdir API to allow getting the branch back from the |
| 74 | # clone call. Or something. 20090224 RBC/spiv. |
| 75 | + # XXX: Should this perhaps clone colocated branches as well, |
| 76 | + # rather than just the default branch? 20100319 JRV |
| 77 | if revision_id is None: |
| 78 | revision_id = self.last_revision() |
| 79 | dir_to = self.bzrdir.clone_on_transport(to_transport, |
| 80 | @@ -1510,7 +1542,7 @@ |
| 81 | """Return the current default format.""" |
| 82 | return klass._default_format |
| 83 | |
| 84 | - def get_reference(self, a_bzrdir): |
| 85 | + def get_reference(self, a_bzrdir, branch_name=None): |
| 86 | """Get the target reference of the branch in a_bzrdir. |
| 87 | |
| 88 | format probing must have been completed before calling |
| 89 | @@ -1518,12 +1550,13 @@ |
| 90 | in a_bzrdir is correct. |
| 91 | |
| 92 | :param a_bzrdir: The bzrdir to get the branch data from. |
| 93 | + :param branch_name: Name of the colocated branch to fetch |
| 94 | :return: None if the branch is not a reference branch. |
| 95 | """ |
| 96 | return None |
| 97 | |
| 98 | @classmethod |
| 99 | - def set_reference(self, a_bzrdir, to_branch): |
| 100 | + def set_reference(self, a_bzrdir, branch_name, to_branch): |
| 101 | """Set the target reference of the branch in a_bzrdir. |
| 102 | |
| 103 | format probing must have been completed before calling |
| 104 | @@ -1531,6 +1564,7 @@ |
| 105 | in a_bzrdir is correct. |
| 106 | |
| 107 | :param a_bzrdir: The bzrdir to set the branch reference for. |
| 108 | + :param branch_name: Name of colocated branch to set, None for default |
| 109 | :param to_branch: branch that the checkout is to reference |
| 110 | """ |
| 111 | raise NotImplementedError(self.set_reference) |
| 112 | @@ -2048,14 +2082,14 @@ |
| 113 | """See BranchFormat.get_format_description().""" |
| 114 | return "Checkout reference format 1" |
| 115 | |
| 116 | - def get_reference(self, a_bzrdir): |
| 117 | + def get_reference(self, a_bzrdir, branch_name=None): |
| 118 | """See BranchFormat.get_reference().""" |
| 119 | - transport = a_bzrdir.get_branch_transport(None) |
| 120 | + transport = a_bzrdir.get_branch_transport(branch_name) |
| 121 | return transport.get_bytes('location') |
| 122 | |
| 123 | - def set_reference(self, a_bzrdir, to_branch): |
| 124 | + def set_reference(self, a_bzrdir, branch_name, to_branch): |
| 125 | """See BranchFormat.set_reference().""" |
| 126 | - transport = a_bzrdir.get_branch_transport(None) |
| 127 | + transport = a_bzrdir.get_branch_transport(branch_name) |
| 128 | location = transport.put_bytes('location', to_branch.base) |
| 129 | |
| 130 | def initialize(self, a_bzrdir, name=None, target_branch=None): |
| 131 | @@ -2110,7 +2144,7 @@ |
| 132 | raise AssertionError("wrong format %r found for %r" % |
| 133 | (format, self)) |
| 134 | if location is None: |
| 135 | - location = self.get_reference(a_bzrdir) |
| 136 | + location = self.get_reference(a_bzrdir, name) |
| 137 | real_bzrdir = bzrdir.BzrDir.open( |
| 138 | location, possible_transports=possible_transports) |
| 139 | result = real_bzrdir.open_branch(name=name, |
| 140 | |
| 141 | === modified file 'bzrlib/builtins.py' |
| 142 | --- bzrlib/builtins.py 2010-04-15 15:03:15 +0000 |
| 143 | +++ bzrlib/builtins.py 2010-04-17 17:41:17 +0000 |
| 144 | @@ -52,7 +52,11 @@ |
| 145 | urlutils, |
| 146 | views, |
| 147 | ) |
| 148 | -from bzrlib.branch import Branch |
| 149 | +from bzrlib.branch import ( |
| 150 | + Branch, |
| 151 | + join_colocated_name, |
| 152 | + split_colocated_name, |
| 153 | + ) |
| 154 | from bzrlib.conflicts import ConflictList |
| 155 | from bzrlib.transport import memory |
| 156 | from bzrlib.revisionspec import RevisionSpec, RevisionInfo |
| 157 | @@ -1230,23 +1234,24 @@ |
| 158 | revision_id = br_from.last_revision() |
| 159 | if to_location is None: |
| 160 | to_location = urlutils.derive_to_location(from_location) |
| 161 | - to_transport = transport.get_transport(to_location) |
| 162 | + to_base, to_branch_name = split_colocated_name(to_location) |
| 163 | + to_transport = transport.get_transport(to_base) |
| 164 | try: |
| 165 | to_transport.mkdir('.') |
| 166 | except errors.FileExists: |
| 167 | if not use_existing_dir: |
| 168 | raise errors.BzrCommandError('Target directory "%s" ' |
| 169 | - 'already exists.' % to_location) |
| 170 | + 'already exists.' % to_base) |
| 171 | else: |
| 172 | try: |
| 173 | bzrdir.BzrDir.open_from_transport(to_transport) |
| 174 | except errors.NotBranchError: |
| 175 | pass |
| 176 | else: |
| 177 | - raise errors.AlreadyBranchError(to_location) |
| 178 | + raise errors.AlreadyBranchError(to_base) |
| 179 | except errors.NoSuchFile: |
| 180 | raise errors.BzrCommandError('Parent of "%s" does not exist.' |
| 181 | - % to_location) |
| 182 | + % to_base) |
| 183 | try: |
| 184 | # preserve whatever source format we have. |
| 185 | dir = br_from.bzrdir.sprout(to_transport.base, revision_id, |
| 186 | @@ -1256,7 +1261,7 @@ |
| 187 | force_new_repo=standalone, |
| 188 | create_tree_if_local=not no_tree, |
| 189 | source_branch=br_from) |
| 190 | - branch = dir.open_branch() |
| 191 | + branch = dir.open_branch(to_branch_name) |
| 192 | except errors.NoSuchRevision: |
| 193 | to_transport.delete_tree('.') |
| 194 | msg = "The branch %s has no revision %s." % (from_location, |
| 195 | @@ -1731,7 +1736,8 @@ |
| 196 | if location is None: |
| 197 | location = u'.' |
| 198 | |
| 199 | - to_transport = transport.get_transport(location) |
| 200 | + (base, branch_name) = split_colocated_name(location) |
| 201 | + to_transport = transport.get_transport(base) |
| 202 | |
| 203 | # The path has to exist to initialize a |
| 204 | # branch inside of it. |
| 205 | @@ -1746,7 +1752,7 @@ |
| 206 | " does not exist." |
| 207 | "\nYou may supply --create-prefix to create all" |
| 208 | " leading parent directories." |
| 209 | - % location) |
| 210 | + % base) |
| 211 | to_transport.create_prefix() |
| 212 | |
| 213 | try: |
| 214 | @@ -1754,17 +1760,18 @@ |
| 215 | except errors.NotBranchError: |
| 216 | # really a NotBzrDir error... |
| 217 | create_branch = bzrdir.BzrDir.create_branch_convenience |
| 218 | - branch = create_branch(to_transport.base, format=format, |
| 219 | - possible_transports=[to_transport]) |
| 220 | + branch = create_branch( |
| 221 | + join_colocated_name(to_transport.base, branch_name), |
| 222 | + format=format, possible_transports=[to_transport]) |
| 223 | a_bzrdir = branch.bzrdir |
| 224 | else: |
| 225 | from bzrlib.transport.local import LocalTransport |
| 226 | - if a_bzrdir.has_branch(): |
| 227 | + if a_bzrdir.has_branch(branch_name): |
| 228 | if (isinstance(to_transport, LocalTransport) |
| 229 | and not a_bzrdir.has_workingtree()): |
| 230 | raise errors.BranchExistsWithoutWorkingTree(location) |
| 231 | raise errors.AlreadyBranchError(location) |
| 232 | - branch = a_bzrdir.create_branch() |
| 233 | + branch = a_bzrdir.create_branch(branch_name) |
| 234 | a_bzrdir.create_workingtree() |
| 235 | if append_revisions_only: |
| 236 | try: |
| 237 | @@ -2365,8 +2372,7 @@ |
| 238 | location = revision[0].get_branch() |
| 239 | else: |
| 240 | location = '.' |
| 241 | - dir, relpath = bzrdir.BzrDir.open_containing(location) |
| 242 | - b = dir.open_branch() |
| 243 | + b, relpath = Branch.open_containing(location) |
| 244 | b.lock_read() |
| 245 | self.add_cleanup(b.unlock) |
| 246 | rev1, rev2 = _get_revision_range(revision, b, self.name()) |
| 247 | @@ -4732,13 +4738,8 @@ |
| 248 | revision=None, force=False, local=False): |
| 249 | if location is None: |
| 250 | location = u'.' |
| 251 | - control, relpath = bzrdir.BzrDir.open_containing(location) |
| 252 | - try: |
| 253 | - tree = control.open_workingtree() |
| 254 | - b = tree.branch |
| 255 | - except (errors.NoWorkingTree, errors.NotLocalUrl): |
| 256 | - tree = None |
| 257 | - b = control.open_branch() |
| 258 | + tree, b, relpath = bzrdir.BzrDir.open_containing_tree_or_branch( |
| 259 | + location) |
| 260 | |
| 261 | if tree is not None: |
| 262 | tree.lock_write() |
| 263 | @@ -5543,51 +5544,53 @@ |
| 264 | from bzrlib import switch |
| 265 | tree_location = '.' |
| 266 | revision = _get_one_revision('switch', revision) |
| 267 | - control_dir = bzrdir.BzrDir.open_containing(tree_location)[0] |
| 268 | + tree_base, branch_name = split_colocated_name(tree_location) |
| 269 | + control_dir = bzrdir.BzrDir.open_containing(tree_base)[0] |
| 270 | if to_location is None: |
| 271 | if revision is None: |
| 272 | raise errors.BzrCommandError('You must supply either a' |
| 273 | ' revision or a location') |
| 274 | to_location = '.' |
| 275 | try: |
| 276 | - branch = control_dir.open_branch() |
| 277 | + branch = control_dir.open_branch(branch_name) |
| 278 | had_explicit_nick = branch.get_config().has_explicit_nickname() |
| 279 | except errors.NotBranchError: |
| 280 | branch = None |
| 281 | had_explicit_nick = False |
| 282 | + to_base, to_branch_name = split_colocated_name(to_location) |
| 283 | if create_branch: |
| 284 | if branch is None: |
| 285 | raise errors.BzrCommandError('cannot create branch without' |
| 286 | ' source branch') |
| 287 | - to_location = directory_service.directories.dereference( |
| 288 | - to_location) |
| 289 | - if '/' not in to_location and '\\' not in to_location: |
| 290 | + to_base = directory_service.directories.dereference( |
| 291 | + to_base) |
| 292 | + if '/' not in to_base and '\\' not in to_base: |
| 293 | # This path is meant to be relative to the existing branch |
| 294 | this_url = self._get_branch_location(control_dir) |
| 295 | - to_location = urlutils.join(this_url, '..', to_location) |
| 296 | - to_branch = branch.bzrdir.sprout(to_location, |
| 297 | - possible_transports=[branch.bzrdir.root_transport], |
| 298 | - source_branch=branch).open_branch() |
| 299 | + to_base = urlutils.join(this_url, '..', to_base) |
| 300 | + to_branch = branch.bzrdir.sprout(to_base, |
| 301 | + possible_transports=[branch.bzrdir.root_transport], |
| 302 | + source_branch=branch).open_branch(to_branch_name) |
| 303 | else: |
| 304 | try: |
| 305 | to_branch = Branch.open(to_location) |
| 306 | except errors.NotBranchError: |
| 307 | - this_url = self._get_branch_location(control_dir) |
| 308 | + this_url = self._get_branch_location(control_dir, branch_name) |
| 309 | to_branch = Branch.open( |
| 310 | urlutils.join(this_url, '..', to_location)) |
| 311 | if revision is not None: |
| 312 | revision = revision.as_revision_id(to_branch) |
| 313 | switch.switch(control_dir, to_branch, force, revision_id=revision) |
| 314 | if had_explicit_nick: |
| 315 | - branch = control_dir.open_branch() #get the new branch! |
| 316 | + branch = control_dir.open_branch(to_branch_name) #get the new branch! |
| 317 | branch.nick = to_branch.nick |
| 318 | note('Switched to branch: %s', |
| 319 | urlutils.unescape_for_display(to_branch.base, 'utf-8')) |
| 320 | |
| 321 | - def _get_branch_location(self, control_dir): |
| 322 | + def _get_branch_location(self, control_dir, branch_name): |
| 323 | """Return location of branch for this control dir.""" |
| 324 | try: |
| 325 | - this_branch = control_dir.open_branch() |
| 326 | + this_branch = control_dir.open_branch(branch_name) |
| 327 | # This may be a heavy checkout, where we want the master branch |
| 328 | master_location = this_branch.get_bound_location() |
| 329 | if master_location is not None: |
| 330 | @@ -5597,7 +5600,7 @@ |
| 331 | except errors.NotBranchError: |
| 332 | format = control_dir.find_branch_format() |
| 333 | if getattr(format, 'get_reference', None) is not None: |
| 334 | - return format.get_reference(control_dir) |
| 335 | + return format.get_reference(control_dir, branch_name) |
| 336 | else: |
| 337 | return control_dir.root_transport.base |
| 338 | |
| 339 | |
| 340 | === modified file 'bzrlib/bzrdir.py' |
| 341 | --- bzrlib/bzrdir.py 2010-04-15 15:03:15 +0000 |
| 342 | +++ bzrlib/bzrdir.py 2010-04-17 17:41:17 +0000 |
| 343 | @@ -220,7 +220,7 @@ |
| 344 | except errors.NoRepositoryPresent: |
| 345 | local_repo = None |
| 346 | try: |
| 347 | - local_branch = self.open_branch() |
| 348 | + local_branch = self.open_branch() # XXX Deal with colocated branches |
| 349 | except errors.NotBranchError: |
| 350 | local_branch = None |
| 351 | else: |
| 352 | @@ -704,13 +704,18 @@ |
| 353 | raise errors.NoRepositoryPresent(self) |
| 354 | return found_repo |
| 355 | |
| 356 | - def get_branch_reference(self): |
| 357 | + def get_branch_reference(self, name=None): |
| 358 | """Return the referenced URL for the branch in this bzrdir. |
| 359 | |
| 360 | + :param name: Optional colocated branch name |
| 361 | :raises NotBranchError: If there is no Branch. |
| 362 | + :raises NoColocatedBranchSupport: If a branch name was specified |
| 363 | + but colocated branches are not supported. |
| 364 | :return: The URL the branch in this bzrdir references if it is a |
| 365 | reference branch, or None for regular branches. |
| 366 | """ |
| 367 | + if name is not None: |
| 368 | + raise errors.NoColocatedBranchSupport(self) |
| 369 | return None |
| 370 | |
| 371 | def get_branch_transport(self, branch_format, name=None): |
| 372 | @@ -951,9 +956,11 @@ |
| 373 | raise errors.NotBranchError(path=url) |
| 374 | a_transport = new_t |
| 375 | |
| 376 | - def _get_tree_branch(self): |
| 377 | + def _get_tree_branch(self, name=None): |
| 378 | """Return the branch and tree, if any, for this bzrdir. |
| 379 | |
| 380 | + :param name: Name of colocated branch to open. |
| 381 | + |
| 382 | Return None for tree if not present or inaccessible. |
| 383 | Raise NotBranchError if no branch is present. |
| 384 | :return: (tree, branch) |
| 385 | @@ -962,9 +969,12 @@ |
| 386 | tree = self.open_workingtree() |
| 387 | except (errors.NoWorkingTree, errors.NotLocalUrl): |
| 388 | tree = None |
| 389 | - branch = self.open_branch() |
| 390 | + branch = self.open_branch(name=name) |
| 391 | else: |
| 392 | - branch = tree.branch |
| 393 | + if name is not None: |
| 394 | + branch = self.open_branch(name=name) |
| 395 | + else: |
| 396 | + branch = tree.branch |
| 397 | return tree, branch |
| 398 | |
| 399 | @classmethod |
| 400 | @@ -976,8 +986,10 @@ |
| 401 | raised |
| 402 | :return: (tree, branch) |
| 403 | """ |
| 404 | + from bzrlib.branch import split_colocated_name |
| 405 | + (location, branch_name) = split_colocated_name(location) |
| 406 | bzrdir = klass.open(location) |
| 407 | - return bzrdir._get_tree_branch() |
| 408 | + return bzrdir._get_tree_branch(branch_name) |
| 409 | |
| 410 | @classmethod |
| 411 | def open_containing_tree_or_branch(klass, location): |
| 412 | @@ -989,8 +1001,10 @@ |
| 413 | raised |
| 414 | relpath is the portion of the path that is contained by the branch. |
| 415 | """ |
| 416 | + from bzrlib.branch import split_colocated_name |
| 417 | + (location, branch_name) = split_colocated_name(location) |
| 418 | bzrdir, relpath = klass.open_containing(location) |
| 419 | - tree, branch = bzrdir._get_tree_branch() |
| 420 | + tree, branch = bzrdir._get_tree_branch(branch_name) |
| 421 | return tree, branch, relpath |
| 422 | |
| 423 | @classmethod |
| 424 | @@ -1261,13 +1275,13 @@ |
| 425 | return result |
| 426 | |
| 427 | def push_branch(self, source, revision_id=None, overwrite=False, |
| 428 | - remember=False, create_prefix=False): |
| 429 | + remember=False, create_prefix=False, branch_name=None): |
| 430 | """Push the source branch into this BzrDir.""" |
| 431 | br_to = None |
| 432 | # If we can open a branch, use its direct repository, otherwise see |
| 433 | # if there is a repository without a branch. |
| 434 | try: |
| 435 | - br_to = self.open_branch() |
| 436 | + br_to = self.open_branch(branch_name) |
| 437 | except errors.NotBranchError: |
| 438 | # Didn't find a branch, can we find a repository? |
| 439 | repository_to = self.find_repository() |
| 440 | @@ -1652,13 +1666,13 @@ |
| 441 | def destroy_workingtree_metadata(self): |
| 442 | self.transport.delete_tree('checkout') |
| 443 | |
| 444 | - def find_branch_format(self): |
| 445 | + def find_branch_format(self, name=None): |
| 446 | """Find the branch 'format' for this bzrdir. |
| 447 | |
| 448 | This might be a synthetic object for e.g. RemoteBranch and SVN. |
| 449 | """ |
| 450 | from bzrlib.branch import BranchFormat |
| 451 | - return BranchFormat.find_format(self) |
| 452 | + return BranchFormat.find_format(self, name=name) |
| 453 | |
| 454 | def _get_mkdir_mode(self): |
| 455 | """Figure out the mode to use when creating a bzrdir subdir.""" |
| 456 | @@ -1666,11 +1680,11 @@ |
| 457 | lockable_files.TransportLock) |
| 458 | return temp_control._dir_mode |
| 459 | |
| 460 | - def get_branch_reference(self): |
| 461 | + def get_branch_reference(self, name=None): |
| 462 | """See BzrDir.get_branch_reference().""" |
| 463 | from bzrlib.branch import BranchFormat |
| 464 | - format = BranchFormat.find_format(self) |
| 465 | - return format.get_reference(self) |
| 466 | + format = BranchFormat.find_format(self, name=name) |
| 467 | + return format.get_reference(self, name=name) |
| 468 | |
| 469 | def get_branch_transport(self, branch_format, name=None): |
| 470 | """See BzrDir.get_branch_transport().""" |
| 471 | @@ -1770,7 +1784,7 @@ |
| 472 | def open_branch(self, name=None, unsupported=False, |
| 473 | ignore_fallbacks=False): |
| 474 | """See BzrDir.open_branch.""" |
| 475 | - format = self.find_branch_format() |
| 476 | + format = self.find_branch_format(name=name) |
| 477 | self._check_supported(format, unsupported) |
| 478 | return format.open(self, name=name, |
| 479 | _found=True, ignore_fallbacks=ignore_fallbacks) |
| 480 | |
| 481 | === modified file 'bzrlib/errors.py' |
| 482 | --- bzrlib/errors.py 2010-03-24 14:15:01 +0000 |
| 483 | +++ bzrlib/errors.py 2010-04-17 17:41:17 +0000 |
| 484 | @@ -3134,3 +3134,10 @@ |
| 485 | def __init__(self, bzrdir): |
| 486 | self.bzrdir = bzrdir |
| 487 | |
| 488 | + |
| 489 | +class InvalidColocatedBranchName(BzrError): |
| 490 | + |
| 491 | + _fmt = "%(branch_name)s is an invalid name for a colocated branch." |
| 492 | + |
| 493 | + def __init__(self, branch_name): |
| 494 | + self.branch_name = branch_name |
| 495 | |
| 496 | === modified file 'bzrlib/reconcile.py' |
| 497 | --- bzrlib/reconcile.py 2010-02-17 17:11:16 +0000 |
| 498 | +++ bzrlib/reconcile.py 2010-04-17 17:41:17 +0000 |
| 499 | @@ -80,19 +80,16 @@ |
| 500 | |
| 501 | def _reconcile(self): |
| 502 | """Helper function for performing reconciliation.""" |
| 503 | - self._reconcile_branch() |
| 504 | + self._reconcile_branches() |
| 505 | self._reconcile_repository() |
| 506 | |
| 507 | - def _reconcile_branch(self): |
| 508 | - try: |
| 509 | - self.branch = self.bzrdir.open_branch() |
| 510 | - except errors.NotBranchError: |
| 511 | - # Nothing to check here |
| 512 | - self.fixed_branch_history = None |
| 513 | - return |
| 514 | - ui.ui_factory.note('Reconciling branch %s' % self.branch.base) |
| 515 | - branch_reconciler = self.branch.reconcile(thorough=True) |
| 516 | - self.fixed_branch_history = branch_reconciler.fixed_history |
| 517 | + def _reconcile_branches(self): |
| 518 | + self.fixed_branch_history = True |
| 519 | + for branch in self.bzrdir.list_branches(): |
| 520 | + ui.ui_factory.note('Reconciling branch %s' % branch.base) |
| 521 | + branch_reconciler = branch.reconcile(thorough=True) |
| 522 | + self.fixed_branch_history = (self.fixed_branch_history and |
| 523 | + branch_reconciler.fixed_history) |
| 524 | |
| 525 | def _reconcile_repository(self): |
| 526 | self.repo = self.bzrdir.find_repository() |
| 527 | |
| 528 | === modified file 'bzrlib/remote.py' |
| 529 | --- bzrlib/remote.py 2010-03-21 21:39:33 +0000 |
| 530 | +++ bzrlib/remote.py 2010-04-17 17:41:17 +0000 |
| 531 | @@ -271,16 +271,19 @@ |
| 532 | def create_workingtree(self, revision_id=None, from_branch=None): |
| 533 | raise errors.NotLocalUrl(self.transport.base) |
| 534 | |
| 535 | - def find_branch_format(self): |
| 536 | + def find_branch_format(self, name=None): |
| 537 | """Find the branch 'format' for this bzrdir. |
| 538 | |
| 539 | This might be a synthetic object for e.g. RemoteBranch and SVN. |
| 540 | """ |
| 541 | - b = self.open_branch() |
| 542 | + b = self.open_branch(name=name) |
| 543 | return b._format |
| 544 | |
| 545 | - def get_branch_reference(self): |
| 546 | + def get_branch_reference(self, name=None): |
| 547 | """See BzrDir.get_branch_reference().""" |
| 548 | + if name is not None: |
| 549 | + # XXX JRV20100304: Support opening colocated branches |
| 550 | + raise errors.NoColocatedBranchSupport(self) |
| 551 | response = self._get_branch_reference() |
| 552 | if response[0] == 'ref': |
| 553 | return response[1] |
| 554 | @@ -317,9 +320,9 @@ |
| 555 | raise errors.UnexpectedSmartServerResponse(response) |
| 556 | return response |
| 557 | |
| 558 | - def _get_tree_branch(self): |
| 559 | + def _get_tree_branch(self, name=None): |
| 560 | """See BzrDir._get_tree_branch().""" |
| 561 | - return None, self.open_branch() |
| 562 | + return None, self.open_branch(name=name) |
| 563 | |
| 564 | def open_branch(self, name=None, unsupported=False, |
| 565 | ignore_fallbacks=False): |
| 566 | |
| 567 | === modified file 'bzrlib/switch.py' |
| 568 | --- bzrlib/switch.py 2010-02-17 17:11:16 +0000 |
| 569 | +++ bzrlib/switch.py 2010-04-17 17:41:17 +0000 |
| 570 | @@ -70,7 +70,7 @@ |
| 571 | branch_format = control.find_branch_format() |
| 572 | if branch_format.get_reference(control) is not None: |
| 573 | # Lightweight checkout: update the branch reference |
| 574 | - branch_format.set_reference(control, to_branch) |
| 575 | + branch_format.set_reference(control, None, to_branch) |
| 576 | else: |
| 577 | b = control.open_branch() |
| 578 | bound_branch = b.get_bound_location() |
| 579 | |
| 580 | === modified file 'bzrlib/tests/per_branch/test_branch.py' |
| 581 | --- bzrlib/tests/per_branch/test_branch.py 2010-03-02 22:25:58 +0000 |
| 582 | +++ bzrlib/tests/per_branch/test_branch.py 2010-04-17 17:41:17 +0000 |
| 583 | @@ -668,7 +668,8 @@ |
| 584 | this_branch = self.make_branch('this') |
| 585 | other_branch = self.make_branch('other') |
| 586 | try: |
| 587 | - this_branch._format.set_reference(this_branch.bzrdir, other_branch) |
| 588 | + this_branch._format.set_reference(this_branch.bzrdir, None, |
| 589 | + other_branch) |
| 590 | except NotImplementedError: |
| 591 | # that's ok |
| 592 | pass |
| 593 | |
| 594 | === modified file 'bzrlib/tests/per_bzrdir_colo/test_unsupported.py' |
| 595 | --- bzrlib/tests/per_bzrdir_colo/test_unsupported.py 2010-04-11 19:40:23 +0000 |
| 596 | +++ bzrlib/tests/per_bzrdir_colo/test_unsupported.py 2010-04-17 17:41:17 +0000 |
| 597 | @@ -35,15 +35,7 @@ |
| 598 | |
| 599 | class TestNoColocatedSupport(TestCaseWithBzrDir): |
| 600 | |
| 601 | - def test_destroy_colocated_branch(self): |
| 602 | - branch = self.make_branch('branch') |
| 603 | - # Colocated branches should not be supported *or* |
| 604 | - # destroy_branch should not be supported at all |
| 605 | - self.assertRaises( |
| 606 | - (errors.NoColocatedBranchSupport, errors.UnsupportedOperation), |
| 607 | - branch.bzrdir.destroy_branch, 'colo') |
| 608 | - |
| 609 | - def test_create_colo_branch(self): |
| 610 | + def make_bzrdir_with_repo(self): |
| 611 | # a bzrdir can construct a branch and repository for itself. |
| 612 | if not self.bzrdir_format.is_supported(): |
| 613 | # unsupported formats are not loopback testable |
| 614 | @@ -53,7 +45,27 @@ |
| 615 | t = get_transport(self.get_url()) |
| 616 | made_control = self.bzrdir_format.initialize(t.base) |
| 617 | made_repo = made_control.create_repository() |
| 618 | + return made_control |
| 619 | + |
| 620 | + def test_destroy_colocated_branch(self): |
| 621 | + branch = self.make_branch('branch') |
| 622 | + # Colocated branches should not be supported *or* |
| 623 | + # destroy_branch should not be supported at all |
| 624 | + self.assertRaises( |
| 625 | + (errors.NoColocatedBranchSupport, errors.UnsupportedOperation), |
| 626 | + branch.bzrdir.destroy_branch, 'colo') |
| 627 | + |
| 628 | + def test_create_colo_branch(self): |
| 629 | + made_control = self.make_bzrdir_with_repo() |
| 630 | self.assertRaises(errors.NoColocatedBranchSupport, |
| 631 | made_control.create_branch, "colo") |
| 632 | |
| 633 | + def test_branch_transport(self): |
| 634 | + made_control = self.make_bzrdir_with_repo() |
| 635 | + self.assertRaises(errors.NoColocatedBranchSupport, |
| 636 | + made_control.get_branch_transport, None, "colo") |
| 637 | |
| 638 | + def test_get_branch_reference(self): |
| 639 | + made_control = self.make_bzrdir_with_repo() |
| 640 | + self.assertRaises(errors.NoColocatedBranchSupport, |
| 641 | + made_control.get_branch_reference, "colo") |
| 642 | |
| 643 | === modified file 'bzrlib/tests/test_branch.py' |
| 644 | --- bzrlib/tests/test_branch.py 2010-03-11 13:14:37 +0000 |
| 645 | +++ bzrlib/tests/test_branch.py 2010-04-17 17:41:17 +0000 |
| 646 | @@ -577,3 +577,40 @@ |
| 647 | self.assertEqual(['lock_write', 'func called', 'unlock'], self._calls) |
| 648 | |
| 649 | |
| 650 | +class TestExtractColocatedName(tests.TestCase): |
| 651 | + |
| 652 | + def test_no_colocated(self): |
| 653 | + self.assertEquals(("/some/path", None), |
| 654 | + _mod_branch.split_colocated_name("/some/path")) |
| 655 | + |
| 656 | + def test_colocated(self): |
| 657 | + self.assertEquals(("/some/path", "tip"), |
| 658 | + _mod_branch.split_colocated_name("/some/path,tip")) |
| 659 | + |
| 660 | + def test_colocated_uses_right_comma(self): |
| 661 | + self.assertEquals(("/some,dir/path", "tip"), |
| 662 | + _mod_branch.split_colocated_name("/some,dir/path,tip")) |
| 663 | + |
| 664 | + def test_colocated_allows_slashes(self): |
| 665 | + self.assertEquals(("/somedir/path", "heads%2Ftip"), |
| 666 | + _mod_branch.split_colocated_name("/somedir/path,heads%2Ftip")) |
| 667 | + |
| 668 | + def test_none(self): |
| 669 | + self.assertEquals((None, None), |
| 670 | + _mod_branch.split_colocated_name(None)) |
| 671 | + |
| 672 | + |
| 673 | +class TestJoinColocatedName(tests.TestCase): |
| 674 | + |
| 675 | + def test_default(self): |
| 676 | + self.assertEquals("/somedir/path", |
| 677 | + _mod_branch.join_colocated_name("/somedir/path", None)) |
| 678 | + |
| 679 | + def test_colocated(self): |
| 680 | + self.assertEquals("/somedir/path,brrr", |
| 681 | + _mod_branch.join_colocated_name("/somedir/path", "brrr")) |
| 682 | + |
| 683 | + def test_invalid_branch_name(self): |
| 684 | + self.assertRaises(errors.InvalidColocatedBranchName, |
| 685 | + _mod_branch.join_colocated_name, "/somedir/path", "brr,brr,brr") |
| 686 | + |
| 687 | |
| 688 | === modified file 'bzrlib/tests/test_remote.py' |
| 689 | --- bzrlib/tests/test_remote.py 2010-02-23 07:43:11 +0000 |
| 690 | +++ bzrlib/tests/test_remote.py 2010-04-17 17:41:17 +0000 |
| 691 | @@ -584,7 +584,7 @@ |
| 692 | # _get_tree_branch is a form of open_branch, but it should only ask for |
| 693 | # branch opening, not any other network requests. |
| 694 | calls = [] |
| 695 | - def open_branch(): |
| 696 | + def open_branch(name=None): |
| 697 | calls.append("Called") |
| 698 | return "a-branch" |
| 699 | transport = MemoryTransport() |
| 700 | |
| 701 | === modified file 'bzrlib/tests/test_urlutils.py' |
| 702 | --- bzrlib/tests/test_urlutils.py 2010-02-17 17:11:16 +0000 |
| 703 | +++ bzrlib/tests/test_urlutils.py 2010-04-17 17:41:17 +0000 |
| 704 | @@ -270,6 +270,17 @@ |
| 705 | self.assertRaises(InvalidURLJoin, urlutils.joinpath, '/', '..') |
| 706 | self.assertRaises(InvalidURLJoin, urlutils.joinpath, '/', '/..') |
| 707 | |
| 708 | + def test_join_subsegments(self): |
| 709 | + join_subsegments = urlutils.join_subsegments |
| 710 | + self.assertEquals("/somedir/path", |
| 711 | + join_subsegments("/somedir/path")) |
| 712 | + self.assertEquals("/somedir/path,brrr", |
| 713 | + join_subsegments("/somedir/path", "brrr")) |
| 714 | + self.assertRaises(InvalidURLJoin, |
| 715 | + join_subsegments, "/somedir/path", "brr,brr,brr") |
| 716 | + self.assertEquals("/somedir/path,bla,bar", |
| 717 | + join_subsegments("/somedir/path", "bla", "bar")) |
| 718 | + |
| 719 | def test_function_type(self): |
| 720 | if sys.platform == 'win32': |
| 721 | self.assertEqual(urlutils._win32_local_path_to_url, urlutils.local_path_to_url) |
| 722 | @@ -412,6 +423,19 @@ |
| 723 | self.assertEqual(('path/..', 'foo'), split('path/../foo')) |
| 724 | self.assertEqual(('../path', 'foo'), split('../path/foo')) |
| 725 | |
| 726 | + def test_split_subsegments(self): |
| 727 | + split_subsegments = urlutils.split_subsegments |
| 728 | + self.assertEquals(("/some/path", []), |
| 729 | + split_subsegments("/some/path")) |
| 730 | + self.assertEquals(("/some/path", ["tip"]), |
| 731 | + split_subsegments("/some/path,tip")) |
| 732 | + self.assertEquals(("/some,dir/path", ["tip"]), |
| 733 | + split_subsegments("/some,dir/path,tip")) |
| 734 | + self.assertEquals(("/somedir/path", ["heads%2Ftip"]), |
| 735 | + split_subsegments("/somedir/path,heads%2Ftip")) |
| 736 | + self.assertEquals(("/somedir/path", ["heads%2Ftip", "bar"]), |
| 737 | + split_subsegments("/somedir/path,heads%2Ftip,bar")) |
| 738 | + |
| 739 | def test_win32_strip_local_trailing_slash(self): |
| 740 | strip = urlutils._win32_strip_local_trailing_slash |
| 741 | self.assertEqual('file://', strip('file://')) |
| 742 | |
| 743 | === modified file 'bzrlib/urlutils.py' |
| 744 | --- bzrlib/urlutils.py 2010-02-17 17:11:16 +0000 |
| 745 | +++ bzrlib/urlutils.py 2010-04-17 17:41:17 +0000 |
| 746 | @@ -469,6 +469,32 @@ |
| 747 | return url_base + head, tail |
| 748 | |
| 749 | |
| 750 | +def split_subsegments(url): |
| 751 | + """Split the subsegment of the last segment of a URL. |
| 752 | + |
| 753 | + :param url: A relative or absolute URL |
| 754 | + :return: (url, subsegments) |
| 755 | + """ |
| 756 | + (parent_url, child_dir) = split(url) |
| 757 | + subsegments = child_dir.split(",") |
| 758 | + if len(subsegments) == 1: |
| 759 | + return (url, []) |
| 760 | + return (join(parent_url, subsegments[0]), subsegments[1:]) |
| 761 | + |
| 762 | + |
| 763 | +def join_subsegments(base, *subsegments): |
| 764 | + """Create a new URL by adding subsegments to an existing one. |
| 765 | + |
| 766 | + """ |
| 767 | + if not subsegments: |
| 768 | + return base |
| 769 | + for subsegment in subsegments: |
| 770 | + if "," in subsegment: |
| 771 | + raise errors.InvalidURLJoin(", exists in subsegments", |
| 772 | + base, subsegments) |
| 773 | + return ",".join((base,) + subsegments) |
| 774 | + |
| 775 | + |
| 776 | def _win32_strip_local_trailing_slash(url): |
| 777 | """Strip slashes after the drive letter""" |
| 778 | if len(url) > WIN32_MIN_ABS_FILEURL_LENGTH: |
This is ready but I'd be interested to hear how we should handle comma's in URLs. I think it's reasonable to require that colocated branch names never can contain a comma in a URL.