Bazaar Version Control System

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

Proposed by Jelmer Vernooij on 2012-02-17
Status: Merged
Approved by: Vincent Ladeuil on 2012-02-20
Approved revision: no longer in the source branch.
Merged at revision: 6470
Proposed branch: lp:~jelmer/bzr/less-inventory-access
Merge into: lp:bzr
Diff against target: 184 lines (+22/-39) 7 files modified
To merge this branch: bzr merge lp:~jelmer/bzr/less-inventory-access
Reviewer Review Type Date Requested Status
Vincent Ladeuil 2012-02-17 Approve on 2012-02-20
Review via email: mp+93612@code.launchpad.net

Commit Message

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

Description of the Change

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.

To post a comment you must log in.
lp:~jelmer/bzr/less-inventory-access updated on 2012-02-18
6469. By Patch Queue Manager <email address hidden> on 2012-02-18

(jelmer) Merge the 2.5 branch. (Jelmer Vernooij)

Vincent Ladeuil (vila) wrote :

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
lp:~jelmer/bzr/less-inventory-access updated on 2012-02-20
6470. By Patch Queue Manager <email address hidden> on 2012-02-20

(jelmer) Avoid direct access of tree inventories in a couple more places.
 (Jelmer Vernooij)

Preview Diff

1=== modified file 'bzrlib/builtins.py'
2--- bzrlib/builtins.py 2012-02-14 17:22:37 +0000
3+++ bzrlib/builtins.py 2012-02-17 17:04:19 +0000
4@@ -1650,10 +1650,8 @@
5 def run(self, dir=u'.'):
6 tree = WorkingTree.open_containing(dir)[0]
7 self.add_cleanup(tree.lock_read().unlock)
8- new_inv = tree.root_inventory
9 old_tree = tree.basis_tree()
10 self.add_cleanup(old_tree.lock_read().unlock)
11- old_inv = old_tree.root_inventory
12 renames = []
13 iterator = tree.iter_changes(old_tree, include_unchanged=True)
14 for f, paths, c, v, p, n, k, e in iterator:
15
16=== modified file 'bzrlib/bundle/bundle_data.py'
17--- bzrlib/bundle/bundle_data.py 2012-01-24 16:19:04 +0000
18+++ bzrlib/bundle/bundle_data.py 2012-02-17 17:04:19 +0000
19@@ -687,10 +687,9 @@
20 if new_path not in self.patches:
21 # If the entry does not have a patch, then the
22 # contents must be the same as in the base_tree
23- ie = self.base_tree.root_inventory[file_id]
24- if ie.text_size is None:
25- return ie.text_size, ie.text_sha1
26- return int(ie.text_size), ie.text_sha1
27+ text_size = self.base_tree.get_file_size(file_id)
28+ text_sha1 = self.base_tree.get_file_sha1(file_id)
29+ return text_size, text_sha1
30 fileobj = self.get_file(file_id)
31 content = fileobj.read()
32 return len(content), sha_string(content)
33@@ -701,7 +700,6 @@
34 This need to be called before ever accessing self.inventory
35 """
36 from os.path import dirname, basename
37- base_inv = self.base_tree.root_inventory
38 inv = Inventory(None, self.revision_id)
39
40 def add_entry(file_id):
41@@ -750,9 +748,9 @@
42
43 root_inventory = property(_get_inventory)
44
45- def __iter__(self):
46- for path, entry in self.inventory.iter_entries():
47- yield entry.file_id
48+ def all_file_ids(self):
49+ return set(
50+ [entry.file_id for path, entry in self.inventory.iter_entries()])
51
52 def list_files(self, include_root=False, from_dir=None, recursive=True):
53 # The only files returned by this are those from the version
54
55=== modified file 'bzrlib/commit.py'
56--- bzrlib/commit.py 2012-02-06 23:38:33 +0000
57+++ bzrlib/commit.py 2012-02-17 17:04:19 +0000
58@@ -834,11 +834,9 @@
59 deleted_paths = {}
60 # XXX: Note that entries may have the wrong kind because the entry does
61 # not reflect the status on disk.
62- # FIXME: Nested trees
63- work_inv = self.work_tree.root_inventory
64 # NB: entries will include entries within the excluded ids/paths
65 # because iter_entries_by_dir has no 'exclude' facility today.
66- entries = work_inv.iter_entries_by_dir(
67+ entries = self.work_tree.iter_entries_by_dir(
68 specific_file_ids=self.specific_file_ids, yield_parents=True)
69 for path, existing_ie in entries:
70 file_id = existing_ie.file_id
71
72=== modified file 'bzrlib/inventory.py'
73--- bzrlib/inventory.py 2011-12-18 12:46:49 +0000
74+++ bzrlib/inventory.py 2012-02-17 17:04:19 +0000
75@@ -850,22 +850,6 @@
76 descend(self.root, u'')
77 return accum
78
79- def directories(self):
80- """Return (path, entry) pairs for all directories, including the root.
81- """
82- accum = []
83- def descend(parent_ie, parent_path):
84- accum.append((parent_path, parent_ie))
85-
86- kids = [(ie.name, ie) for ie in parent_ie.children.itervalues() if ie.kind == 'directory']
87- kids.sort()
88-
89- for name, child_ie in kids:
90- child_path = osutils.pathjoin(parent_path, name)
91- descend(child_ie, child_path)
92- descend(self.root, u'')
93- return accum
94-
95 def path2id(self, relpath):
96 """Walk down through directories to return entry of last component.
97
98
99=== modified file 'bzrlib/merge.py'
100--- bzrlib/merge.py 2012-01-24 16:19:04 +0000
101+++ bzrlib/merge.py 2012-02-17 17:04:19 +0000
102@@ -1260,9 +1260,9 @@
103
104 def merge_names(self, file_id):
105 def get_entry(tree):
106- if tree.has_id(file_id):
107+ try:
108 return tree.root_inventory[file_id]
109- else:
110+ except errors.NoSuchId:
111 return None
112 this_entry = get_entry(self.this_tree)
113 other_entry = get_entry(self.other_tree)
114
115=== modified file 'bzrlib/tests/test_bundle.py'
116--- bzrlib/tests/test_bundle.py 2012-01-30 14:18:22 +0000
117+++ bzrlib/tests/test_bundle.py 2012-02-17 17:04:19 +0000
118@@ -80,9 +80,6 @@
119 def get_root_id(self):
120 return self.root.file_id
121
122- def get_root_id(self):
123- return self.root.file_id
124-
125 def all_file_ids(self):
126 return set(self.paths.keys())
127
128@@ -114,8 +111,8 @@
129 return kind
130
131 def make_entry(self, file_id, path):
132- from bzrlib.inventory import (InventoryEntry, InventoryFile
133- , InventoryDirectory, InventoryLink)
134+ from bzrlib.inventory import (InventoryFile , InventoryDirectory,
135+ InventoryLink)
136 name = os.path.basename(path)
137 kind = self.kind(file_id)
138 parent_id = self.parent_id(file_id)
139@@ -158,6 +155,12 @@
140 def get_file_revision(self, file_id):
141 return self.inventory[file_id].revision
142
143+ def get_file_size(self, file_id):
144+ return self.inventory[file_id].text_size
145+
146+ def get_file_sha1(self, file_id):
147+ return self.inventory[file_id].text_sha1
148+
149 def contents_stats(self, file_id):
150 if file_id not in self.contents:
151 return None, None
152@@ -326,7 +329,7 @@
153 self.assertTrue(btree.path2id("grandparent/parent/file") is None)
154
155 def sorted_ids(self, tree):
156- ids = list(tree)
157+ ids = list(tree.all_file_ids())
158 ids.sort()
159 return ids
160
161
162=== modified file 'bzrlib/workingtree.py'
163--- bzrlib/workingtree.py 2012-01-30 14:18:22 +0000
164+++ bzrlib/workingtree.py 2012-02-17 17:04:19 +0000
165@@ -2084,7 +2084,7 @@
166 return osutils.lexists(self.abspath(path))
167
168 def has_or_had_id(self, file_id):
169- if file_id == self.root_inventory.root.file_id:
170+ if file_id == self.get_root_id():
171 return True
172 inv, inv_file_id = self._unpack_file_id(file_id)
173 return inv.has_id(inv_file_id)
174@@ -2912,7 +2912,9 @@
175 This is the same order used by 'osutils.walkdirs'.
176 """
177 ## TODO: Work from given directory downwards
178- for path, dir_entry in self.root_inventory.directories():
179+ for path, dir_entry in self.iter_entries_by_dir():
180+ if dir_entry.kind != 'directory':
181+ continue
182 # mutter("search for unknowns in %r", path)
183 dirabs = self.abspath(path)
184 if not isdir(dirabs):