[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