Merge lp:~jelmer/brz/plan-merge into lp:brz

[Jelmer Vernooij]

Move Tree.plan_file_merge to InventoryTree.plan_file_merge.

Move Tree.plan_file_merge to InventoryTree.plan_file_merge.

Also, mark merge types that require a merge plan as such so that 'bzr merge'
can error out when trying to use --weave or --lca merge with git trees.

[Martin Packman (gz)]

Looks good.

review: Approve

=== modified file 'breezy/builtins.py'
--- breezy/builtins.py	2018-02-21 13:19:44 +0000
+++ breezy/builtins.py	2018-02-24 18:57:47 +0000
@@ -4381,7 +4381,7 @@
     committed to record the result of the merge.
 
     merge refuses to run if there are any uncommitted changes, unless
-    --force is given.  If --force is given, then the changes from the source 
+    --force is given.  If --force is given, then the changes from the source
     will be merged with the current working tree, including any uncommitted
     changes in the tree.  The --force option can also be used to create a
     merge revision which has more than two parents.
@@ -4610,6 +4610,15 @@
             raise errors.BzrCommandError(gettext("Cannot do conflict reduction and"
                                          " show base."))
 
+        if (merger.merge_type.requires_file_merge_plan and
+            (not getattr(merger.this_tree, 'plan_file_merge', None) or
+             not getattr(merger.other_tree, 'plan_file_merge', None) or
+             (merger.base_tree is not None and
+                 not getattr(merger.base_tree, 'plan_file_merge', None)))):
+            raise errors.BzrCommandError(
+                 gettext('Plan file merge unsupported: '
+                         'Merge type incompatible with tree formats.'))
+
     def _get_merger_from_branch(self, tree, location, revision, remember,
                                 possible_transports, pb):
         """Produce a merger from a location, assuming it refers to a branch."""
 
=== modified file 'breezy/bzr/inventorytree.py'
--- breezy/bzr/inventorytree.py	2018-01-12 08:52:43 +0000
+++ breezy/bzr/inventorytree.py	2018-02-24 18:57:47 +0000
@@ -258,6 +258,63 @@
         for child in viewvalues(getattr(entry, 'children', {})):
             yield child.file_id
 
+    def _get_plan_merge_data(self, file_id, other, base):
+        from . import versionedfile
+        vf = versionedfile._PlanMergeVersionedFile(file_id)
+        last_revision_a = self._get_file_revision(
+                self.id2path(file_id), file_id, vf, 'this:')
+        last_revision_b = other._get_file_revision(
+                other.id2path(file_id), file_id, vf, 'other:')
+        if base is None:
+            last_revision_base = None
+        else:
+            last_revision_base = base._get_file_revision(
+                    base.id2path(file_id), file_id, vf, 'base:')
+        return vf, last_revision_a, last_revision_b, last_revision_base
+
+    def plan_file_merge(self, file_id, other, base=None):
+        """Generate a merge plan based on annotations.
+
+        If the file contains uncommitted changes in this tree, they will be
+        attributed to the 'current:' pseudo-revision.  If the file contains
+        uncommitted changes in the other tree, they will be assigned to the
+        'other:' pseudo-revision.
+        """
+        data = self._get_plan_merge_data(file_id, other, base)
+        vf, last_revision_a, last_revision_b, last_revision_base = data
+        return vf.plan_merge(last_revision_a, last_revision_b,
+                             last_revision_base)
+
+    def plan_file_lca_merge(self, file_id, other, base=None):
+        """Generate a merge plan based lca-newness.
+
+        If the file contains uncommitted changes in this tree, they will be
+        attributed to the 'current:' pseudo-revision.  If the file contains
+        uncommitted changes in the other tree, they will be assigned to the
+        'other:' pseudo-revision.
+        """
+        data = self._get_plan_merge_data(file_id, other, base)
+        vf, last_revision_a, last_revision_b, last_revision_base = data
+        return vf.plan_lca_merge(last_revision_a, last_revision_b,
+                                 last_revision_base)
+
+    def _get_file_revision(self, path, file_id, vf, tree_revision):
+        """Ensure that file_id, tree_revision is in vf to plan the merge."""
+        if getattr(self, '_repository', None) is None:
+            last_revision = tree_revision
+            parent_keys = [(file_id, t.get_file_revision(path, file_id)) for t in
+                self._iter_parent_trees()]
+            vf.add_lines((file_id, last_revision), parent_keys,
+                         self.get_file_lines(path, file_id))
+            repo = self.branch.repository
+            base_vf = repo.texts
+        else:
+            last_revision = self.get_file_revision(path, file_id)
+            base_vf = self._repository.texts
+        if base_vf not in vf.fallback_versionedfiles:
+            vf.fallback_versionedfiles.append(base_vf)
+        return last_revision
+
 
 class MutableInventoryTree(MutableTree, InventoryTree):
 
 
=== modified file 'breezy/bzr/versionedfile.py'
--- breezy/bzr/versionedfile.py	2017-12-04 23:25:45 +0000
+++ breezy/bzr/versionedfile.py	2018-02-24 18:57:47 +0000
@@ -669,7 +669,7 @@
         """
         raise NotImplementedError(self.iter_lines_added_or_present_in_versions)
 
-    def plan_merge(self, ver_a, ver_b):
+    def plan_merge(self, ver_a, ver_b, base=None):
         """Return pseudo-annotation indicating how the two versions merge.
 
         This is computed between versions a and b and their common
 
=== modified file 'breezy/merge.py'
--- breezy/merge.py	2017-11-13 22:51:34 +0000
+++ breezy/merge.py	2018-02-24 18:57:47 +0000
@@ -94,7 +94,7 @@
     """PerFileMerger objects are used by plugins extending merge for breezy.
 
     See ``breezy.plugins.news_merge.news_merge`` for an example concrete class.
-    
+
     :ivar merger: The Merge3Merger performing the merge.
     """
 
@@ -104,7 +104,7 @@
 
     def merge_contents(self, merge_params):
         """Attempt to merge the contents of a single file.
-        
+
         :param merge_params: A breezy.merge.MergeFileHookParams
         :return: A tuple of (status, chunks), where status is one of
             'not_applicable', 'success', 'conflicted', or 'delete'.  If status
@@ -174,13 +174,13 @@
     classes should implement ``merge_text``.
 
     See ``breezy.plugins.news_merge.news_merge`` for an example concrete class.
-    
+
     :ivar affected_files: The configured file paths to merge.
 
     :cvar name_prefix: The prefix to use when looking up configuration
         details. <name_prefix>_merge_files describes the files targeted by the
         hook for example.
-        
+
     :cvar default_files: The default file paths to merge when no configuration
         is present.
     """
@@ -718,6 +718,7 @@
     supports_reverse_cherrypick = True
     winner_idx = {"this": 2, "other": 1, "conflict": 1}
     supports_lca_trees = True
+    requires_file_merge_plan = False
 
     def __init__(self, working_tree, this_tree, base_tree, other_tree,
                  interesting_ids=None, reprocess=False, show_base=False,
@@ -1408,7 +1409,7 @@
     def merge_contents(self, merge_hook_params):
         """Fallback merge logic after user installed hooks."""
         # This function is used in merge hooks as the fallback instance.
-        # Perhaps making this function and the functions it calls be a 
+        # Perhaps making this function and the functions it calls be a
         # a separate class would be better.
         if merge_hook_params.winner == 'other':
             # OTHER is a straight winner, so replace this contents with other
@@ -1483,7 +1484,6 @@
                                               other_lines)
             file_group.append(trans_id)
 
-
     def _get_filter_tree_path(self, file_id):
         if self.this_tree.supports_content_filtering():
             # We get the path from the working tree if it exists.
@@ -1667,6 +1667,7 @@
     supports_show_base = False
     supports_reverse_cherrypick = False
     history_based = True
+    requires_file_merge_plan = True
 
     def _generate_merge_plan(self, file_id, base):
         return self.this_tree.plan_file_merge(file_id, self.other_tree,
@@ -1721,6 +1722,8 @@
 
 class LCAMerger(WeaveMerger):
 
+    requires_file_merge_plan = True
+
     def _generate_merge_plan(self, file_id, base):
         return self.this_tree.plan_file_lca_merge(file_id, self.other_tree,
                                                   base=base)
@@ -1728,15 +1731,14 @@
 class Diff3Merger(Merge3Merger):
     """Three-way merger using external diff3 for text merging"""
 
+    requires_file_merge_plan = False
+
     def dump_file(self, temp_dir, name, tree, file_id):
         out_path = osutils.pathjoin(temp_dir, name)
-        out_file = open(out_path, "wb")
-        try:
+        with open(out_path, "wb") as out_file:
             in_file = tree.get_file(tree.id2path(file_id), file_id)
             for line in in_file:
                 out_file.write(line)
-        finally:
-            out_file.close()
         return out_path
 
     def text_merge(self, file_id, trans_id):
@@ -1754,11 +1756,8 @@
             status = breezy.patch.diff3(new_file, this, base, other)
             if status not in (0, 1):
                 raise errors.BzrError("Unhandled diff3 exit code")
-            f = open(new_file, 'rb')
-            try:
+            with open(new_file, 'rb') as f:
                 self.tt.create_file(f, trans_id)
-            finally:
-                f.close()
             if status == 1:
                 name = self.tt.final_name(trans_id)
                 parent_id = self.tt.final_parent(trans_id)
@@ -1837,7 +1836,7 @@
 
 class _MergeTypeParameterizer(object):
     """Wrap a merge-type class to provide extra parameters.
-    
+
     This is hack used by MergeIntoMerger to pass some extra parameters to its
     merge_type.  Merger.do_merge() sets up its own set of parameters to pass to
     the 'merge_type' member.  It is difficult override do_merge without
 
=== modified file 'breezy/tests/per_tree/test_tree.py'
--- breezy/tests/per_tree/test_tree.py	2018-02-16 19:38:39 +0000
+++ breezy/tests/per_tree/test_tree.py	2018-02-24 18:57:47 +0000
@@ -56,6 +56,8 @@
         work_b = work_a.controldir.sprout('wtb').open_workingtree()
         self.build_tree_contents([('wta/file', 'b\nc\nd\ne\n')])
         tree_a = self.workingtree_to_test_tree(work_a)
+        if getattr(tree_a, 'plan_file_merge', None) is None:
+            raise tests.TestNotApplicable('Tree does not support plan_file_merge')
         tree_a.lock_read()
         self.addCleanup(tree_a.unlock)
         self.build_tree_contents([('wtb/file', 'a\nc\nd\nf\n')])
 
=== modified file 'breezy/tree.py'
--- breezy/tree.py	2018-02-17 02:57:14 +0000
+++ breezy/tree.py	2018-02-24 18:57:47 +0000
@@ -476,46 +476,6 @@
         """
         raise NotImplementedError(self.annotate_iter)
 
-    def _get_plan_merge_data(self, file_id, other, base):
-        from .bzr import versionedfile
-        vf = versionedfile._PlanMergeVersionedFile(file_id)
-        last_revision_a = self._get_file_revision(
-                self.id2path(file_id), file_id, vf, 'this:')
-        last_revision_b = other._get_file_revision(
-                other.id2path(file_id), file_id, vf, 'other:')
-        if base is None:
-            last_revision_base = None
-        else:
-            last_revision_base = base._get_file_revision(
-                    base.id2path(file_id), file_id, vf, 'base:')
-        return vf, last_revision_a, last_revision_b, last_revision_base
-
-    def plan_file_merge(self, file_id, other, base=None):
-        """Generate a merge plan based on annotations.
-
-        If the file contains uncommitted changes in this tree, they will be
-        attributed to the 'current:' pseudo-revision.  If the file contains
-        uncommitted changes in the other tree, they will be assigned to the
-        'other:' pseudo-revision.
-        """
-        data = self._get_plan_merge_data(file_id, other, base)
-        vf, last_revision_a, last_revision_b, last_revision_base = data
-        return vf.plan_merge(last_revision_a, last_revision_b,
-                             last_revision_base)
-
-    def plan_file_lca_merge(self, file_id, other, base=None):
-        """Generate a merge plan based lca-newness.
-
-        If the file contains uncommitted changes in this tree, they will be
-        attributed to the 'current:' pseudo-revision.  If the file contains
-        uncommitted changes in the other tree, they will be assigned to the
-        'other:' pseudo-revision.
-        """
-        data = self._get_plan_merge_data(file_id, other, base)
-        vf, last_revision_a, last_revision_b, last_revision_base = data
-        return vf.plan_lca_merge(last_revision_a, last_revision_b,
-                                 last_revision_base)
-
     def _iter_parent_trees(self):
         """Iterate through parent trees, defaulting to Tree.revision_tree."""
         for revision_id in self.get_parent_ids():
@@ -524,23 +484,6 @@
             except errors.NoSuchRevisionInTree:
                 yield self.repository.revision_tree(revision_id)
 
-    def _get_file_revision(self, path, file_id, vf, tree_revision):
-        """Ensure that file_id, tree_revision is in vf to plan the merge."""
-        if getattr(self, '_repository', None) is None:
-            last_revision = tree_revision
-            parent_keys = [(file_id, t.get_file_revision(path, file_id)) for t in
-                self._iter_parent_trees()]
-            vf.add_lines((file_id, last_revision), parent_keys,
-                         self.get_file_lines(path, file_id))
-            repo = self.branch.repository
-            base_vf = repo.texts
-        else:
-            last_revision = self.get_file_revision(path, file_id)
-            base_vf = self._repository.texts
-        if base_vf not in vf.fallback_versionedfiles:
-            vf.fallback_versionedfiles.append(base_vf)
-        return last_revision
-
     def _check_retrieved(self, ie, f):
         if not __debug__:
             return