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

[Jelmer Vernooij]

Refuse removing active branches (branches associated with a working tree) in 'bzr rmbranch'.

Users can force the removal of such branches with the new --force argument to 'bzr rmbranch'.

[Martin Packman (gz)]

As per the prerequisite, if this should now be landing on dev as the targeting suggests, the release notes need moving. However, this seems like a colo ui issue that would be nice to fix on the 2.5 branch.

+ gettext("Use --force to remove active branch."))

This should probably state what it has (not) done, rather than just how to do the foot shooting.

Perhaps "Keeping active branch, use --force to really remove it." or something.

Code and tests look fine.

review: Needs Information

[Jelmer Vernooij (jelmer)]

Error message updated and branch retargeted against bzr.dev.

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2012-02-24 14:50:50 +0000
+++ bzrlib/builtins.py	2012-02-24 14:50:52 +0000
@@ -6477,12 +6477,22 @@
 
     takes_args = ["location?"]
 
-    takes_options = ['directory']
+    takes_options = ['directory',
+        Option('force', help='Remove branch even if it is the active branch.')]
 
     aliases = ["rmbranch"]
 
-    def run(self, directory=None, location=None):
+    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("Use --force to remove active branch."))
         br.bzrdir.destroy_branch(br.name)
 
 

=== modified file 'bzrlib/tests/blackbox/test_rmbranch.py'
--- bzrlib/tests/blackbox/test_rmbranch.py	2012-02-24 14:50:50 +0000
+++ bzrlib/tests/blackbox/test_rmbranch.py	2012-02-24 14:50:52 +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(['Use --force to remove active branch.\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(['Use --force to remove active branch.\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')
+        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)
@@ -72,7 +84,7 @@
 
     def test_remove_colo_directory(self):
         # Remove a colocated branch.
-        tree = self.example_branch('a')
+        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)
@@ -80,6 +92,17 @@
         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(['Use --force to remove active branch.\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):
 

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2012-02-24 14:50:50 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2012-02-24 14:50:52 +0000
@@ -37,6 +37,9 @@
 * ``bzr rmbranch`` now supports removing colocated branches.
   (Jelmer Vernooij, #920653)
 
+* ``bzr rmbranch`` no longer removes active branches unless ``--force``
+  is specified. (Jelmer Vernooij, #922953)
+
 * Show locks in ``bzr info`` on control directories without a
   repository. (Jelmer Vernooij, #936767)