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

gh-93939: Add script to check extension modules #94545

Merged
merged 9 commits into from
Jul 5, 2022

Conversation

tiran
Copy link
Member

@tiran tiran commented Jul 4, 2022

Add script Tools/scripts/check_modules.py to check and validate builtin
and shared extension modules. The script also handles Modules/Setup and
will eventually replace setup.py.

Add script ``Tools/scripts/check_modules.py`` to check and validate builtin
and shared extension modules. The script also handles ``Modules/Setup`` and
will eventually replace ``setup.py``.
@tiran tiran marked this pull request as ready for review July 4, 2022 09:26
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

"check modules" is very generic. Would you mind to elaborate the purpose of the script? I understand that the purpose is to check which modules are built successfully and which ones are disabled. Maybe say "check modules build"?

It seems like the script also renames extensions which cannot be imported.

Please elaborate the docstring of the script also.

@tiran
Copy link
Member Author

tiran commented Jul 4, 2022

Do you have an idea for a better name? check_extension_modules maybe?

@vstinner
Copy link
Member

vstinner commented Jul 4, 2022

Do you have an idea for a better name?

check_extensions_build or check_built_extensions.

@tiran
Copy link
Member Author

tiran commented Jul 4, 2022

The script does the same as setup.py methods check_extension_import and summary + LIST_MODULE_NAMES hack in a more generalized way. The main difference is that the new script handles Makefile variables and all makesetup files instead of relying on distutils.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Thanks for the updated docstring.

Tools/scripts/check_extension_modules.py Outdated Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Outdated Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Outdated Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Outdated Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Show resolved Hide resolved
Makefile.pre.in Show resolved Hide resolved
tiran and others added 5 commits July 4, 2022 19:04
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

I took only a quick glance for now. If this can replace setup.py, this is a very nice improvement to the build system!

Should we add a check that adds a user-friendly warning if you try to run this script in the source directory? Someone will create an issue about that in the future.

Tools/scripts/check_extension_modules.py Outdated Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Outdated Show resolved Hide resolved
Tools/scripts/check_extension_modules.py Outdated Show resolved Hide resolved
tiran and others added 2 commits July 5, 2022 08:55
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>
@tiran tiran merged commit 7bd67d1 into python:main Jul 5, 2022
@tiran tiran deleted the gh-93939-check-modules branch July 5, 2022 07:25
Sign up for free to 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