Merge lp:~gz/bzr/2.5_backport_rmbranch_fixes into lp:bzr/2.5

[Martin Packman]

Backport fixes to rmbranch on colocated branches from 2.6

Backports from 2.6 fixes related to rmbranch which have caught out several people when working with colocated branches. Both bugs are included, as the second which fixes the specific issue of accidentally removing your active branch depends on some changes made in the first.

[Jelmer Vernooij (jelmer)]

review: Approve (code)

[Martin Packman (gz)]

sent to pqm by email

[Martin Packman (gz)]

Failed on pqm as it failed to branch over http, contents of stdout were just:

    Could not determine branch type for 'http://bazaar.launchpad.net/~gz/bzr/2.5_backport_rmbranch_fixes'

Due current to launchpad database work causing lag, as the http rewriting uses a slave database.

[Martin Packman (gz)]

sent to pqm by email

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2012-02-16 16:42:43 +0000
+++ bzrlib/builtins.py	2012-05-22 08:15:24 +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)))
@@ -6247,7 +6265,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)
@@ -6450,13 +6473,23 @@
 
     takes_args = ["location?"]
 
+    takes_options = ['directory',
+        Option('force', help='Remove branch even if it is the active branch.')]
+
     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, force=False):
+        br = open_nearby_branch(near=directory, location=location)
+        if not force and br.bzrdir.has_workingtree():
+            try:
+                active_branch = br.bzrdir.open_branch(name="")
+            except errors.NotBranchError:
+                active_branch = None
+            if (active_branch is not None and
+                br.control_url == active_branch.control_url):
+                raise errors.BzrCommandError(
+                    gettext("Branch is active. Use --force to remove it."))
+        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-05-22 08:15:24 +0000
@@ -28,7 +28,7 @@
 
 class TestRemoveBranch(TestCaseWithTransport):
 
-    def example_branch(self, path='.', format=None):
+    def example_tree(self, path='.', format=None):
         tree = self.make_branch_and_tree(path, format=format)
         self.build_tree_contents([(path + '/hello', 'foo')])
         tree.add('hello')
@@ -40,29 +40,41 @@
 
     def test_remove_local(self):
         # Remove a local branch.
-        self.example_branch('a')
-        self.run_bzr('rmbranch a')
+        tree = self.example_tree('a')
+        self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
+            'rmbranch a')
+        self.run_bzr('rmbranch --force a')
         dir = bzrdir.BzrDir.open('a')
         self.assertFalse(dir.has_branch())
         self.assertPathExists('a/hello')
         self.assertPathExists('a/goodbye')
 
     def test_no_branch(self):
-        # No branch in the current directory. 
+        # No branch in the current directory.
         self.make_repository('a')
         self.run_bzr_error(['Not a branch'],
             'rmbranch a')
 
+    def test_no_tree(self):
+        # removing the active branch is possible if there is no tree
+        tree = self.example_tree('a')
+        tree.bzrdir.destroy_workingtree()
+        self.run_bzr('rmbranch', working_dir='a')
+        dir = bzrdir.BzrDir.open('a')
+        self.assertFalse(dir.has_branch())
+
     def test_no_arg(self):
         # location argument defaults to current directory
-        self.example_branch('a')
-        self.run_bzr('rmbranch', working_dir='a')
+        self.example_tree('a')
+        self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
+            'rmbranch a')
+        self.run_bzr('rmbranch --force', working_dir='a')
         dir = bzrdir.BzrDir.open('a')
         self.assertFalse(dir.has_branch())
 
     def test_remove_colo(self):
         # Remove a colocated branch.
-        tree = self.example_branch('a', format='development-colo')
+        tree = self.example_tree('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 +82,27 @@
         self.assertFalse(dir.has_branch('otherbranch'))
         self.assertTrue(dir.has_branch())
 
+    def test_remove_colo_directory(self):
+        # Remove a colocated branch.
+        tree = self.example_tree('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())
+
+    def test_remove_active_colo_branch(self):
+        # Remove a colocated branch.
+        dir = self.make_repository('a').bzrdir
+        branch = dir.create_branch('otherbranch')
+        branch.create_checkout('a')
+        self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
+            'rmbranch otherbranch -d %s' % branch.bzrdir.user_url)
+        self.assertTrue(dir.has_branch('otherbranch'))
+        self.run_bzr('rmbranch --force otherbranch -d %s' % branch.bzrdir.user_url)
+        self.assertFalse(dir.has_branch('otherbranch'))
+
 
 class TestSmartServerRemoveBranch(TestCaseWithTransport):
 
@@ -83,6 +116,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-03 12:47:10 +0000
+++ bzrlib/tests/blackbox/test_switch.py	2012-05-22 08:15:24 +0000
@@ -472,5 +472,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.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2012-04-30 11:12:00 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2012-05-22 08:15:24 +0000
@@ -26,6 +26,12 @@
 .. Improvements to existing commands, especially improved performance 
    or memory usage, or better results.
 
+* ``bzr rmbranch`` now supports removing colocated branches.
+  (Jelmer Vernooij, #920653)
+
+* ``bzr rmbranch`` no longer removes active branches unless ``--force``
+  is specified. (Jelmer Vernooij, #922953)
+
 Bug Fixes
 *********