Skip to content

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

Closed
Closed
Define Py_BUILD_CORE_MODULE in extensions instead of setup.py and Modules/Setup#88140
Assignees
tiran
Labels
3.11only security fixesbuildThe build process and cross-buildextension-modulesC modules in the Modules dirtype-bugAn unexpected behavior, bug, or error

Description

@tiran
tiran
opened on 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']

    Activity

    1. tiran commented on Apr 29, 2021

      @tiran
      tiran
      on Apr 29, 2021
      MemberAuthor

      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.

    2. tiran
      added
      3.10 (EOL)end of life
      3.11only security fixes
      on Apr 29, 2021
    3. tiran
      self-assigned this
      on Apr 29, 2021
    4. tiran
      added
      buildThe build process and cross-build
      type-bugAn unexpected behavior, bug, or error
      3.10 (EOL)end of life
      3.11only security fixes
      on Apr 29, 2021
    5. tiran
      self-assigned this
      on Apr 29, 2021
    6. tiran
      added
      buildThe build process and cross-build
      type-bugAn unexpected behavior, bug, or error
      on Apr 29, 2021

    7. tiran commented on Apr 29, 2021

      @tiran
      tiran
      on Apr 29, 2021
      MemberAuthor

      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

    8. vstinner commented on Apr 29, 2021

      @vstinner
      vstinner
      on Apr 29, 2021
      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.

    9. vstinner commented on Apr 29, 2021

      @vstinner
      vstinner
      on Apr 29, 2021
      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.

    10. tiran commented on May 1, 2021

      @tiran
      tiran
      on May 1, 2021
      MemberAuthor

      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.

    11. tiran commented on Oct 22, 2021

      @tiran
      tiran
      on Oct 22, 2021
      MemberAuthor

      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.

    12. tiran
      added
      extension-modulesC modules in the Modules dir
      and removed
      3.10 (EOL)end of life
      on Oct 22, 2021
    13. 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[/+] on Oct 22, 2021
    14. tiran
      added
      extension-modulesC modules in the Modules dir
      and removed
      3.10 (EOL)end of life
      on Oct 22, 2021
    15. 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[/+] on Oct 22, 2021

    16. erlend-aasland commented on Oct 22, 2021

      @erlend-aasland
      erlend-aasland
      on Oct 22, 2021
      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.

    17. tiran commented on Oct 22, 2021

      @tiran
      tiran
      on Oct 22, 2021
      MemberAuthor

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

    18. tiran
      closed this as completedon Oct 22, 2021
    19. tiran
      closed this as completedon Oct 22, 2021
    20. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    3.11only security fixesbuildThe build process and cross-buildextension-modulesC modules in the Modules dirtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions