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

Proposed by Martin Packman
Status: Merged
Approved by: Martin Packman
Approved revision: 6501
Merged at revision: 6500
Proposed branch: lp:~gz/bzr/2.5_backport_rmbranch_fixes
Merge into: lp:bzr/2.5
Diff against target: 220 lines (+95/-23)
4 files modified
bzrlib/builtins.py (+47/-14)
bzrlib/tests/blackbox/test_rmbranch.py (+41/-8)
bzrlib/tests/blackbox/test_switch.py (+1/-1)
doc/en/release-notes/bzr-2.5.txt (+6/-0)
To merge this branch: bzr merge lp:~gz/bzr/2.5_backport_rmbranch_fixes
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) code Approve
Review via email: mp+106753@code.launchpad.net

Commit message

Backport fixes to rmbranch on colocated branches from 2.6

Description of the change

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.

To post a comment you must log in.
6501. By Jelmer Vernooij

Cherrypick requiring --force to remove an active branch with rmbranch

Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve (code)
Revision history for this message
Martin Packman (gz) wrote :

sent to pqm by email

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

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.

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

sent to pqm by email

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-16 16:42:43 +0000
3+++ bzrlib/builtins.py 2012-05-22 08:15:24 +0000
4@@ -149,9 +149,9 @@
5 return location
6
7
8-def lookup_sibling_branch(control_dir, location, possible_transports=None):
9- """Lookup sibling branch.
10-
11+def open_sibling_branch(control_dir, location, possible_transports=None):
12+ """Open a branch, possibly a sibling.
13+
14 :param control_dir: Control directory relative to which to lookup the
15 location.
16 :param location: Location to look up
17@@ -162,13 +162,31 @@
18 return control_dir.open_branch(location,
19 possible_transports=possible_transports)
20 except (errors.NotBranchError, errors.NoColocatedBranchSupport):
21+ this_url = _get_branch_location(control_dir)
22+ return Branch.open(
23+ urlutils.join(
24+ this_url, '..', urlutils.escape(location)))
25+
26+
27+def open_nearby_branch(near=None, location=None, possible_transports=None):
28+ """Open a nearby branch.
29+
30+ :param near: Optional location of container from which to open branch
31+ :param location: Location of the branch
32+ :return: Branch instance
33+ """
34+ if near is None:
35+ if location is None:
36+ location = "."
37 try:
38- return Branch.open(location)
39+ return Branch.open(location,
40+ possible_transports=possible_transports)
41 except errors.NotBranchError:
42- this_url = _get_branch_location(control_dir)
43- return Branch.open(
44- urlutils.join(
45- this_url, '..', urlutils.escape(location)))
46+ near = "."
47+ cdir = controldir.ControlDir.open(near,
48+ possible_transports=possible_transports)
49+ return open_sibling_branch(cdir, location,
50+ possible_transports=possible_transports)
51
52
53 @symbol_versioning.deprecated_function(symbol_versioning.deprecated_in((2, 3, 0)))
54@@ -6247,7 +6265,12 @@
55 possible_transports=possible_transports,
56 source_branch=branch).open_branch()
57 else:
58- to_branch = lookup_sibling_branch(control_dir, to_location)
59+ try:
60+ to_branch = Branch.open(to_location,
61+ possible_transports=possible_transports)
62+ except errors.NotBranchError:
63+ to_branch = open_sibling_branch(control_dir, to_location,
64+ possible_transports=possible_transports)
65 if revision is not None:
66 revision = revision.as_revision_id(to_branch)
67 switch.switch(control_dir, to_branch, force, revision_id=revision)
68@@ -6450,13 +6473,23 @@
69
70 takes_args = ["location?"]
71
72+ takes_options = ['directory',
73+ Option('force', help='Remove branch even if it is the active branch.')]
74+
75 aliases = ["rmbranch"]
76
77- def run(self, location=None):
78- if location is None:
79- location = "."
80- cdir = controldir.ControlDir.open_containing(location)[0]
81- cdir.destroy_branch()
82+ def run(self, directory=None, location=None, force=False):
83+ br = open_nearby_branch(near=directory, location=location)
84+ if not force and br.bzrdir.has_workingtree():
85+ try:
86+ active_branch = br.bzrdir.open_branch(name="")
87+ except errors.NotBranchError:
88+ active_branch = None
89+ if (active_branch is not None and
90+ br.control_url == active_branch.control_url):
91+ raise errors.BzrCommandError(
92+ gettext("Branch is active. Use --force to remove it."))
93+ br.bzrdir.destroy_branch(br.name)
94
95
96 class cmd_shelve(Command):
97
98=== modified file 'bzrlib/tests/blackbox/test_rmbranch.py'
99--- bzrlib/tests/blackbox/test_rmbranch.py 2012-01-25 17:31:04 +0000
100+++ bzrlib/tests/blackbox/test_rmbranch.py 2012-05-22 08:15:24 +0000
101@@ -28,7 +28,7 @@
102
103 class TestRemoveBranch(TestCaseWithTransport):
104
105- def example_branch(self, path='.', format=None):
106+ def example_tree(self, path='.', format=None):
107 tree = self.make_branch_and_tree(path, format=format)
108 self.build_tree_contents([(path + '/hello', 'foo')])
109 tree.add('hello')
110@@ -40,29 +40,41 @@
111
112 def test_remove_local(self):
113 # Remove a local branch.
114- self.example_branch('a')
115- self.run_bzr('rmbranch a')
116+ tree = self.example_tree('a')
117+ self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
118+ 'rmbranch a')
119+ self.run_bzr('rmbranch --force a')
120 dir = bzrdir.BzrDir.open('a')
121 self.assertFalse(dir.has_branch())
122 self.assertPathExists('a/hello')
123 self.assertPathExists('a/goodbye')
124
125 def test_no_branch(self):
126- # No branch in the current directory.
127+ # No branch in the current directory.
128 self.make_repository('a')
129 self.run_bzr_error(['Not a branch'],
130 'rmbranch a')
131
132+ def test_no_tree(self):
133+ # removing the active branch is possible if there is no tree
134+ tree = self.example_tree('a')
135+ tree.bzrdir.destroy_workingtree()
136+ self.run_bzr('rmbranch', working_dir='a')
137+ dir = bzrdir.BzrDir.open('a')
138+ self.assertFalse(dir.has_branch())
139+
140 def test_no_arg(self):
141 # location argument defaults to current directory
142- self.example_branch('a')
143- self.run_bzr('rmbranch', working_dir='a')
144+ self.example_tree('a')
145+ self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
146+ 'rmbranch a')
147+ self.run_bzr('rmbranch --force', working_dir='a')
148 dir = bzrdir.BzrDir.open('a')
149 self.assertFalse(dir.has_branch())
150
151 def test_remove_colo(self):
152 # Remove a colocated branch.
153- tree = self.example_branch('a', format='development-colo')
154+ tree = self.example_tree('a')
155 tree.bzrdir.create_branch(name="otherbranch")
156 self.assertTrue(tree.bzrdir.has_branch('otherbranch'))
157 self.run_bzr('rmbranch %s,branch=otherbranch' % tree.bzrdir.user_url)
158@@ -70,6 +82,27 @@
159 self.assertFalse(dir.has_branch('otherbranch'))
160 self.assertTrue(dir.has_branch())
161
162+ def test_remove_colo_directory(self):
163+ # Remove a colocated branch.
164+ tree = self.example_tree('a')
165+ tree.bzrdir.create_branch(name="otherbranch")
166+ self.assertTrue(tree.bzrdir.has_branch('otherbranch'))
167+ self.run_bzr('rmbranch otherbranch -d %s' % tree.bzrdir.user_url)
168+ dir = bzrdir.BzrDir.open('a')
169+ self.assertFalse(dir.has_branch('otherbranch'))
170+ self.assertTrue(dir.has_branch())
171+
172+ def test_remove_active_colo_branch(self):
173+ # Remove a colocated branch.
174+ dir = self.make_repository('a').bzrdir
175+ branch = dir.create_branch('otherbranch')
176+ branch.create_checkout('a')
177+ self.run_bzr_error(['Branch is active. Use --force to remove it.\n'],
178+ 'rmbranch otherbranch -d %s' % branch.bzrdir.user_url)
179+ self.assertTrue(dir.has_branch('otherbranch'))
180+ self.run_bzr('rmbranch --force otherbranch -d %s' % branch.bzrdir.user_url)
181+ self.assertFalse(dir.has_branch('otherbranch'))
182+
183
184 class TestSmartServerRemoveBranch(TestCaseWithTransport):
185
186@@ -83,6 +116,6 @@
187 # being too low. If rpc_count increases, more network roundtrips have
188 # become necessary for this use case. Please do not adjust this number
189 # upwards without agreement from bzr's network support maintainers.
190- self.assertLength(2, self.hpss_calls)
191+ self.assertLength(5, self.hpss_calls)
192 self.assertLength(1, self.hpss_connections)
193 self.assertThat(self.hpss_calls, ContainsNoVfsCalls)
194
195=== modified file 'bzrlib/tests/blackbox/test_switch.py'
196--- bzrlib/tests/blackbox/test_switch.py 2012-02-03 12:47:10 +0000
197+++ bzrlib/tests/blackbox/test_switch.py 2012-05-22 08:15:24 +0000
198@@ -472,5 +472,5 @@
199 # become necessary for this use case. Please do not adjust this number
200 # upwards without agreement from bzr's network support maintainers.
201 self.assertLength(24, self.hpss_calls)
202- self.assertLength(5, self.hpss_connections)
203+ self.assertLength(4, self.hpss_connections)
204 self.assertThat(self.hpss_calls, ContainsNoVfsCalls)
205
206=== modified file 'doc/en/release-notes/bzr-2.5.txt'
207--- doc/en/release-notes/bzr-2.5.txt 2012-04-30 11:12:00 +0000
208+++ doc/en/release-notes/bzr-2.5.txt 2012-05-22 08:15:24 +0000
209@@ -26,6 +26,12 @@
210 .. Improvements to existing commands, especially improved performance
211 or memory usage, or better results.
212
213+* ``bzr rmbranch`` now supports removing colocated branches.
214+ (Jelmer Vernooij, #920653)
215+
216+* ``bzr rmbranch`` no longer removes active branches unless ``--force``
217+ is specified. (Jelmer Vernooij, #922953)
218+
219 Bug Fixes
220 *********
221

Subscribers

People subscribed via source and target branches