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

Proposed by Jelmer Vernooij
Status: Merged
Approved by: Martin Packman
Approved revision: 6483
Merged at revision: 6482
Proposed branch: lp:~jelmer/bzr/rmbranch-active
Merge into: lp:bzr
Prerequisite: lp:~jelmer/bzr/rmbranch-colo
Diff against target: 130 lines (+46/-10)
3 files modified
bzrlib/builtins.py (+12/-2)
bzrlib/tests/blackbox/test_rmbranch.py (+31/-8)
doc/en/release-notes/bzr-2.6.txt (+3/-0)
To merge this branch: bzr merge lp:~jelmer/bzr/rmbranch-active
Reviewer Review Type Date Requested Status
Martin Packman (community) Approve
Review via email: mp+94932@code.launchpad.net

This proposal supersedes a proposal from 2012-02-24.

Commit message

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

Description of the change

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'.

To post a comment you must log in.
Revision history for this message
Martin Packman (gz) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Jelmer Vernooij (jelmer) wrote : Posted in a previous version of this proposal

Error message updated and branch retargeted against bzr.dev.

Revision history for this message
Martin Packman (gz) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/builtins.py'
2--- bzrlib/builtins.py 2012-02-28 10:14:18 +0000
3+++ bzrlib/builtins.py 2012-02-28 10:14:18 +0000
4@@ -6477,12 +6477,22 @@
5
6 takes_args = ["location?"]
7
8- takes_options = ['directory']
9+ takes_options = ['directory',
10+ Option('force', help='Remove branch even if it is the active branch.')]
11
12 aliases = ["rmbranch"]
13
14- def run(self, directory=None, location=None):
15+ def run(self, directory=None, location=None, force=False):
16 br = open_nearby_branch(near=directory, location=location)
17+ if not force and br.bzrdir.has_workingtree():
18+ try:
19+ active_branch = br.bzrdir.open_branch(name="")
20+ except errors.NotBranchError:
21+ active_branch = None
22+ if (active_branch is not None and
23+ br.control_url == active_branch.control_url):
24+ raise errors.BzrCommandError(
25+ gettext("Branch is active. Use --force to remove it."))
26 br.bzrdir.destroy_branch(br.name)
27
28
29
30=== modified file 'bzrlib/tests/blackbox/test_rmbranch.py'
31--- bzrlib/tests/blackbox/test_rmbranch.py 2012-02-28 10:14:18 +0000
32+++ bzrlib/tests/blackbox/test_rmbranch.py 2012-02-28 10:14:18 +0000
33@@ -28,7 +28,7 @@
34
35 class TestRemoveBranch(TestCaseWithTransport):
36
37- def example_branch(self, path='.', format=None):
38+ def example_tree(self, path='.', format=None):
39 tree = self.make_branch_and_tree(path, format=format)
40 self.build_tree_contents([(path + '/hello', 'foo')])
41 tree.add('hello')
42@@ -40,29 +40,41 @@
43
44 def test_remove_local(self):
45 # Remove a local branch.
46- self.example_branch('a')
47- self.run_bzr('rmbranch a')
48+ tree = self.example_tree('a')
49+ self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
50+ 'rmbranch a')
51+ self.run_bzr('rmbranch --force a')
52 dir = bzrdir.BzrDir.open('a')
53 self.assertFalse(dir.has_branch())
54 self.assertPathExists('a/hello')
55 self.assertPathExists('a/goodbye')
56
57 def test_no_branch(self):
58- # No branch in the current directory.
59+ # No branch in the current directory.
60 self.make_repository('a')
61 self.run_bzr_error(['Not a branch'],
62 'rmbranch a')
63
64+ def test_no_tree(self):
65+ # removing the active branch is possible if there is no tree
66+ tree = self.example_tree('a')
67+ tree.bzrdir.destroy_workingtree()
68+ self.run_bzr('rmbranch', working_dir='a')
69+ dir = bzrdir.BzrDir.open('a')
70+ self.assertFalse(dir.has_branch())
71+
72 def test_no_arg(self):
73 # location argument defaults to current directory
74- self.example_branch('a')
75- self.run_bzr('rmbranch', working_dir='a')
76+ self.example_tree('a')
77+ self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
78+ 'rmbranch a')
79+ self.run_bzr('rmbranch --force', working_dir='a')
80 dir = bzrdir.BzrDir.open('a')
81 self.assertFalse(dir.has_branch())
82
83 def test_remove_colo(self):
84 # Remove a colocated branch.
85- tree = self.example_branch('a')
86+ tree = self.example_tree('a')
87 tree.bzrdir.create_branch(name="otherbranch")
88 self.assertTrue(tree.bzrdir.has_branch('otherbranch'))
89 self.run_bzr('rmbranch %s,branch=otherbranch' % tree.bzrdir.user_url)
90@@ -72,7 +84,7 @@
91
92 def test_remove_colo_directory(self):
93 # Remove a colocated branch.
94- tree = self.example_branch('a')
95+ tree = self.example_tree('a')
96 tree.bzrdir.create_branch(name="otherbranch")
97 self.assertTrue(tree.bzrdir.has_branch('otherbranch'))
98 self.run_bzr('rmbranch otherbranch -d %s' % tree.bzrdir.user_url)
99@@ -80,6 +92,17 @@
100 self.assertFalse(dir.has_branch('otherbranch'))
101 self.assertTrue(dir.has_branch())
102
103+ def test_remove_active_colo_branch(self):
104+ # Remove a colocated branch.
105+ dir = self.make_repository('a').bzrdir
106+ branch = dir.create_branch('otherbranch')
107+ branch.create_checkout('a')
108+ self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
109+ 'rmbranch otherbranch -d %s' % branch.bzrdir.user_url)
110+ self.assertTrue(dir.has_branch('otherbranch'))
111+ self.run_bzr('rmbranch --force otherbranch -d %s' % branch.bzrdir.user_url)
112+ self.assertFalse(dir.has_branch('otherbranch'))
113+
114
115 class TestSmartServerRemoveBranch(TestCaseWithTransport):
116
117
118=== modified file 'doc/en/release-notes/bzr-2.6.txt'
119--- doc/en/release-notes/bzr-2.6.txt 2012-02-28 10:14:18 +0000
120+++ doc/en/release-notes/bzr-2.6.txt 2012-02-28 10:14:18 +0000
121@@ -37,6 +37,9 @@
122 * ``bzr rmbranch`` now supports removing colocated branches.
123 (Jelmer Vernooij, #920653)
124
125+* ``bzr rmbranch`` no longer removes active branches unless ``--force``
126+ is specified. (Jelmer Vernooij, #922953)
127+
128 * Two new command hooks, ``pre_command`` and ``post_command``,
129 provide notification before and after a command has been run.
130 (Brian de Alwis, Jelmer Vernooij)