Merge lp:~jelmer/brz/changes-from-entries into lp:brz

[Jelmer Vernooij]

Avoid id2path calls in Tree.

Avoid id2path calls in Tree.

[Martin Packman (gz)]

Thanks.

review: Approve

=== modified file 'breezy/tests/per_intertree/test_file_content_matches.py'
--- breezy/tests/per_intertree/test_file_content_matches.py	2017-05-21 18:10:28 +0000
+++ breezy/tests/per_intertree/test_file_content_matches.py	2018-03-18 01:17:08 +0000
@@ -32,7 +32,9 @@
         tree2.add('file', 'file-id-2')
         tree1, tree2 = self.mutable_trees_to_test_trees(self, tree1, tree2)
         inter = self.intertree_class(tree1, tree2)
-        self.assertTrue(inter.file_content_matches('file-id-1', 'file-id-2'))
+        self.assertTrue(inter.file_content_matches(
+            'file', 'file', 'file-id-1', 'file-id-2'))
+        self.assertTrue(inter.file_content_matches('file', 'file'))
 
     def test_different_contents_and_same_verifier(self):
         tree1 = self.make_branch_and_tree('1')
@@ -45,4 +47,7 @@
         tree2.add('file', 'file-id-2')
         tree1, tree2 = self.mutable_trees_to_test_trees(self, tree1, tree2)
         inter = self.intertree_class(tree1, tree2)
-        self.assertFalse(inter.file_content_matches('file-id-1', 'file-id-2'))
+        self.assertFalse(inter.file_content_matches(
+            'file', 'file', 'file-id-1', 'file-id-2'))
+        self.assertFalse(inter.file_content_matches(
+            'file', 'file'))

=== modified file 'breezy/tree.py'
--- breezy/tree.py	2018-03-02 00:38:34 +0000
+++ breezy/tree.py	2018-03-18 01:17:08 +0000
@@ -783,16 +783,14 @@
         # it works for all trees.
         return True
 
-    def _changes_from_entries(self, source_entry, target_entry,
-        source_path=None, target_path=None):
+    def _changes_from_entries(self, source_entry, target_entry, source_path,
+                              target_path):
         """Generate a iter_changes tuple between source_entry and target_entry.
 
         :param source_entry: An inventory entry from self.source, or None.
         :param target_entry: An inventory entry from self.target, or None.
-        :param source_path: The path of source_entry, if known. If not known
-            it will be looked up.
-        :param target_path: The path of target_entry, if known. If not known
-            it will be looked up.
+        :param source_path: The path of source_entry.
+        :param target_path: The path of target_entry.
         :return: A tuple, item 0 of which is an iter_changes result tuple, and
             item 1 is True if there are any changes in the result tuple.
         """
@@ -806,8 +804,6 @@
             source_versioned = True
             source_name = source_entry.name
             source_parent = source_entry.parent_id
-            if source_path is None:
-                source_path = self.source.id2path(file_id)
             source_kind, source_executable, source_stat = \
                 self.source._comparison_data(source_entry, source_path)
         else:
@@ -820,8 +816,6 @@
             target_versioned = True
             target_name = target_entry.name
             target_parent = target_entry.parent_id
-            if target_path is None:
-                target_path = self.target.id2path(file_id)
             target_kind, target_executable, target_stat = \
                 self.target._comparison_data(target_entry, target_path)
         else:
@@ -836,8 +830,9 @@
         if source_kind != target_kind:
             changed_content = True
         elif source_kind == 'file':
-            if not self.file_content_matches(file_id, file_id, source_path,
-                    target_path, source_stat, target_stat):
+            if not self.file_content_matches(
+                    source_path, target_path,
+                    file_id, file_id, source_stat, target_stat):
                 changed_content = True
         elif source_kind == 'symlink':
             if (self.source.get_symlink_target(source_path, file_id) !=
@@ -1132,8 +1127,16 @@
                 if result is None:
                     old_entry = self._get_entry(self.source, file_id)
                     new_entry = self._get_entry(self.target, file_id)
+                    try:
+                        source_path = self.source.id2path(file_id)
+                    except errors.NoSuchId:
+                        source_path = None
+                    try:
+                        target_path = self.target.id2path(file_id)
+                    except errors.NoSuchId:
+                        target_path = None
                     result, changes = self._changes_from_entries(
-                        old_entry, new_entry)
+                        old_entry, new_entry, source_path, target_path)
                 else:
                     changes = True
                 # Get this parents parent to examine.
@@ -1153,26 +1156,23 @@
                     yield result
 
     def file_content_matches(
-            self, source_file_id, target_file_id, source_path=None,
-            target_path=None, source_stat=None, target_stat=None):
+            self, source_path, target_path,
+            source_file_id=None, target_file_id=None,
+            source_stat=None, target_stat=None):
         """Check if two files are the same in the source and target trees.
 
         This only checks that the contents of the files are the same,
         it does not touch anything else.
 
-        :param source_file_id: File id of the file in the source tree
-        :param target_file_id: File id of the file in the target tree
         :param source_path: Path of the file in the source tree
         :param target_path: Path of the file in the target tree
+        :param source_file_id: Optional file id of the file in the source tree
+        :param target_file_id: Optional file id of the file in the target tree
         :param source_stat: Optional stat value of the file in the source tree
         :param target_stat: Optional stat value of the file in the target tree
         :return: Boolean indicating whether the files have the same contents
         """
         with self.lock_read():
-            if source_path is None:
-                source_path = self.source.id2path(source_file_id)
-            if target_path is None:
-                target_path = self.target.id2path(target_file_id)
             source_verifier_kind, source_verifier_data = (
                     self.source.get_file_verifier(
                         source_path, source_file_id, source_stat))