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

[Jelmer Vernooij]

Support removing colocated branches in 'bzr rmbranch'.

Support removing colocated branches in 'bzr rmbranch'.

Also includes a drive-by fix to reduce the number of HPSS connections for 'bzr switch' in lightweight checkouts by one.

[Martin Packman (gz)]

Substative changes all seems fine.

Is this intended to go on 2.5 or dev? Either needs retargetting or release notes need moving (and conflict to resolve there anyway).

-def lookup_sibling_branch(control_dir, location, possible_transports=None):
- """Lookup sibling branch.
-
+def open_sibling_branch(control_dir, location, possible_transports=None):
+ """Open a branch, possibly a sibling.

This looks technically like an api change if it's what's in 2.5.0 but as it's pretty new I guess it's okay to do.

- self.assertLength(2, self.hpss_calls)
+ self.assertLength(5, self.hpss_calls)

Sneaking this up is from looking at more possible branch locations?

review: Needs Information

[Jelmer Vernooij (jelmer)]

On 02/26/2012 11:17 PM, Martin Packman wrote:
> Review: Needs Information
>
> Substative changes all seems fine.
>
> Is this intended to go on 2.5 or dev? Either needs retargetting or release notes need moving (and conflict to resolve there anyway).
Yeah, mostly because it's a bug that people can remove active colocated
branches.

Alternatively, I could propose it against bzr.dev. colo support in 2.5
isn't fully polished anyway. That would mean fewer (risky) changes in
2.5. What do you think?
>
> -def lookup_sibling_branch(control_dir, location, possible_transports=None):
> - """Lookup sibling branch.
> -
> +def open_sibling_branch(control_dir, location, possible_transports=None):
> + """Open a branch, possibly a sibling.
>
> This looks technically like an api change if it's what's in 2.5.0 but as it's pretty new I guess it's okay to do.
I think that should be fine considering it's a fairly recent change,
indeed - and it's something that can't really be something that's used
by outsiders yet.
>
> - self.assertLength(2, self.hpss_calls)
> + self.assertLength(5, self.hpss_calls)
>
> Sneaking this up is from looking at more possible branch locations?
Yeah.

Cheers,

Jelmer

[Jelmer Vernooij (jelmer)]

Now updated to target 2.6.

[Martin Packman (gz)]

Thanks for the extra details. This change does not seem unreasonable for 2.5, and the dependant branch probably does want fixing there. It can always be backported though, so this should probably just land as is.

review: Approve

[Jelmer Vernooij (jelmer)]

sent to pqm by email

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2012-02-20 12:40:02 +0000
+++ bzrlib/builtins.py	2012-02-28 10:07:20 +0000
@@ -149,9 +149,9 @@
     return location
 
 
-def lookup_sibling_branch(control_dir, location, possible_transports=None):
-    """Lookup sibling branch.
-    
+def open_sibling_branch(control_dir, location, possible_transports=None):
+    """Open a branch, possibly a sibling.
+
     :param control_dir: Control directory relative to which to lookup the
         location.
     :param location: Location to look up
@@ -162,13 +162,31 @@
         return control_dir.open_branch(location, 
             possible_transports=possible_transports)
     except (errors.NotBranchError, errors.NoColocatedBranchSupport):
+        this_url = _get_branch_location(control_dir)
+        return Branch.open(
+            urlutils.join(
+                this_url, '..', urlutils.escape(location)))
+
+
+def open_nearby_branch(near=None, location=None, possible_transports=None):
+    """Open a nearby branch.
+
+    :param near: Optional location of container from which to open branch
+    :param location: Location of the branch
+    :return: Branch instance
+    """
+    if near is None:
+        if location is None:
+            location = "."
         try:
-            return Branch.open(location)
+            return Branch.open(location,
+                possible_transports=possible_transports)
         except errors.NotBranchError:
-            this_url = _get_branch_location(control_dir)
-            return Branch.open(
-                urlutils.join(
-                    this_url, '..', urlutils.escape(location)))
+            near = "."
+    cdir = controldir.ControlDir.open(near,
+        possible_transports=possible_transports)
+    return open_sibling_branch(cdir, location,
+        possible_transports=possible_transports)
 
 
 @symbol_versioning.deprecated_function(symbol_versioning.deprecated_in((2, 3, 0)))
@@ -6251,7 +6269,12 @@
                  possible_transports=possible_transports,
                  source_branch=branch).open_branch()
         else:
-            to_branch = lookup_sibling_branch(control_dir, to_location)
+            try:
+                to_branch = Branch.open(to_location,
+                    possible_transports=possible_transports)
+            except errors.NotBranchError:
+                to_branch = open_sibling_branch(control_dir, to_location,
+                    possible_transports=possible_transports)
         if revision is not None:
             revision = revision.as_revision_id(to_branch)
         switch.switch(control_dir, to_branch, force, revision_id=revision)
@@ -6454,13 +6477,13 @@
 
     takes_args = ["location?"]
 
+    takes_options = ['directory']
+
     aliases = ["rmbranch"]
 
-    def run(self, location=None):
-        if location is None:
-            location = "."
-        cdir = controldir.ControlDir.open_containing(location)[0]
-        cdir.destroy_branch()
+    def run(self, directory=None, location=None):
+        br = open_nearby_branch(near=directory, location=location)
+        br.bzrdir.destroy_branch(br.name)
 
 
 class cmd_shelve(Command):

=== modified file 'bzrlib/tests/blackbox/test_rmbranch.py'
--- bzrlib/tests/blackbox/test_rmbranch.py	2012-01-25 17:31:04 +0000
+++ bzrlib/tests/blackbox/test_rmbranch.py	2012-02-28 10:07:20 +0000
@@ -62,7 +62,7 @@
 
     def test_remove_colo(self):
         # Remove a colocated branch.
-        tree = self.example_branch('a', format='development-colo')
+        tree = self.example_branch('a')
         tree.bzrdir.create_branch(name="otherbranch")
         self.assertTrue(tree.bzrdir.has_branch('otherbranch'))
         self.run_bzr('rmbranch %s,branch=otherbranch' % tree.bzrdir.user_url)
@@ -70,6 +70,16 @@
         self.assertFalse(dir.has_branch('otherbranch'))
         self.assertTrue(dir.has_branch())
 
+    def test_remove_colo_directory(self):
+        # Remove a colocated branch.
+        tree = self.example_branch('a')
+        tree.bzrdir.create_branch(name="otherbranch")
+        self.assertTrue(tree.bzrdir.has_branch('otherbranch'))
+        self.run_bzr('rmbranch otherbranch -d %s' % tree.bzrdir.user_url)
+        dir = bzrdir.BzrDir.open('a')
+        self.assertFalse(dir.has_branch('otherbranch'))
+        self.assertTrue(dir.has_branch())
+
 
 class TestSmartServerRemoveBranch(TestCaseWithTransport):
 
@@ -83,6 +93,6 @@
         # being too low. If rpc_count increases, more network roundtrips have
         # become necessary for this use case. Please do not adjust this number
         # upwards without agreement from bzr's network support maintainers.
-        self.assertLength(2, self.hpss_calls)
+        self.assertLength(5, self.hpss_calls)
         self.assertLength(1, self.hpss_connections)
         self.assertThat(self.hpss_calls, ContainsNoVfsCalls)

=== modified file 'bzrlib/tests/blackbox/test_switch.py'
--- bzrlib/tests/blackbox/test_switch.py	2012-02-18 16:55:04 +0000
+++ bzrlib/tests/blackbox/test_switch.py	2012-02-28 10:07:20 +0000
@@ -475,5 +475,5 @@
         # become necessary for this use case. Please do not adjust this number
         # upwards without agreement from bzr's network support maintainers.
         self.assertLength(24, self.hpss_calls)
-        self.assertLength(5, self.hpss_connections)
+        self.assertLength(4, self.hpss_connections)
         self.assertThat(self.hpss_calls, ContainsNoVfsCalls)

=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- doc/en/release-notes/bzr-2.6.txt	2012-02-25 14:13:19 +0000
+++ doc/en/release-notes/bzr-2.6.txt	2012-02-28 10:07:20 +0000
@@ -34,6 +34,9 @@
 * Avoid 'Invalid range access' errors when whole files are retrieved with
   transport.http.get() . (Vincent Ladeuil, #924746)
 
+* ``bzr rmbranch`` now supports removing colocated branches.
+  (Jelmer Vernooij, #920653)
+
 * Two new command hooks, ``pre_command`` and ``post_command``,
   provide notification before and after a command has been run.
   (Brian de Alwis, Jelmer Vernooij)