Merge lp:~vila/bzr/426344-check-pending-merges into lp:bzr

Proposed by Vincent Ladeuil on 2009-09-08
Status: Merged
Approved by: John A Meinel on 2009-09-08
Approved revision: 4676
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
Reviewer Review Type Date Requested Status
John A Meinel 2009-09-08 Approve on 2009-09-08
Review via email: mp+11378@code.launchpad.net
To post a comment you must log in.
Vincent Ladeuil (vila) wrote :

This fixes the last two places where we were checking uncommitted changes
while not taking pending merges into account: merge and remove-tree.

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_tree.has_changes()

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://enigmail.mozdev.org/

iEYEARECAAYFAkqmmPcACgkQJdeBCYSNAANcQACfTLaWXv/YvIx79zte6lhB2CWg
3JUAoJKgX7MQd/GI06BE4XCc0vKn3qkJ
=LDTT
-----END PGP SIGNATURE-----

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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')