Bazaar

Merge lp:~bzr/bzr/faster-ls into lp:~bzr/bzr/trunk-old

  1. faster-ls
  2. Merge into trunk-old
Proposed by Ian Clatworthy
Status: Superseded
Proposed branch: lp:~bzr/bzr/faster-ls
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 497 lines (has conflicts) 
Text conflict in bzrlib/builtins.py
To merge this branch: bzr merge lp:~bzr/bzr/faster-ls
Reviewer Review Type Date Requested Status
John A Meinel Needs Information
Review via email: mp+6670@code.launchpad.net

This proposal has been superseded by a proposal from 2009-06-17.

To post a comment you must log in.
Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

This patch makes ls faster. In particular, ls on a historical revision is faster: ls -r-1 on the OpenOffice trunk drops from 62 seconds to 1.3 seconds. Plain ls on a subdirectory gets faster as well, dropping from 3-4 seconds to around 1 second.

As well as improving performance, I've moved most of the ls logic out of builtins.py to ls.py. New unit tests are needed for this new public API. Before I add those, I'd like a reviewer to confirm:

1. the new API looks right
2. my changes to the blackbox tests are correct.

In the latter case, I believe that 'ls dir --from-root' ought to show just the stuff in dir, not everything (as the current test expects).

Revision history for this message
John A Meinel (jameinel) wrote :

I would *like* to see 'ls' work on top of "iter_entries_by_dir()" rather than list_files().

As near as I can tell, the only thing that list_files gives is "kind character" (though perhaps it also does more with unversioned files?)

I'd like to know if you think that is possible, or if it is too different of an api.

I'm not sure about having a file named "ls.py", maybe it's okay. I don't really have a much better name for it.

+# Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Canonical Ltd

I'm pretty sure 'ls.py' is only (C) 2009 :)

+def ls(tree, outf, from_dir=None, recursive=False, kind=None, unknown=False,
+ versioned=False, ignored=False, verbose=False, null=False, show_ids=False,
+ prefix=None, from_root=False):

^- When I see a function with that many parameters, I wonder if it wouldn't be better implemented as some sort of class... Not saying that is true here, but it seems like it might.

I think we should also think about whether it would be possible to add "bzr ls --added" more easily with this change or not. As near as I can tell, it would be ~ the same effort, so at least it isn't worse.

The main difficulty we've had is that "Versioned" is a state of a single tree, but "Added" is a diff between two trees. So you need an api that can give you the difference among 3 trees:
1) basis_tree
2) current working tree (versioned info)
3) filesystem

Since that gives you "Newly Versioned" "Renamed" "Ignored" etc.

As for the blackbox changes... You changed everything to be '--recurse' to '--no-recurse' (since the default changed), but you didn't add back any "--recurse" tests.

Also, is this the correct behavior:
        self.ls_equals('subdir/b\n'
                       , '--from-root')

I'm thinking it probably is, but I'll note that it is a change in behavior. 'bzr ls --from-root' used to be equivalent to 'bzr ls ../../' until you got to the root. We now don't have a way from a subdirectory to say "ls everything" without giving the full path to the root.

While doing "bzr ls --from-root ." would be an easy way to specify the opposite.

review: Needs Information
Revision history for this message
John A Meinel (jameinel) wrote :

Sorry, I hit 'Save' before I was done.

=== modified file 'bzrlib/transform.py'

I really wonder if this should be in "bzrlib/preview_tree.py". Not that you have to do that.

I noticed you didn't add "bzrlib/tests/test_ls.py". Since the code is now refactored, it would be nice if most of the 'blackbox/tests_ls.py' was actually done as whitebox tests. Again, not required, but if you at least added the test file and some very basic tests, it would be a place that could be obviously expanded later.

So overall, I think it is worth landing something like this.

Revision history for this message
Robert Collins (lifeless) wrote :

Just a couple of comments.

ls.py doesn't seem to know if its UI layer or logic layer.

I'd personally rather have a clear logic layer - something that takes
the needed parameters and provides an iterator over [semi-]structured
data.

then the command can be
for thing in [dols]:
    self.output.write("%s%s\n" % (thing[0], thing[1]))

This would make this useful for GUI's as well.

I think a good home for this would be tree.py, rather than a new python
module, as it is very tree specific code (in fact, in refactoring terms,
the first parameter 'tree' stands out like a ForeignMethod).

-Rob

Revision history for this message
Martin Pool (mbp) wrote :

> This patch makes ls faster. In particular, ls on a historical revision is
> faster: ls -r-1 on the OpenOffice trunk drops from 62 seconds to 1.3 seconds.
> Plain ls on a subdirectory gets faster as well, dropping from 3-4 seconds to
> around 1 second.

Just a meta-comment: also explaining in the cover letter what you changed to do that will help with reviews. People may have comments only on the approach without reading the patches and also it helps verify the patch does what was intended.

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

> Just a couple of comments.
>
> ls.py doesn't seem to know if its UI layer or logic layer.
>
> I'd personally rather have a clear logic layer - something that takes
> the needed parameters and provides an iterator over [semi-]structured
> data.
>
> then the command can be
> for thing in [dols]:
> self.output.write("%s%s\n" % (thing[0], thing[1]))
>
> This would make this useful for GUI's as well.
>
> I think a good home for this would be tree.py, rather than a new python
> module, as it is very tree specific code (in fact, in refactoring terms,
> the first parameter 'tree' stands out like a ForeignMethod).

All good points, though the use case for a GUI ls client isn't really there. (The functionality tends to be provided by far more powerful GUIs like qbrowse and Olive). I'll resubmit this in a less ambitious form, just focusing on the logic bug and performance issue for now.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/builtins.py'
2--- bzrlib/builtins.py 2009-06-11 06:54:33 +0000
3+++ bzrlib/builtins.py 2009-06-16 02:37:53 +0000
4@@ -2357,42 +2357,32 @@
5 recursive=False, from_root=False,
6 unknown=False, versioned=False, ignored=False,
7 null=False, kind=None, show_ids=False, path=None):
8-
9+ # Validate the command line options
10 if kind and kind not in ('file', 'directory', 'symlink'):
11 raise errors.BzrCommandError('invalid kind specified')
12-
13 if verbose and null:
14 raise errors.BzrCommandError('Cannot set both --verbose and --null')
15- all = not (unknown or versioned or ignored)
16-
17- selection = {'I':ignored, '?':unknown, 'V':versioned}
18-
19 if path is None:
20- fs_path = '.'
21- prefix = ''
22- else:
23- if from_root:
24- raise errors.BzrCommandError('cannot specify both --from-root'
25- ' and PATH')
26- fs_path = path
27- prefix = path
28- tree, branch, relpath = bzrdir.BzrDir.open_containing_tree_or_branch(
29- fs_path)
30- if from_root:
31- relpath = u''
32- elif relpath:
33- relpath += '/'
34+ path = '.'
35+ elif from_root:
36+ raise errors.BzrCommandError('cannot specify both --from-root'
37+ ' and PATH')
38+
39+ # Get the tree
40+ tree, branch, dir = bzrdir.BzrDir.open_containing_tree_or_branch(path)
41+ mutter("ls dir is %s", dir)
42 if revision is not None or tree is None:
43 tree = _get_one_revision_tree('ls', revision, branch=branch)
44
45- apply_view = False
46- if isinstance(tree, WorkingTree) and tree.supports_views():
47- view_files = tree.views.lookup_view()
48- if view_files:
49- apply_view = True
50- view_str = views.view_display_str(view_files)
51- note("Ignoring files outside view. View is %s" % view_str)
52+ # Calculate the prefix to use
53+ prefix = None
54+ if from_root:
55+ if dir:
56+ prefix = dir + '/'
57+ elif path != '.':
58+ prefix = path + '/'
59
60+<<<<<<< TREE
61 tree.lock_read()
62 try:
63 for fp, fc, fkind, fid, entry in tree.list_files(include_root=False):
64@@ -2436,6 +2426,13 @@
65 self.outf.write(outstring + '\n')
66 finally:
67 tree.unlock()
68+=======
69+ # Display the files
70+ from bzrlib import ls
71+ ls.ls(tree, self.outf, from_dir=dir, recursive=recursive,
72+ kind=kind, unknown=unknown, versioned=versioned, ignored=ignored,
73+ verbose=verbose, null=null, show_ids=show_ids, prefix=prefix)
74+>>>>>>> MERGE-SOURCE
75
76
77 class cmd_unknowns(Command):
78
79=== added file 'bzrlib/ls.py'
80--- bzrlib/ls.py 1970-01-01 00:00:00 +0000
81+++ bzrlib/ls.py 2009-06-16 02:37:53 +0000
82@@ -0,0 +1,115 @@
83+# Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Canonical Ltd
84+#
85+# This program is free software; you can redistribute it and/or modify
86+# it under the terms of the GNU General Public License as published by
87+# the Free Software Foundation; either version 2 of the License, or
88+# (at your option) any later version.
89+#
90+# This program is distributed in the hope that it will be useful,
91+# but WITHOUT ANY WARRANTY; without even the implied warranty of
92+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
93+# GNU General Public License for more details.
94+#
95+# You should have received a copy of the GNU General Public License
96+# along with this program; if not, write to the Free Software
97+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
98+
99+"""List files in a tree."""
100+
101+
102+from bzrlib.lazy_import import lazy_import
103+lazy_import(globals(), """
104+from bzrlib import (
105+ errors,
106+ osutils,
107+ )
108+from bzrlib.trace import mutter, note
109+from bzrlib.workingtree import WorkingTree
110+""")
111+
112+
113+def ls(tree, outf, from_dir=None, recursive=False, kind=None, unknown=False,
114+ versioned=False, ignored=False, verbose=False, null=False, show_ids=False,
115+ prefix=None, from_root=False):
116+ """List files for a tree.
117+
118+ If unknown, versioned and ignored are all False, then all are displayed.
119+
120+ :param tree: the tree to display files for
121+ :param outf: the output stream
122+ :param from_dir: just from this directory
123+ :param recursive: whether to recurse into subdirectories or not
124+ :param kind: one of 'file', 'symlink', 'directory' or None for all
125+ :param unknown: include unknown files or not
126+ :param versioned: include versioned files or not
127+ :param ignored: include ignored files or not
128+ :param verbose: show file kinds, not just paths
129+ :param null: separate entries with null characters instead of newlines
130+ :param show_ids: show file_ids or not
131+ :param prefix: prefix paths with this string or None for no prefix
132+ :param from_root: show paths from the root instead of relative
133+ """
134+ mutter("ls from: %s" % (from_dir,))
135+ # Tell the user if a view if being applied
136+ apply_view = False
137+ if isinstance(tree, WorkingTree) and tree.supports_views():
138+ view_files = tree.views.lookup_view()
139+ if view_files:
140+ apply_view = True
141+ view_str = views.view_display_str(view_files)
142+ note("Ignoring files outside view. View is %s" % view_str)
143+
144+ # Find and display the files
145+ all = not (unknown or versioned or ignored)
146+ selection = {'I':ignored, '?':unknown, 'V':versioned}
147+ #if from_dir and from_root:
148+ # prefix = from_dir
149+ #else:
150+ # prefix = None
151+ tree.lock_read()
152+ try:
153+ for fp, fc, fkind, fid, entry in tree.list_files(include_root=False,
154+ from_dir=from_dir, recursive=recursive):
155+ # Apply additional masking
156+ if not all and not selection[fc]:
157+ continue
158+ if kind is not None and fkind != kind:
159+ continue
160+ if apply_view:
161+ if from_dir:
162+ fullpath = osutils.pathjoin(from_dir, fp)
163+ else:
164+ fullpath = fp
165+ try:
166+ views.check_path_in_view(tree, fullpath)
167+ except errors.FileOutsideView:
168+ continue
169+
170+ # Output the entry
171+ kindch = entry.kind_character()
172+ if prefix is not None:
173+ fp = osutils.pathjoin(prefix, fp)
174+ outstring = fp + kindch
175+ if verbose:
176+ outstring = '%-8s %s' % (fc, outstring)
177+ if show_ids and fid is not None:
178+ outstring = "%-50s %s" % (outstring, fid)
179+ outf.write(outstring + '\n')
180+ elif null:
181+ outf.write(fp + '\0')
182+ if show_ids:
183+ if fid is not None:
184+ outf.write(fid)
185+ outf.write('\0')
186+ outf.flush()
187+ else:
188+ if show_ids:
189+ if fid is not None:
190+ my_id = fid
191+ else:
192+ my_id = ''
193+ outf.write('%-50s %s\n' % (outstring, my_id))
194+ else:
195+ outf.write(outstring + '\n')
196+ finally:
197+ tree.unlock()
198
199=== modified file 'bzrlib/revisiontree.py'
200--- bzrlib/revisiontree.py 2009-06-10 03:56:49 +0000
201+++ bzrlib/revisiontree.py 2009-06-16 02:37:53 +0000
202@@ -114,11 +114,18 @@
203 def has_filename(self, filename):
204 return bool(self.inventory.path2id(filename))
205
206- def list_files(self, include_root=False):
207+ def list_files(self, include_root=False, from_dir=None, recursive=True):
208 # The only files returned by this are those from the version
209- entries = self.inventory.iter_entries()
210- # skip the root for compatability with the current apis.
211- if self.inventory.root is not None and not include_root:
212+ inv = self.inventory
213+ if from_dir is None:
214+ from_dir_id = None
215+ else:
216+ from_dir_id = inv.path2id(from_dir)
217+ if from_dir_id is None:
218+ # Directory not versioned
219+ return
220+ entries = inv.iter_entries(from_dir=from_dir_id, recursive=recursive)
221+ if inv.root is not None and not include_root and from_dir is None:
222 # skip the root for compatability with the current apis.
223 entries.next()
224 for path, entry in entries:
225
226=== modified file 'bzrlib/tests/blackbox/test_ls.py'
227--- bzrlib/tests/blackbox/test_ls.py 2009-05-08 13:39:32 +0000
228+++ bzrlib/tests/blackbox/test_ls.py 2009-06-16 02:37:53 +0000
229@@ -129,19 +129,11 @@
230 self.ls_equals('b\n')
231 self.ls_equals('b\0'
232 , '--null')
233- self.ls_equals('.bzrignore\n'
234- 'a\n'
235- 'subdir/\n'
236- 'subdir/b\n'
237+ self.ls_equals('subdir/b\n'
238 , '--from-root')
239- self.ls_equals('.bzrignore\0'
240- 'a\0'
241- 'subdir\0'
242- 'subdir/b\0'
243+ self.ls_equals('subdir/b\0'
244 , '--from-root --null')
245- self.ls_equals('.bzrignore\n'
246- 'a\n'
247- 'subdir/\n'
248+ self.ls_equals('subdir/b\n'
249 , '--from-root', recursive=False)
250
251 def test_ls_path(self):
252
253=== modified file 'bzrlib/tests/tree_implementations/test_list_files.py'
254--- bzrlib/tests/tree_implementations/test_list_files.py 2009-03-23 14:59:43 +0000
255+++ bzrlib/tests/tree_implementations/test_list_files.py 2009-06-16 02:37:53 +0000
256@@ -49,7 +49,68 @@
257 try:
258 actual = [(path, status, kind, file_id)
259 for path, status, kind, file_id, ie in
260- tree.list_files(include_root=False)]
261+ tree.list_files()]
262+ finally:
263+ tree.unlock()
264+ self.assertEqual(expected, actual)
265+
266+ def test_list_files_with_root_no_recurse(self):
267+ work_tree = self.make_branch_and_tree('wt')
268+ tree = self.get_tree_no_parents_abc_content(work_tree)
269+ expected = [('', 'V', 'directory', 'root-id'),
270+ ('a', 'V', 'file', 'a-id'),
271+ ('b', 'V', 'directory', 'b-id'),
272+ ]
273+ tree.lock_read()
274+ try:
275+ actual = [(path, status, kind, file_id)
276+ for path, status, kind, file_id, ie in
277+ tree.list_files(include_root=True, recursive=False)]
278+ finally:
279+ tree.unlock()
280+ self.assertEqual(expected, actual)
281+
282+ def test_list_files_no_root_no_recurse(self):
283+ work_tree = self.make_branch_and_tree('wt')
284+ tree = self.get_tree_no_parents_abc_content(work_tree)
285+ expected = [('a', 'V', 'file', 'a-id'),
286+ ('b', 'V', 'directory', 'b-id'),
287+ ]
288+ tree.lock_read()
289+ try:
290+ actual = [(path, status, kind, file_id)
291+ for path, status, kind, file_id, ie in
292+ tree.list_files(recursive=False)]
293+ finally:
294+ tree.unlock()
295+ self.assertEqual(expected, actual)
296+
297+ def test_list_files_from_dir(self):
298+ work_tree = self.make_branch_and_tree('wt')
299+ tree = self.get_tree_no_parents_abc_content(work_tree)
300+ expected = [('c', 'V', 'file', 'c-id'),
301+ ]
302+ tree.lock_read()
303+ try:
304+ actual = [(path, status, kind, file_id)
305+ for path, status, kind, file_id, ie in
306+ tree.list_files(from_dir='b')]
307+ finally:
308+ tree.unlock()
309+ self.assertEqual(expected, actual)
310+
311+ def test_list_files_from_dir_no_recurse(self):
312+ # The test trees don't have much nesting so test with an explicit root
313+ work_tree = self.make_branch_and_tree('wt')
314+ tree = self.get_tree_no_parents_abc_content(work_tree)
315+ expected = [('a', 'V', 'file', 'a-id'),
316+ ('b', 'V', 'directory', 'b-id'),
317+ ]
318+ tree.lock_read()
319+ try:
320+ actual = [(path, status, kind, file_id)
321+ for path, status, kind, file_id, ie in
322+ tree.list_files(from_dir='', recursive=False)]
323 finally:
324 tree.unlock()
325 self.assertEqual(expected, actual)
326
327=== modified file 'bzrlib/transform.py'
328--- bzrlib/transform.py 2009-06-10 03:56:49 +0000
329+++ bzrlib/transform.py 2009-06-16 02:37:53 +0000
330@@ -1748,7 +1748,7 @@
331 if self._transform.final_file_id(trans_id) is None:
332 yield self._final_paths._determine_path(trans_id)
333
334- def _make_inv_entries(self, ordered_entries, specific_file_ids):
335+ def _make_inv_entries(self, ordered_entries, specific_file_ids=None):
336 for trans_id, parent_file_id in ordered_entries:
337 file_id = self._transform.final_file_id(trans_id)
338 if file_id is None:
339@@ -1791,14 +1791,41 @@
340 specific_file_ids):
341 yield unicode(self._final_paths.get_path(trans_id)), entry
342
343- def list_files(self, include_root=False):
344- """See Tree.list_files."""
345+ def _iter_entries_for_dir(self, dir_path):
346+ """Return path, entry for items in a directory without recursing down."""
347+ dir_file_id = self.path2id(dir_path)
348+ ordered_ids = []
349+ for file_id in self.iter_children(dir_file_id):
350+ trans_id = self._transform.trans_id_file_id(file_id)
351+ ordered_ids.append((trans_id, file_id))
352+ for entry, trans_id in self._make_inv_entries(ordered_ids):
353+ yield unicode(self._final_paths.get_path(trans_id)), entry
354+
355+ def list_files(self, include_root=False, from_dir=None, recursive=True):
356+ """See WorkingTree.list_files."""
357 # XXX This should behave like WorkingTree.list_files, but is really
358 # more like RevisionTree.list_files.
359- for path, entry in self.iter_entries_by_dir():
360- if entry.name == '' and not include_root:
361- continue
362- yield path, 'V', entry.kind, entry.file_id, entry
363+ if recursive:
364+ prefix = None
365+ if from_dir:
366+ prefix = from_dir + '/'
367+ entries = self.iter_entries_by_dir()
368+ for path, entry in entries:
369+ if entry.name == '' and not include_root:
370+ continue
371+ if prefix:
372+ if not path.startswith(prefix):
373+ continue
374+ path = path[len(prefix):]
375+ yield path, 'V', entry.kind, entry.file_id, entry
376+ else:
377+ if from_dir is None and include_root is True:
378+ root_entry = inventory.make_entry('directory', '',
379+ ROOT_PARENT, self.get_root_id())
380+ yield '', 'V', 'directory', root_entry.file_id, root_entry
381+ entries = self._iter_entries_for_dir(from_dir or '')
382+ for path, entry in entries:
383+ yield path, 'V', entry.kind, entry.file_id, entry
384
385 def kind(self, file_id):
386 trans_id = self._transform.trans_id_file_id(file_id)
387
388=== modified file 'bzrlib/workingtree.py'
389--- bzrlib/workingtree.py 2009-06-15 15:47:45 +0000
390+++ bzrlib/workingtree.py 2009-06-16 02:37:53 +0000
391@@ -1115,15 +1115,16 @@
392 def _kind(self, relpath):
393 return osutils.file_kind(self.abspath(relpath))
394
395- def list_files(self, include_root=False):
396- """Recursively list all files as (path, class, kind, id, entry).
397+ def list_files(self, include_root=False, from_dir=None, recursive=True):
398+ """List all files as (path, class, kind, id, entry).
399
400 Lists, but does not descend into unversioned directories.
401-
402 This does not include files that have been deleted in this
403- tree.
404+ tree. Skips the control directory.
405
406- Skips the control directory.
407+ :param include_root: if True, do not return an entry for the root
408+ :param from_dir: start from this directory or None for the root
409+ :param recursive: whether to recurse into subdirectories or not
410 """
411 # list_files is an iterator, so @needs_read_lock doesn't work properly
412 # with it. So callers should be careful to always read_lock the tree.
413@@ -1131,7 +1132,7 @@
414 raise errors.ObjectNotLocked(self)
415
416 inv = self.inventory
417- if include_root is True:
418+ if from_dir is None and include_root is True:
419 yield ('', 'V', 'directory', inv.root.file_id, inv.root)
420 # Convert these into local objects to save lookup times
421 pathjoin = osutils.pathjoin
422@@ -1144,13 +1145,22 @@
423 fk_entries = {'directory':TreeDirectory, 'file':TreeFile, 'symlink':TreeLink}
424
425 # directory file_id, relative path, absolute path, reverse sorted children
426- children = os.listdir(self.basedir)
427+ if from_dir is not None:
428+ from_dir_id = inv.path2id(from_dir)
429+ if from_dir_id is None:
430+ # Directory not versioned
431+ return
432+ from_dir_abspath = pathjoin(self.basedir, from_dir)
433+ else:
434+ from_dir_id = inv.root.file_id
435+ from_dir_abspath = self.basedir
436+ children = os.listdir(from_dir_abspath)
437 children.sort()
438 # jam 20060527 The kernel sized tree seems equivalent whether we
439 # use a deque and popleft to keep them sorted, or if we use a plain
440 # list and just reverse() them.
441 children = collections.deque(children)
442- stack = [(inv.root.file_id, u'', self.basedir, children)]
443+ stack = [(from_dir_id, u'', from_dir_abspath, children)]
444 while stack:
445 from_dir_id, from_dir_relpath, from_dir_abspath, children = stack[-1]
446
447@@ -1214,14 +1224,15 @@
448 if fk != 'directory':
449 continue
450
451- # But do this child first
452- new_children = os.listdir(fap)
453- new_children.sort()
454- new_children = collections.deque(new_children)
455- stack.append((f_ie.file_id, fp, fap, new_children))
456- # Break out of inner loop,
457- # so that we start outer loop with child
458- break
459+ # But do this child first if recursing down
460+ if recursive:
461+ new_children = os.listdir(fap)
462+ new_children.sort()
463+ new_children = collections.deque(new_children)
464+ stack.append((f_ie.file_id, fp, fap, new_children))
465+ # Break out of inner loop,
466+ # so that we start outer loop with child
467+ break
468 else:
469 # if we finished all children, pop it off the stack
470 stack.pop()
471
472=== modified file 'bzrlib/workingtree_4.py'
473--- bzrlib/workingtree_4.py 2009-06-11 04:23:53 +0000
474+++ bzrlib/workingtree_4.py 2009-06-16 02:37:53 +0000
475@@ -1844,12 +1844,19 @@
476 return None
477 return ie.executable
478
479- def list_files(self, include_root=False):
480+ def list_files(self, include_root=False, from_dir=None, recursive=True):
481 # We use a standard implementation, because DirStateRevisionTree is
482 # dealing with one of the parents of the current state
483 inv = self._get_inventory()
484- entries = inv.iter_entries()
485- if self.inventory.root is not None and not include_root:
486+ if from_dir is None:
487+ from_dir_id = None
488+ else:
489+ from_dir_id = inv.path2id(from_dir)
490+ if from_dir_id is None:
491+ # Directory not versioned
492+ return
493+ entries = inv.iter_entries(from_dir=from_dir_id, recursive=recursive)
494+ if inv.root is not None and not include_root and from_dir is None:
495 entries.next()
496 for path, entry in entries:
497 yield path, 'V', entry.kind, entry.file_id, entry