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

[Vincent Ladeuil]

[Vincent Ladeuil (vila)]

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)]

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

=== modified file 'NEWS'
--- NEWS	2009-09-04 00:49:55 +0000
+++ NEWS	2009-09-08 16:46:40 +0000
@@ -30,6 +30,10 @@
   longer report incorrect errors about ``Missing inventory ('TREE_ROOT', ...)``
   (Robert Collins, #416732)
 
+* ``bzr merge`` and ``bzr remove-tree`` now requires --force if pending
+  merges are present in the working tree.
+  (Vincent Ladeuil, #426344)
+
 * Don't restrict the command name used to run the test suite.
   (Vincent Ladeuil, #419950)
 
 
=== 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 @@
             raise errors.BzrCommandError("You cannot remove the working tree"
                                          " of a remote path")
         if not force:
-            # XXX: What about pending merges ? -- vila 20090629
-            if working.has_changes(working.basis_tree()):
+            if (working.has_changes(working.basis_tree())
+                or len(working.get_parent_ids()) > 1):
                 raise errors.UncommittedChanges(working)
 
         working_path = working.bzrdir.root_transport.base
@@ -3653,13 +3653,14 @@
         verified = 'inapplicable'
         tree = WorkingTree.open_containing(directory)[0]
 
-        # die as quickly as possible if there are uncommitted changes
         try:
             basis_tree = tree.revision_tree(tree.last_revision())
         except errors.NoSuchRevision:
             basis_tree = tree.basis_tree()
+
+        # die as quickly as possible if there are uncommitted changes
         if not force:
-            if tree.has_changes(basis_tree):
+            if tree.has_changes(basis_tree) or len(tree.get_parent_ids()) > 1:
                 raise errors.UncommittedChanges(tree)
 
         view_info = _get_view_info_for_change_reporter(tree)
 
=== 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 @@
 
 import os
 
-from bzrlib import merge_directive
-from bzrlib.branch import Branch
-from bzrlib.bzrdir import BzrDir
-from bzrlib.conflicts import ConflictList, ContentsConflict
-from bzrlib.osutils import abspath, file_kind, pathjoin
-from bzrlib.tests.blackbox import ExternalBase
-import bzrlib.urlutils as urlutils
-from bzrlib.workingtree import WorkingTree
-
-
-class TestMerge(ExternalBase):
+from bzrlib import (
+    branch,
+    bzrdir,
+    conflicts,
+    errors,
+    merge_directive,
+    osutils,
+    tests,
+    urlutils,
+    workingtree,
+    )
+
+
+class TestMerge(tests.TestCaseWithTransport):
 
     def example_branch(self, path='.'):
         tree = self.make_branch_and_tree(path)
         self.build_tree_contents([
-            (pathjoin(path, 'hello'), 'foo'),
-            (pathjoin(path, 'goodbye'), 'baz')])
+            (osutils.pathjoin(path, 'hello'), 'foo'),
+            (osutils.pathjoin(path, 'goodbye'), 'baz')])
         tree.add('hello')
         tree.commit(message='setup')
         tree.add('goodbye')
@@ -63,13 +66,11 @@
         return tree, other
 
     def test_merge_reprocess(self):
-        d = BzrDir.create_standalone_workingtree('.')
+        d = bzrdir.BzrDir.create_standalone_workingtree('.')
         d.commit('h')
         self.run_bzr('merge . --reprocess --merge-type weave')
 
     def test_merge(self):
-        from bzrlib.branch import Branch
-
         a_tree = self.example_branch('a')
         ancestor = a_tree.branch.revno()
         b_tree = a_tree.bzrdir.sprout('b').open_workingtree()
@@ -80,7 +81,7 @@
         # We can't merge when there are in-tree changes
         os.chdir('a')
         self.run_bzr('merge ../b', retcode=3)
-        a = WorkingTree.open('.')
+        a = workingtree.WorkingTree.open('.')
         a_tip = a.commit("Like an epidemic of u's")
         self.run_bzr('merge ../b -r last:1..last:1 --merge-type blooof',
                     retcode=3)
@@ -99,7 +100,7 @@
         self.run_bzr('merge ../b -r last:1')
         self.check_file_contents('goodbye', 'quux')
         # Merging a branch pulls its revision into the tree
-        b = Branch.open('../b')
+        b = branch.Branch.open('../b')
         b_tip = b.last_revision()
         self.failUnless(a.branch.repository.has_revision(b_tip))
         self.assertEqual([a_tip, b_tip], a.get_parent_ids())
@@ -249,8 +250,8 @@
 
         base = urlutils.local_path_from_url(branch_a.base)
         self.assertEndsWith(err, '+N  b\nAll changes applied successfully.\n')
-        self.assertEquals(abspath(branch_b.get_submit_branch()),
-                          abspath(parent))
+        self.assertEquals(osutils.abspath(branch_b.get_submit_branch()),
+                          osutils.abspath(parent))
         # test implicit --remember when committing new file
         self.build_tree(['e'])
         tree_b.add('e')
@@ -265,8 +266,8 @@
         out, err = self.run_bzr('merge ../branch_c --remember')
         self.assertEquals(out, '')
         self.assertEquals(err, '+N  c\nAll changes applied successfully.\n')
-        self.assertEquals(abspath(branch_b.get_submit_branch()),
-                          abspath(branch_c.bzrdir.root_transport.base))
+        self.assertEquals(osutils.abspath(branch_b.get_submit_branch()),
+                          osutils.abspath(branch_c.bzrdir.root_transport.base))
         # re-open tree as external run_bzr modified it
         tree_b = branch_b.bzrdir.open_workingtree()
         tree_b.commit('merge branch_c')
@@ -294,7 +295,7 @@
                                               tree_b.get_parent_ids()[0])
         self.assertEqualDiff(testament_a.as_text(),
                          testament_b.as_text())
-        tree_a.set_conflicts(ConflictList())
+        tree_a.set_conflicts(conflicts.ConflictList())
         tree_a.commit('message')
         # it is legal to attempt to merge an already-merged bundle
         output = self.run_bzr('merge ../bundle')[1]
@@ -349,7 +350,7 @@
         os.chdir('a')
         (out, err) = self.run_bzr('merge --pull ../b')
         self.assertContainsRe(out, 'Now on revision 2\\.')
-        tree_a = WorkingTree.open('.')
+        tree_a = workingtree.WorkingTree.open('.')
         self.assertEqual([self.id2], tree_a.get_parent_ids())
 
     def test_merge_kind_change(self):
@@ -363,14 +364,15 @@
         tree_a.commit('changed file to directory')
         os.chdir('tree_b')
         self.run_bzr('merge ../tree_a')
-        self.assertEqual('directory', file_kind('file'))
+        self.assertEqual('directory', osutils.file_kind('file'))
         tree_b.revert()
-        self.assertEqual('file', file_kind('file'))
+        self.assertEqual('file', osutils.file_kind('file'))
         self.build_tree_contents([('file', 'content_2')])
         tree_b.commit('content change')
         self.run_bzr('merge ../tree_a', retcode=1)
         self.assertEqual(tree_b.conflicts(),
-                         [ContentsConflict('file', file_id='file-id')])
+                         [conflicts.ContentsConflict('file',
+                                                     file_id='file-id')])
 
     def test_directive_cherrypick(self):
         source = self.make_branch_and_tree('source')
@@ -496,18 +498,6 @@
         out, err = self.run_bzr(['merge', '-d', 'a', 'b'])
         self.assertContainsRe(err, 'Warning: criss-cross merge encountered.')
 
-    def test_merge_force(self):
-        tree_a = self.make_branch_and_tree('a')
-        self.build_tree(['a/foo'])
-        tree_a.add(['foo'])
-        tree_a.commit('add file')
-        tree_b = tree_a.bzrdir.sprout('b').open_workingtree()
-        self.build_tree_contents([('a/foo', 'change 1')])
-        tree_a.commit('change file')
-        tree_b.merge_from_branch(tree_a.branch)
-        tree_a.commit('empty change to allow merge to run')
-        self.run_bzr(['merge', '../a', '--force'], working_dir='b')
-
     def test_merge_from_submit(self):
         tree_a = self.make_branch_and_tree('a')
         tree_b = tree_a.bzrdir.sprout('b').open_workingtree()
@@ -560,11 +550,11 @@
         tree_a.merge_from_branch(tree_b.branch)
         self.build_tree_contents([('a/file',
                                    'base-contents\nthis-contents\n')])
-        tree_a.set_conflicts(ConflictList())
+        tree_a.set_conflicts(conflicts.ConflictList())
         tree_b.merge_from_branch(tree_a.branch)
         self.build_tree_contents([('b/file',
                                    'base-contents\nother-contents\n')])
-        tree_b.set_conflicts(ConflictList())
+        tree_b.set_conflicts(conflicts.ConflictList())
         tree_a.commit('', rev_id='rev3a')
         tree_b.commit('', rev_id='rev3b')
         out, err = self.run_bzr(['merge', '-d', 'a', 'b', '--lca'], retcode=1)
@@ -598,3 +588,33 @@
         other.commit('rev1b')
         self.run_bzr('merge -d this other -r0..')
         self.failUnlessExists('this/other_file')
+
+
+class TestMergeForce(tests.TestCaseWithTransport):
+
+    def setUp(self):
+        super(TestMergeForce, self).setUp()
+        self.tree_a = self.make_branch_and_tree('a')
+        self.build_tree(['a/foo'])
+        self.tree_a.add(['foo'])
+        self.tree_a.commit('add file')
+        self.tree_b = self.tree_a.bzrdir.sprout('b').open_workingtree()
+        self.build_tree_contents([('a/foo', 'change 1')])
+        self.tree_a.commit('change file')
+        self.tree_b.merge_from_branch(self.tree_a.branch)
+
+    def test_merge_force(self):
+        self.tree_a.commit('empty change to allow merge to run')
+        # Second merge on top if the uncommitted one
+        self.run_bzr(['merge', '../a', '--force'], working_dir='b')
+
+
+    def test_merge_with_uncommitted_changes(self):
+        self.run_bzr_error(['Working tree .* has uncommitted changes'],
+                           ['merge', '../a'], working_dir='b')
+
+    def test_merge_with_pending_merges(self):
+        # Revert the changes keeping the pending merge
+        self.run_bzr(['revert', 'b'])
+        self.run_bzr_error(['Working tree .* has uncommitted changes'],
+                           ['merge', '../a'], working_dir='b')
 
=== 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 @@
         self.run_bzr('remove-tree branch1 --force')
         self.failIfExists('branch1/foo')
         self.failUnlessExists('branch1/bar')
+
+    def test_remove_tree_pending_merges(self):
+        self.run_bzr(['branch', 'branch1', 'branch2'])
+        self.build_tree(['branch1/bar'])
+        self.tree.add('bar')
+        self.tree.commit('2')
+        self.failUnlessExists('branch1/bar')
+        self.run_bzr(['merge', '../branch1'], working_dir='branch2')
+        self.failUnlessExists('branch2/bar')
+        self.run_bzr(['revert', '.'], working_dir='branch2')
+        self.failIfExists('branch2/bar')
+        output = self.run_bzr_error(["Working tree .* has uncommitted changes"],
+                                    'remove-tree branch2', retcode=3)
+
+    def test_remove_tree_pending_merges_force(self):
+        self.run_bzr(['branch', 'branch1', 'branch2'])
+        self.build_tree(['branch1/bar'])
+        self.tree.add('bar')
+        self.tree.commit('2')
+        self.failUnlessExists('branch1/bar')
+        self.run_bzr(['merge', '../branch1'], working_dir='branch2')
+        self.failUnlessExists('branch2/bar')
+        self.run_bzr(['revert', '.'], working_dir='branch2')
+        self.failIfExists('branch2/bar')
+        self.run_bzr('remove-tree branch2 --force')
+        self.failIfExists('branch2/foo')
+        self.failIfExists('branch2/bar')