Drop HAVE_FSTAT: require fstat() to compile/use Python

[vstinner]

BPO	23753
Nosy	@pitrou, @vstinner, @serhiy-storchaka
Files	stat.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 2015-03-24.11:23:35.400>
created_at = <Date 2015-03-23.17:16:39.919>
labels = []
title = 'Drop HAVE_FSTAT: require fstat() to compile/use Python'
updated_at = <Date 2015-03-24.11:23:35.399>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2015-03-24.11:23:35.399>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-03-24.11:23:35.400>
closer = 'vstinner'
components = []
creation = <Date 2015-03-23.17:16:39.919>
creator = 'vstinner'
dependencies = []
files = ['38660']
hgrepos = []
issue_num = 23753
keywords = ['patch']
message_count = 12.0
messages = ['239047', '239061', '239064', '239068', '239075', '239076', '239078', '239102', '239103', '239105', '239111', '239112']
nosy_count = 5.0
nosy_names = ['pitrou', 'vstinner', 'neologix', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue23753'
versions = ['Python 3.5']

[vstinner]

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.

[vstinner]

stat.patch: Stop pretending that Python works without stat() nor fstat(), consider that these functions are always available.

[serhiy-storchaka]

See bpo-22623 for moving in opposite direction.

[vstinner]

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(-)

[neologix]

+1 from me, fstat() has always been par of POSIX.
It's really likely Python won't build anyway on such systems.

[serhiy-storchaka]

See also bpo-12082.

[neologix]

Serhiy Storchaka added the comment:

See also bpo-12082.

Yes, but I don't think we want to clutter the code to support exotic
niche platforms.

[vstinner]

Antoine and Charles-François are in favor of removing these #ifdef.

Serhiy wrote:

See bpo-22623 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."

[python-dev]

New changeset a84eae63b4cd by Victor Stinner in branch 'default':
Issue bpo-23753: Python doesn't support anymore platforms without stat() or
https://hg.python.org/cpython/rev/a84eae63b4cd

[serhiy-storchaka]

-#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)

[python-dev]

New changeset ad5521dd7b80 by Victor Stinner in branch 'default':
Issue bpo-23753: Move _Py_wstat() from Python/fileutils.c to Modules/getpath.c
https://hg.python.org/cpython/rev/ad5521dd7b80

[vstinner]

-#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.