Returning Windows file attribute information via os.stat()

[benhoyt]

BPO	21719
Nosy	@loewis, @vstinner, @benhoyt, @ethanfurman, @JimJJewett, @zware
Files	issue21719.patch issue21719-2.patch issue21719-3.patch issue21719-4.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 = 'https://github.com/zware'
closed_at = <Date 2014-06-19.15:02:20.309>
created_at = <Date 2014-06-11.13:22:41.541>
labels = ['type-feature', 'library']
title = 'Returning Windows file attribute information via os.stat()'
updated_at = <Date 2014-06-20.09:18:58.030>
user = 'https://github.com/benhoyt'

bugs.python.org fields:

activity = <Date 2014-06-20.09:18:58.030>
actor = 'vstinner'
assignee = 'zach.ware'
closed = True
closed_date = <Date 2014-06-19.15:02:20.309>
closer = 'zach.ware'
components = ['Library (Lib)']
creation = <Date 2014-06-11.13:22:41.541>
creator = 'benhoyt'
dependencies = []
files = ['35615', '35621', '35622', '35632']
hgrepos = []
issue_num = 21719
keywords = ['patch']
message_count = 17.0
messages = ['220266', '220272', '220274', '220275', '220416', '220439', '220464', '220467', '220497', '220498', '220499', '220563', '220989', '220990', '221062', '221064', '221081']
nosy_count = 6.0
nosy_names = ['loewis', 'vstinner', 'benhoyt', 'ethan.furman', 'Jim.Jewett', 'zach.ware']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue21719'
versions = ['Python 3.5']

[benhoyt]

I asked recently on python-dev [1] about adding a "st_winattrs" attribute to stat result objects on Windows, to return the full set of Windows file attribute bits, such as "hidden" or "compressed" status. Copying from that thread for a bit more context here:

Python's os.stat() simply discards most of the file attribute
information fetched via the Win32 system calls. On Windows, os.stat()
calls CreateFile to open the file and get the dwFileAttributes value,
but it throws it all away except the FILE_ATTRIBUTE_DIRECTORY and
FILE_ATTRIBUTE_READONLY bits. See CPython source at [2].

Given that os.stat() returns extended, platform-specific file
attributes on Linux and OS X platforms (for example,
st_blocks, st_rsize, etc), it seems that Windows is something of a
second-class citizen here.

There are several questions on StackOverflow about how to get this
information on Windows, and one has to resort to ctypes. For example,
[3].

To solve this problem, I think we should add a "st_winattrs" attribute to the object returned by os.stat() on
Windows. And we should add the relevant Win32 FILE_ATTRIBUTE_* constants to the "stat" module.

Then, similarly to existing code like hasattr(st, 'st_blocks') on
Linux, you could write a cross-platform function to determine if a
file was hidden, something like so:

    def is_hidden(path):
        if startswith(os.path.basename(path), '.'):
            return True
        st = os.stat(path)
        return getattr(st, 'st_winattrs', 0) & FILE_ATTRIBUTE_HIDDEN != 0

There was general support for this idea on python-dev (see [4] [5] [6]), so I'd love to see this in Python 3.5.

Basically we need to add a "st_winattrs" integer attribute to the win32_stat struct in posixmodule.c and add the FILE_ATTRIBUTE_* constants to _stat.c, as well as adding Windows-specific tests and documentation.

I've never compiled CPython on Windows, but I aim to provide a patch for this at some stage soonish. Feedback and other patches welcome in the meantime. :-)

[1] https://mail.python.org/pipermail/python-dev/2014-June/134990.html
[2] https://github.com/python/cpython/blob/master/Modules/posixmodule.c#L1462
[3] http://stackoverflow.com/a/6365265
[4] https://mail.python.org/pipermail/python-dev/2014-June/134993.html
[5] https://mail.python.org/pipermail/python-dev/2014-June/135006.html
[6] https://mail.python.org/pipermail/python-dev/2014-June/135007.html

[loewis]

Instead of the somewhat cryptic name "winattrs", I suggest to call it st_file_attributes, as it is called in the Windows API (actually, GetFileAttributesEx calls it dwFileAttributes, but st_file_attributes could be considered a Pythonic spelling of that).

[benhoyt]

Fair call -- "st_file_attributes" sounds good to me, and matches the prefix of the FILE_ATTRIBUTES_* constants better too.

[vstinner]

Instead of the somewhat cryptic name "winattrs", I suggest to call it st_file_attributes (...)

On Windows, os.stat() calls GetFileInformationByHandle() which fills a BY_HANDLE_FILE_INFORMATION structure, and this structure has a dwFileAttributes attribute. The os.stat() calls also GetFileType().
http://msdn.microsoft.com/en-us/library/windows/desktop/aa364952%28v=vs.85%29.aspx

So I agree that os.stat().st_file_attributes is less surprising for Windows developers and it is more homogenous with FILE_ATTRIBUTE_xxx constants.

[benhoyt]

I've got a patch for this. Need to finish the docs and add tests, and then I'll post here.

[benhoyt]

Full patch to add this to Python 3.5 attached:

• code changes to posixmodule.c and _stat.c
• tests added in test_os.py and test_stat.py
• docs added to os.rst and stat.rst

[jimjjewett]

Added version 3.5; is 3.4 somehow still correct?

Set stage to patch review as there are still comments to deal with, but it looks pretty close to commit review.

[zware]

Not sure what you were going for on the version, Jim; you added 3.4, but this is a new feature for 3.5.

[benhoyt]

Updated patch attached based on code reviews.

I'm replying to the code review here as when I tried to reply on bugs.python.org/review I got a Python exception, "AttributeError: NoneType has no attribute something or other". FYI, it seems Django is set up in debug mode, because it was the full colourful traceback with local vars etc. I didn't save this file, sorry!

Zach Ware
=========

Either way, these should probably be defined in stat.py as well (sorry; I should
have said 'as well as' instead of 'instead of' when I mentioned _stat.c in the
email thread).

I don't think so -- I thought the purpose of moving them to _stat.c was so you could use the windows.h constants rather than hard-coded integers? But if they're in stat.py, why do they need to be in _stat.c as well?

http://bugs.python.org/review/21719/diff/12149/Modules/_stat.c#newcode563

Modules/_stat.c:563: // For some reason FILE_ATTRIBUTE_INTEGRITY_STREAM and
PEP-7 bans C++ style comments.

Good point.

stoneleaf
=========

'covert' should be 'convert' - can you fix this as part of your patch? ;)

Fixed in latest patch.

Instead of spelling out FILE_ATTRIBUTE_ each time, can we abbreviate that part
to FA_ ?

I disagree. I realize FILE_ATTRIBUTE_* is longish, but it's what it's called in the Windows API, so will make these constants much more findable and obvious to Windows programmers (consistent with the Win32 documentation).

On non-Windows machines 'st_file_attributes' will be 0?

No, it won't be present at all (like the other non-cross-platform attributes).

There's no attribute for the value 8?

Correct -- see the Win32 API documentation and loewis's comment.

haypo
=====

It would be nice to document these options here :)

I don't think we should duplicate the Win32 documentation. Ours will just be less thorough and potentially get out of date. I agree we should add a link to the Win32 docs though.

Instead of == 0, you can use assertFalse.

Could, but I don't like this when I'm doing bit/integer comparisons. Explicit is better.

The purpose of the _stat module is to only provide data from the OS, to not
hardcode constants. So -1 to provide these constants on all platforms.

Agreed. See above.

If the constant is only available on new Visual Studio version, we may hardcode
the constant to avoid the dependency on a specific Visual Studio version. For
example, in unicodeobject.c I used:

#ifndef WC_ERR_INVALID_CHARS

define WC_ERR_INVALID_CHARS 0x0080

#endif

That's a good way of doing it. I'll use this approach.

Modules/posixmodule.c:1437: unsigned long st_file_attributes;
Should we use a DWORD here?

I don't think so. None of the other values in win32_stat (e.g., st_dev) are defined using DWORD or Win32-style types. See also loewis's comment.

loewis
======

We shouldn't replicate Microsoft's documentation (if for no other reason than
copyright). Instead, linking to

http://msdn.microsoft.com/en-us/library/windows/desktop/gg258117.aspx

might be helpful (as long as Microsoft preserves this URL).

Agreed. I'll make this change.

[zware]

Ben Hoyt wrote:

I'm replying to the code review here as when I tried to reply on
bugs.python.org/review I got a Python exception, "AttributeError:
NoneType has no attribute something or other".

Generally when you get that you can just hit back and try again, it will work by the 4th try ;)

I don't think so -- I thought the purpose of moving them to _stat.c
was so you could use the windows.h constants rather than hard-coded
integers? But if they're in stat.py, why do they need to be in
_stat.c as well?

The idea is that _stat.c will use system-provided values wherever possible, but stat.py should be as accurate as we can make it to provide a backup for when _stat isn't around (either when it's just not built, which isn't an issue on Windows, or in another Python implementation).

[benhoyt]

The idea is that _stat.c will use system-provided values wherever
possible, but stat.py should be as accurate as we can make it to
provide a backup for when _stat isn't around (either when it's just
not built, which isn't an issue on Windows, or in another Python
implementation).

Fair call. I've added the constants to stat.py as well. Updated patch attached.

[benhoyt]

Uploading a (hopefully final! :-) patch to fix Zach Ware's points from the code review:

1. use stat.FILE_ATTRIBUTE_DIRECTORY constant in test_os.py
2. break line length in stat.rst doc source

[zware]

Committed as 706fab0213db (with the wrong issue number), with just a couple of comment tweaks (mostly to shorten a couple more lines) and some committer drudge-work.

Thanks for your contribution, Ben!