Merge lp:~jelmer/brz/iter-children into lp:brz

[Jelmer Vernooij]

Remove Tree.iter_children in favor of Tree.iter_child_entries.

Remove Tree.iter_children in favor of Tree.iter_child_entries.

[Martin Packman (gz)]

Thanks!

review: Approve

=== modified file 'breezy/bzr/inventorytree.py'
--- breezy/bzr/inventorytree.py	2018-03-02 00:38:34 +0000
+++ breezy/bzr/inventorytree.py	2018-03-18 13:40:22 +0000
@@ -77,7 +77,8 @@
         adjusted to account for existing elements that match case
         insensitively.
         """
-        return list(self._yield_canonical_inventory_paths(paths))
+        with self.lock_read():
+            return list(self._yield_canonical_inventory_paths(paths))
 
     def get_canonical_inventory_path(self, path):
         """Returns the first inventory item that case-insensitively matches path.
@@ -97,7 +98,8 @@
         :return: The input path adjusted to account for existing elements
         that match case insensitively.
         """
-        return next(self._yield_canonical_inventory_paths([path]))
+        with self.lock_read():
+            return next(self._yield_canonical_inventory_paths([path]))
 
     def _yield_canonical_inventory_paths(self, paths):
         for path in paths:
@@ -112,31 +114,34 @@
             for elt in bit_iter:
                 lelt = elt.lower()
                 new_path = None
-                for child in self.iter_children(cur_id):
-                    try:
-                        # XXX: it seem like if the child is known to be in the
-                        # tree, we shouldn't need to go from its id back to
-                        # its path -- mbp 2010-02-11
-                        #
-                        # XXX: it seems like we could be more efficient
-                        # by just directly looking up the original name and
-                        # only then searching all children; also by not
-                        # chopping paths so much. -- mbp 2010-02-11
-                        child_base = os.path.basename(self.id2path(child))
-                        if (child_base == elt):
-                            # if we found an exact match, we can stop now; if
-                            # we found an approximate match we need to keep
-                            # searching because there might be an exact match
-                            # later.  
-                            cur_id = child
-                            new_path = osutils.pathjoin(cur_path, child_base)
-                            break
-                        elif child_base.lower() == lelt:
-                            cur_id = child
-                            new_path = osutils.pathjoin(cur_path, child_base)
-                    except errors.NoSuchId:
-                        # before a change is committed we can see this error...
-                        continue
+                try:
+                    for child in self.iter_child_entries(self.id2path(cur_id), cur_id):
+                        try:
+                            # XXX: it seem like if the child is known to be in the
+                            # tree, we shouldn't need to go from its id back to
+                            # its path -- mbp 2010-02-11
+                            #
+                            # XXX: it seems like we could be more efficient
+                            # by just directly looking up the original name and
+                            # only then searching all children; also by not
+                            # chopping paths so much. -- mbp 2010-02-11
+                            child_base = os.path.basename(self.id2path(child.file_id))
+                            if (child_base == elt):
+                                # if we found an exact match, we can stop now; if
+                                # we found an approximate match we need to keep
+                                # searching because there might be an exact match
+                                # later.  
+                                cur_id = child.file_id
+                                new_path = osutils.pathjoin(cur_path, child_base)
+                                break
+                            elif child_base.lower() == lelt:
+                                cur_id = child.file_id
+                                new_path = osutils.pathjoin(cur_path, child_base)
+                        except errors.NoSuchId:
+                            # before a change is committed we can see this error...
+                            continue
+                except errors.NotADirectory:
+                    pass
                 if new_path:
                     cur_path = new_path
                 else:
@@ -248,15 +253,13 @@
         with self.lock_read():
             inv, inv_file_id = self._path2inv_file_id(path, file_id)
             try:
-                return iter(viewvalues(inv[inv_file_id].children))
+                ie = inv[inv_file_id]
             except errors.NoSuchId:
                 raise errors.NoSuchFile(path)
-
-    def iter_children(self, file_id, path=None):
-        """See Tree.iter_children."""
-        entry = self.iter_entries_by_dir([file_id]).next()[1]
-        for child in viewvalues(getattr(entry, 'children', {})):
-            yield child.file_id
+            else:
+                if ie.kind != 'directory':
+                    raise errors.NotADirectory(path)
+                return iter(viewvalues(ie.children))
 
     def _get_plan_merge_data(self, file_id, other, base):
         from . import versionedfile

=== modified file 'breezy/transform.py'
--- breezy/transform.py	2018-03-02 00:50:50 +0000
+++ breezy/transform.py	2018-03-18 13:40:22 +0000
@@ -2123,7 +2123,7 @@
         self._all_children_cache[trans_id] = children
         return children
 
-    def iter_children(self, file_id):
+    def _iter_children(self, file_id):
         trans_id = self._transform.trans_id_file_id(file_id)
         for child_trans_id in self._all_children(trans_id):
             yield self._transform.final_file_id(child_trans_id)
@@ -2197,7 +2197,7 @@
         """Return path, entry for items in a directory without recursing down."""
         dir_file_id = self.path2id(dir_path)
         ordered_ids = []
-        for file_id in self.iter_children(dir_file_id):
+        for file_id in self._iter_children(dir_file_id):
             trans_id = self._transform.trans_id_file_id(file_id)
             ordered_ids.append((trans_id, file_id))
         for entry, trans_id in self._make_inv_entries(ordered_ids):

=== modified file 'breezy/tree.py'
--- breezy/tree.py	2018-03-02 00:38:34 +0000
+++ breezy/tree.py	2018-03-18 13:40:22 +0000
@@ -537,14 +537,6 @@
         """
         return find_ids_across_trees(paths, [self] + list(trees), require_versioned)
 
-    def iter_children(self, file_id):
-        """Iterate over the file ids of the children of an entry.
-
-        :param file_id: File id of the entry
-        :return: Iterator over child file ids.
-        """
-        raise NotImplementedError(self.iter_children)
-
     def lock_read(self):
         """Lock this tree for multiple read only operations.
 
@@ -746,13 +738,18 @@
     # 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:
-                if not tree.has_or_had_id(file_id):
+        for tree in trees:
+            for file_id in pending:
+                try:
+                    path = tree.id2path(file_id)
+                except errors.NoSuchId:
                     continue
-                for child_id in tree.iter_children(file_id):
-                    if child_id not in interesting_ids:
-                        new_pending.add(child_id)
+                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
@@ -1148,7 +1145,8 @@
                             # Reusing a discarded change.
                             old_entry = self._get_entry(self.source, file_id)
                         precise_file_ids.update(
-                                self.source.iter_children(file_id))
+                                child.file_id
+                                for child in self.source.iter_child_entries(result[1][0]))
                     changed_file_ids.add(result[0])
                     yield result