Skip to content

Conversation

@tiran
Copy link
Member

@tirantiran commented Nov 18, 2021

@tirantiranforce-pushed the bpo-45573-setup-core branch 2 times, most recently from be740b2 to 0db8a04CompareNovember 18, 2021 13:37
@tirantiran marked this pull request as ready for review November 18, 2021 15:34
@tirantiranforce-pushed the bpo-45573-setup-core branch from 0db8a04 to 05c1b88CompareNovember 18, 2021 16:11
Copy link
Contributor

@erlend-aaslanderlend-aasland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement. Not all modules that #define Py_BUILD_CORE_MODULE are included in Modules/Setup.core. Is Modules/Setup.builtin a more "correct" name?

Modules that define Py_BUILD_CORE_MODULE
$ grep -r "#.*define.*Py_BUILD_CORE_MODULE" Modules | sort Modules/_abc.c:# define Py_BUILD_CORE_MODULE 1 Modules/_asynciomodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/_blake2/blake2b_impl.c:# define Py_BUILD_CORE_MODULE 1 Modules/_blake2/blake2module.c:# define Py_BUILD_CORE_MODULE 1 Modules/_blake2/blake2s_impl.c:# define Py_BUILD_CORE_MODULE 1 Modules/_ctypes/_ctypes.c:# define Py_BUILD_CORE_MODULE 1 Modules/_ctypes/callbacks.c:# define Py_BUILD_CORE_MODULE 1 Modules/_ctypes/cfield.c:# define Py_BUILD_CORE_MODULE 1 Modules/_ctypes/stgdict.c:# define Py_BUILD_CORE_MODULE 1 Modules/_cursesmodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/_datetimemodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/_decimal/_decimal.c:# define Py_BUILD_CORE_MODULE 1 Modules/_hashopenssl.c:# define Py_BUILD_CORE_MODULE 1 Modules/_heapqmodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/_json.c:# define Py_BUILD_CORE_MODULE 1 Modules/_lsprof.c:# define Py_BUILD_CORE_MODULE 1 Modules/_pickle.c:# define Py_BUILD_CORE_MODULE 1 Modules/_posixsubprocess.c:# define Py_BUILD_CORE_MODULE 1 Modules/_queuemodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/_randommodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/_sha3/sha3module.c:# define Py_BUILD_CORE_MODULE 1 Modules/_struct.c:# define Py_BUILD_CORE_MODULE 1 Modules/_testinternalcapi.c:# define Py_BUILD_CORE_MODULE 1 Modules/_testmultiphase.c:# define Py_BUILD_CORE_MODULE 1 Modules/_xxsubinterpretersmodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/_zoneinfo.c:# define Py_BUILD_CORE_MODULE 1 Modules/arraymodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/binascii.c:# define Py_BUILD_CORE_MODULE 1 Modules/cmathmodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/mathmodule.c:# define Py_BUILD_CORE_MODULE 1 Modules/md5module.c:# define Py_BUILD_CORE_MODULE 1 Modules/sha1module.c:# define Py_BUILD_CORE_MODULE 1 Modules/sha256module.c:# define Py_BUILD_CORE_MODULE 1 Modules/sha512module.c:# define Py_BUILD_CORE_MODULE 1 Modules/unicodedata.c:# define Py_BUILD_CORE_MODULE 1

configure.ac Outdated
Comment on lines 6008 to 6009
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the alias?

@tiran
Copy link
MemberAuthor

This is a nice improvement. Not all modules that #define Py_BUILD_CORE_MODULE are included in Modules/Setup.core. Is Modules/Setup.builtin a more "correct" name?

The term core is overloaded. I have not found a better term, though.

Py_BUILD_CORE_MODULE enables some private APIs that not available to 3rd party extension modules. The flag is orthogonal to static / shared state. You can build a shared C extension that uses core module APIs.

Setup.builtin does not fit well either. Other modules can be build as builtin modules, too. Setup.mandatory is also incorrect. Some modules like _abc could be build as shared extensions. Maybe Setup.bootstrap ?

@erlend-aasland
Copy link
Contributor

Yeah, Setup.bootstrap sounds like the best candidate.

@tirantiran changed the title bpo-45573: Move mandatory core modules to Modules/Setup.corebpo-45573: Move mandatory core modules to Modules/Setup.bootstrapNov 19, 2021
@tirantiranforce-pushed the bpo-45573-setup-core branch from 05c1b88 to a1b915fCompareNovember 19, 2021 08:50
@tirantiranforce-pushed the bpo-45573-setup-core branch from ff2d1e0 to db3e45eCompareNovember 19, 2021 15:01
Copy link
Contributor

@erlend-aaslanderlend-aasland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! LGTM

@tirantiran changed the title bpo-45573: Move mandatory core modules to Modules/Setup.bootstrapbpo-45573: Move mandatory core modules to Modules/Setup.bootstrap (GH-29616)Nov 19, 2021
@tirantiran merged commit 7e44dc0 into python:mainNov 19, 2021
@tirantiran deleted the bpo-45573-setup-core branch November 19, 2021 15:40
remykarem pushed a commit to remykarem/cpython that referenced this pull request Dec 7, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tiran@erlend-aasland@the-knights-who-say-ni@bedevere-bot