[RFC] detecting "file not found" and "directory not found" exceptions on Windows

John Arbash Meinel john at arbash-meinel.com
Tue Aug 12 00:09:30 BST 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark Hammond wrote:
> I was tracking down some test failures on Windows, a'la:
> 
> | WindowsError: [Error 267] The directory name is invalid:
> u'O:/window~1/testbzr-3ml2mo.tmp/nd_conflicts_wrong_parent_kind/work/parent\
> \*.*'
> 
> and came across an issue that might be worth fixing in a slightly broader
> context.
> 
> The short story is that code in osutils.py and transform.py is assuming
> os.listdir raises an exception with errno set to ENOTDIR - but this isn't
> true on Windows.  Looking a little further, I see that John and Alex already
> struck this issue in a slightly different context - eg, workingtree_4.py has
> the block:
> 
> """
> ...
>             except OSError, e:
>                 # on win32, python2.4 has e.errno == ERROR_DIRECTORY, but
>                 # python 2.5 has e.errno == EINVAL,
>                 #            and e.winerror == ERROR_DIRECTORY
>                 e_winerror = getattr(e, 'winerror', None)
>                 win_errors = (ERROR_DIRECTORY, ERROR_PATH_NOT_FOUND)
>                 # there may be directories in the inventory even though
>                 # this path is not a file on disk: so mark it as end of
>                 # iterator
>                 if e.errno in (errno.ENOENT, errno.ENOTDIR, errno.EINVAL):
>                     current_dir_info = None
>                 elif (sys.platform == 'win32'
> ...
> """
> 
> All os.listdir() calls, and potentially others, should be wrapped with an
> exception handler similar to this.  The listdir calls mentioned are easy to
> fix - but a quick grep shows other possibly suspect cases.  Sometimes we are
> sure the parent directory exists meaning ENOENT is OK (such as
> fancy_rename), but sometimes its not completely clear how it will be used
> (eg, Transport._translate_error()).
> 
> I've attached a patch that fixes just the test failures.  Before this patch
> my test suite says:
> 
> | Ran 13687 tests
> | FAILED (failures=66, errors=201, known_failure_count=14)
> | 960 tests skipped
> 
> and after I see:
> 
> | Ran 13687 tests
> | FAILED (failures=51, errors=106, known_failure_count=14)
> | 960 tests skipped
> 
> While this might be a good approach for the 1.6 branch, I think that causing
> such code to spread may be error prone long term (and there are already
> other inconsistencies in the handling).  I'm wondering it we should try and
> abstract this a little, introducing a new (osutils?) helper function that
> takes an exception object and indicates if it means "directory not found",
> then consolidating the similar code in workingtree_4, osutils and transform?
> Note that the "file not found" handling does seem portable between windows
> and linux, but that might be worth abstracting too?  If so, any advice on
> names, places, etc?  Even better, anyone want to take it on <wink>?
> 
> Thanks,
>  
> Mark
> 

I think it should be abstracted out to an osutils.list_dir() function,
and then we should spread that throughout the codebase.

By the way, even though this is RFC, please use "MERGE/RFC", especially
since I can't read your patches inline (because of Outlook), it would be
nice to at least be able to read them through Bundle Buggy.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkigxqoACgkQJdeBCYSNAAO4eQCgqe8HOmfT8Jvv5v4PsSOCqZCt
em4AnRQHao52h1Y8Ms4AiD5SnS9pf3RN
=9pkK
-----END PGP SIGNATURE-----



More information about the bazaar mailing list