Merge lp:~jelmer/brz/extract-paths2ids into lp:brz

[Jelmer Vernooij]

Add a new Tree.find_related_paths_across_trees() method and pass files rather than file ids in various places.

Add a new Tree.find_related_paths_across_trees() method.

This finds a set of paths in a specific tree that are related to a set of specified paths in trees to look up in.

This allows further eliminating the use of file ids in public APIs:

* Tree.paths2ids can now be InventoryTree-specific.
* Merge now only takes a list of interesting files rather than a list of interesting file ids or interesting files.
* Tree.iter_entries_by_dir() now takes a list of specific_files

[Martin Packman (gz)]

Thanks, all looks sensible.

review: Approve

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

Merging failed
https://ci.breezy-vcs.org/job/brz-dev/26/

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

Merging failed
https://ci.breezy-vcs.org/job/brz-dev/28/

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

Running landing tests failed
https://ci.breezy-vcs.org/job/brz-dev/31/

=== modified file 'breezy/builtins.py'
--- breezy/builtins.py	2018-03-24 00:39:56 +0000
+++ breezy/builtins.py	2018-03-24 01:23:20 +0000
@@ -53,6 +53,7 @@
     symbol_versioning,
     timestamp,
     transport,
+    tree as _mod_tree,
     ui,
     urlutils,
     views,
@@ -960,11 +961,11 @@
 
         self.add_cleanup(tree.lock_read().unlock)
         if file_list is not None:
-            file_ids = tree.paths2ids(file_list, trees=extra_trees,
-                                      require_versioned=True)
+            paths = tree.find_related_paths_across_trees(
+                    file_list, extra_trees, require_versioned=True)
             # find_ids_across_trees may include some paths that don't
             # exist in 'tree'.
-            entries = tree.iter_entries_by_dir(specific_file_ids=file_ids)
+            entries = tree.iter_entries_by_dir(specific_files=paths)
         else:
             entries = tree.iter_entries_by_dir()
 
@@ -4775,29 +4776,27 @@
                                          " merges.  Not cherrypicking or"
                                          " multi-merges."))
         repository = tree.branch.repository
-        interesting_ids = None
+        interesting_files = None
         new_conflicts = []
         conflicts = tree.conflicts()
         if file_list is not None:
-            interesting_ids = set()
+            interesting_files = set()
             for filename in file_list:
-                file_id = tree.path2id(filename)
-                if file_id is None:
+                if not tree.is_versioned(filename):
                     raise errors.NotVersionedError(filename)
-                interesting_ids.add(file_id)
-                if tree.kind(filename, file_id) != "directory":
+                interesting_files.add(filename)
+                if tree.kind(filename) != "directory":
                     continue
 
-                # FIXME: Support nested trees
-                for name, ie in tree.root_inventory.iter_entries(file_id):
-                    interesting_ids.add(ie.file_id)
+                for path, ie in tree.iter_entries_by_dir(specific_files=[filename]):
+                    interesting_files.add(path)
             new_conflicts = conflicts.select_conflicts(tree, file_list)[0]
         else:
             # Remerge only supports resolving contents conflicts
             allowed_conflicts = ('text conflict', 'contents conflict')
             restore_files = [c.path for c in conflicts
                              if c.typestring in allowed_conflicts]
-        _mod_merge.transform_tree(tree, tree.basis_tree(), interesting_ids)
+        _mod_merge.transform_tree(tree, tree.basis_tree(), interesting_files)
         tree.set_conflicts(ConflictList(new_conflicts))
         if file_list is not None:
             restore_files = file_list
@@ -4814,7 +4813,7 @@
         tree.set_parent_ids(parents[:1])
         try:
             merger = _mod_merge.Merger.from_revision_ids(tree, parents[1])
-            merger.interesting_ids = interesting_ids
+            merger.interesting_files = interesting_files
             merger.merge_type = merge_type
             merger.show_base = show_base
             merger.reprocess = reprocess

=== modified file 'breezy/bzr/inventory.py'
--- breezy/bzr/inventory.py	2018-03-10 15:17:30 +0000
+++ breezy/bzr/inventory.py	2018-03-24 01:23:20 +0000
@@ -720,12 +720,12 @@
 
     def _preload_cache(self):
         """Populate any caches, we are about to access all items.
-        
+
         The default implementation does nothing, because CommonInventory doesn't
         have a cache.
         """
         pass
-    
+
     def iter_entries_by_dir(self, from_dir=None, specific_file_ids=None,
         yield_parents=False):
         """Iterate over the entries in a directory first order.

=== modified file 'breezy/bzr/inventorytree.py'
--- breezy/bzr/inventorytree.py	2018-03-24 00:16:27 +0000
+++ breezy/bzr/inventorytree.py	2018-03-24 01:23:20 +0000
@@ -170,6 +170,53 @@
             file_id = file_id[0]
         return self.root_inventory, file_id
 
+    def find_related_paths_across_trees(self, paths, trees=[],
+            require_versioned=True):
+        """Find related paths in tree corresponding to specified filenames in any
+        of `lookup_trees`.
+
+        All matches in all trees will be used, and all children of matched
+        directories will be used.
+
+        :param paths: The filenames to find related paths for (if None, returns
+            None)
+        :param trees: The trees to find file_ids within
+        :param require_versioned: if true, all specified filenames must occur in
+            at least one tree.
+        :return: a set of paths for the specified filenames and their children
+            in `tree`
+        """
+        if paths is None:
+            return None;
+        file_ids = self.paths2ids(
+                paths, trees, require_versioned=require_versioned)
+        ret = set()
+        for file_id in file_ids:
+            try:
+                ret.add(self.id2path(file_id))
+            except errors.NoSuchId:
+                pass
+        return ret
+
+    def paths2ids(self, paths, trees=[], require_versioned=True):
+        """Return all the ids that can be reached by walking from paths.
+
+        Each path is looked up in this tree and any extras provided in
+        trees, and this is repeated recursively: the children in an extra tree
+        of a directory that has been renamed under a provided path in this tree
+        are all returned, even if none exist under a provided path in this
+        tree, and vice versa.
+
+        :param paths: An iterable of paths to start converting to ids from.
+            Alternatively, if paths is None, no ids should be calculated and None
+            will be returned. This is offered to make calling the api unconditional
+            for code that *might* take a list of files.
+        :param trees: Additional trees to consider.
+        :param require_versioned: If False, do not raise NotVersionedError if
+            an element of paths is not versioned in this tree and all of trees.
+        """
+        return find_ids_across_trees(paths, [self] + list(trees), require_versioned)
+
     def path2id(self, path):
         """Return the id for path in this tree."""
         with self.lock_read():
@@ -222,7 +269,7 @@
         # are not versioned.
         return set((p for p in paths if self.path2id(p) is None))
 
-    def iter_entries_by_dir(self, specific_file_ids=None, yield_parents=False):
+    def iter_entries_by_dir(self, specific_files=None, yield_parents=False):
         """Walk the tree in 'by_dir' order.
 
         This will yield each entry in the tree as a (path, entry) tuple.
@@ -231,20 +278,20 @@
         See Tree.iter_entries_by_dir for details.
 
         :param yield_parents: If True, yield the parents from the root leading
-            down to specific_file_ids that have been requested. This has no
-            impact if specific_file_ids is None.
+            down to specific_files that have been requested. This has no
+            impact if specific_files is None.
         """
         with self.lock_read():
-            if specific_file_ids is None:
-                inventory_file_ids = None
-            else:
+            if specific_files is not None:
                 inventory_file_ids = []
-                for tree_file_id in specific_file_ids:
-                    inventory, inv_file_id = self._unpack_file_id(tree_file_id)
+                for path in specific_files:
+                    inventory, inv_file_id = self._path2inv_file_id(path)
                     if not inventory is self.root_inventory: # for now
                         raise AssertionError("%r != %r" % (
                             inventory, self.root_inventory))
                     inventory_file_ids.append(inv_file_id)
+            else:
+                inventory_file_ids = None
             # FIXME: Handle nested trees
             return self.root_inventory.iter_entries_by_dir(
                 specific_file_ids=inventory_file_ids, yield_parents=yield_parents)
@@ -319,6 +366,84 @@
         return last_revision
 
 
+def find_ids_across_trees(filenames, trees, require_versioned=True):
+    """Find the ids corresponding to specified filenames.
+
+    All matches in all trees will be used, and all children of matched
+    directories will be used.
+
+    :param filenames: The filenames to find file_ids for (if None, returns
+        None)
+    :param trees: The trees to find file_ids within
+    :param require_versioned: if true, all specified filenames must occur in
+        at least one tree.
+    :return: a set of file ids for the specified filenames and their children.
+    """
+    if not filenames:
+        return None
+    specified_path_ids = _find_ids_across_trees(filenames, trees,
+        require_versioned)
+    return _find_children_across_trees(specified_path_ids, trees)
+
+
+def _find_ids_across_trees(filenames, trees, require_versioned):
+    """Find the ids corresponding to specified filenames.
+
+    All matches in all trees will be used, but subdirectories are not scanned.
+
+    :param filenames: The filenames to find file_ids for
+    :param trees: The trees to find file_ids within
+    :param require_versioned: if true, all specified filenames must occur in
+        at least one tree.
+    :return: a set of file ids for the specified filenames
+    """
+    not_versioned = []
+    interesting_ids = set()
+    for tree_path in filenames:
+        not_found = True
+        for tree in trees:
+            file_id = tree.path2id(tree_path)
+            if file_id is not None:
+                interesting_ids.add(file_id)
+                not_found = False
+        if not_found:
+            not_versioned.append(tree_path)
+    if len(not_versioned) > 0 and require_versioned:
+        raise errors.PathsNotVersionedError(not_versioned)
+    return interesting_ids
+
+
+def _find_children_across_trees(specified_ids, trees):
+    """Return a set including specified ids and their children.
+
+    All matches in all trees will be used.
+
+    :param trees: The trees to find file_ids within
+    :return: a set containing all specified ids and their children
+    """
+    interesting_ids = set(specified_ids)
+    pending = interesting_ids
+    # now handle children of interesting ids
+    # we loop so that we handle all children of each id in both trees
+    while len(pending) > 0:
+        new_pending = set()
+        for file_id in pending:
+            for tree in trees:
+                try:
+                    path = tree.id2path(file_id)
+                except errors.NoSuchId:
+                    continue
+                try:
+                    for child in tree.iter_child_entries(path, file_id):
+                        if child.file_id not in interesting_ids:
+                            new_pending.add(child.file_id)
+                except errors.NotADirectory:
+                    pass
+        interesting_ids.update(new_pending)
+        pending = new_pending
+    return interesting_ids
+
+
 class MutableInventoryTree(MutableTree, InventoryTree):
 
     def apply_inventory_delta(self, changes):
@@ -442,10 +567,11 @@
             return entry[3]
         # Find a 'best fit' match if the filesystem is case-insensitive
         inv_path = self.tree._fix_case_of_inventory_path(inv_path)
-        file_id = self.tree.path2id(inv_path)
-        if file_id is not None:
-            return self.tree.iter_entries_by_dir([file_id]).next()[1]
-        return None
+        try:
+            return self.tree.iter_entries_by_dir(
+                    specific_files=[inv_path]).next()[1]
+        except StopIteration:
+            return None
 
     def _convert_to_directory(self, this_ie, inv_path):
         """Convert an entry to a directory.

=== modified file 'breezy/bzr/workingtree.py'
--- breezy/bzr/workingtree.py	2018-03-04 19:11:21 +0000
+++ breezy/bzr/workingtree.py	2018-03-24 01:23:20 +0000
@@ -652,10 +652,7 @@
         file_ids are in a WorkingTree if they are in the working inventory
         and the working file exists.
         """
-        ret = set()
-        for path, ie in self.iter_entries_by_dir():
-            ret.add(ie.file_id)
-        return ret
+        return {ie.file_id for path, ie in self.iter_entries_by_dir()}
 
     def all_versioned_paths(self):
         return {path for path, ie in self.iter_entries_by_dir()}
@@ -1699,13 +1696,13 @@
                 blocked_parent_ids.add(ie.file_id)
             yield path, ie
 
-    def iter_entries_by_dir(self, specific_file_ids=None, yield_parents=False):
+    def iter_entries_by_dir(self, specific_files=None,
+                            yield_parents=False):
         """See Tree.iter_entries_by_dir()"""
         # The only trick here is that if we supports_tree_reference then we
         # need to detect if a directory becomes a tree-reference.
         iterator = super(WorkingTree, self).iter_entries_by_dir(
-                specific_file_ids=specific_file_ids,
-                yield_parents=yield_parents)
+                specific_files=specific_files, yield_parents=yield_parents)
         if not self.supports_tree_reference():
             return iterator
         else:

=== modified file 'breezy/filter_tree.py'
--- breezy/filter_tree.py	2018-03-23 23:32:28 +0000
+++ breezy/filter_tree.py	2018-03-24 01:23:20 +0000
@@ -58,14 +58,14 @@
     def is_executable(self, path, file_id=None):
         return self.backing_tree.is_executable(path, file_id)
 
-    def iter_entries_by_dir(self, specific_file_ids=None, yield_parents=None):
+    def iter_entries_by_dir(self, specific_files=None, yield_parents=None):
         # NB: This simply returns the parent tree's entries; the length may be
         # wrong but it can't easily be calculated without filtering the whole
         # text.  Currently all callers cope with this; perhaps they should be
         # updated to a narrower interface that only provides things guaranteed
         # cheaply available across all trees. -- mbp 20110705
         return self.backing_tree.iter_entries_by_dir(
-            specific_file_ids=specific_file_ids,
+            specific_files=specific_files,
             yield_parents=yield_parents)
 
     def lock_read(self):

=== modified file 'breezy/merge.py'
--- breezy/merge.py	2018-03-24 00:39:56 +0000
+++ breezy/merge.py	2018-03-24 01:23:20 +0000
@@ -56,12 +56,13 @@
 # TODO: Report back as changes are merged in
 
 
-def transform_tree(from_tree, to_tree, interesting_ids=None):
+def transform_tree(from_tree, to_tree, interesting_files=None):
     from_tree.lock_tree_write()
     operation = cleanup.OperationWithCleanups(merge_inner)
     operation.add_cleanup(from_tree.unlock)
     operation.run_simple(from_tree.branch, to_tree, from_tree,
-        ignore_zero=True, interesting_ids=interesting_ids, this_tree=from_tree)
+        ignore_zero=True, this_tree=from_tree,
+        interesting_files=interesting_files)
 
 
 class MergeHooks(hooks.Hooks):
@@ -295,7 +296,6 @@
         self.base_tree = base_tree
         self.ignore_zero = False
         self.backup_files = False
-        self.interesting_ids = None
         self.interesting_files = None
         self.show_base = False
         self.reprocess = False
@@ -610,7 +610,6 @@
     def make_merger(self):
         kwargs = {'working_tree': self.this_tree, 'this_tree': self.this_tree,
                   'other_tree': self.other_tree,
-                  'interesting_ids': self.interesting_ids,
                   'interesting_files': self.interesting_files,
                   'this_branch': self.this_branch,
                   'other_branch': self.other_branch,
@@ -722,7 +721,7 @@
     requires_file_merge_plan = False
 
     def __init__(self, working_tree, this_tree, base_tree, other_tree,
-                 interesting_ids=None, reprocess=False, show_base=False,
+                 reprocess=False, show_base=False,
                  change_reporter=None, interesting_files=None, do_merge=True,
                  cherrypick=False, lca_trees=None, this_branch=None,
                  other_branch=None):
@@ -735,30 +734,22 @@
         :param this_branch: The branch associated with this_tree.  Defaults to
             this_tree.branch if not supplied.
         :param other_branch: The branch associated with other_tree, if any.
-        :param interesting_ids: The file_ids of files that should be
-            participate in the merge.  May not be combined with
-            interesting_files.
         :param: reprocess If True, perform conflict-reduction processing.
         :param show_base: If True, show the base revision in text conflicts.
             (incompatible with reprocess)
         :param change_reporter: An object that should report changes made
         :param interesting_files: The tree-relative paths of files that should
             participate in the merge.  If these paths refer to directories,
-            the contents of those directories will also be included.  May not
-            be combined with interesting_ids.  If neither interesting_files nor
-            interesting_ids is specified, all files may participate in the
+            the contents of those directories will also be included.  If not
+            specified, all files may participate in the
             merge.
         :param lca_trees: Can be set to a dictionary of {revision_id:rev_tree}
             if the ancestry was found to include a criss-cross merge.
             Otherwise should be None.
         """
         object.__init__(self)
-        if interesting_files is not None and interesting_ids is not None:
-            raise ValueError(
-                'specify either interesting_ids or interesting_files')
         if this_branch is None:
             this_branch = this_tree.branch
-        self.interesting_ids = interesting_ids
         self.interesting_files = interesting_files
         self.working_tree = working_tree
         self.this_tree = this_tree
@@ -874,14 +865,13 @@
         iterator = self.other_tree.iter_changes(self.base_tree,
                 specific_files=self.interesting_files,
                 extra_trees=[self.this_tree])
+        this_interesting_files = self.this_tree.find_related_paths_across_trees(
+                self.interesting_files, trees=[self.other_tree])
         this_entries = dict((e.file_id, e) for p, e in
                             self.this_tree.iter_entries_by_dir(
-                            self.interesting_ids))
+                            specific_files=this_interesting_files))
         for (file_id, paths, changed, versioned, parents, names, kind,
              executable) in iterator:
-            if (self.interesting_ids is not None and
-                file_id not in self.interesting_ids):
-                continue
             entry = this_entries.get(file_id)
             if entry is not None:
                 this_name = entry.name
@@ -919,10 +909,10 @@
             lookup_trees = [self.this_tree, self.base_tree]
             lookup_trees.extend(self._lca_trees)
             # I think we should include the lca trees as well
-            interesting_ids = self.other_tree.paths2ids(self.interesting_files,
-                                                        lookup_trees)
+            interesting_files = self.other_tree.find_related_paths_across_trees(
+                    self.interesting_files, lookup_trees)
         else:
-            interesting_ids = self.interesting_ids
+            interesting_files = None
         result = []
         walker = _mod_tree.MultiWalker(self.other_tree, self._lca_trees)
 
@@ -932,7 +922,7 @@
             # Is this modified at all from any of the other trees?
             if other_ie is None:
                 other_ie = _none_entry
-            if interesting_ids is not None and file_id not in interesting_ids:
+            if interesting_files is not None and path not in interesting_files:
                 continue
 
             # If other_revision is found in any of the lcas, that means this
@@ -1815,7 +1805,7 @@
         self.merge_type = Merge3Merger
         self.show_base = False
         self.reprocess = False
-        self.interesting_ids = None
+        self.interesting_files = None
         self.merge_type = _MergeTypeParameterizer(MergeIntoMergeType,
               target_subdir=self._target_subdir,
               source_subpath=self._source_subpath)
@@ -1926,7 +1916,6 @@
 def merge_inner(this_branch, other_tree, base_tree, ignore_zero=False,
                 backup_files=False,
                 merge_type=Merge3Merger,
-                interesting_ids=None,
                 show_base=False,
                 reprocess=False,
                 other_rev_id=None,
@@ -1947,13 +1936,8 @@
                     change_reporter=change_reporter)
     merger.backup_files = backup_files
     merger.merge_type = merge_type
-    merger.interesting_ids = interesting_ids
     merger.ignore_zero = ignore_zero
-    if interesting_files:
-        if interesting_ids:
-            raise ValueError('Only supply interesting_ids'
-                             ' or interesting_files')
-        merger.interesting_files = interesting_files
+    merger.interesting_files = interesting_files
     merger.show_base = show_base
     merger.reprocess = reprocess
     merger.other_rev_id = other_rev_id

=== modified file 'breezy/rename_map.py'
--- breezy/rename_map.py	2018-03-12 01:24:43 +0000
+++ breezy/rename_map.py	2018-03-24 01:23:20 +0000
@@ -26,6 +26,7 @@
 from .sixish import (
     BytesIO,
     viewitems,
+    viewvalues,
     )
 from .ui import ui_factory
 
@@ -237,7 +238,14 @@
     def _make_inventory_delta(self, matches):
         delta = []
         file_id_matches = dict((f, p) for p, f in viewitems(matches))
-        for old_path, entry in self.tree.iter_entries_by_dir(file_id_matches):
+        file_id_query = []
+        for f in viewvalues(matches):
+            try:
+                file_id_query.append(self.tree.id2path(f))
+            except errors.NoSuchId:
+                pass
+        for old_path, entry in self.tree.iter_entries_by_dir(
+                specific_files=file_id_query):
             new_path = file_id_matches[entry.file_id]
             parent_path, new_name = osutils.split(new_path)
             parent_id = matches.get(parent_path)

=== modified file 'breezy/tests/per_intertree/test_compare.py'
--- breezy/tests/per_intertree/test_compare.py	2017-11-21 20:37:41 +0000
+++ breezy/tests/per_intertree/test_compare.py	2018-03-24 01:23:20 +0000
@@ -557,7 +557,9 @@
 
     @staticmethod
     def get_path_entry(tree, file_id):
-        iterator = tree.iter_entries_by_dir(specific_file_ids=[file_id])
+        with tree.lock_read():
+            path = tree.id2path(file_id)
+        iterator = tree.iter_entries_by_dir(specific_files=[path])
         try:
             return next(iterator)
         except StopIteration:

=== modified file 'breezy/tests/per_tree/test_inv.py'
--- breezy/tests/per_tree/test_inv.py	2017-08-01 01:44:41 +0000
+++ breezy/tests/per_tree/test_inv.py	2018-03-24 01:23:20 +0000
@@ -33,8 +33,8 @@
     )
 
 
-def get_entry(tree, file_id):
-    return tree.iter_entries_by_dir([file_id]).next()[1]
+def get_entry(tree, path):
+    return tree.iter_entries_by_dir(specific_files=[path]).next()[1]
 
 
 class TestInventoryWithSymlinks(per_tree.TestCaseWithTree):
@@ -52,7 +52,7 @@
             raise TestSkipped(
                 'symlinks not accurately represented in working trees and'
                 ' preview trees')
-        entry = get_entry(self.tree, self.tree.path2id('symlink'))
+        entry = get_entry(self.tree, 'symlink')
         self.assertEqual(entry.symlink_target, 'link-target')
 
     def test_symlink_target_tree(self):
@@ -64,7 +64,7 @@
         self.assertIs(None, self.tree.get_file_size('symlink'))
 
     def test_symlink(self):
-        entry = get_entry(self.tree, self.tree.path2id('symlink'))
+        entry = get_entry(self.tree, 'symlink')
         self.assertEqual(entry.kind, 'symlink')
         self.assertEqual(None, entry.text_size)
 
@@ -76,6 +76,9 @@
         self.build_tree(['tree/dir/', 'tree/dir/file'])
         work_tree.add(['dir', 'dir/file'])
         tree = self._convert_tree(work_tree)
+        if not isinstance(tree, InventoryTree):
+            raise tests.TestNotApplicable(
+                "test not applicable on non-inventory tests")
         tree.lock_read()
         self.addCleanup(tree.unlock)
         self.assertEqual({tree.path2id('dir'), tree.path2id('dir/file')},
@@ -88,6 +91,9 @@
         work_tree.commit('commit old state')
         work_tree.remove('file')
         tree = self._convert_tree(work_tree)
+        if not isinstance(tree, InventoryTree):
+            raise tests.TestNotApplicable(
+                "test not applicable on non-inventory tests")
         tree.lock_read()
         self.addCleanup(tree.unlock)
         self.assertEqual(set([]), tree.paths2ids(['file'],

=== modified file 'breezy/tests/per_tree/test_test_trees.py'
--- breezy/tests/per_tree/test_test_trees.py	2018-02-26 13:36:15 +0000
+++ breezy/tests/per_tree/test_test_trees.py	2018-03-24 01:23:20 +0000
@@ -228,11 +228,8 @@
                         (u'ba\N{Euro Sign}r/ba\N{Euro Sign}z',
                          baz_id, bar_id, revision_id),
                        ]
-        tree.lock_read()
-        try:
+        with tree.lock_read():
             path_entries = list(tree.iter_entries_by_dir())
-        finally:
-            tree.unlock()
 
         for expected, (path, ie) in zip(path_and_ids, path_entries):
             self.assertEqual(expected[0], path) # Paths should match
@@ -276,11 +273,8 @@
                         (u'ba\N{Euro Sign}r/qu\N{Euro Sign}x',
                          qux_id, bar_id, revision_id_2),
                        ]
-        tree.lock_read()
-        try:
+        with tree.lock_read():
             path_entries = list(tree.iter_entries_by_dir())
-        finally:
-            tree.unlock()
 
         for (epath, efid, eparent, erev), (path, ie) in zip(path_and_ids,
                                                             path_entries):

=== modified file 'breezy/tests/per_workingtree/test_inv.py'
--- breezy/tests/per_workingtree/test_inv.py	2018-02-15 19:38:33 +0000
+++ breezy/tests/per_workingtree/test_inv.py	2018-03-24 01:23:20 +0000
@@ -178,6 +178,6 @@
         # wt.current_dirstate()'s idea about what files are where.
         ie = base.inventory['subdir-id']
         self.assertEqual('directory', ie.kind)
-        path, ie = next(base.iter_entries_by_dir(['subdir-id']))
+        path, ie = next(base.iter_entries_by_dir(specific_files=['subdir']))
         self.assertEqual('subdir', path)
         self.assertEqual('tree-reference', ie.kind)

=== modified file 'breezy/tests/per_workingtree/test_nested_specifics.py'
--- breezy/tests/per_workingtree/test_nested_specifics.py	2018-02-16 19:38:39 +0000
+++ breezy/tests/per_workingtree/test_nested_specifics.py	2018-03-24 01:23:20 +0000
@@ -79,5 +79,5 @@
 
     def test_iter_entries_by_dir_autodetects_subtree(self):
         tree = self.prepare_with_subtree()
-        path, ie = next(tree.iter_entries_by_dir(['subtree-id']))
+        path, ie = next(tree.iter_entries_by_dir(specific_files=['subtree']))
         self.assertEqual('tree-reference', ie.kind)

=== modified file 'breezy/tests/per_workingtree/test_paths2ids.py'
--- breezy/tests/per_workingtree/test_paths2ids.py	2018-02-03 13:39:29 +0000
+++ breezy/tests/per_workingtree/test_paths2ids.py	2018-03-24 01:23:20 +0000
@@ -22,6 +22,7 @@
 """
 
 from breezy import errors
+from breezy.bzr.inventorytree import InventoryTree
 from breezy.tests import (
     features,
     TestNotApplicable,
@@ -62,16 +63,30 @@
 
     def test_paths_none_result_none(self):
         tree = self.make_branch_and_tree('tree')
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable(
+                "test not applicable on non-inventory tests")
+
         tree.lock_read()
         self.assertEqual(None, tree.paths2ids(None))
         tree.unlock()
 
     def test_find_single_root(self):
         tree = self.make_branch_and_tree('tree')
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable(
+                "test not applicable on non-inventory tests")
+
+
         self.assertExpectedIds([tree.path2id('')], tree, [''])
 
     def test_find_tree_and_clone_roots(self):
         tree = self.make_branch_and_tree('tree')
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable(
+                "test not applicable on non-inventory tests")
+
+
         clone = tree.controldir.clone('clone').open_workingtree()
         clone.lock_tree_write()
         clone_root_id = 'new-id'
@@ -136,6 +151,11 @@
           new-child because its under dir in new.
         """
         tree = self.make_branch_and_tree('tree')
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable(
+                "test not applicable on non-inventory tests")
+
+
         self.build_tree(
             ['tree/dir/', 'tree/dir/child-moves', 'tree/dir/child-stays',
              'tree/dir/child-goes'])
@@ -161,6 +181,11 @@
 
     def test_unversioned_one_tree(self):
         tree = self.make_branch_and_tree('tree')
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable(
+                "test not applicable on non-inventory tests")
+
+
         self.build_tree(['tree/unversioned'])
         self.assertExpectedIds([], tree, ['unversioned'], require_versioned=False)
         tree.lock_read()
@@ -172,6 +197,10 @@
         # should not raise an error: it must be unversioned in *all* trees to
         # error.
         tree = self.make_branch_and_tree('tree')
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable(
+                "test not applicable on non-inventory tests")
+
         if not tree.supports_setting_file_ids():
             raise TestNotApplicable('tree does not support setting file ids')
         tree.commit('make basis')
@@ -185,6 +214,10 @@
         # should not raise an error: it must be unversioned in *all* trees to
         # error.
         tree = self.make_branch_and_tree('tree')
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable(
+                "test not applicable on non-inventory tests")
+
         tree.commit('make basis')
         basis = tree.basis_tree()
         self.assertExpectedIds([], tree, ['unversioned'], [basis],
@@ -201,6 +234,10 @@
     def test_unversioned_non_ascii_one_tree(self):
         self.requireFeature(features.UnicodeFilenameFeature)
         tree = self.make_branch_and_tree('.')
+        if not isinstance(tree, InventoryTree):
+            raise TestNotApplicable(
+                "test not applicable on non-inventory tests")
+
         self.build_tree([u"\xa7"])
         self.assertExpectedIds([], tree, [u"\xa7"], require_versioned=False)
         self.addCleanup(tree.lock_read().unlock)

=== modified file 'breezy/tests/test_merge.py'
--- breezy/tests/test_merge.py	2018-03-20 00:30:39 +0000
+++ breezy/tests/test_merge.py	2018-03-24 01:23:20 +0000
@@ -1265,8 +1265,7 @@
         builder.build_snapshot(['B-id', 'C-id'], [], revision_id='D-id')
         return builder
 
-    def make_Merger(self, builder, other_revision_id,
-                    interesting_files=None, interesting_ids=None):
+    def make_Merger(self, builder, other_revision_id, interesting_files=None):
         """Make a Merger object from a branch builder"""
         mem_tree = memorytree.MemoryTree.create_on_branch(builder.get_branch())
         mem_tree.lock_write()
@@ -1274,8 +1273,6 @@
         merger = _mod_merge.Merger.from_revision_ids(
             mem_tree, other_revision_id)
         merger.set_interesting_files(interesting_files)
-        # It seems there is no matching function for set_interesting_ids
-        merger.interesting_ids = interesting_ids
         merger.merge_type = _mod_merge.Merge3Merger
         return merger
 
@@ -1400,10 +1397,9 @@
 class TestMergerEntriesLCA(TestMergerBase):
 
     def make_merge_obj(self, builder, other_revision_id,
-                       interesting_files=None, interesting_ids=None):
+                       interesting_files=None):
         merger = self.make_Merger(builder, other_revision_id,
-            interesting_files=interesting_files,
-            interesting_ids=interesting_ids)
+            interesting_files=interesting_files)
         return merger.make_merger()
 
     def test_simple(self):
@@ -2099,7 +2095,7 @@
                            ((False, [False, False]), False, False)),
                          ], entries)
 
-    def test_interesting_ids(self):
+    def test_interesting_files(self):
         # Two files modified, but we should filter one of them
         builder = self.get_builder()
         builder.build_snapshot(None,
@@ -2114,7 +2110,7 @@
              ('modify', ('b-id', 'new-content\n'))], revision_id='E-id')
         builder.build_snapshot(['B-id', 'C-id'], [], revision_id='D-id')
         merge_obj = self.make_merge_obj(builder, 'E-id',
-                                        interesting_ids=['b-id'])
+                                        interesting_files=['b'])
         entries = list(merge_obj._entries_lca())
         root_id = 'a-root-id'
         self.assertEqual([('b-id', True,

=== modified file 'breezy/tests/test_merge_core.py'
--- breezy/tests/test_merge_core.py	2018-03-09 19:52:32 +0000
+++ breezy/tests/test_merge_core.py	2018-03-24 01:23:20 +0000
@@ -77,8 +77,8 @@
             if option is True:
                 new_file(tt)
 
-    def merge(self, merge_type=Merge3Merger, interesting_ids=None, **kwargs):
-        merger = self.make_merger(merge_type, interesting_ids, **kwargs)
+    def merge(self, merge_type=Merge3Merger, interesting_files=None, **kwargs):
+        merger = self.make_merger(merge_type, interesting_files, **kwargs)
         merger.do_merge()
         return merger.cooked_conflicts
 
@@ -86,7 +86,7 @@
         merger = self.make_merger(Merge3Merger, None, this_revision_tree=True)
         return merger.make_preview_transform()
 
-    def make_merger(self, merge_type, interesting_ids,
+    def make_merger(self, merge_type, interesting_files,
             this_revision_tree=False, **kwargs):
         self.base_tt.apply()
         self.base.commit('base commit')
@@ -110,7 +110,7 @@
         else:
             this_tree = self.this
         merger = merge_type(this_tree, self.this, self.base, other_basis,
-                            interesting_ids=interesting_ids, do_merge=False,
+                            interesting_files=interesting_files, do_merge=False,
                             this_branch=self.this.branch, **kwargs)
         return merger
 
@@ -251,7 +251,7 @@
         builder.change_contents("1", other="text4")
         builder.add_file("2", builder.tree_root, "name2", "hello1", True)
         builder.change_contents("2", other="text4")
-        builder.merge(interesting_ids=["1"])
+        builder.merge(interesting_files=["name1"])
         self.assertEqual(builder.this.get_file("name1").read(), "text4" )
         self.assertEqual(builder.this.get_file("name2").read(), "hello1" )
         builder.cleanup()

=== modified file 'breezy/tests/test_shelf.py'
--- breezy/tests/test_shelf.py	2017-12-12 01:19:18 +0000
+++ breezy/tests/test_shelf.py	2018-03-24 01:23:20 +0000
@@ -206,7 +206,7 @@
 
     def check_shelve_creation(self, creator, tree):
         self.assertRaises(StopIteration,
-                          next, tree.iter_entries_by_dir(['foo-id']))
+                          next, tree.iter_entries_by_dir(specific_files=['foo']))
         s_trans_id = creator.shelf_transform.trans_id_file_id('foo-id')
         self.assertEqual('foo-id',
                          creator.shelf_transform.final_file_id(s_trans_id))
@@ -349,7 +349,7 @@
         creator.shelve_creation('foo-id')
         creator.transform()
         self.assertRaises(StopIteration,
-                          next, tree.iter_entries_by_dir(['foo-id']))
+                          next, tree.iter_entries_by_dir(specific_files=['foo']))
         self.assertShelvedFileEqual('', creator, 'foo-id')
         s_trans_id = creator.shelf_transform.trans_id_file_id('foo-id')
         self.assertEqual('foo-id',

=== modified file 'breezy/tests/test_transform.py'
--- breezy/tests/test_transform.py	2018-03-24 00:39:56 +0000
+++ breezy/tests/test_transform.py	2018-03-24 01:23:20 +0000
@@ -2937,13 +2937,14 @@
         self.assertFalse(preview_tree.is_versioned('old_name/child'))
         self.assertEqual('child-id', preview_tree.path2id('new_name/child'))
 
-    def assertMatchingIterEntries(self, tt, specific_file_ids=None):
+    def assertMatchingIterEntries(self, tt, specific_files=None):
         preview_tree = tt.get_preview_tree()
         preview_result = list(preview_tree.iter_entries_by_dir(
-                              specific_file_ids))
+                              specific_files=specific_files))
         tree = tt._tree
         tt.apply()
-        actual_result = list(tree.iter_entries_by_dir(specific_file_ids))
+        actual_result = list(tree.iter_entries_by_dir(
+            specific_files=specific_files))
         self.assertEqual(actual_result, preview_result)
 
     def test_iter_entries_by_dir_new(self):
@@ -2983,7 +2984,7 @@
         self.build_tree(['tree/parent/', 'tree/parent/child'])
         tree.add(['parent', 'parent/child'], ['parent-id', 'child-id'])
         tt = TreeTransform(tree)
-        self.assertMatchingIterEntries(tt, ['tree-root-id', 'child-id'])
+        self.assertMatchingIterEntries(tt, ['', 'parent/child'])
 
     def test_symlink_content_summary(self):
         self.requireFeature(SymlinkFeature)

=== modified file 'breezy/tests/test_workingtree.py'
--- breezy/tests/test_workingtree.py	2018-02-11 17:07:38 +0000
+++ breezy/tests/test_workingtree.py	2018-03-24 01:23:20 +0000
@@ -257,7 +257,8 @@
         subtree = self.make_branch_and_tree('tree/a/b')
         self.assertEqual([('tree-reference', 'b-id')],
                          [(ie.kind, ie.file_id)
-                          for path, ie in tree.iter_entries_by_dir(['b-id'])])
+                          for path, ie in tree.iter_entries_by_dir(
+                              specific_files=['a/b'])])
 
     def test_direct_subtree(self):
         tree = self.make_simple_tree()

=== modified file 'breezy/transform.py'
--- breezy/transform.py	2018-03-24 00:39:56 +0000
+++ breezy/transform.py	2018-03-24 01:23:20 +0000
@@ -894,7 +894,7 @@
                 to_trans_ids[to_file_id] = trans_id
         return from_trans_ids, to_trans_ids
 
-    def _from_file_data(self, from_trans_id, from_versioned, file_id):
+    def _from_file_data(self, from_trans_id, from_versioned, from_path):
         """Get data about a file in the from (tree) state
 
         Return a (name, parent, kind, executable) tuple
@@ -902,7 +902,8 @@
         from_path = self._tree_id_paths.get(from_trans_id)
         if from_versioned:
             # get data from working tree if versioned
-            from_entry = self._tree.iter_entries_by_dir([file_id]).next()[1]
+            from_entry = self._tree.iter_entries_by_dir(
+                    specific_files=[from_path]).next()[1]
             from_name = from_entry.name
             from_parent = from_entry.parent_id
         else:
@@ -971,12 +972,6 @@
             else:
                 to_versioned = True
 
-            from_name, from_parent, from_kind, from_executable = \
-                self._from_file_data(from_trans_id, from_versioned, file_id)
-
-            to_name, to_parent, to_kind, to_executable = \
-                self._to_file_data(to_trans_id, from_trans_id, from_executable)
-
             if not from_versioned:
                 from_path = None
             else:
@@ -985,6 +980,13 @@
                 to_path = None
             else:
                 to_path = final_paths.get_path(to_trans_id)
+
+            from_name, from_parent, from_kind, from_executable = \
+                self._from_file_data(from_trans_id, from_versioned, from_path)
+
+            to_name, to_parent, to_kind, to_executable = \
+                self._to_file_data(to_trans_id, from_trans_id, from_executable)
+
             if from_kind != to_kind:
                 modified = True
             elif to_kind in ('file', 'symlink') and (
@@ -1763,9 +1765,6 @@
                 inventory_delta.append((path, None, file_id, None))
             new_path_file_ids = dict((t, self.final_file_id(t)) for p, t in
                                      new_paths)
-            entries = self._tree.iter_entries_by_dir(
-                viewvalues(new_path_file_ids))
-            old_paths = dict((e.file_id, p) for p, e in entries)
             final_kinds = {}
             for num, (path, trans_id) in enumerate(new_paths):
                 if (num % 10) == 0:
@@ -1793,7 +1792,10 @@
                     new_entry = inventory.make_entry(kind,
                         self.final_name(trans_id),
                         parent_file_id, file_id)
-                old_path = old_paths.get(new_entry.file_id)
+                try:
+                    old_path = self._tree.id2path(new_entry.file_id)
+                except errors.NoSuchId:
+                    old_path = None
                 new_executability = self._new_executability.get(trans_id)
                 if new_executability is not None:
                     new_entry.executable = new_executability
@@ -1944,10 +1946,8 @@
             path = self._tree_id_paths[parent_id]
         except KeyError:
             return
-        file_id = self.tree_file_id(parent_id)
-        if file_id is None:
-            return
-        entry = self._tree.iter_entries_by_dir([file_id]).next()[1]
+        entry = self._tree.iter_entries_by_dir(
+                specific_files=[path]).next()[1]
         children = getattr(entry, 'children', {})
         for child in children:
             childpath = joinpath(path, child)
@@ -2127,14 +2127,14 @@
             if self._transform.final_file_id(trans_id) is None:
                 yield self._final_paths._determine_path(trans_id)
 
-    def _make_inv_entries(self, ordered_entries, specific_file_ids=None,
+    def _make_inv_entries(self, ordered_entries, specific_files=None,
         yield_parents=False):
         for trans_id, parent_file_id in ordered_entries:
             file_id = self._transform.final_file_id(trans_id)
             if file_id is None:
                 continue
-            if (specific_file_ids is not None
-                and file_id not in specific_file_ids):
+            if (specific_files is not None and
+                unicode(self._final_paths.get_path(trans_id)) not in specific_files):
                 continue
             kind = self._transform.final_kind(trans_id)
             if kind is None:
@@ -2172,7 +2172,7 @@
         for entry, trans_id in self._make_inv_entries(todo):
             yield entry
 
-    def iter_entries_by_dir(self, specific_file_ids=None, yield_parents=False):
+    def iter_entries_by_dir(self, specific_files=None, yield_parents=False):
         # This may not be a maximally efficient implementation, but it is
         # reasonably straightforward.  An implementation that grafts the
         # TreeTransform changes onto the tree's iter_entries_by_dir results
@@ -2180,7 +2180,7 @@
         # position.
         ordered_ids = self._list_files_by_dir()
         for entry, trans_id in self._make_inv_entries(ordered_ids,
-            specific_file_ids, yield_parents=yield_parents):
+            specific_files, yield_parents=yield_parents):
             yield unicode(self._final_paths.get_path(trans_id)), entry
 
     def _iter_entries_for_dir(self, dir_path):
@@ -3081,7 +3081,7 @@
                         if file_id is None:
                             file_id = tt.inactive_file_id(trans_id)
                         _, entry = next(path_tree.iter_entries_by_dir(
-                            [file_id]))
+                            specific_files=[path_tree.id2path(file_id)]))
                         # special-case the other tree root (move its
                         # children to current root)
                         if entry.parent_id is None:

=== modified file 'breezy/tree.py'
--- breezy/tree.py	2018-03-24 00:39:56 +0000
+++ breezy/tree.py	2018-03-24 01:23:20 +0000
@@ -213,7 +213,7 @@
         """
         raise NotImplementedError(self.id2path)
 
-    def iter_entries_by_dir(self, specific_file_ids=None, yield_parents=False):
+    def iter_entries_by_dir(self, specific_files=None, yield_parents=False):
         """Walk the tree in 'by_dir' order.
 
         This will yield each entry in the tree as a (path, entry) tuple.
@@ -239,8 +239,8 @@
           a, f, a/b, a/d, a/b/c, a/d/e, f/g
 
         :param yield_parents: If True, yield the parents from the root leading
-            down to specific_file_ids that have been requested. This has no
-            impact if specific_file_ids is None.
+            down to specific_files that have been requested. This has no
+            impact if specific_files is None.
         """
         raise NotImplementedError(self.iter_entries_by_dir)
 
@@ -501,24 +501,23 @@
         """
         return self.path2id(path) is not None
 
-    def paths2ids(self, paths, trees=[], require_versioned=True):
-        """Return all the ids that can be reached by walking from paths.
-
-        Each path is looked up in this tree and any extras provided in
-        trees, and this is repeated recursively: the children in an extra tree
-        of a directory that has been renamed under a provided path in this tree
-        are all returned, even if none exist under a provided path in this
-        tree, and vice versa.
-
-        :param paths: An iterable of paths to start converting to ids from.
-            Alternatively, if paths is None, no ids should be calculated and None
-            will be returned. This is offered to make calling the api unconditional
-            for code that *might* take a list of files.
-        :param trees: Additional trees to consider.
-        :param require_versioned: If False, do not raise NotVersionedError if
-            an element of paths is not versioned in this tree and all of trees.
+    def find_related_paths_across_trees(self, paths, trees=[],
+            require_versioned=True):
+        """Find related paths in tree corresponding to specified filenames in any
+        of `lookup_trees`.
+
+        All matches in all trees will be used, and all children of matched
+        directories will be used.
+
+        :param paths: The filenames to find related paths for (if None, returns
+            None)
+        :param trees: The trees to find file_ids within
+        :param require_versioned: if true, all specified filenames must occur in
+            at least one tree.
+        :return: a set of paths for the specified filenames and their children
+            in `tree`
         """
-        return find_ids_across_trees(paths, [self] + list(trees), require_versioned)
+        raise NotImplementedError(self.find_related_paths_across_trees)
 
     def lock_read(self):
         """Lock this tree for multiple read only operations.
@@ -657,84 +656,6 @@
         return searcher
 
 
-def find_ids_across_trees(filenames, trees, require_versioned=True):
-    """Find the ids corresponding to specified filenames.
-
-    All matches in all trees will be used, and all children of matched
-    directories will be used.
-
-    :param filenames: The filenames to find file_ids for (if None, returns
-        None)
-    :param trees: The trees to find file_ids within
-    :param require_versioned: if true, all specified filenames must occur in
-        at least one tree.
-    :return: a set of file ids for the specified filenames and their children.
-    """
-    if not filenames:
-        return None
-    specified_path_ids = _find_ids_across_trees(filenames, trees,
-        require_versioned)
-    return _find_children_across_trees(specified_path_ids, trees)
-
-
-def _find_ids_across_trees(filenames, trees, require_versioned):
-    """Find the ids corresponding to specified filenames.
-
-    All matches in all trees will be used, but subdirectories are not scanned.
-
-    :param filenames: The filenames to find file_ids for
-    :param trees: The trees to find file_ids within
-    :param require_versioned: if true, all specified filenames must occur in
-        at least one tree.
-    :return: a set of file ids for the specified filenames
-    """
-    not_versioned = []
-    interesting_ids = set()
-    for tree_path in filenames:
-        not_found = True
-        for tree in trees:
-            file_id = tree.path2id(tree_path)
-            if file_id is not None:
-                interesting_ids.add(file_id)
-                not_found = False
-        if not_found:
-            not_versioned.append(tree_path)
-    if len(not_versioned) > 0 and require_versioned:
-        raise errors.PathsNotVersionedError(not_versioned)
-    return interesting_ids
-
-
-def _find_children_across_trees(specified_ids, trees):
-    """Return a set including specified ids and their children.
-
-    All matches in all trees will be used.
-
-    :param trees: The trees to find file_ids within
-    :return: a set containing all specified ids and their children
-    """
-    interesting_ids = set(specified_ids)
-    pending = interesting_ids
-    # now handle children of interesting ids
-    # we loop so that we handle all children of each id in both trees
-    while len(pending) > 0:
-        new_pending = set()
-        for tree in trees:
-            for file_id in pending:
-                try:
-                    path = tree.id2path(file_id)
-                except errors.NoSuchId:
-                    continue
-                try:
-                    for child in tree.iter_child_entries(path, file_id):
-                        if child.file_id not in interesting_ids:
-                            new_pending.add(child.file_id)
-                except errors.NotADirectory:
-                    pass
-        interesting_ids.update(new_pending)
-        pending = new_pending
-    return interesting_ids
-
-
 class InterTree(InterObject):
     """This class represents operations taking place between two Trees.
 
@@ -854,18 +775,6 @@
         if extra_trees is not None:
             trees = trees + tuple(extra_trees)
         with self.lock_read():
-            # target is usually the newer tree:
-            specific_file_ids = self.target.paths2ids(specific_files, trees,
-                require_versioned=require_versioned)
-            if specific_files and not specific_file_ids:
-                # All files are unversioned, so just return an empty delta
-                # _compare_trees would think we want a complete delta
-                result = delta.TreeDelta()
-                fake_entry = inventory.InventoryFile('unused', 'unused', 'unused')
-                result.unversioned = [(path, None,
-                    self.target._comparison_data(fake_entry, path)[0]) for path in
-                    specific_files]
-                return result
             return delta._compare_trees(self.source, self.target, want_unchanged,
                 specific_files, include_root, extra_trees=extra_trees,
                 require_versioned=require_versioned,
@@ -907,17 +816,23 @@
             output. An unversioned file is defined as one with (False, False)
             for the versioned pair.
         """
-        lookup_trees = [self.source]
-        if extra_trees:
-             lookup_trees.extend(extra_trees)
+        if not extra_trees:
+             extra_trees = []
+        else:
+             extra_trees = list(extra_trees)
         # The ids of items we need to examine to insure delta consistency.
         precise_file_ids = set()
         changed_file_ids = []
         if specific_files == []:
-            specific_file_ids = []
+            target_specific_files = []
+            source_specific_files = []
         else:
-            specific_file_ids = self.target.paths2ids(specific_files,
-                lookup_trees, require_versioned=require_versioned)
+            target_specific_files = self.target.find_related_paths_across_trees(
+                    specific_files, [self.source] + extra_trees,
+                    require_versioned=require_versioned)
+            source_specific_files = self.source.find_related_paths_across_trees(
+                    specific_files, [self.target] + extra_trees,
+                    require_versioned=require_versioned)
         if specific_files is not None:
             # reparented or added entries must have their parents included
             # so that valid deltas can be created. The seen_parents set
@@ -937,10 +852,10 @@
             all_unversioned = collections.deque()
         to_paths = {}
         from_entries_by_dir = list(self.source.iter_entries_by_dir(
-            specific_file_ids=specific_file_ids))
+            specific_files=source_specific_files))
         from_data = dict((e.file_id, (p, e)) for p, e in from_entries_by_dir)
         to_entries_by_dir = list(self.target.iter_entries_by_dir(
-            specific_file_ids=specific_file_ids))
+            specific_files=target_specific_files))
         num_entries = len(from_entries_by_dir) + len(to_entries_by_dir)
         entry_count = 0
         # the unversioned path lookup only occurs on real trees - where there
@@ -969,7 +884,7 @@
             if pb is not None:
                 pb.update('comparing files', entry_count, num_entries)
             if changes or include_unchanged:
-                if specific_file_ids is not None:
+                if specific_files is not None:
                     new_parent_id = result[4][1]
                     precise_file_ids.add(new_parent_id)
                     changed_file_ids.append(result[0])
@@ -1017,12 +932,12 @@
             yield(file_id, (path, to_path), changed_content, versioned, parent,
                   name, kind, executable)
         changed_file_ids = set(changed_file_ids)
-        if specific_file_ids is not None:
+        if specific_files is not None:
             for result in self._handle_precise_ids(precise_file_ids,
                 changed_file_ids):
                 yield result
 
-    def _get_entry(self, tree, file_id):
+    def _get_entry(self, tree, path):
         """Get an inventory entry from a tree, with missing entries as None.
 
         If the tree raises NotImplementedError on accessing .inventory, then
@@ -1032,20 +947,12 @@
         :param tree: The tree to lookup the entry in.
         :param file_id: The file_id to lookup.
         """
+        # No inventory available.
         try:
-            inventory = tree.root_inventory
-        except (AttributeError, NotImplementedError):
-            # No inventory available.
-            try:
-                iterator = tree.iter_entries_by_dir(specific_file_ids=[file_id])
-                return iterator.next()[1]
-            except StopIteration:
-                return None
-        else:
-            try:
-                return inventory[file_id]
-            except errors.NoSuchId:
-                return None
+            iterator = tree.iter_entries_by_dir(specific_files=[path])
+            return iterator.next()[1]
+        except StopIteration:
+            return None
 
     def _handle_precise_ids(self, precise_file_ids, changed_file_ids,
         discarded_changes=None):
@@ -1093,22 +1000,26 @@
                 # Examine file_id
                 if discarded_changes:
                     result = discarded_changes.get(file_id)
-                    old_entry = None
+                    source_entry = None
                 else:
                     result = None
                 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
+                        source_entry = None
+                    else:
+                        source_entry = self._get_entry(self.source, source_path)
                     try:
                         target_path = self.target.id2path(file_id)
                     except errors.NoSuchId:
                         target_path = None
+                        target_entry = None
+                    else:
+                        target_entry = self._get_entry(self.target, target_path)
                     result, changes = self._changes_from_entries(
-                        old_entry, new_entry, source_path, target_path)
+                        source_entry, target_entry, source_path, target_path)
                 else:
                     changes = True
                 # Get this parents parent to examine.
@@ -1119,9 +1030,9 @@
                             result[6][1] != 'directory'):
                         # This stopped being a directory, the old children have
                         # to be included.
-                        if old_entry is None:
+                        if source_entry is None:
                             # Reusing a discarded change.
-                            old_entry = self._get_entry(self.source, file_id)
+                            source_entry = self._get_entry(self.source, result[1][0])
                         precise_file_ids.update(
                                 child.file_id
                                 for child in self.source.iter_child_entries(result[1][0]))