Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
vstinner opened this issue Mar 23, 2015 · 12 comments
Closed

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

vstinner opened this issue Mar 23, 2015 · 12 comments

Comments

@vstinner
Copy link
Member

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
    Copy link
    Member Author

    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
    Copy link
    Member Author

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

    @serhiy-storchaka
    Copy link
    Member

    See bpo-22623 for moving in opposite direction.

    @vstinner
    Copy link
    Member Author

    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
    Copy link
    Mannequin

    neologix mannequin commented Mar 23, 2015

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

    @serhiy-storchaka
    Copy link
    Member

    See also bpo-12082.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Mar 23, 2015

    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
    Copy link
    Member Author

    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
    Copy link
    Mannequin

    python-dev mannequin commented Mar 24, 2015

    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
    Copy link
    Member

    -#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
    Copy link
    Mannequin

    python-dev mannequin commented Mar 24, 2015

    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
    Copy link
    Member Author

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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    None yet
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @serhiy-storchaka