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

Remove the concept of platform-specific directories #72233

Closed
zware opened this issue Sep 9, 2016 · 35 comments
Closed

Remove the concept of platform-specific directories #72233

zware opened this issue Sep 9, 2016 · 35 comments
Labels
3.7 (EOL) end of life build The build process and cross-build type-feature A feature request or enhancement

Comments

@zware
Copy link
Member

zware commented Sep 9, 2016

BPO 28046
Nosy @doko42, @vstinner, @ned-deily, @xdegaye, @zware, @moreati, @yan12125
PRs
  • bpo-28046: Remove MACHDEPPATH from Modules/Setup.dist #5289
  • Files
  • remove_plat.diff
  • get_sysconfigdata_name.diff
  • sysconfigdata_env_var.diff
  • sysconfigdata_ABIFLAGS.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 2018-01-24.16:20:41.981>
    created_at = <Date 2016-09-09.17:58:33.545>
    labels = ['type-feature', '3.7', 'build']
    title = 'Remove the concept of platform-specific directories'
    updated_at = <Date 2018-01-24.16:20:41.980>
    user = 'https://github.com/zware'

    bugs.python.org fields:

    activity = <Date 2018-01-24.16:20:41.980>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-01-24.16:20:41.981>
    closer = 'vstinner'
    components = ['Build', 'Cross-Build']
    creation = <Date 2016-09-09.17:58:33.545>
    creator = 'zach.ware'
    dependencies = []
    files = ['44506', '44530', '44532', '45232']
    hgrepos = []
    issue_num = 28046
    keywords = ['patch']
    message_count = 35.0
    messages = ['275360', '275363', '275427', '275429', '275430', '275433', '275436', '275438', '275442', '275475', '275477', '275518', '275519', '275532', '275592', '275599', '275635', '275638', '275642', '275643', '275650', '275664', '275667', '275677', '275711', '275845', '275846', '275850', '276245', '279508', '279592', '303426', '310539', '310605', '310608']
    nosy_count = 8.0
    nosy_names = ['doko', 'vstinner', 'ned.deily', 'xdegaye', 'python-dev', 'zach.ware', 'Alex.Willmer', 'yan12125']
    pr_nums = ['5289']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue28046'
    versions = ['Python 3.6', 'Python 3.7']

    @zware
    Copy link
    Member Author

    zware commented Sep 9, 2016

    As a follow-on to bpo-28027, the attached patch removes the concept of platform-specific directories (Lib/plat-darwin, Lib/plat-x86_64-linux-gnu, etc). I believe this should be fine for cross-builds (at least as fine as the previous solution for _sysconfigdata), but I'd like someone to test it for me since I have no experience with cross-builds.

    @zware zware self-assigned this Sep 9, 2016
    @zware zware added build The build process and cross-build type-feature A feature request or enhancement labels Sep 9, 2016
    @vstinner
    Copy link
    Member

    vstinner commented Sep 9, 2016

    You might also remove Lib/_sysconfigdata.py from .gitignore.

    @doko42
    Copy link
    Member

    doko42 commented Sep 9, 2016

    not sure if I fully understand:

    • The patch encodes the multiarch triplet into the _sysconfigdata
      name and installs it into the standard lib dir.
    • It removes the PLATDIR macro
    • what happens to the constants modules which are currently
      found in the platdir? These vary across architectures and
      should not be install in the standard lib dir.

    @zware
    Copy link
    Member Author

    zware commented Sep 9, 2016

    Those were removed in bpo-28027, by BDFL decree.

    @doko42
    Copy link
    Member

    doko42 commented Sep 9, 2016

    this comes as a surprise. I can't remember that discussion and decision.

    @zware
    Copy link
    Member Author

    zware commented Sep 9, 2016

    Sorry, it happened at the sprint at Instagram. Do you have a use for those modules that we need to reconsider?

    @vstinner
    Copy link
    Member

    vstinner commented Sep 9, 2016

    this comes as a surprise. I can't remember that discussion and decision.

    Removing these modules is not a new idea: I just posted a message to the issue bpo-28027 to complete the history of this change.

    @doko42
    Copy link
    Member

    doko42 commented Sep 9, 2016

    The removal is not following the guidelines to first deprecate these for a release, and then to remove them. I know that at least the RTLD_* constants are used in a number of places, which will just stop working.

    I'm not against removing these in 3.7, however they should be deprecated in 3.6 first. There is no harm done with this approach.

    @vstinner
    Copy link
    Member

    vstinner commented Sep 9, 2016

    Matthias Klose added the comment:

    The removal is not following the guidelines to first deprecate these for a release, and then to remove them. I know that at least the RTLD_* constants are used in a number of places, which will just stop working.

    Please continue the discussion in the issue bpo-28027, reopen it if you want.

    @zware
    Copy link
    Member Author

    zware commented Sep 9, 2016

    Xavier: Could you please make sure this doesn't break anything for Android/cross-builds?

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Sep 9, 2016

    Sure, I was already planning to do it :)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 10, 2016

    New changeset 90396ec9a2f8 by Zachary Ware in branch 'default':
    Issue bpo-28046: Remove platform-specific directories from sys.path
    https://hg.python.org/cpython/rev/90396ec9a2f8

    @zware
    Copy link
    Member Author

    zware commented Sep 10, 2016

    Xavier: I'm still interested in hearing whether this breaks anything for you, but I went ahead and pushed it.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 10, 2016

    New changeset 2bfe63a3eb5c by Zachary Ware in branch 'default':
    Issue bpo-28046: Fix distutils
    https://hg.python.org/cpython/rev/2bfe63a3eb5c

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Sep 10, 2016

    Seems after this changeset Python is broken:

    shell@ASUS_Z00E_2:/data/local/tmp/python3 $ python3.6
    Failed to import the site module
    Traceback (most recent call last):
      File "/data/local/tmp/python3/lib/python3.6/site.py", line 549, in <module>
        main()
      File "/data/local/tmp/python3/lib/python3.6/site.py", line 536, in main
        known_paths = addusersitepackages(known_paths)
      File "/data/local/tmp/python3/lib/python3.6/site.py", line 281, in addusersitepackages
        user_site = getusersitepackages()
      File "/data/local/tmp/python3/lib/python3.6/site.py", line 257, in getusersitepackages
        user_base = getuserbase() # this will also set USER_BASE
      File "/data/local/tmp/python3/lib/python3.6/site.py", line 247, in getuserbase
        USER_BASE = get_config_var('userbase')
      File "/data/local/tmp/python3/lib/python3.6/sysconfig.py", line 600, in get_config_var
        return get_config_vars().get(name)
      File "/data/local/tmp/python3/lib/python3.6/sysconfig.py", line 549, in get_config_vars
        _init_posix(_CONFIG_VARS)
      File "/data/local/tmp/python3/lib/python3.6/sysconfig.py", line 420, in _init_posix
        _temp = __import__(name, globals(), locals(), ['build_time_vars'], 0)
    ModuleNotFoundError: No module named '_sysconfigdata_m_linux_aarch64-linux-android'

    sysconfig data is installed as lib-dynload/_sysconfigdata_m_linux_x86_64-linux-gnu.py. It should be at lib-dynload/_sysconfigdata_m_linux_aarch64-linux-android.py?

    PS: I'm building for Android AArch64 on Linux x86_64

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Sep 10, 2016

    Got it. When calling python -m sysconfig --generate-posix-vars, the host (Linux x86_64) Python is used, so _get_sysconfigdata_name() returns the name for Linux instead of for Android. Maybe _get_sysconfigdata_name() should check whether it's used for cross-compiling or not.

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Sep 10, 2016

    Confirming the problem reported by Chi Hsuan Yen. The attached patch fixes this.

    Another problem is that the shared libraries names of the extension modules are now suffixed with the wrong triplet, i.e. with the build system triplet instead of the target host triplet. So they cannot be imported. Not sure where this regression has been introduced.

    @zware
    Copy link
    Member Author

    zware commented Sep 10, 2016

    Xavier, that change looks good to me, please commit it.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 10, 2016

    New changeset 1d71ce4531ac by Xavier de Gaye in branch 'default':
    Issue bpo-28046: Fix get_sysconfigdata_name().
    https://hg.python.org/cpython/rev/1d71ce4531ac

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Sep 10, 2016

    With this patch no extensions in Modules/ can be built. Let me check whether there's a bug in my build script.

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Sep 10, 2016

    Hmm things are quite complicated. Brief: the build is broken for out-of-source cross-compiling if the host Python is an in-source build.

    (Below $build_dir refers to the directory that invokes $source_dir/configure)

    In an out-of-source build, setup.py relies on sysconfig.get_config_var('srcdir') to get correct filenames. In _init_posix(), _get_sysconfigdata_name() still returns an incorrect name (for example, _sysconfigdata_m_linux_x86_64-linux-gnu, on Linux x86_64). There's no _sysconfigdata_m_linux_x86_64-linux-gnu.py in $build_dir/build/lib.linux-aarch64-3.6, so $PYTHON_FOR_BUILD imports the one from the host python, which has srcdir == '.' As a result, build_ext can't find source files:

    building 'xxlimited' extension
    /home/yen/Projects/python3-android/clang-bin/cc -fPIC -Wno-unused-result -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -g -fvar-tracking-assignments -g -fvar-tracking-assignments -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -g -fvar-tracking-assignments -g -fvar-tracking-assignments -std=c99 -DPy_LIMITED_API=0x03050000 -I./Include -I. -IInclude -I/usr/include -I/usr/local/include -I/home/yen/Projects/python3-android/src/cpython/build-target/Include -I/home/yen/Projects/python3-android/src/cpython/build-target -c xxlimited.c -o build/temp.linux-aarch64-3.6/xxlimited.o
    clang: error: unknown argument: '-fvar-tracking-assignments'
    clang: error: unknown argument: '-fvar-tracking-assignments'
    clang: error: unknown argument: '-fvar-tracking-assignments'
    clang: error: unknown argument: '-fvar-tracking-assignments'
    clang: error: no such file or directory: 'xxlimited.c'
    clang: error: no input files

    Ironically, before 1d71ce4531ac, $build_dir/build/lib.linux-aarch64-3.6 has _sysconfigdata_m_linux_x86_64-linux-gnu.py, so building is OK.

    Again, I don't know to fix it :(

    [1] https://hg.python.org/cpython/file/1d71ce4531ac/setup.py#l218

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Sep 10, 2016

    With the attached patch, the python test suite runs smoothly on android and without errors on linux. On android, there are two new failed test cases in test_sysconfig that I will look into later (in the frame of this issue, I guess ?), but otherwise about the same tests fail that were failing before.

    Here are some details about the fix:

    Another problem is that the shared libraries names of the extension modules are now suffixed with the wrong triplet.

    This is because _init_posix() in Lib/distutils/sysconfig.py still imports the native python sysconfigdata module instead of the newly built one for the target host.

    After _init_posix() is fixed, another problem occurs: extension modules are now built by including /usr/include from the native system and this leads to some unresolved dlopen references at run time on android. The reason is that add_gcc_paths() in setup.py calls sysconfig.get_config_var('CC') and this returns the native compiler instead of the cross-compiler.

    So a solution is to set the _SYSCONFIGDATA_NAME environment variable in PYTHON_FOR_BUILD.

    @zware
    Copy link
    Member Author

    zware commented Sep 10, 2016

    LGTM, thank you Xavier!

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Sep 10, 2016

    Much thanks! It's now building fine and running fine with my configuration. The test suite is still running and I believe there won't be more surprise than failed tests.

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Sep 10, 2016

    One of the test_sysconfig failed test is caused by the changes made in bpo-27917. I will fix it there. The other one is caused by a change in my android build setup that does not install anymore include/python3.6m/pyconfig.h, this is fixed now and the test does not fail anymore.

    Pending code reviews, I will commit the patch after changing the env var name from _SYSCONFIGDATA_NAME to _PYTHON_SYSCONFIGDATA_NAME.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 11, 2016

    New changeset 04b679dd11eb by Xavier de Gaye in branch 'default':
    Issue bpo-28046: get_sysconfigdata_name() uses the _PYTHON_SYSCONFIGDATA_NAME
    https://hg.python.org/cpython/rev/04b679dd11eb

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Sep 11, 2016

    Thanks for your help in testing these changes Chi Hsuan Yen.

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Sep 11, 2016

    No No, it's me who should say thank you. Thanks for all efforts on maintaining the Android port of CPython!

    @doko42
    Copy link
    Member

    doko42 commented Sep 13, 2016

    I don't like the _PYTHON_SYSCONFIGDATA_NAME hack. This should be based on the target, not on a name for just a particular file. Following up on that in bpo-28125.

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Oct 26, 2016

    'make install' fails to remove the sysconfigdata module from lib-dynload and prints now instead:

    rm: cannot remove '/path/to/install/lib/python3.7/lib-dynload/_sysconfigdata_m.py': No such file or directory
    

    The patch fixes this. It also removes a useless and now incorrect line that was meant to echo the previously executed command.

    @xdegaye xdegaye mannequin added the 3.7 (EOL) end of life label Oct 26, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 28, 2016

    New changeset 86577b7100a4 by Xavier de Gaye in branch '3.6':
    Issue bpo-28046: Fix the removal of the sysconfigdata module
    https://hg.python.org/cpython/rev/86577b7100a4

    New changeset 28205c4cf20b by Xavier de Gaye in branch 'default':
    Issue bpo-28046: Merge with 3.6.
    https://hg.python.org/cpython/rev/28205c4cf20b

    @zware
    Copy link
    Member Author

    zware commented Sep 30, 2017

    Are there any remaining outstanding issues here?

    @zware zware removed their assignment Sep 30, 2017
    @zware zware closed this as completed Nov 26, 2017
    @vstinner
    Copy link
    Member

    The cleanup is not complete, so I reopen the issue: #5289

    @vstinner vstinner reopened this Jan 24, 2018
    @vstinner
    Copy link
    Member

    New changeset 5de15f1 by Victor Stinner in branch 'master':
    bpo-28046: Remove MACHDEPPATH from Modules/Setup.dist (bpo-5289)
    5de15f1

    @vstinner
    Copy link
    Member

    Fixed. I close again the issue. I don't want to backport this change, it doesn't hurt Python 3.6.

    @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.7 (EOL) end of life build The build process and cross-build type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants