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

[Jelmer Vernooij]

Add support for addressing colocated branches using comma's.

[Jelmer Vernooij (jelmer)]

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.

[Andrew Bennetts (spiv)]

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://host/path,branchname
   * branch location: http://host/path
   * branch name: branchname
 * http://host/path,branch%2Cname
   * branch location: http://host/path
   * branch name: branch,name
 * http://host/path,branchname/README.txt
   * branch location: http://host/path
   * 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/colocated-branches.txt?)

review: Needs Information

[lp:~jelmer/bzr/colo-urls]

5070. By Jelmer Vernooij on 2010-04-03

Add branch name argument in a couple more places.

5071. By Jelmer Vernooij on 2010-04-10

Fix two test failures.

5072. By Jelmer Vernooij on 2010-04-10

Merge updated colo spec.

5073. By Jelmer Vernooij on 2010-04-11

merge colocated branch tests.

5074. By Jelmer Vernooij on 2010-04-16

Merge more-colo.

5075. By Jelmer Vernooij on 2010-04-17

merge more-colo

5076. By Jelmer Vernooij on 2010-04-17

Clarify parsing of subsegments.

5077. By Jelmer Vernooij on 2010-04-17

Remove unused error.

5078. By Jelmer Vernooij on 2010-04-17

merge urlutils subsegment improvements.

5079. By Jelmer Vernooij on 2010-04-17

use new subsegments urlutils functions.

[Martin Pool (mbp)]

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._format.set_reference(this_branch.bzrdir, other_branch)
+ this_branch._format.set_reference(this_branch.bzrdir, None,
+ 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?

[Robert Collins (lifeless)]

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.

[Vincent Ladeuil (vila)]

@Robert: Did you unpack your comb ? :-)

[Jelmer Vernooij (jelmer)]

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.

[Robert Collins (lifeless)]

Am looking at this, iz not forgotten.

[Robert Collins (lifeless)]

Oh, and @spiv - yes, , should be per segment in the path, not whole-string - thats the point of this ;)

[Robert Collins (lifeless)]

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.open_from_transport, and so on.
 - 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

Unmerged revisions

5079. By Jelmer Vernooij on 2010-04-17

use new subsegments urlutils functions.

5078. By Jelmer Vernooij on 2010-04-17

merge urlutils subsegment improvements.

5077. By Jelmer Vernooij on 2010-04-17

Remove unused error.

5076. By Jelmer Vernooij on 2010-04-17

Clarify parsing of subsegments.

5075. By Jelmer Vernooij on 2010-04-17

merge more-colo

5074. By Jelmer Vernooij on 2010-04-16

Merge more-colo.

5073. By Jelmer Vernooij on 2010-04-11

merge colocated branch tests.

5072. By Jelmer Vernooij on 2010-04-10

Merge updated colo spec.

5071. By Jelmer Vernooij on 2010-04-10

Fix two test failures.

5070. By Jelmer Vernooij on 2010-04-03

Add branch name argument in a couple more places.

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2010-04-10 18:03:54 +0000
+++ bzrlib/branch.py	2010-04-17 17:41:17 +0000
@@ -63,6 +63,34 @@
 BZR_BRANCH_FORMAT_6 = "Bazaar Branch Format 6 (bzr 0.15)\n"
 
 
+def split_colocated_name(url):
+    """Extract the colocated branch name from a URL.
+
+    :param url: The original URL
+    :return: Tuple with new URL and colocated branch name (None if not specified)
+    """
+    if url is None:
+        return (None, None)
+    (base, subsegments) = urlutils.split_subsegments(url)
+    if len(subsegments) == 0:
+        return (url, None)
+    if len(subsegments) > 1:
+        raise errors.InvalidURL(url)
+    return (base, subsegments[0])
+
+
+def join_colocated_name(base, branch_name):
+    """Combine a location and a colocated branch name URL.
+
+    :param base: Base location
+    :param branch_name: Co-located branch name
+    :return: A URL that addresses a colocated branch
+    """
+    if branch_name is None:
+        return base
+    return urlutils.join_subsegments(base, branch_name)
+
+
 # TODO: Maybe include checks for common corruption of newlines, etc?
 
 # TODO: Some operations like log might retrieve the same revisions
@@ -159,15 +187,16 @@
         return [('revision-existence', revid), ('lefthand-distance', revid)]
 
     @staticmethod
-    def open(base, _unsupported=False, possible_transports=None):
+    def open(url, _unsupported=False, possible_transports=None):
         """Open the branch rooted at base.
 
         For instance, if the branch is at URL/.bzr/branch,
         Branch.open(URL) -> a Branch instance.
         """
+        (base, branch_name) = split_colocated_name(url)
         control = bzrdir.BzrDir.open(base, _unsupported,
                                      possible_transports=possible_transports)
-        return control.open_branch(unsupported=_unsupported)
+        return control.open_branch(name=branch_name, unsupported=_unsupported)
 
     @staticmethod
     def open_from_transport(transport, name=None, _unsupported=False):
@@ -187,9 +216,10 @@
         format, UnknownFormatError or UnsupportedFormatError are raised.
         If there is one, it is returned, along with the unused portion of url.
         """
-        control, relpath = bzrdir.BzrDir.open_containing(url,
+        (base, branch_name) = split_colocated_name(url)
+        control, relpath = bzrdir.BzrDir.open_containing(base,
                                                          possible_transports)
-        return control.open_branch(), relpath
+        return control.open_branch(name=branch_name), relpath
 
     def _push_should_merge_tags(self):
         """Should _basic_push merge this branch's tags into the target?
@@ -1335,6 +1365,8 @@
         """
         # XXX: Fix the bzrdir API to allow getting the branch back from the
         # clone call. Or something. 20090224 RBC/spiv.
+        # XXX: Should this perhaps clone colocated branches as well, 
+        # rather than just the default branch? 20100319 JRV
         if revision_id is None:
             revision_id = self.last_revision()
         dir_to = self.bzrdir.clone_on_transport(to_transport,
@@ -1510,7 +1542,7 @@
         """Return the current default format."""
         return klass._default_format
 
-    def get_reference(self, a_bzrdir):
+    def get_reference(self, a_bzrdir, branch_name=None):
         """Get the target reference of the branch in a_bzrdir.
 
         format probing must have been completed before calling
@@ -1518,12 +1550,13 @@
         in a_bzrdir is correct.
 
         :param a_bzrdir: The bzrdir to get the branch data from.
+        :param branch_name: Name of the colocated branch to fetch
         :return: None if the branch is not a reference branch.
         """
         return None
 
     @classmethod
-    def set_reference(self, a_bzrdir, to_branch):
+    def set_reference(self, a_bzrdir, branch_name, to_branch):
         """Set the target reference of the branch in a_bzrdir.
 
         format probing must have been completed before calling
@@ -1531,6 +1564,7 @@
         in a_bzrdir is correct.
 
         :param a_bzrdir: The bzrdir to set the branch reference for.
+        :param branch_name: Name of colocated branch to set, None for default
         :param to_branch: branch that the checkout is to reference
         """
         raise NotImplementedError(self.set_reference)
@@ -2048,14 +2082,14 @@
         """See BranchFormat.get_format_description()."""
         return "Checkout reference format 1"
 
-    def get_reference(self, a_bzrdir):
+    def get_reference(self, a_bzrdir, branch_name=None):
         """See BranchFormat.get_reference()."""
-        transport = a_bzrdir.get_branch_transport(None)
+        transport = a_bzrdir.get_branch_transport(branch_name)
         return transport.get_bytes('location')
 
-    def set_reference(self, a_bzrdir, to_branch):
+    def set_reference(self, a_bzrdir, branch_name, to_branch):
         """See BranchFormat.set_reference()."""
-        transport = a_bzrdir.get_branch_transport(None)
+        transport = a_bzrdir.get_branch_transport(branch_name)
         location = transport.put_bytes('location', to_branch.base)
 
     def initialize(self, a_bzrdir, name=None, target_branch=None):
@@ -2110,7 +2144,7 @@
                 raise AssertionError("wrong format %r found for %r" %
                     (format, self))
         if location is None:
-            location = self.get_reference(a_bzrdir)
+            location = self.get_reference(a_bzrdir, name)
         real_bzrdir = bzrdir.BzrDir.open(
             location, possible_transports=possible_transports)
         result = real_bzrdir.open_branch(name=name, 

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2010-04-15 15:03:15 +0000
+++ bzrlib/builtins.py	2010-04-17 17:41:17 +0000
@@ -52,7 +52,11 @@
     urlutils,
     views,
     )
-from bzrlib.branch import Branch
+from bzrlib.branch import (
+    Branch,
+    join_colocated_name,
+    split_colocated_name,
+    )
 from bzrlib.conflicts import ConflictList
 from bzrlib.transport import memory
 from bzrlib.revisionspec import RevisionSpec, RevisionInfo
@@ -1230,23 +1234,24 @@
             revision_id = br_from.last_revision()
         if to_location is None:
             to_location = urlutils.derive_to_location(from_location)
-        to_transport = transport.get_transport(to_location)
+        to_base, to_branch_name = split_colocated_name(to_location)
+        to_transport = transport.get_transport(to_base)
         try:
             to_transport.mkdir('.')
         except errors.FileExists:
             if not use_existing_dir:
                 raise errors.BzrCommandError('Target directory "%s" '
-                    'already exists.' % to_location)
+                    'already exists.' % to_base)
             else:
                 try:
                     bzrdir.BzrDir.open_from_transport(to_transport)
                 except errors.NotBranchError:
                     pass
                 else:
-                    raise errors.AlreadyBranchError(to_location)
+                    raise errors.AlreadyBranchError(to_base)
         except errors.NoSuchFile:
             raise errors.BzrCommandError('Parent of "%s" does not exist.'
-                                         % to_location)
+                                         % to_base)
         try:
             # preserve whatever source format we have.
             dir = br_from.bzrdir.sprout(to_transport.base, revision_id,
@@ -1256,7 +1261,7 @@
                                         force_new_repo=standalone,
                                         create_tree_if_local=not no_tree,
                                         source_branch=br_from)
-            branch = dir.open_branch()
+            branch = dir.open_branch(to_branch_name)
         except errors.NoSuchRevision:
             to_transport.delete_tree('.')
             msg = "The branch %s has no revision %s." % (from_location,
@@ -1731,7 +1736,8 @@
         if location is None:
             location = u'.'
 
-        to_transport = transport.get_transport(location)
+        (base, branch_name) = split_colocated_name(location)
+        to_transport = transport.get_transport(base)
 
         # The path has to exist to initialize a
         # branch inside of it.
@@ -1746,7 +1752,7 @@
                     " does not exist."
                     "\nYou may supply --create-prefix to create all"
                     " leading parent directories."
-                    % location)
+                    % base)
             to_transport.create_prefix()
 
         try:
@@ -1754,17 +1760,18 @@
         except errors.NotBranchError:
             # really a NotBzrDir error...
             create_branch = bzrdir.BzrDir.create_branch_convenience
-            branch = create_branch(to_transport.base, format=format,
-                                   possible_transports=[to_transport])
+            branch = create_branch(
+                join_colocated_name(to_transport.base, branch_name), 
+                format=format, possible_transports=[to_transport])
             a_bzrdir = branch.bzrdir
         else:
             from bzrlib.transport.local import LocalTransport
-            if a_bzrdir.has_branch():
+            if a_bzrdir.has_branch(branch_name):
                 if (isinstance(to_transport, LocalTransport)
                     and not a_bzrdir.has_workingtree()):
                         raise errors.BranchExistsWithoutWorkingTree(location)
                 raise errors.AlreadyBranchError(location)
-            branch = a_bzrdir.create_branch()
+            branch = a_bzrdir.create_branch(branch_name)
             a_bzrdir.create_workingtree()
         if append_revisions_only:
             try:
@@ -2365,8 +2372,7 @@
                 location = revision[0].get_branch()
             else:
                 location = '.'
-            dir, relpath = bzrdir.BzrDir.open_containing(location)
-            b = dir.open_branch()
+            b, relpath = Branch.open_containing(location)
             b.lock_read()
             self.add_cleanup(b.unlock)
             rev1, rev2 = _get_revision_range(revision, b, self.name())
@@ -4732,13 +4738,8 @@
             revision=None, force=False, local=False):
         if location is None:
             location = u'.'
-        control, relpath = bzrdir.BzrDir.open_containing(location)
-        try:
-            tree = control.open_workingtree()
-            b = tree.branch
-        except (errors.NoWorkingTree, errors.NotLocalUrl):
-            tree = None
-            b = control.open_branch()
+        tree, b, relpath = bzrdir.BzrDir.open_containing_tree_or_branch(
+            location)
 
         if tree is not None:
             tree.lock_write()
@@ -5543,51 +5544,53 @@
         from bzrlib import switch
         tree_location = '.'
         revision = _get_one_revision('switch', revision)
-        control_dir = bzrdir.BzrDir.open_containing(tree_location)[0]
+        tree_base, branch_name = split_colocated_name(tree_location)
+        control_dir = bzrdir.BzrDir.open_containing(tree_base)[0]
         if to_location is None:
             if revision is None:
                 raise errors.BzrCommandError('You must supply either a'
                                              ' revision or a location')
             to_location = '.'
         try:
-            branch = control_dir.open_branch()
+            branch = control_dir.open_branch(branch_name)
             had_explicit_nick = branch.get_config().has_explicit_nickname()
         except errors.NotBranchError:
             branch = None
             had_explicit_nick = False
+        to_base, to_branch_name = split_colocated_name(to_location)
         if create_branch:
             if branch is None:
                 raise errors.BzrCommandError('cannot create branch without'
                                              ' source branch')
-            to_location = directory_service.directories.dereference(
-                              to_location)
-            if '/' not in to_location and '\\' not in to_location:
+            to_base = directory_service.directories.dereference(
+                              to_base)
+            if '/' not in to_base and '\\' not in to_base:
                 # This path is meant to be relative to the existing branch
                 this_url = self._get_branch_location(control_dir)
-                to_location = urlutils.join(this_url, '..', to_location)
-            to_branch = branch.bzrdir.sprout(to_location,
-                                 possible_transports=[branch.bzrdir.root_transport],
-                                 source_branch=branch).open_branch()
+                to_base = urlutils.join(this_url, '..', to_base)
+            to_branch = branch.bzrdir.sprout(to_base,
+                possible_transports=[branch.bzrdir.root_transport],
+                source_branch=branch).open_branch(to_branch_name)
         else:
             try:
                 to_branch = Branch.open(to_location)
             except errors.NotBranchError:
-                this_url = self._get_branch_location(control_dir)
+                this_url = self._get_branch_location(control_dir, branch_name)
                 to_branch = Branch.open(
                     urlutils.join(this_url, '..', to_location))
         if revision is not None:
             revision = revision.as_revision_id(to_branch)
         switch.switch(control_dir, to_branch, force, revision_id=revision)
         if had_explicit_nick:
-            branch = control_dir.open_branch() #get the new branch!
+            branch = control_dir.open_branch(to_branch_name) #get the new branch!
             branch.nick = to_branch.nick
         note('Switched to branch: %s',
             urlutils.unescape_for_display(to_branch.base, 'utf-8'))
 
-    def _get_branch_location(self, control_dir):
+    def _get_branch_location(self, control_dir, branch_name):
         """Return location of branch for this control dir."""
         try:
-            this_branch = control_dir.open_branch()
+            this_branch = control_dir.open_branch(branch_name)
             # This may be a heavy checkout, where we want the master branch
             master_location = this_branch.get_bound_location()
             if master_location is not None:
@@ -5597,7 +5600,7 @@
         except errors.NotBranchError:
             format = control_dir.find_branch_format()
             if getattr(format, 'get_reference', None) is not None:
-                return format.get_reference(control_dir)
+                return format.get_reference(control_dir, branch_name)
             else:
                 return control_dir.root_transport.base
 

=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py	2010-04-15 15:03:15 +0000
+++ bzrlib/bzrdir.py	2010-04-17 17:41:17 +0000
@@ -220,7 +220,7 @@
         except errors.NoRepositoryPresent:
             local_repo = None
         try:
-            local_branch = self.open_branch()
+            local_branch = self.open_branch() # XXX Deal with colocated branches
         except errors.NotBranchError:
             local_branch = None
         else:
@@ -704,13 +704,18 @@
             raise errors.NoRepositoryPresent(self)
         return found_repo
 
-    def get_branch_reference(self):
+    def get_branch_reference(self, name=None):
         """Return the referenced URL for the branch in this bzrdir.
 
+        :param name: Optional colocated branch name
         :raises NotBranchError: If there is no Branch.
+        :raises NoColocatedBranchSupport: If a branch name was specified
+            but colocated branches are not supported.
         :return: The URL the branch in this bzrdir references if it is a
             reference branch, or None for regular branches.
         """
+        if name is not None:
+            raise errors.NoColocatedBranchSupport(self)
         return None
 
     def get_branch_transport(self, branch_format, name=None):
@@ -951,9 +956,11 @@
                 raise errors.NotBranchError(path=url)
             a_transport = new_t
 
-    def _get_tree_branch(self):
+    def _get_tree_branch(self, name=None):
         """Return the branch and tree, if any, for this bzrdir.
 
+        :param name: Name of colocated branch to open.
+
         Return None for tree if not present or inaccessible.
         Raise NotBranchError if no branch is present.
         :return: (tree, branch)
@@ -962,9 +969,12 @@
             tree = self.open_workingtree()
         except (errors.NoWorkingTree, errors.NotLocalUrl):
             tree = None
-            branch = self.open_branch()
+            branch = self.open_branch(name=name)
         else:
-            branch = tree.branch
+            if name is not None:
+                branch = self.open_branch(name=name)
+            else:
+                branch = tree.branch
         return tree, branch
 
     @classmethod
@@ -976,8 +986,10 @@
         raised
         :return: (tree, branch)
         """
+        from bzrlib.branch import split_colocated_name
+        (location, branch_name) = split_colocated_name(location)
         bzrdir = klass.open(location)
-        return bzrdir._get_tree_branch()
+        return bzrdir._get_tree_branch(branch_name)
 
     @classmethod
     def open_containing_tree_or_branch(klass, location):
@@ -989,8 +1001,10 @@
         raised
         relpath is the portion of the path that is contained by the branch.
         """
+        from bzrlib.branch import split_colocated_name
+        (location, branch_name) = split_colocated_name(location)
         bzrdir, relpath = klass.open_containing(location)
-        tree, branch = bzrdir._get_tree_branch()
+        tree, branch = bzrdir._get_tree_branch(branch_name)
         return tree, branch, relpath
 
     @classmethod
@@ -1261,13 +1275,13 @@
         return result
 
     def push_branch(self, source, revision_id=None, overwrite=False, 
-        remember=False, create_prefix=False):
+        remember=False, create_prefix=False, branch_name=None):
         """Push the source branch into this BzrDir."""
         br_to = None
         # If we can open a branch, use its direct repository, otherwise see
         # if there is a repository without a branch.
         try:
-            br_to = self.open_branch()
+            br_to = self.open_branch(branch_name)
         except errors.NotBranchError:
             # Didn't find a branch, can we find a repository?
             repository_to = self.find_repository()
@@ -1652,13 +1666,13 @@
     def destroy_workingtree_metadata(self):
         self.transport.delete_tree('checkout')
 
-    def find_branch_format(self):
+    def find_branch_format(self, name=None):
         """Find the branch 'format' for this bzrdir.
 
         This might be a synthetic object for e.g. RemoteBranch and SVN.
         """
         from bzrlib.branch import BranchFormat
-        return BranchFormat.find_format(self)
+        return BranchFormat.find_format(self, name=name)
 
     def _get_mkdir_mode(self):
         """Figure out the mode to use when creating a bzrdir subdir."""
@@ -1666,11 +1680,11 @@
                                      lockable_files.TransportLock)
         return temp_control._dir_mode
 
-    def get_branch_reference(self):
+    def get_branch_reference(self, name=None):
         """See BzrDir.get_branch_reference()."""
         from bzrlib.branch import BranchFormat
-        format = BranchFormat.find_format(self)
-        return format.get_reference(self)
+        format = BranchFormat.find_format(self, name=name)
+        return format.get_reference(self, name=name)
 
     def get_branch_transport(self, branch_format, name=None):
         """See BzrDir.get_branch_transport()."""
@@ -1770,7 +1784,7 @@
     def open_branch(self, name=None, unsupported=False,
                     ignore_fallbacks=False):
         """See BzrDir.open_branch."""
-        format = self.find_branch_format()
+        format = self.find_branch_format(name=name)
         self._check_supported(format, unsupported)
         return format.open(self, name=name,
             _found=True, ignore_fallbacks=ignore_fallbacks)

=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	2010-03-24 14:15:01 +0000
+++ bzrlib/errors.py	2010-04-17 17:41:17 +0000
@@ -3134,3 +3134,10 @@
     def __init__(self, bzrdir):
         self.bzrdir = bzrdir
 
+
+class InvalidColocatedBranchName(BzrError):
+
+    _fmt = "%(branch_name)s is an invalid name for a colocated branch."
+
+    def __init__(self, branch_name):
+        self.branch_name = branch_name

=== modified file 'bzrlib/reconcile.py'
--- bzrlib/reconcile.py	2010-02-17 17:11:16 +0000
+++ bzrlib/reconcile.py	2010-04-17 17:41:17 +0000
@@ -80,19 +80,16 @@
 
     def _reconcile(self):
         """Helper function for performing reconciliation."""
-        self._reconcile_branch()
+        self._reconcile_branches()
         self._reconcile_repository()
 
-    def _reconcile_branch(self):
-        try:
-            self.branch = self.bzrdir.open_branch()
-        except errors.NotBranchError:
-            # Nothing to check here
-            self.fixed_branch_history = None
-            return
-        ui.ui_factory.note('Reconciling branch %s' % self.branch.base)
-        branch_reconciler = self.branch.reconcile(thorough=True)
-        self.fixed_branch_history = branch_reconciler.fixed_history
+    def _reconcile_branches(self):
+        self.fixed_branch_history = True
+        for branch in self.bzrdir.list_branches():
+            ui.ui_factory.note('Reconciling branch %s' % branch.base)
+            branch_reconciler = branch.reconcile(thorough=True)
+            self.fixed_branch_history = (self.fixed_branch_history and 
+                branch_reconciler.fixed_history)
 
     def _reconcile_repository(self):
         self.repo = self.bzrdir.find_repository()

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2010-03-21 21:39:33 +0000
+++ bzrlib/remote.py	2010-04-17 17:41:17 +0000
@@ -271,16 +271,19 @@
     def create_workingtree(self, revision_id=None, from_branch=None):
         raise errors.NotLocalUrl(self.transport.base)
 
-    def find_branch_format(self):
+    def find_branch_format(self, name=None):
         """Find the branch 'format' for this bzrdir.
 
         This might be a synthetic object for e.g. RemoteBranch and SVN.
         """
-        b = self.open_branch()
+        b = self.open_branch(name=name)
         return b._format
 
-    def get_branch_reference(self):
+    def get_branch_reference(self, name=None):
         """See BzrDir.get_branch_reference()."""
+        if name is not None:
+            # XXX JRV20100304: Support opening colocated branches
+            raise errors.NoColocatedBranchSupport(self)
         response = self._get_branch_reference()
         if response[0] == 'ref':
             return response[1]
@@ -317,9 +320,9 @@
             raise errors.UnexpectedSmartServerResponse(response)
         return response
 
-    def _get_tree_branch(self):
+    def _get_tree_branch(self, name=None):
         """See BzrDir._get_tree_branch()."""
-        return None, self.open_branch()
+        return None, self.open_branch(name=name)
 
     def open_branch(self, name=None, unsupported=False,
                     ignore_fallbacks=False):

=== modified file 'bzrlib/switch.py'
--- bzrlib/switch.py	2010-02-17 17:11:16 +0000
+++ bzrlib/switch.py	2010-04-17 17:41:17 +0000
@@ -70,7 +70,7 @@
     branch_format = control.find_branch_format()
     if branch_format.get_reference(control) is not None:
         # Lightweight checkout: update the branch reference
-        branch_format.set_reference(control, to_branch)
+        branch_format.set_reference(control, None, to_branch)
     else:
         b = control.open_branch()
         bound_branch = b.get_bound_location()

=== modified file 'bzrlib/tests/per_branch/test_branch.py'
--- bzrlib/tests/per_branch/test_branch.py	2010-03-02 22:25:58 +0000
+++ bzrlib/tests/per_branch/test_branch.py	2010-04-17 17:41:17 +0000
@@ -668,7 +668,8 @@
         this_branch = self.make_branch('this')
         other_branch = self.make_branch('other')
         try:
-            this_branch._format.set_reference(this_branch.bzrdir, other_branch)
+            this_branch._format.set_reference(this_branch.bzrdir, None,
+                other_branch)
         except NotImplementedError:
             # that's ok
             pass

=== 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-17 17:41:17 +0000
@@ -35,15 +35,7 @@
 
 class TestNoColocatedSupport(TestCaseWithBzrDir):
 
-    def test_destroy_colocated_branch(self):
-        branch = self.make_branch('branch')
-        # Colocated branches should not be supported *or* 
-        # destroy_branch should not be supported at all
-        self.assertRaises(
-            (errors.NoColocatedBranchSupport, errors.UnsupportedOperation),
-            branch.bzrdir.destroy_branch, 'colo')
-
-    def test_create_colo_branch(self):
+    def make_bzrdir_with_repo(self):
         # a bzrdir can construct a branch and repository for itself.
         if not self.bzrdir_format.is_supported():
             # unsupported formats are not loopback testable
@@ -53,7 +45,27 @@
         t = get_transport(self.get_url())
         made_control = self.bzrdir_format.initialize(t.base)
         made_repo = made_control.create_repository()
+        return made_control
+
+    def test_destroy_colocated_branch(self):
+        branch = self.make_branch('branch')
+        # Colocated branches should not be supported *or* 
+        # destroy_branch should not be supported at all
+        self.assertRaises(
+            (errors.NoColocatedBranchSupport, errors.UnsupportedOperation),
+            branch.bzrdir.destroy_branch, 'colo')
+
+    def test_create_colo_branch(self):
+        made_control = self.make_bzrdir_with_repo()
         self.assertRaises(errors.NoColocatedBranchSupport, 
             made_control.create_branch, "colo")
 
+    def test_branch_transport(self):
+        made_control = self.make_bzrdir_with_repo()
+        self.assertRaises(errors.NoColocatedBranchSupport,
+            made_control.get_branch_transport, None, "colo")
 
+    def test_get_branch_reference(self):
+        made_control = self.make_bzrdir_with_repo()
+        self.assertRaises(errors.NoColocatedBranchSupport,
+            made_control.get_branch_reference, "colo")

=== modified file 'bzrlib/tests/test_branch.py'
--- bzrlib/tests/test_branch.py	2010-03-11 13:14:37 +0000
+++ bzrlib/tests/test_branch.py	2010-04-17 17:41:17 +0000
@@ -577,3 +577,40 @@
         self.assertEqual(['lock_write', 'func called', 'unlock'], self._calls)
 
 
+class TestExtractColocatedName(tests.TestCase):
+
+    def test_no_colocated(self):
+        self.assertEquals(("/some/path", None),
+            _mod_branch.split_colocated_name("/some/path"))
+
+    def test_colocated(self):
+        self.assertEquals(("/some/path", "tip"),
+            _mod_branch.split_colocated_name("/some/path,tip"))
+
+    def test_colocated_uses_right_comma(self):
+        self.assertEquals(("/some,dir/path", "tip"),
+            _mod_branch.split_colocated_name("/some,dir/path,tip"))
+
+    def test_colocated_allows_slashes(self):
+        self.assertEquals(("/somedir/path", "heads%2Ftip"),
+            _mod_branch.split_colocated_name("/somedir/path,heads%2Ftip"))
+
+    def test_none(self):
+        self.assertEquals((None, None),
+            _mod_branch.split_colocated_name(None))
+
+
+class TestJoinColocatedName(tests.TestCase):
+
+    def test_default(self):
+        self.assertEquals("/somedir/path", 
+            _mod_branch.join_colocated_name("/somedir/path", None))
+
+    def test_colocated(self):
+        self.assertEquals("/somedir/path,brrr", 
+            _mod_branch.join_colocated_name("/somedir/path", "brrr"))
+
+    def test_invalid_branch_name(self):
+        self.assertRaises(errors.InvalidColocatedBranchName,
+            _mod_branch.join_colocated_name, "/somedir/path", "brr,brr,brr")
+

=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py	2010-02-23 07:43:11 +0000
+++ bzrlib/tests/test_remote.py	2010-04-17 17:41:17 +0000
@@ -584,7 +584,7 @@
         # _get_tree_branch is a form of open_branch, but it should only ask for
         # branch opening, not any other network requests.
         calls = []
-        def open_branch():
+        def open_branch(name=None):
             calls.append("Called")
             return "a-branch"
         transport = MemoryTransport()

=== modified file 'bzrlib/tests/test_urlutils.py'
--- bzrlib/tests/test_urlutils.py	2010-02-17 17:11:16 +0000
+++ bzrlib/tests/test_urlutils.py	2010-04-17 17:41:17 +0000
@@ -270,6 +270,17 @@
         self.assertRaises(InvalidURLJoin, urlutils.joinpath, '/', '..')
         self.assertRaises(InvalidURLJoin, urlutils.joinpath, '/', '/..')
 
+    def test_join_subsegments(self):
+        join_subsegments = urlutils.join_subsegments
+        self.assertEquals("/somedir/path", 
+            join_subsegments("/somedir/path"))
+        self.assertEquals("/somedir/path,brrr", 
+            join_subsegments("/somedir/path", "brrr"))
+        self.assertRaises(InvalidURLJoin,
+            join_subsegments, "/somedir/path", "brr,brr,brr")
+        self.assertEquals("/somedir/path,bla,bar",
+            join_subsegments("/somedir/path", "bla", "bar"))
+
     def test_function_type(self):
         if sys.platform == 'win32':
             self.assertEqual(urlutils._win32_local_path_to_url, urlutils.local_path_to_url)
@@ -412,6 +423,19 @@
         self.assertEqual(('path/..', 'foo'), split('path/../foo'))
         self.assertEqual(('../path', 'foo'), split('../path/foo'))
 
+    def test_split_subsegments(self):
+        split_subsegments = urlutils.split_subsegments
+        self.assertEquals(("/some/path", []),
+            split_subsegments("/some/path"))
+        self.assertEquals(("/some/path", ["tip"]),
+            split_subsegments("/some/path,tip"))
+        self.assertEquals(("/some,dir/path", ["tip"]),
+            split_subsegments("/some,dir/path,tip"))
+        self.assertEquals(("/somedir/path", ["heads%2Ftip"]),
+            split_subsegments("/somedir/path,heads%2Ftip"))
+        self.assertEquals(("/somedir/path", ["heads%2Ftip", "bar"]),
+            split_subsegments("/somedir/path,heads%2Ftip,bar"))
+
     def test_win32_strip_local_trailing_slash(self):
         strip = urlutils._win32_strip_local_trailing_slash
         self.assertEqual('file://', strip('file://'))

=== modified file 'bzrlib/urlutils.py'
--- bzrlib/urlutils.py	2010-02-17 17:11:16 +0000
+++ bzrlib/urlutils.py	2010-04-17 17:41:17 +0000
@@ -469,6 +469,32 @@
     return url_base + head, tail
 
 
+def split_subsegments(url):
+    """Split the subsegment of the last segment of a URL.
+
+    :param url: A relative or absolute URL
+    :return: (url, subsegments)
+    """
+    (parent_url, child_dir) = split(url)
+    subsegments = child_dir.split(",")
+    if len(subsegments) == 1:
+        return (url, [])
+    return (join(parent_url, subsegments[0]), subsegments[1:])
+
+
+def join_subsegments(base, *subsegments):
+    """Create a new URL by adding subsegments to an existing one.
+
+    """
+    if not subsegments:
+        return base
+    for subsegment in subsegments:
+        if "," in subsegment:
+            raise errors.InvalidURLJoin(", exists in subsegments",
+                                        base, subsegments)
+    return ",".join((base,) + subsegments)
+
+
 def _win32_strip_local_trailing_slash(url):
     """Strip slashes after the drive letter"""
     if len(url) > WIN32_MIN_ABS_FILEURL_LENGTH: