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

[Jelmer Vernooij]

Require --force to remove active branches in 'bzr rmbranch'.

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.

[Martin Packman (gz)]

Changed message is nice and clear. As mentioned in the prerequisite branch, this fix probably does want to end up on 2.5 but landing this on trunk first won't hurt.

review: Approve

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2012-02-28 10:14:18 +0000
+++ bzrlib/builtins.py	2012-02-28 10:14:18 +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("Branch is active. Use --force to remove it."))
         br.bzrdir.destroy_branch(br.name)
 
 

=== modified file 'bzrlib/tests/blackbox/test_rmbranch.py'
--- bzrlib/tests/blackbox/test_rmbranch.py	2012-02-28 10:14:18 +0000
+++ bzrlib/tests/blackbox/test_rmbranch.py	2012-02-28 10:14:18 +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')
+        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(['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):
 

=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- doc/en/release-notes/bzr-2.6.txt	2012-02-28 10:14:18 +0000
+++ doc/en/release-notes/bzr-2.6.txt	2012-02-28 10:14:18 +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)
+
 * Two new command hooks, ``pre_command`` and ``post_command``,
   provide notification before and after a command has been run.
   (Brian de Alwis, Jelmer Vernooij)