FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-45019: Add a tool to generate list of modules to include for frozen modules by ericsnowcurrently · Pull Request #27980 · python/cpython · GitHub

Repository navigation

bpo-45019: Add a tool to generate list of modules to include for frozen modules - #27980

Merged
ericsnowcurrently merged 41 commits into
python:mainfrom
ericsnowcurrently:frozen-modules-cleanup
Aug 30, 2021
Merged

ericsnowcurrently merged 41 commits into
python:mainfrom
ericsnowcurrently:frozen-modules-cleanup

Conversation

ericsnowcurrently commented Aug 26, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Frozen modules must be added to several files in order to work properly. Before this change this had to be done manually. Here we add a tool to generate the relevant lines in those files instead. This helps us avoid mistakes and omissions.

https://bugs.python.org/issue45019

Copy link
Copy Markdown
Member

Is this ready for review (apart from the failing test)?

gvanrossum self-requested a review August 27, 2021 03:45

Copy link
Copy Markdown
Member Author

Is this ready for review (apart from the failing test)?

Yes.

(I need to figure out why "make regen-keywords" isn't working for me locally. Something about ModuleNotFoundError('dataclasses').)

brettcannon changed the title bpo-45019: Add a tool to generate lines for frozen modules. bpo-45019: Add a tool to generate list of modules to include for frozen modules Aug 27, 2021

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I've only gotten to frozen.c, I'll review the rest later. So far this is great, I just have a few nits about outdated comments.

Comment thread Python/frozen.c Outdated
Comment thread PCbuild/readme.txt Outdated
Comment thread Programs/_freeze_module.c Outdated
Comment thread Programs/_freeze_module.c Outdated
Comment thread Programs/_freeze_module.c Outdated
Comment thread Python/frozen.c Outdated
Comment thread Python/frozen.c Outdated

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Here's the rest. It's looking much better than the version I used for my "codegen" experiments!

Comment thread Tools/scripts/freeze_modules.py Outdated
Comment thread Tools/scripts/freeze_modules.py Outdated
Comment thread Tools/scripts/freeze_modules.py Outdated

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM! Do you want any other reviewers? Pablo?

Copy link
Copy Markdown
Member Author

This change is focused on cleanup, so other reviewers probably aren't necessary. The next PR (freezing some of the stdlib modules) might deserve more than one.

ericsnowcurrently merged commit 044e8d8 into python:main Aug 30, 2021
ericsnowcurrently deleted the frozen-modules-cleanup branch August 31, 2021 14:08
ericsnowcurrently added a commit that referenced this pull request Sep 13, 2021
There are a few things I missed in gh-27980. This is a follow-up that will make subsequent PRs cleaner. It includes fixes to tests and tools that reference the frozen modules.

https://bugs.python.org/issue45019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL