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.

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

change sys.platform() to just "aix" for AIX #80769

Closed
aixtools opened this issue Apr 10, 2019 · 11 comments
Closed

change sys.platform() to just "aix" for AIX #80769

aixtools opened this issue Apr 10, 2019 · 11 comments
Labels
3.8 build The build process and cross-build

Comments

@aixtools
Copy link
Contributor

aixtools commented Apr 10, 2019

BPO 36588
Nosy @vstinner, @aixtools
PRs
  • bpo-36588: For AIX remove major version from sys.platform #12787
  • 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 2019-04-12.14:16:10.774>
    created_at = <Date 2019-04-10.15:04:50.538>
    labels = ['build', '3.8']
    title = 'change sys.platform() to just "aix" for AIX'
    updated_at = <Date 2019-04-13.12:28:44.062>
    user = 'https://github.com/aixtools'

    bugs.python.org fields:

    activity = <Date 2019-04-13.12:28:44.062>
    actor = 'Michael.Felt'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-04-12.14:16:10.774>
    closer = 'vstinner'
    components = ['Build']
    creation = <Date 2019-04-10.15:04:50.538>
    creator = 'Michael.Felt'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 36588
    keywords = ['patch']
    message_count = 11.0
    messages = ['339869', '339870', '339872', '339895', '339983', '340048', '340049', '340061', '340064', '340112', '340156']
    nosy_count = 2.0
    nosy_names = ['vstinner', 'Michael.Felt']
    pr_nums = ['12787']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue36588'
    versions = ['Python 3.8']

    @aixtools
    Copy link
    Contributor Author

    aixtools commented Apr 10, 2019

    This is something that probably shouts - boring - but back in 2012 it was a hot topic for linux2 and linux3.

    Maybe - as far back as 1996 (when AIX4 was new) "aix3" and "aix4" made sense. Whether that is true, or not - is pointless these days - for Python3.

    In the python code I have reviewed - for various reasons - I have never seen any code with "sys.platform() == "aixX" (where X is any of 5, 6, 7).

    There was a reference to "aix3" and "aix4" in setup.py (recently removed, and there is no replacement for aix5, aix6, or aix7 - not needed!)

    What I mostly see is sys.platform.startswith("aix"). The other form of the same test is sys.platform[:3] == 'aix'

    sys.platform is "build" related, e.g., potentially bound to libc issues. Even if this was the case (AIX offers since 2007 - official binary compatibility from old to new (when libc is dynamically linked, not static linked), was "unofficial" for aix3 and aix4).

    Yes, I am sure there are arguments along the line of "why change" since we have been updating it - always, and/or the documentation says so.

    linux2 had to stay, because there was known code that compared with linux2 (and that code was having problems when python was built on linux3 - hence the change to make sys.platform return linux2 for all Python3.2 and younger).

    FYI: in Cpython (master) there are no references to "aixX".

    All the references there are (in .py) are:

    michael@x071:[/data/prj/python/git/cpython-master]find . -name \*.py | xargs egrep "[\"']aix"
    ./Lib/asyncio/unix_events.py: if is_socket or (is_fifo and not sys.platform.startswith("aix")):
    ./Lib/ctypes/init.py: if _sys.platform.startswith("aix"):
    ./Lib/ctypes/util.py:elif sys.platform.startswith("aix"):
    ./Lib/ctypes/util.py: elif sys.platform.startswith("aix"):
    ./Lib/distutils/command/build_ext.py: elif sys.platform[:3] == 'aix':
    ./Lib/distutils/util.py: elif osname[:3] == "aix":
    ./Lib/sysconfig.py: elif osname[:3] == "aix":
    ./Lib/test/test_asyncio/test_events.py: if sys.platform.startswith("aix"):
    ./Lib/test/test_faulthandler.py: @unittest.skipIf(sys.platform.startswith('aix'),
    ./Lib/test/test_strftime.py: or sys.platform.startswith(("aix", "sunos", "solaris"))):
    ./Lib/test/test_strptime.py: @unittest.skipIf(sys.platform.startswith('aix'),
    ./Lib/test/test_locale.py: @unittest.skipIf(sys.platform.startswith('aix'),
    ./Lib/test/test_locale.py: @unittest.skipIf(sys.platform.startswith('aix'),
    ./Lib/test/test_fileio.py: not sys.platform.startswith(('sunos', 'aix')):
    ./Lib/test/test_tools/test_i18n.py: @unittest.skipIf(sys.platform.startswith('aix'),
    ./Lib/test/test_wait4.py: if sys.platform.startswith('aix'):
    ./Lib/test/test_c_locale_coercion.py:elif sys.platform.startswith("aix"):
    ./Lib/test/test_shutil.py:AIX = sys.platform[:3] == 'aix'
    ./Lib/test/test_utf8_mode.py: elif sys.platform.startswith("aix"):

    I'll write the patch - if I recall it should be a one-liner in configure.ac, but I think some discussion (or blessing) first is appropriate.

    Maybe even review whether other platforms no longer rely on the X for the platform.

    Hoping this helps!

    @aixtools aixtools added 3.9 3.8 build The build process and cross-build labels Apr 10, 2019
    @vstinner
    Copy link
    Member

    vstinner commented Apr 10, 2019

    I like the idea. Would you like to propose a patch? I suggest to only make such change in Python 3.8 and properly document it.

    @aixtools
    Copy link
    Contributor Author

    aixtools commented Apr 10, 2019

    On 10/04/2019 17:05, STINNER Victor wrote:

    STINNER Victor <vstinner@redhat.com> added the comment:

    I like the idea. Would you like to propose a patch? I suggest to only make such change in Python 3.8 and properly document it.

    ----------
    nosy: +vstinner


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue36588\>


    If I understand correctly, the change should be quite simple:

    diff --git a/configure.ac b/configure.ac
    index 73ee71c..9632add 100644
    --- a/configure.ac
    +++ b/configure.ac
    @@ -404,6 +404,7 @@ then
         MACHDEP="$ac_md_system$ac_md_release"

         case $MACHDEP in
    +       aix*) MACHDEP="aix";;
            linux*) MACHDEP="linux";;
            cygwin*) MACHDEP="cygwin";;
            darwin*) MACHDEP="darwin";;

    However, I am less familiar with (where) the appropriate documentation
    is. A pointer to the documentation is appreciated.

    @vstinner
    Copy link
    Member

    vstinner commented Apr 10, 2019

    However, I am less familiar with (where) the appropriate documentation is. A pointer to the documentation is appreciated.

    It would be nice to add a note to:

    In the source code, there are the files:

    @aixtools
    Copy link
    Contributor Author

    aixtools commented Apr 11, 2019

    Was:
    root@x064:[/data/prj/python/python3-3.8]./python
    Python 3.8.0a3+ (heads/bpo-28009-2-dirty:2fb2bc81c3, Apr 11 2019, 07:09:55) [C] on aix6
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import sys
    >>> sys.platform
    'aix6'
    >>> import os
    >>> os.uname()[3]
    '7'

    ./python ../git/python3-3.8/Tools/scripts/patchcheck.py
    Getting base branch for PR ... origin/master
    Getting the list of files that have been added/changed ... 6 files
    Fixing Python file whitespace ... 0 files
    Fixing C file whitespace ... 0 files
    Fixing docs whitespace ... 0 files
    Docs modified ... yes
    Misc/ACKS updated ... NO
    Misc/NEWS.d updated with blurb ... yes
    configure regenerated ... yes
    pyconfig.h.in regenerated ... no

    On system I built on:
    root@x066:[/data/prj/python/python3-3.8]./python
    Python 3.8.0a3+ (heads/bpo-36588-aix-platform-dirty:2021d40faa, Apr 11 2019, 15:16:05) [C] on aix
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import os,sys
    >>> sys.platform
    'aix'
    >>> os.uname()[3]
    '6'
    
    And on AIX7 system:
    root@x064:[/data/prj/python/python3-3.8]./python
    Python 3.8.0a3+ (heads/bpo-36588-aix-platform-dirty:2021d40faa, Apr 11 2019, 15:16:05) [C] on aix
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import os,sys
    >>> sys.platform
    'aix'
    >>> os.uname()[3]
    '7'

    @vstinner
    Copy link
    Member

    vstinner commented Apr 12, 2019

    New changeset 9d949f7 by Victor Stinner (Michael Felt) in branch 'master':
    bpo-36588: On AIX, remove major version from sys.platform (GH-12787)
    9d949f7

    @vstinner vstinner removed the 3.9 label Apr 12, 2019
    @vstinner
    Copy link
    Member

    vstinner commented Apr 12, 2019

    Do you want to work on a change to replace sys.platform.startswith("aix") to cleanup the stdlib and tests? Not sure if it's needed :-) It's up to you.

    @aixtools
    Copy link
    Contributor Author

    aixtools commented Apr 12, 2019

    On 12/04/2019 16:16, STINNER Victor wrote:

    STINNER Victor <vstinner@redhat.com> added the comment:

    Do you want to work on a change to replace sys.platform.startswith("aix") to cleanup the stdlib and tests? Not sure if it's needed :-) It's up to you.

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue36588\>


    Sure. I'll do that. I shoul open an issue along the lines of "AIX:
    cleanup stdlib and tests and how sys.platform is utilized".

    But, should I just continue standard practice (sys.platform), or would
    this be a moment to move towards platform.system() (i.e., set the
    example to be to use "run-time" rather than "build-time").

    Michael

    @vstinner
    Copy link
    Member

    vstinner commented Apr 12, 2019

    But, should I just continue standard practice (sys.platform), or would
    this be a moment to move towards platform.system() (i.e., set the
    example to be to use "run-time" rather than "build-time").

    Oh, now I'm confused :-) I checked the Python test suite: some tests use sys.platform == "linux" or sys.platform in ("linux", ...), some tests uses sys.platform.startswith("linux").

    In case of doubt, I suggest to do nothing :-) Leave the code unchanged :-)

    @aixtools
    Copy link
    Contributor Author

    aixtools commented Apr 12, 2019

    On 12/04/2019 17:34, STINNER Victor wrote:

    STINNER Victor <vstinner@redhat.com> added the comment:

    > But, should I just continue standard practice (sys.platform), or would
    > this be a moment to move towards platform.system() (i.e., set the
    > example to be to use "run-time" rather than "build-time").
    Oh, now I'm confused :-) I checked the Python test suite: some tests use sys.platform == "linux" or sys.platform in ("linux", ...), some tests uses sys.platform.startswith("linux").

    In case of doubt, I suggest to do nothing :-) Leave the code unchanged :-)

    Agreed, in case of doubt - leave alone (never change a winning team).

    And, to make it a short reply - I'll get started, and we see where it
    leads us.

    Michael

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue36588\>


    @aixtools
    Copy link
    Contributor Author

    aixtools commented Apr 13, 2019

    On 12/04/2019 23:16, Michael Felt wrote:

    Agreed, in case of doubt - leave alone (never change a winning team).

    And, to make it a short reply - I'll get started, and we see where it
    leads us.

    I opened bpo-36624 (https://bugs.python.org/issue36624) - before I
    "take off", some comments/discussion on what is desirable and achievable
    would be welcome.

    @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
    Labels
    3.8 build The build process and cross-build
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants