classification
Title: Drop HAVE_FSTAT: require fstat() to compile/use Python
Type: Stage:
Components: Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: neologix, pitrou, python-dev, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2015-03-23 17:16 by vstinner, last changed 2015-03-24 11:23 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
stat.patch vstinner, 2015-03-23 20:31 review
Messages (12)
msg239047 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-03-23 17:16
Topic previously discussed at:
https://mail.python.org/pipermail/python-dev/2013-May/126285.html

Related issue:
http://bugs.python.org/issue12082

Antoine Pitrou wrote in the issue:
"I would personally like to remove HAVE_FSTAT and make Python unconditionally use fstat(). It will make the code quite simpler in some places."

I agree. I'm quite sure that Python doesn't work on such platform, and it would require much more changes than just making fstat optional.

So I'm in favor of dropping the check on fstat() and expect it to be always available.

Examples of Python modules of the standard library using os.fstat:

- fileinput
- genericpath
- netrc
- os which contains "set.add(stat) # fstat always works"
- _pyio (the call is optional, it catchs AttributeError)
- shutil
- socket
- tarfile
- asyncio
- http.client (optional, catch AttributeError)
- http.server
- logging
- io
- etc.
msg239061 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-03-23 20:31
stat.patch: Stop pretending that Python works without stat() nor fstat(), consider that these functions are always available.
msg239064 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-23 20:48
See issue22623 for moving in opposite direction.
msg239068 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-03-23 21:06
My changes only *removes* code and so make it simpler ;-)

$ diffstat stat.patch 
 Include/fileutils.h  |    6 ------
 Include/pyport.h     |   22 ----------------------
 Modules/_io/fileio.c |   20 --------------------
 Modules/mmapmodule.c |    4 ----
 Python/fileutils.c   |   16 ----------------
 Python/marshal.c     |    4 ----
 6 files changed, 72 deletions(-)
msg239075 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2015-03-23 21:35
+1 from me, fstat() has always been par of POSIX.
It's really likely Python won't build anyway on such systems.
msg239076 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-23 21:50
See also issue12082.
msg239078 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2015-03-23 22:38
> Serhiy Storchaka added the comment:
>
> See also issue12082.

Yes, but I don't think we want to clutter the code to support exotic
niche platforms.
msg239102 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-03-24 09:29
Antoine and Charles-François are in favor of removing these #ifdef.

Serhiy wrote:
> See issue22623 for moving in opposite direction.

Not exactly, Link Mauve wrote "On those two platforms, fstat() is correctly defined and works fine, so it shouldn’t be a problem to drop its #ifdef."
msg239103 - (view) Author: Roundup Robot (python-dev) Date: 2015-03-24 09:29
New changeset a84eae63b4cd by Victor Stinner in branch 'default':
Issue #23753: Python doesn't support anymore platforms without stat() or
https://hg.python.org/cpython/rev/a84eae63b4cd
msg239105 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-24 10:12
> -#if defined(HAVE_STAT) && !defined(MS_WINDOWS)

This doesn't look correct. An equivalent replacement is 

-#if defined(HAVE_STAT) && !defined(MS_WINDOWS)
+#if !defined(MS_WINDOWS)
msg239111 - (view) Author: Roundup Robot (python-dev) Date: 2015-03-24 11:19
New changeset ad5521dd7b80 by Victor Stinner in branch 'default':
Issue #23753: Move _Py_wstat() from Python/fileutils.c to Modules/getpath.c
https://hg.python.org/cpython/rev/ad5521dd7b80
msg239112 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-03-24 11:23
> -#if defined(HAVE_STAT) && !defined(MS_WINDOWS)
> This doesn't look correct. An equivalent replacement is 

Oh, I missed the "!". Only _Py_wstat() uses this test. Windows has _wstat(), so _Py_wstat() could use it.

I added deliberately "!defined(MS_WINDOWS)" because _Py_wstat() is only used in Modules/getpath.c and this file is not compiled on Windows.

Instead of modifying _Py_wstat(), I moved it back to Modules/getpath.c. There is no need to overengineer this function only called 3 times in getpath.c.
History
Date User Action Args
2015-04-09 13:20:48r.david.murraylinkissue12082 superseder
2015-03-24 11:23:35vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg239112
2015-03-24 11:19:30python-devsetmessages: + msg239111
2015-03-24 10:12:50serhiy.storchakasetmessages: + msg239105
2015-03-24 09:29:52python-devsetnosy: + python-dev
messages: + msg239103
2015-03-24 09:29:42vstinnersetmessages: + msg239102
2015-03-23 22:38:57neologixsetmessages: + msg239078
2015-03-23 21:50:42serhiy.storchakasetmessages: + msg239076
2015-03-23 21:35:06neologixsetmessages: + msg239075
2015-03-23 21:06:22vstinnersetmessages: + msg239068
2015-03-23 20:48:31serhiy.storchakasetmessages: + msg239064
2015-03-23 20:32:14vstinnersetnosy: + serhiy.storchaka
2015-03-23 20:31:45vstinnersetfiles: + stat.patch
keywords: + patch
messages: + msg239061
2015-03-23 17:16:39vstinnercreate