Rev 4745: (vila) Don't crash when merging from an already merged other root in file:///home/pqm/archives/thelove/bzr/2.0/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Fri Apr 2 14:55:08 BST 2010


At file:///home/pqm/archives/thelove/bzr/2.0/

------------------------------------------------------------
revno: 4745 [merge]
revision-id: pqm at pqm.ubuntu.com-20100402135506-0tbjujqpqa0bxz7c
parent: pqm at pqm.ubuntu.com-20100325030945-uypmkeucy0sm31un
parent: v.ladeuil+lp at free.fr-20100402124412-ccxov3f1zlu8l7p7
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.0
timestamp: Fri 2010-04-02 14:55:06 +0100
message:
  (vila) Don't crash when merging from an already merged other root
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/merge.py                merge.py-20050513021216-953b65a438527106
  bzrlib/tests/per_workingtree/test_merge_from_branch.py test_merge_from_bran-20060904034200-12jxyk2zlhpufxe1-1
=== modified file 'NEWS'
--- a/NEWS	2010-03-25 02:01:47 +0000
+++ b/NEWS	2010-04-01 14:10:47 +0000
@@ -22,6 +22,9 @@
   permissions as ``.bzr`` directory on a POSIX OS.
   (Parth Malwankar, #262450)
 
+* Additional merges after an unrelated branch has been merged with its
+  history no longer crash when deleted files are involved.
+  (Vincent Ladeuil, John Arbash Meinel, #375898)
 
 bzr 2.0.5
 #########

=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py	2009-10-27 09:45:35 +0000
+++ b/bzrlib/merge.py	2010-04-02 12:44:12 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006, 2008 Canonical Ltd
+# Copyright (C) 2005-2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -901,21 +901,38 @@
         other_root = self.tt.trans_id_file_id(other_root_file_id)
         if other_root == self.tt.root:
             return
+        if self.other_tree.inventory.root.file_id in self.this_tree.inventory:
+            # the other tree's root is a non-root in the current tree (as when
+            # a previously unrelated branch is merged into another)
+            return
         try:
             self.tt.final_kind(other_root)
+            other_root_is_present = True
         except NoSuchFile:
-            return
-        if self.other_tree.inventory.root.file_id in self.this_tree.inventory:
-            # the other tree's root is a non-root in the current tree
-            return
-        self.reparent_children(self.other_tree.inventory.root, self.tt.root)
-        self.tt.cancel_creation(other_root)
-        self.tt.cancel_versioning(other_root)
-
-    def reparent_children(self, ie, target):
-        for thing, child in ie.children.iteritems():
+            # other_root doesn't have a physical representation. We still need
+            # to move any references to the actual root of the tree.
+            other_root_is_present = False
+        # 'other_tree.inventory.root' is not present in this tree. We are
+        # calling adjust_path for children which *want* to be present with a
+        # correct place to go.
+        for thing, child in self.other_tree.inventory.root.children.iteritems():
             trans_id = self.tt.trans_id_file_id(child.file_id)
-            self.tt.adjust_path(self.tt.final_name(trans_id), target, trans_id)
+            if not other_root_is_present:
+                # FIXME: Make final_kind returns None instead of raising
+                # NoSuchFile to avoid the ugly construct below -- vila 20100402
+                try:
+                    self.tt.final_kind(trans_id)
+                    # The item exist in the final tree and has a defined place
+                    # to go already.
+                    continue
+                except errors.NoSuchFile, e:
+                    pass
+            # Move the item into the root
+            self.tt.adjust_path(self.tt.final_name(trans_id),
+                                self.tt.root, trans_id)
+        if other_root_is_present:
+            self.tt.cancel_creation(other_root)
+            self.tt.cancel_versioning(other_root)
 
     def write_modified(self, results):
         modified_hashes = {}

=== modified file 'bzrlib/tests/per_workingtree/test_merge_from_branch.py'
--- a/bzrlib/tests/per_workingtree/test_merge_from_branch.py	2009-07-10 07:14:02 +0000
+++ b/bzrlib/tests/per_workingtree/test_merge_from_branch.py	2010-04-01 14:06:48 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 Canonical Ltd
+# Copyright (C) 2006-2010 Canonical Ltd
 # Authors:  Robert Collins <robert.collins at canonical.com>
 #
 # This program is free software; you can redistribute it and/or modify
@@ -20,13 +20,14 @@
 import os
 
 from bzrlib import (
+    branchbuilder,
     errors,
     merge
     )
-from bzrlib.tests.per_workingtree import TestCaseWithWorkingTree
-
-
-class TestMergeFromBranch(TestCaseWithWorkingTree):
+from bzrlib.tests import per_workingtree
+
+
+class TestMergeFromBranch(per_workingtree.TestCaseWithWorkingTree):
 
     def create_two_trees_for_merging(self):
         """Create two trees that can be merged from.
@@ -114,3 +115,103 @@
                 self.tt.create_file('qux', trans_id)
         this.merge_from_branch(other.branch, merge_type=QuxMerge)
         self.assertEqual('qux', this.get_file_text('foo-id'))
+
+
+class TestMergedBranch(per_workingtree.TestCaseWithWorkingTree):
+
+    def make_branch_builder(self, relpath, format=None):
+        if format is None:
+            format = self.bzrdir_format
+        builder = branchbuilder.BranchBuilder(self.get_transport(),
+                                              format=format)
+        return builder
+
+    def make_inner_branch(self):
+        bld_inner = self.make_branch_builder('inner')
+        bld_inner.start_series()
+        bld_inner.build_snapshot(
+            '1', None,
+            [('add', ('', 'inner-root-id', 'directory', '')),
+             ('add', ('dir', 'dir-id', 'directory', '')),
+             ('add', ('dir/file1', 'file1-id', 'file', 'file1 content\n')),
+             ('add', ('file3', 'file3-id', 'file', 'file3 content\n')),
+             ])
+        bld_inner.build_snapshot(
+            '3', ['1'], [('modify', ('file3-id', 'new file3 contents\n')),])
+        bld_inner.build_snapshot(
+            '2', ['1'],
+            [('add', ('dir/file2', 'file2-id', 'file', 'file2 content\n')),
+             ])
+        bld_inner.finish_series()
+        br = bld_inner.get_branch()
+        return br
+
+    def assertTreeLayout(self, expected, tree):
+        tree.lock_read()
+        try:
+            actual = [e[0] for e in tree.list_files()]
+            # list_files doesn't guarantee order
+            actual = sorted(actual)
+            self.assertEqual(expected, actual)
+        finally:
+            tree.unlock()
+
+    def make_outer_tree(self):
+        outer = self.make_branch_and_tree('outer')
+        self.build_tree_contents([('outer/foo', 'foo')])
+        outer.add('foo', 'foo-id')
+        outer.commit('added foo')
+        inner = self.make_inner_branch()
+        outer.merge_from_branch(inner, to_revision='1', from_revision='null:')
+        outer.commit('merge inner branch')
+        outer.mkdir('dir-outer', 'dir-outer-id')
+        outer.move(['dir', 'file3'], to_dir='dir-outer')
+        outer.commit('rename imported dir and file3 to dir-outer')
+        return outer, inner
+
+    def test_file1_deleted_in_dir(self):
+        outer, inner = self.make_outer_tree()
+        outer.remove(['dir-outer/dir/file1'], keep_files=False)
+        outer.commit('delete file1')
+        outer.merge_from_branch(inner)
+        outer.commit('merge the rest')
+        self.assertTreeLayout(['dir-outer',
+                               'dir-outer/dir',
+                               'dir-outer/dir/file2',
+                               'dir-outer/file3',
+                               'foo'],
+                              outer)
+
+    def test_file3_deleted_in_root(self):
+        # Reproduce bug #375898
+        outer, inner = self.make_outer_tree()
+        outer.remove(['dir-outer/file3'], keep_files=False)
+        outer.commit('delete file3')
+        outer.merge_from_branch(inner)
+        outer.commit('merge the rest')
+        self.assertTreeLayout(['dir-outer',
+                               'dir-outer/dir',
+                               'dir-outer/dir/file1',
+                               'dir-outer/dir/file2',
+                               'foo'],
+                              outer)
+
+
+    def test_file3_in_root_conflicted(self):
+        outer, inner = self.make_outer_tree()
+        outer.remove(['dir-outer/file3'], keep_files=False)
+        outer.commit('delete file3')
+        nb_conflicts = outer.merge_from_branch(inner, to_revision='3')
+        self.assertEqual(4, nb_conflicts)
+        self.assertTreeLayout(['dir-outer',
+                               'dir-outer/dir',
+                               'dir-outer/dir/file1',
+                               # Ideally th conflict helpers should be in
+                               # dir-outer/dir but since we can't easily find
+                               # back the file3 -> outer-dir/dir rename, root
+                               # is good enough -- vila 20100401
+                               'file3.BASE',
+                               'file3.OTHER',
+                               'foo'],
+                              outer)
+




More information about the bazaar-commits mailing list