Skip to content

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

bpo-45019: Add a tool to generate list of modules to include for frozen modules#27980
ericsnowcurrently merged 41 commits into
python:mainfrom
ericsnowcurrently:frozen-modules-cleanup

Conversation

@ericsnowcurrently

@ericsnowcurrentlyericsnowcurrently 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

ericsnowcurrentlyand others added 29 commits August 26, 2021 12:52
@ericsnowcurrently
Clean up _freeze_importlib.c.
a10471a
@ericsnowcurrently
_freeze_importlib -> _freeze_module
6252a37
@ericsnowcurrently
Add tooling to freeze stdlib modules.
b230af8
@ericsnowcurrently
Support freezing a package's submodules.
e4ef8fc
@ericsnowcurrently
Use freeze_modules.py for all frozen modules.
6bcc8c9
@ericsnowcurrently
Regen frozen.c and Makefile.pre.in.
1cae771
@ericsnowcurrently
Add the updating_file_with_tmpfile() "tool".
ba9417c
@ericsnowcurrently
Regen frozen.c in place.
6dcb926
@ericsnowcurrently
Clean up.
8a2b4b7
@ericsnowcurrently
Add --no-regen.
8ad3c1d
@ericsnowcurrently
Be consistent about the frozen module filename (frozen_*.h).
1e88d34
@ericsnowcurrently
Use consistently named make targets for regen-frozen-*.
c4874f2
@ericsnowcurrently
Drop the stdlib placeholders.
06af093
@ericsnowcurrently
Simplify FROZEN a little.
3a036b7
@ericsnowcurrently
Combine FROZEN and MODULES.
6a27a1d
@ericsnowcurrently
Drop the frozen module groups.
e3e07a9
@ericsnowcurrently
Fix a typo.
3d2ebdb
@ericsnowcurrently
Fix the Makefile.
9a5141f
@gvanrossum@ericsnowcurrently
Make freeze_modules.py less fragile.
3687fda
@ericsnowcurrently
Simplify specs.
1331a77
@ericsnowcurrently
Explicitly freeze modules in Makefile.
30972b2
@ericsnowcurrently
Fix a typo.
7a4a876
@ericsnowcurrently
Fix dependencies.
1306a4e
@ericsnowcurrently
Drop some dead code.
f724deb
@ericsnowcurrently
Fix another typo.
9a4d120
@ericsnowcurrently
Generate the frozen file in-place rather than with a temp file.
54b0116
@ericsnowcurrently
Update Windows build config.
16adb8e
@ericsnowcurrently
Move frozen modules into their own directory.
2d51cba
@ericsnowcurrently
Add a NEWS entry.
5cfd853
@ericsnowcurrentlyericsnowcurrently mentioned this pull request Aug 26, 2021
Closed
46 tasks
@gvanrossum

gvanrossum commented Aug 27, 2021

Copy link
Copy Markdown
Member

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

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

ericsnowcurrently commented Aug 27, 2021

Copy link
Copy Markdown
MemberAuthor

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').)

@brettcannonbrettcannon 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 modulesAug 27, 2021
gvanrossum
gvanrossum reviewed Aug 28, 2021
View reviewed changes

@gvanrossumgvanrossum 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.

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 threadPython/frozen.c Outdated
Comment threadPCbuild/readme.txt Outdated
Comment threadPrograms/_freeze_module.c Outdated
Comment threadPrograms/_freeze_module.c Outdated
Comment threadPrograms/_freeze_module.c Outdated
Comment threadPython/frozen.c Outdated
Comment threadPython/frozen.c Outdated
gvanrossum
gvanrossum reviewed Aug 28, 2021
View reviewed changes

@gvanrossumgvanrossum 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.

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

Comment threadPython/frozen_modules/hello.h
Comment threadTools/scripts/freeze_modules.py Outdated
Comment threadTools/scripts/freeze_modules.py Outdated
Comment threadTools/scripts/freeze_modules.py
Comment threadTools/scripts/freeze_modules.py Outdated
Comment threadTools/scripts/update_file.py
gvanrossum
gvanrossum approved these changes Aug 30, 2021
View reviewed changes

@gvanrossumgvanrossum 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.

LGTM! Do you want any other reviewers? Pablo?

@ericsnowcurrently

ericsnowcurrently commented Aug 30, 2021

Copy link
Copy Markdown
MemberAuthor

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
ericsnowcurrently merged commit 044e8d8 into python:mainAug 30, 2021
@ericsnowcurrently
ericsnowcurrently deleted the frozen-modules-cleanup branch August 31, 2021 14:08
@ericsnowcurrentlyericsnowcurrently mentioned this pull request Sep 13, 2021
Merged
ericsnowcurrently added a commit that referenced this pull request Sep 13, 2021
@ericsnowcurrently
bpo-45019: Do some cleanup related to frozen modules. (gh-28319)
a2d8c4b
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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossumgvanrossumgvanrossum approved these changes
@brettcannonbrettcannonAwaiting requested review from brettcannonbrettcannon is a code owner
@encukouencukouAwaiting requested review from encukou
@ncoghlanncoghlanAwaiting requested review from ncoghlanncoghlan is a code owner
@warsawwarsawAwaiting requested review from warsawwarsaw is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ericsnowcurrently@gvanrossum@the-knights-who-say-ni@bedevere-bot