Merge lp:~vila/bzr/426344-check-pending-merges into lp:bzr
- 426344-check-pending-merges
- Merge into bzr.dev
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | John A Meinel | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | not available | ||||
Proposed branch: | lp:~vila/bzr/426344-check-pending-merges | ||||
Merge into: | lp:bzr | ||||
Diff against target: | None lines | ||||
To merge this branch: | bzr merge lp:~vila/bzr/426344-check-pending-merges | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
John A Meinel | Approve | ||
Review via email: mp+11378@code.launchpad.net |
Commit message
Description of the change
Vincent Ladeuil (vila) wrote : | # |
John A Meinel (jameinel) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/426344-check-pending-merges into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> This fixes the last two places where we were checking uncommitted changes
> while not taking pending merges into account: merge and remove-tree.
>
review: approve
merge: approve
However, this is looking more and more like we should have a simple:
working_
Which takes no target, and grabs its own basis_tree, and looks at its
own pending merge list. Which I asked for in the beginning, though, so
perhaps I'm biased.
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkq
3JUAoJKgX7MQd/
=LDTT
-----END PGP SIGNATURE-----
Preview Diff
1 | === modified file 'NEWS' |
2 | --- NEWS 2009-09-04 00:49:55 +0000 |
3 | +++ NEWS 2009-09-08 16:46:40 +0000 |
4 | @@ -30,6 +30,10 @@ |
5 | longer report incorrect errors about ``Missing inventory ('TREE_ROOT', ...)`` |
6 | (Robert Collins, #416732) |
7 | |
8 | +* ``bzr merge`` and ``bzr remove-tree`` now requires --force if pending |
9 | + merges are present in the working tree. |
10 | + (Vincent Ladeuil, #426344) |
11 | + |
12 | * Don't restrict the command name used to run the test suite. |
13 | (Vincent Ladeuil, #419950) |
14 | |
15 | |
16 | === modified file 'bzrlib/builtins.py' |
17 | --- bzrlib/builtins.py 2009-08-28 05:00:33 +0000 |
18 | +++ bzrlib/builtins.py 2009-09-08 16:45:11 +0000 |
19 | @@ -461,8 +461,8 @@ |
20 | raise errors.BzrCommandError("You cannot remove the working tree" |
21 | " of a remote path") |
22 | if not force: |
23 | - # XXX: What about pending merges ? -- vila 20090629 |
24 | - if working.has_changes(working.basis_tree()): |
25 | + if (working.has_changes(working.basis_tree()) |
26 | + or len(working.get_parent_ids()) > 1): |
27 | raise errors.UncommittedChanges(working) |
28 | |
29 | working_path = working.bzrdir.root_transport.base |
30 | @@ -3653,13 +3653,14 @@ |
31 | verified = 'inapplicable' |
32 | tree = WorkingTree.open_containing(directory)[0] |
33 | |
34 | - # die as quickly as possible if there are uncommitted changes |
35 | try: |
36 | basis_tree = tree.revision_tree(tree.last_revision()) |
37 | except errors.NoSuchRevision: |
38 | basis_tree = tree.basis_tree() |
39 | + |
40 | + # die as quickly as possible if there are uncommitted changes |
41 | if not force: |
42 | - if tree.has_changes(basis_tree): |
43 | + if tree.has_changes(basis_tree) or len(tree.get_parent_ids()) > 1: |
44 | raise errors.UncommittedChanges(tree) |
45 | |
46 | view_info = _get_view_info_for_change_reporter(tree) |
47 | |
48 | === modified file 'bzrlib/tests/blackbox/test_merge.py' |
49 | --- bzrlib/tests/blackbox/test_merge.py 2009-08-06 06:58:09 +0000 |
50 | +++ bzrlib/tests/blackbox/test_merge.py 2009-09-08 16:42:51 +0000 |
51 | @@ -21,23 +21,26 @@ |
52 | |
53 | import os |
54 | |
55 | -from bzrlib import merge_directive |
56 | -from bzrlib.branch import Branch |
57 | -from bzrlib.bzrdir import BzrDir |
58 | -from bzrlib.conflicts import ConflictList, ContentsConflict |
59 | -from bzrlib.osutils import abspath, file_kind, pathjoin |
60 | -from bzrlib.tests.blackbox import ExternalBase |
61 | -import bzrlib.urlutils as urlutils |
62 | -from bzrlib.workingtree import WorkingTree |
63 | - |
64 | - |
65 | -class TestMerge(ExternalBase): |
66 | +from bzrlib import ( |
67 | + branch, |
68 | + bzrdir, |
69 | + conflicts, |
70 | + errors, |
71 | + merge_directive, |
72 | + osutils, |
73 | + tests, |
74 | + urlutils, |
75 | + workingtree, |
76 | + ) |
77 | + |
78 | + |
79 | +class TestMerge(tests.TestCaseWithTransport): |
80 | |
81 | def example_branch(self, path='.'): |
82 | tree = self.make_branch_and_tree(path) |
83 | self.build_tree_contents([ |
84 | - (pathjoin(path, 'hello'), 'foo'), |
85 | - (pathjoin(path, 'goodbye'), 'baz')]) |
86 | + (osutils.pathjoin(path, 'hello'), 'foo'), |
87 | + (osutils.pathjoin(path, 'goodbye'), 'baz')]) |
88 | tree.add('hello') |
89 | tree.commit(message='setup') |
90 | tree.add('goodbye') |
91 | @@ -63,13 +66,11 @@ |
92 | return tree, other |
93 | |
94 | def test_merge_reprocess(self): |
95 | - d = BzrDir.create_standalone_workingtree('.') |
96 | + d = bzrdir.BzrDir.create_standalone_workingtree('.') |
97 | d.commit('h') |
98 | self.run_bzr('merge . --reprocess --merge-type weave') |
99 | |
100 | def test_merge(self): |
101 | - from bzrlib.branch import Branch |
102 | - |
103 | a_tree = self.example_branch('a') |
104 | ancestor = a_tree.branch.revno() |
105 | b_tree = a_tree.bzrdir.sprout('b').open_workingtree() |
106 | @@ -80,7 +81,7 @@ |
107 | # We can't merge when there are in-tree changes |
108 | os.chdir('a') |
109 | self.run_bzr('merge ../b', retcode=3) |
110 | - a = WorkingTree.open('.') |
111 | + a = workingtree.WorkingTree.open('.') |
112 | a_tip = a.commit("Like an epidemic of u's") |
113 | self.run_bzr('merge ../b -r last:1..last:1 --merge-type blooof', |
114 | retcode=3) |
115 | @@ -99,7 +100,7 @@ |
116 | self.run_bzr('merge ../b -r last:1') |
117 | self.check_file_contents('goodbye', 'quux') |
118 | # Merging a branch pulls its revision into the tree |
119 | - b = Branch.open('../b') |
120 | + b = branch.Branch.open('../b') |
121 | b_tip = b.last_revision() |
122 | self.failUnless(a.branch.repository.has_revision(b_tip)) |
123 | self.assertEqual([a_tip, b_tip], a.get_parent_ids()) |
124 | @@ -249,8 +250,8 @@ |
125 | |
126 | base = urlutils.local_path_from_url(branch_a.base) |
127 | self.assertEndsWith(err, '+N b\nAll changes applied successfully.\n') |
128 | - self.assertEquals(abspath(branch_b.get_submit_branch()), |
129 | - abspath(parent)) |
130 | + self.assertEquals(osutils.abspath(branch_b.get_submit_branch()), |
131 | + osutils.abspath(parent)) |
132 | # test implicit --remember when committing new file |
133 | self.build_tree(['e']) |
134 | tree_b.add('e') |
135 | @@ -265,8 +266,8 @@ |
136 | out, err = self.run_bzr('merge ../branch_c --remember') |
137 | self.assertEquals(out, '') |
138 | self.assertEquals(err, '+N c\nAll changes applied successfully.\n') |
139 | - self.assertEquals(abspath(branch_b.get_submit_branch()), |
140 | - abspath(branch_c.bzrdir.root_transport.base)) |
141 | + self.assertEquals(osutils.abspath(branch_b.get_submit_branch()), |
142 | + osutils.abspath(branch_c.bzrdir.root_transport.base)) |
143 | # re-open tree as external run_bzr modified it |
144 | tree_b = branch_b.bzrdir.open_workingtree() |
145 | tree_b.commit('merge branch_c') |
146 | @@ -294,7 +295,7 @@ |
147 | tree_b.get_parent_ids()[0]) |
148 | self.assertEqualDiff(testament_a.as_text(), |
149 | testament_b.as_text()) |
150 | - tree_a.set_conflicts(ConflictList()) |
151 | + tree_a.set_conflicts(conflicts.ConflictList()) |
152 | tree_a.commit('message') |
153 | # it is legal to attempt to merge an already-merged bundle |
154 | output = self.run_bzr('merge ../bundle')[1] |
155 | @@ -349,7 +350,7 @@ |
156 | os.chdir('a') |
157 | (out, err) = self.run_bzr('merge --pull ../b') |
158 | self.assertContainsRe(out, 'Now on revision 2\\.') |
159 | - tree_a = WorkingTree.open('.') |
160 | + tree_a = workingtree.WorkingTree.open('.') |
161 | self.assertEqual([self.id2], tree_a.get_parent_ids()) |
162 | |
163 | def test_merge_kind_change(self): |
164 | @@ -363,14 +364,15 @@ |
165 | tree_a.commit('changed file to directory') |
166 | os.chdir('tree_b') |
167 | self.run_bzr('merge ../tree_a') |
168 | - self.assertEqual('directory', file_kind('file')) |
169 | + self.assertEqual('directory', osutils.file_kind('file')) |
170 | tree_b.revert() |
171 | - self.assertEqual('file', file_kind('file')) |
172 | + self.assertEqual('file', osutils.file_kind('file')) |
173 | self.build_tree_contents([('file', 'content_2')]) |
174 | tree_b.commit('content change') |
175 | self.run_bzr('merge ../tree_a', retcode=1) |
176 | self.assertEqual(tree_b.conflicts(), |
177 | - [ContentsConflict('file', file_id='file-id')]) |
178 | + [conflicts.ContentsConflict('file', |
179 | + file_id='file-id')]) |
180 | |
181 | def test_directive_cherrypick(self): |
182 | source = self.make_branch_and_tree('source') |
183 | @@ -496,18 +498,6 @@ |
184 | out, err = self.run_bzr(['merge', '-d', 'a', 'b']) |
185 | self.assertContainsRe(err, 'Warning: criss-cross merge encountered.') |
186 | |
187 | - def test_merge_force(self): |
188 | - tree_a = self.make_branch_and_tree('a') |
189 | - self.build_tree(['a/foo']) |
190 | - tree_a.add(['foo']) |
191 | - tree_a.commit('add file') |
192 | - tree_b = tree_a.bzrdir.sprout('b').open_workingtree() |
193 | - self.build_tree_contents([('a/foo', 'change 1')]) |
194 | - tree_a.commit('change file') |
195 | - tree_b.merge_from_branch(tree_a.branch) |
196 | - tree_a.commit('empty change to allow merge to run') |
197 | - self.run_bzr(['merge', '../a', '--force'], working_dir='b') |
198 | - |
199 | def test_merge_from_submit(self): |
200 | tree_a = self.make_branch_and_tree('a') |
201 | tree_b = tree_a.bzrdir.sprout('b').open_workingtree() |
202 | @@ -560,11 +550,11 @@ |
203 | tree_a.merge_from_branch(tree_b.branch) |
204 | self.build_tree_contents([('a/file', |
205 | 'base-contents\nthis-contents\n')]) |
206 | - tree_a.set_conflicts(ConflictList()) |
207 | + tree_a.set_conflicts(conflicts.ConflictList()) |
208 | tree_b.merge_from_branch(tree_a.branch) |
209 | self.build_tree_contents([('b/file', |
210 | 'base-contents\nother-contents\n')]) |
211 | - tree_b.set_conflicts(ConflictList()) |
212 | + tree_b.set_conflicts(conflicts.ConflictList()) |
213 | tree_a.commit('', rev_id='rev3a') |
214 | tree_b.commit('', rev_id='rev3b') |
215 | out, err = self.run_bzr(['merge', '-d', 'a', 'b', '--lca'], retcode=1) |
216 | @@ -598,3 +588,33 @@ |
217 | other.commit('rev1b') |
218 | self.run_bzr('merge -d this other -r0..') |
219 | self.failUnlessExists('this/other_file') |
220 | + |
221 | + |
222 | +class TestMergeForce(tests.TestCaseWithTransport): |
223 | + |
224 | + def setUp(self): |
225 | + super(TestMergeForce, self).setUp() |
226 | + self.tree_a = self.make_branch_and_tree('a') |
227 | + self.build_tree(['a/foo']) |
228 | + self.tree_a.add(['foo']) |
229 | + self.tree_a.commit('add file') |
230 | + self.tree_b = self.tree_a.bzrdir.sprout('b').open_workingtree() |
231 | + self.build_tree_contents([('a/foo', 'change 1')]) |
232 | + self.tree_a.commit('change file') |
233 | + self.tree_b.merge_from_branch(self.tree_a.branch) |
234 | + |
235 | + def test_merge_force(self): |
236 | + self.tree_a.commit('empty change to allow merge to run') |
237 | + # Second merge on top if the uncommitted one |
238 | + self.run_bzr(['merge', '../a', '--force'], working_dir='b') |
239 | + |
240 | + |
241 | + def test_merge_with_uncommitted_changes(self): |
242 | + self.run_bzr_error(['Working tree .* has uncommitted changes'], |
243 | + ['merge', '../a'], working_dir='b') |
244 | + |
245 | + def test_merge_with_pending_merges(self): |
246 | + # Revert the changes keeping the pending merge |
247 | + self.run_bzr(['revert', 'b']) |
248 | + self.run_bzr_error(['Working tree .* has uncommitted changes'], |
249 | + ['merge', '../a'], working_dir='b') |
250 | |
251 | === modified file 'bzrlib/tests/blackbox/test_remove_tree.py' |
252 | --- bzrlib/tests/blackbox/test_remove_tree.py 2009-03-23 14:59:43 +0000 |
253 | +++ bzrlib/tests/blackbox/test_remove_tree.py 2009-09-08 16:45:11 +0000 |
254 | @@ -122,3 +122,30 @@ |
255 | self.run_bzr('remove-tree branch1 --force') |
256 | self.failIfExists('branch1/foo') |
257 | self.failUnlessExists('branch1/bar') |
258 | + |
259 | + def test_remove_tree_pending_merges(self): |
260 | + self.run_bzr(['branch', 'branch1', 'branch2']) |
261 | + self.build_tree(['branch1/bar']) |
262 | + self.tree.add('bar') |
263 | + self.tree.commit('2') |
264 | + self.failUnlessExists('branch1/bar') |
265 | + self.run_bzr(['merge', '../branch1'], working_dir='branch2') |
266 | + self.failUnlessExists('branch2/bar') |
267 | + self.run_bzr(['revert', '.'], working_dir='branch2') |
268 | + self.failIfExists('branch2/bar') |
269 | + output = self.run_bzr_error(["Working tree .* has uncommitted changes"], |
270 | + 'remove-tree branch2', retcode=3) |
271 | + |
272 | + def test_remove_tree_pending_merges_force(self): |
273 | + self.run_bzr(['branch', 'branch1', 'branch2']) |
274 | + self.build_tree(['branch1/bar']) |
275 | + self.tree.add('bar') |
276 | + self.tree.commit('2') |
277 | + self.failUnlessExists('branch1/bar') |
278 | + self.run_bzr(['merge', '../branch1'], working_dir='branch2') |
279 | + self.failUnlessExists('branch2/bar') |
280 | + self.run_bzr(['revert', '.'], working_dir='branch2') |
281 | + self.failIfExists('branch2/bar') |
282 | + self.run_bzr('remove-tree branch2 --force') |
283 | + self.failIfExists('branch2/foo') |
284 | + self.failIfExists('branch2/bar') |
This fixes the last two places where we were checking uncommitted changes
while not taking pending merges into account: merge and remove-tree.