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

Define Py_BUILD_CORE_MODULE in extensions instead of setup.py and Modules/Setup #88140

Closed
tiran opened this issue Apr 29, 2021 · 8 comments
Closed
Assignees
Labels
3.11 build The build process and cross-build extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@tiran
Copy link
Member

tiran commented Apr 29, 2021

BPO 43974
Nosy @brettcannon, @tiran, @ericsnowcurrently, @pablogsal, @erlend-aasland
PRs
  • bpo-43974: Set Py_BUILD_CORE_MODULE for all core modules #25713
  • bpo-43974: Move Py_BUILD_CORE_MODULE into module code (GH-29157) #29157
  • 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 = 'https://github.com/tiran'
    closed_at = <Date 2021-10-22.13:37:02.817>
    created_at = <Date 2021-04-29.09:02:26.399>
    labels = ['extension-modules', 'type-bug', 'build', '3.11']
    title = 'Define Py_BUILD_CORE_MODULE in extensions instead of setup.py and Modules/Setup'
    updated_at = <Date 2022-02-21.11:46:32.070>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2022-02-21.11:46:32.070>
    actor = 'vstinner'
    assignee = 'christian.heimes'
    closed = True
    closed_date = <Date 2021-10-22.13:37:02.817>
    closer = 'christian.heimes'
    components = ['Build', 'Extension Modules']
    creation = <Date 2021-04-29.09:02:26.399>
    creator = 'christian.heimes'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43974
    keywords = ['patch']
    message_count = 8.0
    messages = ['392293', '392295', '392332', '392333', '392587', '404755', '404757', '404768']
    nosy_count = 5.0
    nosy_names = ['brett.cannon', 'christian.heimes', 'eric.snow', 'pablogsal', 'erlendaasland']
    pr_nums = ['25713', '29157']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue43974'
    versions = ['Python 3.11']

    @tiran
    Copy link
    Member Author

    tiran commented Apr 29, 2021

    CPython's setup.py contains lots of

      extra_compile_args = ['-DPy_BUILD_CORE_MODULE']

    to mark modules as core module. Extra compiler args is the wrong option. It's also tedious and err-prone to define the macro in each and every Extension() class instance.

    The compiler flag should be set automatically for all core extensions and it should use be set using the correct option define_macros.

    @tiran tiran self-assigned this Apr 29, 2021
    @tiran tiran added build The build process and cross-build type-bug An unexpected behavior, bug, or error 3.10 3.11 labels Apr 29, 2021
    @tiran tiran self-assigned this Apr 29, 2021
    @tiran tiran added build The build process and cross-build type-bug An unexpected behavior, bug, or error labels Apr 29, 2021
    @tiran
    Copy link
    Member Author

    tiran commented Apr 29, 2021

    Related to the change:

    It looks like we can also cleanup Modules/Setup and remove -DPy_BUILD_CORE_BUILTIN and -DPy_BUILD_CORE_MODULE. Modules/makesetup adds $PY_BUILTIN_MODULE_CFLAGS in the compile step. The variable is defined as

    PY_BUILTIN_MODULE_CFLAGS= $(PY_STDMODULE_CFLAGS) -DPy_BUILD_CORE_BUILTIN
    

    @vstinner
    Copy link
    Member

    vstinner commented Apr 29, 2021

    I would prefer to limit the usage of the internal C API in extension modules built as dynamic libraries. See bpo-41111: "[C API] Convert a few stdlib extensions to the limited C API (PEP-384)".

    Also, this issue is motived by PR 25653 which requires to use the internal C API in many C extensions. But I proposed a different approach, PR 25710, which prevents that.

    @vstinner
    Copy link
    Member

    vstinner commented Apr 29, 2021

    It looks like we can also cleanup Modules/Setup and remove -DPy_BUILD_CORE_BUILTIN and -DPy_BUILD_CORE_MODULE. Modules/makesetup adds $PY_BUILTIN_MODULE_CFLAGS in the compile step.

    Oh, I didn't notice.

    @tiran
    Copy link
    Member Author

    tiran commented May 1, 2021

    I would prefer to limit the usage of the internal C API in extension modules built as dynamic libraries. See bpo-41111: "[C API] Convert a few stdlib extensions to the limited C API (PEP-384)".

    Let's make this a coordinated effort in 3.11. I suggest that we slowly remove functions from Py_BUILD_CORE_MODULE. For now I'm interested to clean up and simplify setup.py.

    @tiran
    Copy link
    Member Author

    tiran commented Oct 22, 2021

    The proposal is related to Brett's ticket bpo-45548. I no longer think that we should define Py_BUILD_CORE_MODULE unconditionally. Instead I propose to move the defines into each C module. This avoids duplication of macros in setup.py and Modules/Setup.

    @tiran tiran added extension-modules C modules in the Modules dir and removed 3.10 labels Oct 22, 2021
    @tiran tiran changed the title setup.py should set Py_BUILD_CORE_MODULE as defined macro Define Py_BUILD_CORE_MODULE in extensions instead of setup.py and Modules/Setup Oct 22, 2021
    @tiran tiran added extension-modules C modules in the Modules dir and removed 3.10 labels Oct 22, 2021
    @tiran tiran changed the title setup.py should set Py_BUILD_CORE_MODULE as defined macro Define Py_BUILD_CORE_MODULE in extensions instead of setup.py and Modules/Setup Oct 22, 2021
    @erlend-aasland
    Copy link
    Contributor

    erlend-aasland commented Oct 22, 2021

    I no longer think that we should define Py_BUILD_CORE_MODULE
    unconditionally. Instead I propose to move the defines into each C module.

    +1. Explicit is nice.

    @tiran
    Copy link
    Member Author

    tiran commented Oct 22, 2021

    New changeset 03e9f5d by Christian Heimes in branch 'main':
    bpo-43974: Move Py_BUILD_CORE_MODULE into module code (GH-29157)
    03e9f5d

    @tiran tiran closed this as completed Oct 22, 2021
    @tiran tiran closed this as completed Oct 22, 2021
    @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.11 build The build process and cross-build extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants