classification
Title: Define Py_BUILD_CORE_MODULE in extensions instead of setup.py and Modules/Setup
Type: behavior Stage: resolved
Components: Build, Extension Modules Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: brett.cannon, christian.heimes, erlendaasland, pablogsal, vstinner
Priority: normal Keywords: patch

Created on 2021-04-29 09:02 by christian.heimes, last changed 2021-10-22 13:37 by christian.heimes. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25713 closed christian.heimes, 2021-04-29 09:04
PR 29157 merged christian.heimes, 2021-10-22 12:07
Messages (8)
msg392293 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-04-29 09:02
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.
msg392295 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-04-29 09:22
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
msg392332 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-29 16:55
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.
msg392333 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-29 16:55
> 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.
msg392587 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-05-01 11:34
> 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.
msg404755 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-10-22 11:32
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.
msg404757 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-10-22 12:18
> 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.
msg404768 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-10-22 13:36
New changeset 03e9f5dc751b8c441a85f428abc3f432ffe46345 by Christian Heimes in branch 'main':
bpo-43974: Move Py_BUILD_CORE_MODULE into module code (GH-29157)
https://github.com/python/cpython/commit/03e9f5dc751b8c441a85f428abc3f432ffe46345
History
Date User Action Args
2021-10-22 13:37:02christian.heimessetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-10-22 13:36:37christian.heimessetmessages: + msg404768
2021-10-22 12:18:16erlendaaslandsetnosy: + erlendaasland
messages: + msg404757
2021-10-22 12:07:11christian.heimessetstage: patch review
pull_requests: + pull_request27432
2021-10-22 11:32:51christian.heimessettitle: 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
nosy: + brett.cannon

messages: + msg404755

versions: - Python 3.10
components: + Extension Modules
2021-05-01 11:34:54christian.heimessetmessages: + msg392587
2021-04-29 16:55:37vstinnersetmessages: + msg392333
2021-04-29 16:55:05vstinnersetmessages: + msg392332
2021-04-29 09:22:43christian.heimessetmessages: + msg392295
stage: patch review -> (no value)
2021-04-29 09:04:49christian.heimessetkeywords: + patch
stage: patch review
pull_requests: + pull_request24402
2021-04-29 09:02:26christian.heimescreate