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
=== modified file 'NEWS'
--- NEWS 2009-09-04 00:49:55 +0000
+++ NEWS 2009-09-08 16:46:40 +0000
@@ -30,6 +30,10 @@
30 longer report incorrect errors about ``Missing inventory ('TREE_ROOT', ...)``30 longer report incorrect errors about ``Missing inventory ('TREE_ROOT', ...)``
31 (Robert Collins, #416732)31 (Robert Collins, #416732)
3232
33* ``bzr merge`` and ``bzr remove-tree`` now requires --force if pending
34 merges are present in the working tree.
35 (Vincent Ladeuil, #426344)
36
33* Don't restrict the command name used to run the test suite.37* Don't restrict the command name used to run the test suite.
34 (Vincent Ladeuil, #419950)38 (Vincent Ladeuil, #419950)
3539
3640
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2009-08-28 05:00:33 +0000
+++ bzrlib/builtins.py 2009-09-08 16:45:11 +0000
@@ -461,8 +461,8 @@
461 raise errors.BzrCommandError("You cannot remove the working tree"461 raise errors.BzrCommandError("You cannot remove the working tree"
462 " of a remote path")462 " of a remote path")
463 if not force:463 if not force:
464 # XXX: What about pending merges ? -- vila 20090629464 if (working.has_changes(working.basis_tree())
465 if working.has_changes(working.basis_tree()):465 or len(working.get_parent_ids()) > 1):
466 raise errors.UncommittedChanges(working)466 raise errors.UncommittedChanges(working)
467467
468 working_path = working.bzrdir.root_transport.base468 working_path = working.bzrdir.root_transport.base
@@ -3653,13 +3653,14 @@
3653 verified = 'inapplicable'3653 verified = 'inapplicable'
3654 tree = WorkingTree.open_containing(directory)[0]3654 tree = WorkingTree.open_containing(directory)[0]
36553655
3656 # die as quickly as possible if there are uncommitted changes
3657 try:3656 try:
3658 basis_tree = tree.revision_tree(tree.last_revision())3657 basis_tree = tree.revision_tree(tree.last_revision())
3659 except errors.NoSuchRevision:3658 except errors.NoSuchRevision:
3660 basis_tree = tree.basis_tree()3659 basis_tree = tree.basis_tree()
3660
3661 # die as quickly as possible if there are uncommitted changes
3661 if not force:3662 if not force:
3662 if tree.has_changes(basis_tree):3663 if tree.has_changes(basis_tree) or len(tree.get_parent_ids()) > 1:
3663 raise errors.UncommittedChanges(tree)3664 raise errors.UncommittedChanges(tree)
36643665
3665 view_info = _get_view_info_for_change_reporter(tree)3666 view_info = _get_view_info_for_change_reporter(tree)
36663667
=== modified file 'bzrlib/tests/blackbox/test_merge.py'
--- bzrlib/tests/blackbox/test_merge.py 2009-08-06 06:58:09 +0000
+++ bzrlib/tests/blackbox/test_merge.py 2009-09-08 16:42:51 +0000
@@ -21,23 +21,26 @@
2121
22import os22import os
2323
24from bzrlib import merge_directive24from bzrlib import (
25from bzrlib.branch import Branch25 branch,
26from bzrlib.bzrdir import BzrDir26 bzrdir,
27from bzrlib.conflicts import ConflictList, ContentsConflict27 conflicts,
28from bzrlib.osutils import abspath, file_kind, pathjoin28 errors,
29from bzrlib.tests.blackbox import ExternalBase29 merge_directive,
30import bzrlib.urlutils as urlutils30 osutils,
31from bzrlib.workingtree import WorkingTree31 tests,
3232 urlutils,
3333 workingtree,
34class TestMerge(ExternalBase):34 )
35
36
37class TestMerge(tests.TestCaseWithTransport):
3538
36 def example_branch(self, path='.'):39 def example_branch(self, path='.'):
37 tree = self.make_branch_and_tree(path)40 tree = self.make_branch_and_tree(path)
38 self.build_tree_contents([41 self.build_tree_contents([
39 (pathjoin(path, 'hello'), 'foo'),42 (osutils.pathjoin(path, 'hello'), 'foo'),
40 (pathjoin(path, 'goodbye'), 'baz')])43 (osutils.pathjoin(path, 'goodbye'), 'baz')])
41 tree.add('hello')44 tree.add('hello')
42 tree.commit(message='setup')45 tree.commit(message='setup')
43 tree.add('goodbye')46 tree.add('goodbye')
@@ -63,13 +66,11 @@
63 return tree, other66 return tree, other
6467
65 def test_merge_reprocess(self):68 def test_merge_reprocess(self):
66 d = BzrDir.create_standalone_workingtree('.')69 d = bzrdir.BzrDir.create_standalone_workingtree('.')
67 d.commit('h')70 d.commit('h')
68 self.run_bzr('merge . --reprocess --merge-type weave')71 self.run_bzr('merge . --reprocess --merge-type weave')
6972
70 def test_merge(self):73 def test_merge(self):
71 from bzrlib.branch import Branch
72
73 a_tree = self.example_branch('a')74 a_tree = self.example_branch('a')
74 ancestor = a_tree.branch.revno()75 ancestor = a_tree.branch.revno()
75 b_tree = a_tree.bzrdir.sprout('b').open_workingtree()76 b_tree = a_tree.bzrdir.sprout('b').open_workingtree()
@@ -80,7 +81,7 @@
80 # We can't merge when there are in-tree changes81 # We can't merge when there are in-tree changes
81 os.chdir('a')82 os.chdir('a')
82 self.run_bzr('merge ../b', retcode=3)83 self.run_bzr('merge ../b', retcode=3)
83 a = WorkingTree.open('.')84 a = workingtree.WorkingTree.open('.')
84 a_tip = a.commit("Like an epidemic of u's")85 a_tip = a.commit("Like an epidemic of u's")
85 self.run_bzr('merge ../b -r last:1..last:1 --merge-type blooof',86 self.run_bzr('merge ../b -r last:1..last:1 --merge-type blooof',
86 retcode=3)87 retcode=3)
@@ -99,7 +100,7 @@
99 self.run_bzr('merge ../b -r last:1')100 self.run_bzr('merge ../b -r last:1')
100 self.check_file_contents('goodbye', 'quux')101 self.check_file_contents('goodbye', 'quux')
101 # Merging a branch pulls its revision into the tree102 # Merging a branch pulls its revision into the tree
102 b = Branch.open('../b')103 b = branch.Branch.open('../b')
103 b_tip = b.last_revision()104 b_tip = b.last_revision()
104 self.failUnless(a.branch.repository.has_revision(b_tip))105 self.failUnless(a.branch.repository.has_revision(b_tip))
105 self.assertEqual([a_tip, b_tip], a.get_parent_ids())106 self.assertEqual([a_tip, b_tip], a.get_parent_ids())
@@ -249,8 +250,8 @@
249250
250 base = urlutils.local_path_from_url(branch_a.base)251 base = urlutils.local_path_from_url(branch_a.base)
251 self.assertEndsWith(err, '+N b\nAll changes applied successfully.\n')252 self.assertEndsWith(err, '+N b\nAll changes applied successfully.\n')
252 self.assertEquals(abspath(branch_b.get_submit_branch()),253 self.assertEquals(osutils.abspath(branch_b.get_submit_branch()),
253 abspath(parent))254 osutils.abspath(parent))
254 # test implicit --remember when committing new file255 # test implicit --remember when committing new file
255 self.build_tree(['e'])256 self.build_tree(['e'])
256 tree_b.add('e')257 tree_b.add('e')
@@ -265,8 +266,8 @@
265 out, err = self.run_bzr('merge ../branch_c --remember')266 out, err = self.run_bzr('merge ../branch_c --remember')
266 self.assertEquals(out, '')267 self.assertEquals(out, '')
267 self.assertEquals(err, '+N c\nAll changes applied successfully.\n')268 self.assertEquals(err, '+N c\nAll changes applied successfully.\n')
268 self.assertEquals(abspath(branch_b.get_submit_branch()),269 self.assertEquals(osutils.abspath(branch_b.get_submit_branch()),
269 abspath(branch_c.bzrdir.root_transport.base))270 osutils.abspath(branch_c.bzrdir.root_transport.base))
270 # re-open tree as external run_bzr modified it271 # re-open tree as external run_bzr modified it
271 tree_b = branch_b.bzrdir.open_workingtree()272 tree_b = branch_b.bzrdir.open_workingtree()
272 tree_b.commit('merge branch_c')273 tree_b.commit('merge branch_c')
@@ -294,7 +295,7 @@
294 tree_b.get_parent_ids()[0])295 tree_b.get_parent_ids()[0])
295 self.assertEqualDiff(testament_a.as_text(),296 self.assertEqualDiff(testament_a.as_text(),
296 testament_b.as_text())297 testament_b.as_text())
297 tree_a.set_conflicts(ConflictList())298 tree_a.set_conflicts(conflicts.ConflictList())
298 tree_a.commit('message')299 tree_a.commit('message')
299 # it is legal to attempt to merge an already-merged bundle300 # it is legal to attempt to merge an already-merged bundle
300 output = self.run_bzr('merge ../bundle')[1]301 output = self.run_bzr('merge ../bundle')[1]
@@ -349,7 +350,7 @@
349 os.chdir('a')350 os.chdir('a')
350 (out, err) = self.run_bzr('merge --pull ../b')351 (out, err) = self.run_bzr('merge --pull ../b')
351 self.assertContainsRe(out, 'Now on revision 2\\.')352 self.assertContainsRe(out, 'Now on revision 2\\.')
352 tree_a = WorkingTree.open('.')353 tree_a = workingtree.WorkingTree.open('.')
353 self.assertEqual([self.id2], tree_a.get_parent_ids())354 self.assertEqual([self.id2], tree_a.get_parent_ids())
354355
355 def test_merge_kind_change(self):356 def test_merge_kind_change(self):
@@ -363,14 +364,15 @@
363 tree_a.commit('changed file to directory')364 tree_a.commit('changed file to directory')
364 os.chdir('tree_b')365 os.chdir('tree_b')
365 self.run_bzr('merge ../tree_a')366 self.run_bzr('merge ../tree_a')
366 self.assertEqual('directory', file_kind('file'))367 self.assertEqual('directory', osutils.file_kind('file'))
367 tree_b.revert()368 tree_b.revert()
368 self.assertEqual('file', file_kind('file'))369 self.assertEqual('file', osutils.file_kind('file'))
369 self.build_tree_contents([('file', 'content_2')])370 self.build_tree_contents([('file', 'content_2')])
370 tree_b.commit('content change')371 tree_b.commit('content change')
371 self.run_bzr('merge ../tree_a', retcode=1)372 self.run_bzr('merge ../tree_a', retcode=1)
372 self.assertEqual(tree_b.conflicts(),373 self.assertEqual(tree_b.conflicts(),
373 [ContentsConflict('file', file_id='file-id')])374 [conflicts.ContentsConflict('file',
375 file_id='file-id')])
374376
375 def test_directive_cherrypick(self):377 def test_directive_cherrypick(self):
376 source = self.make_branch_and_tree('source')378 source = self.make_branch_and_tree('source')
@@ -496,18 +498,6 @@
496 out, err = self.run_bzr(['merge', '-d', 'a', 'b'])498 out, err = self.run_bzr(['merge', '-d', 'a', 'b'])
497 self.assertContainsRe(err, 'Warning: criss-cross merge encountered.')499 self.assertContainsRe(err, 'Warning: criss-cross merge encountered.')
498500
499 def test_merge_force(self):
500 tree_a = self.make_branch_and_tree('a')
501 self.build_tree(['a/foo'])
502 tree_a.add(['foo'])
503 tree_a.commit('add file')
504 tree_b = tree_a.bzrdir.sprout('b').open_workingtree()
505 self.build_tree_contents([('a/foo', 'change 1')])
506 tree_a.commit('change file')
507 tree_b.merge_from_branch(tree_a.branch)
508 tree_a.commit('empty change to allow merge to run')
509 self.run_bzr(['merge', '../a', '--force'], working_dir='b')
510
511 def test_merge_from_submit(self):501 def test_merge_from_submit(self):
512 tree_a = self.make_branch_and_tree('a')502 tree_a = self.make_branch_and_tree('a')
513 tree_b = tree_a.bzrdir.sprout('b').open_workingtree()503 tree_b = tree_a.bzrdir.sprout('b').open_workingtree()
@@ -560,11 +550,11 @@
560 tree_a.merge_from_branch(tree_b.branch)550 tree_a.merge_from_branch(tree_b.branch)
561 self.build_tree_contents([('a/file',551 self.build_tree_contents([('a/file',
562 'base-contents\nthis-contents\n')])552 'base-contents\nthis-contents\n')])
563 tree_a.set_conflicts(ConflictList())553 tree_a.set_conflicts(conflicts.ConflictList())
564 tree_b.merge_from_branch(tree_a.branch)554 tree_b.merge_from_branch(tree_a.branch)
565 self.build_tree_contents([('b/file',555 self.build_tree_contents([('b/file',
566 'base-contents\nother-contents\n')])556 'base-contents\nother-contents\n')])
567 tree_b.set_conflicts(ConflictList())557 tree_b.set_conflicts(conflicts.ConflictList())
568 tree_a.commit('', rev_id='rev3a')558 tree_a.commit('', rev_id='rev3a')
569 tree_b.commit('', rev_id='rev3b')559 tree_b.commit('', rev_id='rev3b')
570 out, err = self.run_bzr(['merge', '-d', 'a', 'b', '--lca'], retcode=1)560 out, err = self.run_bzr(['merge', '-d', 'a', 'b', '--lca'], retcode=1)
@@ -598,3 +588,33 @@
598 other.commit('rev1b')588 other.commit('rev1b')
599 self.run_bzr('merge -d this other -r0..')589 self.run_bzr('merge -d this other -r0..')
600 self.failUnlessExists('this/other_file')590 self.failUnlessExists('this/other_file')
591
592
593class TestMergeForce(tests.TestCaseWithTransport):
594
595 def setUp(self):
596 super(TestMergeForce, self).setUp()
597 self.tree_a = self.make_branch_and_tree('a')
598 self.build_tree(['a/foo'])
599 self.tree_a.add(['foo'])
600 self.tree_a.commit('add file')
601 self.tree_b = self.tree_a.bzrdir.sprout('b').open_workingtree()
602 self.build_tree_contents([('a/foo', 'change 1')])
603 self.tree_a.commit('change file')
604 self.tree_b.merge_from_branch(self.tree_a.branch)
605
606 def test_merge_force(self):
607 self.tree_a.commit('empty change to allow merge to run')
608 # Second merge on top if the uncommitted one
609 self.run_bzr(['merge', '../a', '--force'], working_dir='b')
610
611
612 def test_merge_with_uncommitted_changes(self):
613 self.run_bzr_error(['Working tree .* has uncommitted changes'],
614 ['merge', '../a'], working_dir='b')
615
616 def test_merge_with_pending_merges(self):
617 # Revert the changes keeping the pending merge
618 self.run_bzr(['revert', 'b'])
619 self.run_bzr_error(['Working tree .* has uncommitted changes'],
620 ['merge', '../a'], working_dir='b')
601621
=== modified file 'bzrlib/tests/blackbox/test_remove_tree.py'
--- bzrlib/tests/blackbox/test_remove_tree.py 2009-03-23 14:59:43 +0000
+++ bzrlib/tests/blackbox/test_remove_tree.py 2009-09-08 16:45:11 +0000
@@ -122,3 +122,30 @@
122 self.run_bzr('remove-tree branch1 --force')122 self.run_bzr('remove-tree branch1 --force')
123 self.failIfExists('branch1/foo')123 self.failIfExists('branch1/foo')
124 self.failUnlessExists('branch1/bar')124 self.failUnlessExists('branch1/bar')
125
126 def test_remove_tree_pending_merges(self):
127 self.run_bzr(['branch', 'branch1', 'branch2'])
128 self.build_tree(['branch1/bar'])
129 self.tree.add('bar')
130 self.tree.commit('2')
131 self.failUnlessExists('branch1/bar')
132 self.run_bzr(['merge', '../branch1'], working_dir='branch2')
133 self.failUnlessExists('branch2/bar')
134 self.run_bzr(['revert', '.'], working_dir='branch2')
135 self.failIfExists('branch2/bar')
136 output = self.run_bzr_error(["Working tree .* has uncommitted changes"],
137 'remove-tree branch2', retcode=3)
138
139 def test_remove_tree_pending_merges_force(self):
140 self.run_bzr(['branch', 'branch1', 'branch2'])
141 self.build_tree(['branch1/bar'])
142 self.tree.add('bar')
143 self.tree.commit('2')
144 self.failUnlessExists('branch1/bar')
145 self.run_bzr(['merge', '../branch1'], working_dir='branch2')
146 self.failUnlessExists('branch2/bar')
147 self.run_bzr(['revert', '.'], working_dir='branch2')
148 self.failIfExists('branch2/bar')
149 self.run_bzr('remove-tree branch2 --force')
150 self.failIfExists('branch2/foo')
151 self.failIfExists('branch2/bar')