Merge lp:~jelmer/bzr/less-inventory-access into lp:bzr

[Jelmer Vernooij]

Avoid direct access of tree inventories in a couple more places.

Remove access to Tree.root_inventory in a couple of places.

In a few places we weren't actually using the inventory after assigning it to a local variable.

I've removed Inventory.directories(). It didn't have any tests and only one caller. Simply using Tree.iter_entries_by_dir() instead and filtering for directories works too, and should be just as quick.

[Vincent Ladeuil (vila)]

Looking at the annotation, directories is so old... I think we're safe to delete it, but you'll get the blame for not deprecating it if somebody was using it ;)

review: Approve

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2012-02-14 17:22:37 +0000
+++ bzrlib/builtins.py	2012-02-17 17:04:19 +0000
@@ -1650,10 +1650,8 @@
     def run(self, dir=u'.'):
         tree = WorkingTree.open_containing(dir)[0]
         self.add_cleanup(tree.lock_read().unlock)
-        new_inv = tree.root_inventory
         old_tree = tree.basis_tree()
         self.add_cleanup(old_tree.lock_read().unlock)
-        old_inv = old_tree.root_inventory
         renames = []
         iterator = tree.iter_changes(old_tree, include_unchanged=True)
         for f, paths, c, v, p, n, k, e in iterator:
 
=== modified file 'bzrlib/bundle/bundle_data.py'
--- bzrlib/bundle/bundle_data.py	2012-01-24 16:19:04 +0000
+++ bzrlib/bundle/bundle_data.py	2012-02-17 17:04:19 +0000
@@ -687,10 +687,9 @@
         if new_path not in self.patches:
             # If the entry does not have a patch, then the
             # contents must be the same as in the base_tree
-            ie = self.base_tree.root_inventory[file_id]
-            if ie.text_size is None:
-                return ie.text_size, ie.text_sha1
-            return int(ie.text_size), ie.text_sha1
+            text_size = self.base_tree.get_file_size(file_id)
+            text_sha1 = self.base_tree.get_file_sha1(file_id)
+            return text_size, text_sha1
         fileobj = self.get_file(file_id)
         content = fileobj.read()
         return len(content), sha_string(content)
@@ -701,7 +700,6 @@
         This need to be called before ever accessing self.inventory
         """
         from os.path import dirname, basename
-        base_inv = self.base_tree.root_inventory
         inv = Inventory(None, self.revision_id)
 
         def add_entry(file_id):
@@ -750,9 +748,9 @@
 
     root_inventory = property(_get_inventory)
 
-    def __iter__(self):
-        for path, entry in self.inventory.iter_entries():
-            yield entry.file_id
+    def all_file_ids(self):
+        return set(
+            [entry.file_id for path, entry in self.inventory.iter_entries()])
 
     def list_files(self, include_root=False, from_dir=None, recursive=True):
         # The only files returned by this are those from the version
 
=== modified file 'bzrlib/commit.py'
--- bzrlib/commit.py	2012-02-06 23:38:33 +0000
+++ bzrlib/commit.py	2012-02-17 17:04:19 +0000
@@ -834,11 +834,9 @@
         deleted_paths = {}
         # XXX: Note that entries may have the wrong kind because the entry does
         # not reflect the status on disk.
-        # FIXME: Nested trees
-        work_inv = self.work_tree.root_inventory
         # NB: entries will include entries within the excluded ids/paths
         # because iter_entries_by_dir has no 'exclude' facility today.
-        entries = work_inv.iter_entries_by_dir(
+        entries = self.work_tree.iter_entries_by_dir(
             specific_file_ids=self.specific_file_ids, yield_parents=True)
         for path, existing_ie in entries:
             file_id = existing_ie.file_id
 
=== modified file 'bzrlib/inventory.py'
--- bzrlib/inventory.py	2011-12-18 12:46:49 +0000
+++ bzrlib/inventory.py	2012-02-17 17:04:19 +0000
@@ -850,22 +850,6 @@
             descend(self.root, u'')
         return accum
 
-    def directories(self):
-        """Return (path, entry) pairs for all directories, including the root.
-        """
-        accum = []
-        def descend(parent_ie, parent_path):
-            accum.append((parent_path, parent_ie))
-
-            kids = [(ie.name, ie) for ie in parent_ie.children.itervalues() if ie.kind == 'directory']
-            kids.sort()
-
-            for name, child_ie in kids:
-                child_path = osutils.pathjoin(parent_path, name)
-                descend(child_ie, child_path)
-        descend(self.root, u'')
-        return accum
-
     def path2id(self, relpath):
         """Walk down through directories to return entry of last component.
 
 
=== modified file 'bzrlib/merge.py'
--- bzrlib/merge.py	2012-01-24 16:19:04 +0000
+++ bzrlib/merge.py	2012-02-17 17:04:19 +0000
@@ -1260,9 +1260,9 @@
 
     def merge_names(self, file_id):
         def get_entry(tree):
-            if tree.has_id(file_id):
+            try:
                 return tree.root_inventory[file_id]
-            else:
+            except errors.NoSuchId:
                 return None
         this_entry = get_entry(self.this_tree)
         other_entry = get_entry(self.other_tree)
 
=== modified file 'bzrlib/tests/test_bundle.py'
--- bzrlib/tests/test_bundle.py	2012-01-30 14:18:22 +0000
+++ bzrlib/tests/test_bundle.py	2012-02-17 17:04:19 +0000
@@ -80,9 +80,6 @@
     def get_root_id(self):
         return self.root.file_id
 
-    def get_root_id(self):
-        return self.root.file_id
-
     def all_file_ids(self):
         return set(self.paths.keys())
 
@@ -114,8 +111,8 @@
         return kind
 
     def make_entry(self, file_id, path):
-        from bzrlib.inventory import (InventoryEntry, InventoryFile
-                                    , InventoryDirectory, InventoryLink)
+        from bzrlib.inventory import (InventoryFile , InventoryDirectory,
+            InventoryLink)
         name = os.path.basename(path)
         kind = self.kind(file_id)
         parent_id = self.parent_id(file_id)
@@ -158,6 +155,12 @@
     def get_file_revision(self, file_id):
         return self.inventory[file_id].revision
 
+    def get_file_size(self, file_id):
+        return self.inventory[file_id].text_size
+
+    def get_file_sha1(self, file_id):
+        return self.inventory[file_id].text_sha1
+
     def contents_stats(self, file_id):
         if file_id not in self.contents:
             return None, None
@@ -326,7 +329,7 @@
         self.assertTrue(btree.path2id("grandparent/parent/file") is None)
 
     def sorted_ids(self, tree):
-        ids = list(tree)
+        ids = list(tree.all_file_ids())
         ids.sort()
         return ids
 
 
=== modified file 'bzrlib/workingtree.py'
--- bzrlib/workingtree.py	2012-01-30 14:18:22 +0000
+++ bzrlib/workingtree.py	2012-02-17 17:04:19 +0000
@@ -2084,7 +2084,7 @@
         return osutils.lexists(self.abspath(path))
 
     def has_or_had_id(self, file_id):
-        if file_id == self.root_inventory.root.file_id:
+        if file_id == self.get_root_id():
             return True
         inv, inv_file_id = self._unpack_file_id(file_id)
         return inv.has_id(inv_file_id)
@@ -2912,7 +2912,9 @@
         This is the same order used by 'osutils.walkdirs'.
         """
         ## TODO: Work from given directory downwards
-        for path, dir_entry in self.root_inventory.directories():
+        for path, dir_entry in self.iter_entries_by_dir():
+            if dir_entry.kind != 'directory':
+                continue
             # mutter("search for unknowns in %r", path)
             dirabs = self.abspath(path)
             if not isdir(dirabs):