Merge lp:~jelmer/brz/lca-merge-git into lp:brz

[Jelmer Vernooij]

Avoid using inventories in breezy.merge when encountering lcas.

Avoid using inventories in breezy.merge when encountering lcas.

This fixes merge for git when encountering lcas.

[Jelmer Vernooij (jelmer)]

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/319/

=== modified file 'breezy/merge.py'
--- breezy/merge.py	2019-06-02 18:45:38 +0000
+++ breezy/merge.py	2019-06-02 18:45:39 +0000
@@ -908,8 +908,6 @@
         from .multiwalker import MultiWalker
         walker = MultiWalker(self.other_tree, self._lca_trees)
 
-        base_inventory = self.base_tree.root_inventory
-        this_inventory = self.this_tree.root_inventory
         for other_path, file_id, other_ie, lca_values in walker.iter_all():
             # Is this modified at all from any of the other trees?
             if other_ie is None:
@@ -948,20 +946,20 @@
                     lca_paths.append(lca_path)
 
             try:
-                base_ie = base_inventory.get_entry(file_id)
+                base_path = self.base_tree.id2path(file_id)
             except errors.NoSuchId:
+                base_path = None
                 base_ie = _none_entry
-                base_path = None
             else:
-                base_path = self.base_tree.id2path(file_id)
+                base_ie = next(self.base_tree.iter_entries_by_dir(specific_files=[base_path]))[1]
 
             try:
-                this_ie = this_inventory.get_entry(file_id)
+                this_path = self.this_tree.id2path(file_id)
             except errors.NoSuchId:
                 this_ie = _none_entry
                 this_path = None
             else:
-                this_path = self.this_tree.id2path(file_id)
+                this_ie = next(self.this_tree.iter_entries_by_dir(specific_files=[this_path]))[1]
 
             lca_kinds = []
             lca_parent_ids = []
@@ -1865,13 +1863,13 @@
 
     def _entries_to_incorporate(self):
         """Yields pairs of (inventory_entry, new_parent)."""
-        other_inv = self.other_tree.root_inventory
-        subdir_id = other_inv.path2id(self._source_subpath)
+        subdir_id = self.other_tree.path2id(self._source_subpath)
         if subdir_id is None:
             # XXX: The error would be clearer if it gave the URL of the source
             # branch, but we don't have a reference to that here.
             raise PathNotInTree(self._source_subpath, "Source tree")
-        subdir = other_inv.get_entry(subdir_id)
+        subdir = next(self.other_tree.iter_entries_by_dir(
+            specific_files=[self._source_subpath]))[1]
         parent_in_target = osutils.dirname(self._target_subdir)
         target_id = self.this_tree.path2id(parent_in_target)
         if target_id is None:
@@ -1893,7 +1891,7 @@
         if subdir.kind != 'directory':
             # No children, so we are done.
             return
-        for path, entry in other_inv.iter_entries_by_dir(subdir_id):
+        for path, entry in self.other_tree.root_inventory.iter_entries_by_dir(subdir_id):
             parent_id = entry.parent_id
             if parent_id == subdir.file_id:
                 # The root's parent ID has changed, so make sure children of

=== modified file 'breezy/multiwalker.py'
--- breezy/multiwalker.py	2019-06-02 18:45:38 +0000
+++ breezy/multiwalker.py	2019-06-02 18:45:39 +0000
@@ -108,6 +108,11 @@
         dirname, basename = osutils.split(path)
         return (dirname.split(u'/'), basename)
 
+    def _lookup_by_master_path(self, extra_entries, other_tree, master_path):
+        return self._lookup_by_file_id(
+            extra_entries, other_tree,
+            self._master_tree.path2id(master_path))
+
     def _lookup_by_file_id(self, extra_entries, other_tree, file_id):
         """Lookup an inventory entry by file_id.
 
@@ -127,8 +132,6 @@
         """
         if file_id in extra_entries:
             return extra_entries.pop(file_id)
-        # TODO: Is id2path better as the first call, or is
-        #       inventory[file_id] better as a first check?
         try:
             cur_path = other_tree.id2path(file_id)
         except errors.NoSuchId:
@@ -137,7 +140,8 @@
             return (None, None)
         else:
             self._out_of_order_processed.add(file_id)
-            cur_ie = other_tree.root_inventory.get_entry(file_id)
+            cur_ie = next(other_tree.iter_entries_by_dir(
+                specific_files=[cur_path]))[1]
             return (cur_path, cur_ie)
 
     def iter_all(self):
@@ -176,17 +180,16 @@
             if not master_has_more:
                 break
 
-            file_id = master_ie.file_id
             other_values = []
             other_values_append = other_values.append
             next_other_entries = []
             next_other_entries_append = next_other_entries.append
             for idx, (other_has_more, other_path, other_ie) in enumerate(other_entries):
                 if not other_has_more:
-                    other_values_append(lookup_by_file_id(
-                        others_extra[idx], self._other_trees[idx], file_id))
+                    other_values_append(self._lookup_by_master_path(
+                        others_extra[idx], self._other_trees[idx], path))
                     next_other_entries_append((False, None, None))
-                elif file_id == other_ie.file_id:
+                elif master_ie.file_id == other_ie.file_id:
                     # This is the critical code path, as most of the entries
                     # should match between most trees.
                     other_values_append((other_path, other_ie))
@@ -203,7 +206,7 @@
                             other_extra[other_file_id] = (other_path, other_ie)
                         other_has_more, other_path, other_ie = \
                             step_one(other_walker)
-                    if other_has_more and other_ie.file_id == file_id:
+                    if other_has_more and other_ie.file_id == master_ie.file_id:
                         # We ended up walking to this point, match and step
                         # again
                         other_values_append((other_path, other_ie))
@@ -212,14 +215,14 @@
                     else:
                         # This record isn't in the normal order, see if it
                         # exists at all.
-                        other_values_append(lookup_by_file_id(
-                            other_extra, self._other_trees[idx], file_id))
+                        other_values_append(self._lookup_by_master_path(
+                            other_extra, self._other_trees[idx], path))
                     next_other_entries_append((other_has_more, other_path,
                                                other_ie))
             other_entries = next_other_entries
 
             # We've matched all the walkers, yield this datapoint
-            yield path, file_id, master_ie, other_values
+            yield path, master_ie.file_id, master_ie, other_values
         self._other_walkers = other_walkers
         self._other_entries = other_entries
         self._others_extra = others_extra
@@ -261,6 +264,3 @@
                     other_values.append(self._lookup_by_file_id(
                         alt_extra, alt_tree, file_id))
                 yield other_path, file_id, None, other_values
-
-
-

=== modified file 'breezy/tests/test_multiwalker.py'
--- breezy/tests/test_multiwalker.py	2019-06-02 18:45:38 +0000
+++ breezy/tests/test_multiwalker.py	2019-06-02 18:45:39 +0000
@@ -229,8 +229,8 @@
         basis_tree, root_id = self.lock_and_get_basis_and_root_id(tree)
         first_tree = tree.branch.repository.revision_tree(b'first-rev-id')
         second_tree = tree.branch.repository.revision_tree(b'second-rev-id')
-        walker = multiwalker.MultiWalker(tree, [basis_tree, first_tree,
-                                              second_tree])
+        walker = multiwalker.MultiWalker(
+            tree, [basis_tree, first_tree, second_tree])
         iterator = walker.iter_all()
         self.assertWalkerNext(u'', root_id, True, [u'', u'', u''], iterator)
         self.assertWalkerNext(u'a', b'a-id', True,