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

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

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

tiran opened this issue Apr 29, 2021 · 8 comments
Assignees
@tiran
Labels
3.11 only security fixes 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 added 3.10 only security fixes 3.11 only security fixes 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 3.10 only security fixes 3.11 only security fixes 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

    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

    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 only security fixes 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 only security fixes 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

    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
    @erlend-aasland erlend-aasland mentioned this issue Jul 27, 2023
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @tiran tiran

    Labels
    3.11 only security fixes 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
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @tiran @erlend-aasland