Regression: os.walk now using os.scandir() breaks bytes filenames on windows

[mont29]

BPO	25911
Nosy	@pfmoore, @db3l, @vstinner, @tjguk, @ideasman42, @benhoyt, @zware, @serhiy-storchaka, @eryksun, @zooba
PRs	[3.4] Backport CI config from master #2475
Files	walk_bytes.patch walk_bytes_2.patch test_walk_bytes_deprecate.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-03-29.21:57:59.389>
created_at = <Date 2015-12-19.15:39:38.268>
labels = ['type-bug', 'library', 'OS-windows']
title = 'Regression: os.walk now using os.scandir() breaks bytes filenames on windows'
updated_at = <Date 2017-06-28.17:13:22.562>
user = 'https://bugs.python.org/mont29'

bugs.python.org fields:

activity = <Date 2017-06-28.17:13:22.562>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2016-03-29.21:57:59.389>
closer = 'vstinner'
components = ['Library (Lib)', 'Windows']
creation = <Date 2015-12-19.15:39:38.268>
creator = 'mont29'
dependencies = []
files = ['41397', '41598', '41897']
hgrepos = []
issue_num = 25911
keywords = ['patch', '3.5regression']
message_count = 37.0
messages = ['256731', '256732', '256755', '256757', '256785', '256787', '256804', '256812', '256820', '256904', '256908', '256909', '256911', '258133', '258134', '258378', '259846', '259847', '259848', '260103', '260787', '261360', '261370', '261735', '262174', '262330', '262332', '262333', '262474', '262475', '262485', '262594', '262599', '262605', '262606', '262607', '262632']
nosy_count = 13.0
nosy_names = ['paul.moore', 'db3l', 'vstinner', 'tim.golden', 'ideasman42', 'SilentGhost', 'benhoyt', 'python-dev', 'zach.ware', 'serhiy.storchaka', 'eryksun', 'steve.dower', 'mont29']
pr_nums = ['2475']
priority = 'high'
resolution = 'fixed'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25911'
versions = ['Python 3.5', 'Python 3.6']

[mont29]

In py3.4 and below we used to be able to use bytes filenames with os.walk(), it’s now impossible under windows due to the limitation of os.scandir().

This issue was reported to Blender tracker (https://developer.blender.org/T47018).

I assume this should be considered as a regression? At least, this should be mentioned in the documentation…

Thanks,
Bastien

[SilentGhost]

Perhaps, I'm looking in the wrong place, but there doesn't seem be any test for bytes filenames.

[vstinner]

Regressions are not cool, but it was a deliberate choice to not support
bytes filenames in os.scandir(). If you use bytes, you can get filenames
which are invalid: you will be unable to use the filename with open() to
read its content for example.

Bytes filename are deprecated since python 3.2 if i recall correctly. It's
time to use the right time, it's also simpler to use on Python 3.

I suggest to document the regression rather than adding bytes support to
os.scandir or don't use scandir in os.walk().

[serhiy-storchaka]

For softer transition we can return old os.walk() implementation for bytes (may be with a deprecation warning). Using os.scandir() is just an optimization, and this shouldn't break existing program.

[ideasman42]

@Haypo, I checked available info online and couldn't find any reference to byte file-paths were deprecated since Python3.2.

On the contrary, the 3.2 release notes 0 state:

"countless fixes regarding bytes/string issues; among them full support for a bytes environment (filenames, environment variables)"

----
Also docs for open 3.2 1 and 3.5 2 say that byte filenames are supported with no mention of deprecation.

Since this is already working properly in 3.5 for other systems besides ms-windows, and worked in 3.4x.
Dropping support on a single platform seems a rather problematic regression.

----

[eryksun]

The ANSI API is problematic because it returns a best-fit encoding to the system codepage. For example:

    >>> os.listdir('.')
    ['ƠƨưƸǀLjǐǘǠǨǰǸ']

    >>> os.listdir(b'.')
    [b'O?u?|?iu?Kj?']

To somewhat work around this problem, listdir and scandir could return the cAlternateFilename of the WIN32_FIND_DATA struct if present. This is the classic 8.3 short name that Microsoft file systems create for MS-DOS compatibility. For NTFS it can be disabled in the registry, or per volume, but I assume whoever does that knows what to expect.

Also, since Python wouldn't need the short name for a wide-character path, there's no point in asking for it. (For NTFS it's a separate entry in the MFT. If it exists, which is known ahead of time, finding the entry requires a second lookup.) In this case it's better to call FindFirstFileExW and request only FindExInfoBasic. Generally the difference is inconsequential, but in a contrived example with 10000 similarly-named files from "ĀāĂă0000" to "ĀāĂă9999" and short names from "0000~1" to "9999~1", skipping the short name lookup shaved about 10% off the total time. For this test, I replaced the FindFirstFileW call in posix_scandir with the following call:

iterator->handle = FindFirstFileExW(path_strW,
                                    FindExInfoBasic,
                                    &iterator->file_data,
                                    FindExSearchNameMatch,
                                    NULL, 0);

[zware]

I like Serhiy's suggestion to use the old walk implementation on Windows if the argument is bytes, with a DeprecationWarning (like all other bytes operations in os on Windows). There was no warning that bytes paths would stop working with os.walk, so we should fix it and I'm bumping up the priority.

bytes filenames have been deprecated since Python 3.3 (bpo-13374); I don't think scandir (new in 3.5) should support bytes.

[vstinner]

Python 3.5.0 and 3.5.1 are out. I don't think that it's worth to add back
bytes support is os.walk() in 3.5.2. I would prefer to do the opposite and
really drop bytes support for all filenames in the whole stdlib in 3.6.

It's really a bad idea to use bytes on Windows. With Python 3, it becomes
possible and simple to handle filenames as Unicode on all platforms and in
all cases (see PEP-393 for undecodable filenames).

[benhoyt]

Just to clarify what Victor and Zachary mentioned: bytes filenames have been deprecated only *on Windows* since Python 3.3; bytes filenames are still fine on POSIX.

[serhiy-storchaka]

Here is a patch that restores the ability of os.walk() to work with bytes filenames on Windows.

[zooba]

Did we want a deprecation warning too? I don't see it in the patch.

Otherwise, LGTM.

[eryksun]

Considering there's no plan to implement bytes paths for scandir on Windows, then the following line in the os docs needs to be modified: "All functions accepting path or file names accept both bytes and string objects, and result in an object of the same type, if a path or file name is returned." It should be changed to acknowledge that some functions on Windows do not support bytes paths, and that existing support is deprecated and may be removed in a future release.

Also, the docs for listdir should warn that using bytes returns invalid results on Windows when filenames contain characters that aren't mapped in the system locale codepage.