classification
Title: Implement stat.st_dev and os.path.samefile on windows
Type: enhancement Stage: resolved
Components: Windows Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brian.curtin Nosy List: amaury.forgeotdarc, brian.curtin, python-dev, serhiy.storchaka
Priority: normal Keywords: needs review, patch

Created on 2011-04-27 16:59 by amaury.forgeotdarc, last changed 2013-01-01 18:32 by brian.curtin. This issue is now closed.

Files
File name Uploaded Description Edit
issue11939.diff brian.curtin, 2012-12-24 19:24 review
issue11939_v2.diff brian.curtin, 2012-12-24 21:33 review
issue11939_v3.diff brian.curtin, 2012-12-24 22:13 review
Messages (20)
msg134595 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-04-27 16:59
Since 9cd1036455e7, os.stat() on Windows fills the st_ino member;
it could fill st_dev just as easily;
then os.path.samefile() could be shared with the posix implementation.
msg134602 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2011-04-27 17:32
I created/assigned #10646 to myself for other samefile issues - I can cover this as well unless someone beats me to it.
msg178090 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-12-24 19:24
Here's a patch that fills st_dev, and while we're at it st_rdev (which is the same value).

I've moved the implementation of samefile/sameopenfile/samestat from Lib/posixpath.py over to Lib/genericpath.py and then removed the implementation from Lib/ntpath.py, so those functions are now common. The same goes for tests - I've rearranged tests towards test_genericpath. I also removed _getfileinformation from Modules/posixmodule.c because it's no longer being used.
msg178101 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-24 21:20
What about macpath? I think test_macpath will fail.
msg178102 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-12-24 21:32
Why do you think that? I don't have a mac so I can't test it.
msg178103 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-12-24 21:33
Here is an updated patch addressing the sameopenfile that remained in Lib/ntpath.py, thanks to Sehriy's comment on the review.
msg178104 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-24 21:55
Hmm. test_macpath will now run the tests for samefile etc. macpath doesn't contain os.path.samefile. But in Modules/posixmodule.c I don't see any special cases for classic MacOS. Actually, I don't know how tests behave on classic MacOS.

Please update the documentation (availability, versionchanged).
msg178105 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-24 22:01
On other hand, you should add "samefile", "sameopenfile" and "samestat" back to __all__ in posixpath and ntpath. __all__ is a list of exported names, not a list of defined names.
msg178108 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-12-24 22:13
Docs and the __all__ changes in V3 patch.
msg178206 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-26 13:03
New changeset 189b21f9bc0c by Brian Curtin in branch 'default':
Fix #11939. Set st_dev attribute on Windows to simplify os.path.samefile.
http://hg.python.org/cpython/rev/189b21f9bc0c
msg178207 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-26 13:11
New changeset 82531b78b719 by Brian Curtin in branch 'default':
Add NEWS entry for fixing #11939
http://hg.python.org/cpython/rev/82531b78b719
msg178208 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-12-26 13:12
Thanks for the reviews.
msg178211 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-26 13:36
New changeset 9e980454b85e by Brian Curtin in branch 'default':
Add tests for Issue #10646.
http://hg.python.org/cpython/rev/9e980454b85e
msg178218 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-26 14:16
Try on Windows:

>>> import os
>>> from os.path import *
>>> samestat(os.stat('.'), os.stat('.'))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'samestat' is not defined
msg178220 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-12-26 14:23
Separate issue. Fixed in #16788.
msg178712 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-31 18:00
New changeset 3738d270c54a by Brian Curtin in branch 'default':
st_dev/st_rdev should be unsigned long as dwVolumeSerialNumber, which it is set to, is a DWORD. This was fixed in #11939 and the overflow was mentioned in #10657 and seen by me on some machines.
http://hg.python.org/cpython/rev/3738d270c54a
msg178727 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-01 09:26
> New changeset 3738d270c54a by Brian Curtin in branch 'default':
> st_dev/st_rdev should be unsigned long as dwVolumeSerialNumber, which it is
> set to, is a DWORD. This was fixed in #11939 and the overflow was
> mentioned in #10657 and seen by me on some machines.
> http://hg.python.org/cpython/rev/3738d270c54a

But than st_dev used as long in _pystat_fromstructstat(). Perhaps you should 
check if st_dev is negative and then select 
PyLong_FromLong/PyLong_FromLongLong or 
PyLong_FromUnsignedLong/PyLong_FromUnsignedLongLong.
msg178739 - (view) Author: Roundup Robot (python-dev) Date: 2013-01-01 18:21
New changeset 61bada808b34 by Brian Curtin in branch 'default':
Set st_dev on Windows as unsigned long to match its DWORD type, related to the change to fix #11939.
http://hg.python.org/cpython/rev/61bada808b34
msg178740 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-01 18:29
Now the code is wrong on non-Windows without PY_LONG_LONG and with signed st_dev.
msg178742 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2013-01-01 18:32
Backed out the changeset. If you have a solution, feel free to fix it.
History
Date User Action Args
2013-01-01 18:32:19brian.curtinsetmessages: + msg178742
2013-01-01 18:29:55serhiy.storchakasetmessages: + msg178740
2013-01-01 18:21:53python-devsetmessages: + msg178739
2013-01-01 09:26:51serhiy.storchakasetmessages: + msg178727
2012-12-31 18:00:05python-devsetmessages: + msg178712
2012-12-31 17:52:19brian.curtinlinkissue10657 superseder
2012-12-26 14:23:08brian.curtinsetmessages: + msg178220
2012-12-26 14:16:38serhiy.storchakasetmessages: + msg178218
2012-12-26 13:36:31python-devsetmessages: + msg178211
2012-12-26 13:12:24brian.curtinsetstatus: open -> closed
resolution: fixed
messages: + msg178208

stage: patch review -> resolved
2012-12-26 13:11:42python-devsetmessages: + msg178207
2012-12-26 13:03:39python-devsetnosy: + python-dev
messages: + msg178206
2012-12-24 22:13:30brian.curtinsetfiles: + issue11939_v3.diff

messages: + msg178108
2012-12-24 22:01:26serhiy.storchakasetmessages: + msg178105
2012-12-24 21:55:00serhiy.storchakasetmessages: + msg178104
2012-12-24 21:33:16brian.curtinsetfiles: + issue11939_v2.diff

messages: + msg178103
2012-12-24 21:32:12brian.curtinsetmessages: + msg178102
2012-12-24 21:20:14serhiy.storchakasetnosy: + serhiy.storchaka

messages: + msg178101
versions: + Python 3.4, - Python 3.3
2012-12-24 19:24:15brian.curtinsetkeywords: + patch, needs review
files: + issue11939.diff
messages: + msg178090

stage: needs patch -> patch review
2011-04-27 17:32:14brian.curtinsetversions: + Python 3.3
nosy: + brian.curtin

messages: + msg134602

assignee: brian.curtin
stage: needs patch
2011-04-27 16:59:42amaury.forgeotdarccreate