[MERGE] Refactor bzrlib.osutils._walkdirs_utf8 to aid API migration in future.

Robert Collins robertc at robertcollins.net
Tue Sep 9 23:21:25 BST 2008


On Tue, 2008-09-09 at 11:19 -0500, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > This patch refactors the guts of _walkdirs_utf8 to be more modular. 
> > 
> > In my testing its no slower than trunk (3ms, the expected overhead of a
> > call-per-dir on mozilla trees is below noise level). I believe my
> > changes to the win32 walker are correct - that is that the win32 feature
> > tests will pass and it will run well on win32; I would appreciate a
> > win32 developer (two are cc'd to this) confirming that this is the case
> > before I submit it to PQM [assuming it gets through review].
> > 
> > Cheers,
> > Rob
> > 
> 
> I'm attaching a patch which updates the code a bit:
> 
> 1) You don't have any direct tests for DirReader implementations. This
> made it difficult for me to understand what you actually expected
> 'top_prefix_to_starting_dir' to return. As the only test for the
> function was in the win32 walkdirs code, and it was failing.
> 
> You actually expect it to return a 5-tuple because you do:
> 
>  relroot, _, _, _, top = pending.pop()
> 
> but you implemented the Win32DirReader as just returning the 2-tuple
> (relroot, top).  I suppose you realized we only need the start and end,
> but didn't have another implementation that returned anything but the
> 5-tuple.
> 
> Also, I *think* in your test you swapped the order of 'prefix' and 'top'.
> 
> I didn't write any other direct tests, though it would probably be good
> to do so at some point.
> 
> 2) You accidentally used "top + XXX" rather than "top_slash + XXX" on a
> few occasions. Which meant we started searching for:
> 
>  foo* rather than foo/*
> 
> and would return paths as
> 
>  fooname rather than foo/name
> 
> 3) There was a failing test because 'read_dir()' isn't an iterator like
> walkdirs_utf8 was, and so an exception was being raised right away,
> rather than after a time.
> 
> 
> With my update, 'test_osutils.py' and 'test__walkdirs...' pass
> successfully on win32.

bb:approve for your tweaks; if you're happy with my code we can land
this.

Thanks immensely for fixing it up on win32.

-Rob


-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080910/06b3720b/attachment.pgp 


More information about the bazaar mailing list