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

[Jelmer Vernooij]

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

[Vincent Ladeuil (vila)]

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

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

66 + format = self.find_branch_format(name)

85 + b = self.open_branch(name)

I may have missed some.

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

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

[Jelmer Vernooij (jelmer)]

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

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

[Vincent Ladeuil (vila)]

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

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

review: Needs Information

[Jelmer Vernooij (jelmer)]

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

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

5153. By Jelmer Vernooij on 2010-04-29

consistency in names

5154. By Jelmer Vernooij on 2010-04-29

merge bzr.dev.

[Vincent Ladeuil (vila)]

Good ! Sorry for the delay.

I'll land it.

review: Approve

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2010-04-28 09:40:23 +0000
+++ bzrlib/branch.py	2010-04-29 09:01:37 +0000
@@ -1346,6 +1346,8 @@
         """
         # 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,
@@ -1521,7 +1523,7 @@
         """Return the current default format."""
         return klass._default_format
 
-    def get_reference(self, a_bzrdir):
+    def get_reference(self, a_bzrdir, name=None):
         """Get the target reference of the branch in a_bzrdir.
 
         format probing must have been completed before calling
@@ -1529,12 +1531,13 @@
         in a_bzrdir is correct.
 
         :param a_bzrdir: The bzrdir to get the branch data from.
+        :param 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, name, to_branch):
         """Set the target reference of the branch in a_bzrdir.
 
         format probing must have been completed before calling
@@ -1542,6 +1545,7 @@
         in a_bzrdir is correct.
 
         :param a_bzrdir: The bzrdir to set the branch reference for.
+        :param 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)
@@ -2157,14 +2161,14 @@
         """See BranchFormat.get_format_description()."""
         return "Checkout reference format 1"
 
-    def get_reference(self, a_bzrdir):
+    def get_reference(self, a_bzrdir, name=None):
         """See BranchFormat.get_reference()."""
-        transport = a_bzrdir.get_branch_transport(None)
+        transport = a_bzrdir.get_branch_transport(None, name=name)
         return transport.get_bytes('location')
 
-    def set_reference(self, a_bzrdir, to_branch):
+    def set_reference(self, a_bzrdir, name, to_branch):
         """See BranchFormat.set_reference()."""
-        transport = a_bzrdir.get_branch_transport(None)
+        transport = a_bzrdir.get_branch_transport(None, name=name)
         location = transport.put_bytes('location', to_branch.base)
 
     def initialize(self, a_bzrdir, name=None, target_branch=None):
@@ -2221,7 +2225,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/bzrdir.py'
--- bzrlib/bzrdir.py	2010-04-28 07:03:38 +0000
+++ bzrlib/bzrdir.py	2010-04-29 09:01:37 +0000
@@ -737,13 +737,18 @@
             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):
@@ -994,9 +999,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)
@@ -1005,9 +1012,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
@@ -1736,13 +1746,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."""
@@ -1750,11 +1760,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()."""
@@ -1854,7 +1864,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/remote.py'
--- bzrlib/remote.py	2010-04-28 07:03:38 +0000
+++ bzrlib/remote.py	2010-04-29 09:01:37 +0000
@@ -272,16 +272,19 @@
     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]
@@ -318,9 +321,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-04-26 23:38:10 +0000
+++ bzrlib/switch.py	2010-04-29 09:01:37 +0000
@@ -78,7 +78,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-04-21 04:12:25 +0000
+++ bzrlib/tests/per_branch/test_branch.py	2010-04-29 09:01:37 +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-29 09:01:37 +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_remote.py'
--- bzrlib/tests/test_remote.py	2010-04-20 00:20:00 +0000
+++ bzrlib/tests/test_remote.py	2010-04-29 09:01:37 +0000
@@ -584,7 +584,7 @@
         # _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()