gh-109515: Allow a large number of frozen modules on Windows by rghe · Pull Request #109516 · python/cpython

rghe · 2023-09-17T14:57:17Z

This PR changes freezing modules compilation on windows by writing freezing module paths into a file instead of the command line.
Non-windows builds are unchanged

Issue: freeze_modules.py on windows hits command line limits when number of frozen modules grows #109515

…vstudio

ghost · 2023-09-17T14:57:19Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2023-09-17T14:57:21Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-app · 2023-09-17T15:12:34Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

zooba · 2023-09-26T16:16:57Z

We always squash merge PRs and rewrite the commit messages, don't worry about the change history.

Co-authored-by: Steve Dower <steve.dower@microsoft.com>

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

zooba

I'm okay with this, but I'm not the maintainer of this code. Not sure who cares about it most, but they can merge it (or I'll get it in a couple of weeks if nobody cares)

zooba · 2023-09-26T20:26:42Z

+ <ItemGroup>
+ <!-- BEGIN freeze mappings -->


It would be nice to fix the indentation on these lines. I don't see where it is though.
Otherwise anyone who edits this file manually is likely to fix it and trigger regeneration.

AA-Turner · 2023-09-26T20:49:13Z

Devguide lists Guido & Kumar as deepfreeze experts.

A

AA-Turner · 2023-09-27T08:52:33Z

 filterlines.append(' </None>')
- deepfreezerules.append(f'\t\t "$(PySourcePath){header}:{src.frozenid}" ^')
- deepfreezerules.append('\t\t "-o" "$(PySourcePath)Python\\deepfreeze\\deepfreeze.c"\'/>' )
+ deepfreezemappings.append(f' <FrozenModule Include="$(PySourcePath)\\{header}" FrozenId="{src.frozenid}" />\n')


I think this would solve Steve's point re indentation
Suggested change
deepfreezemappings.append(f' <FrozenModule Include="$(PySourcePath)\\{header}" FrozenId="{src.frozenid}" />\n')
deepfreezemappings.append(f' <FrozenModule Include="$(PySourcePath)\\{header}" FrozenId="{src.frozenid}" />\n')

reformatted the vcxproj, now everything seems aligned

gvanrossum

Changes to deepfreeze.py LGTM (one nit for the help message). I'll leave the rest to Steve and Adam!

Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>

rghe · 2023-10-28T10:22:45Z

I'm okay with this, but I'm not the maintainer of this code. Not sure who cares about it most, but they can merge it (or I'll get it in a couple of weeks if nobody cares)

ahem, ping?

zooba · 2023-10-30T16:13:45Z

Thanks for the ping!

ericsnowcurrently · 2023-10-30T16:45:22Z

There are a bunch of buildbot failures, but they may be due to gh-110764.

rghe · 2023-10-30T17:42:47Z

There are a bunch of buildbot failures, but they may be due to gh-110764.

well, I don´t think it's due to this change, since it affects only the pre-build frozen module generator on windows builds and all error messages seem to be "define _Py_ThreadId for this platform"

…a list file instead of arguments (pythonGH-109516)

rgheand others added 2 commits September 9, 2023 18:23

fix windows command line limitations when freezing many modules
97c9b8c

Merge branch 'python:main' into allow-large-number-of-frozen-modules-…
5d640cb
…vstudio

bedevere-appBot added the awaiting review label Sep 17, 2023

bedevere-appBot mentioned this pull request Sep 17, 2023
freeze_modules.py on windows hits command line limits when number of frozen modules grows #109515
Closed

fix spacing
ddf9082

blurb-itBotand others added 2 commits September 17, 2023 15:17

📜🤖 Added by blurb_it.
9058b8d

missing regen
e98c3d0

rghe requested a review from a team as a code owner September 17, 2023 16:33

rghe added 3 commits September 17, 2023 18:47

add generated file
86d1818

add buildpath-dependent generated files to .gitignore
ca98aa1

try relative paths
5463e79

AA-Turner changed the title ~~gh-109515 - Allow large number of frozen modules even on windows~~gh-109515: Allow a large number of frozen modules on WindowsSep 21, 2023

AA-Turner added the skip news label Sep 21, 2023

fill mappings file with relative paths from vcxproj
7456fe5

zooba reviewed Sep 21, 2023
View reviewed changes

Comment threadPCbuild/_freeze_module.vcxproj Outdated
Comment threadPCbuild/_freeze_module.vcxproj Outdated
Comment threadTools/build/deepfreeze.py Outdated
Comment threadMisc/NEWS.d/next/Build/2023-09-17-15-17-53.gh-issue-109515.zpEECA.rst Outdated

rgheand others added 4 commits September 23, 2023 18:31

Implement S.Dower review suggestions
031ea3e

Merge branch 'main' into allow-large-number-of-frozen-modules-vstudio
e6fb7f5

clean gitignore
b78bb97

Merge branch 'main' into allow-large-number-of-frozen-modules-vstudio
93750fa

zooba reviewed Sep 26, 2023
View reviewed changes

Comment threadTools/build/freeze_modules.py Outdated

Update Tools/build/freeze_modules.py
d584d2d
Co-authored-by: Steve Dower <steve.dower@microsoft.com>

AA-Turner reviewed Sep 26, 2023
View reviewed changes

Comment threadTools/build/deepfreeze.py Outdated
Comment threadTools/build/deepfreeze.py Outdated
Comment threadTools/build/deepfreeze.py Outdated
Comment threadTools/build/freeze_modules.py Outdated

rgheand others added 4 commits September 26, 2023 19:28

Update Tools/build/deepfreeze.py
48112ab
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

Update Tools/build/deepfreeze.py
74a255a
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

Update Tools/build/deepfreeze.py
ecc334f
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

expand \t and align vcxfile by AA-Turner suggestion
bb42f32

typo
4cc3458

zooba approved these changes Sep 26, 2023
View reviewed changes

bedevere-appBot added awaiting core review and removed awaiting review labels Sep 26, 2023

AA-Turner requested review from gvanrossum and kumaraditya303 September 26, 2023 20:48

AA-Turner reviewed Sep 27, 2023
View reviewed changes

reformat vcxproj
1c20fcd

gvanrossum reviewed Sep 27, 2023
View reviewed changes

Comment threadTools/build/deepfreeze.py Outdated

Update Tools/build/deepfreeze.py
5620f94
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>

zooba merged commit 8eaa206 into python:mainOct 30, 2023

bedevere-appBot removed the awaiting core review label Oct 30, 2023

rghe deleted the allow-large-number-of-frozen-modules-vstudio branch October 30, 2023 17:42

FullteaR pushed a commit to FullteaR/cpython that referenced this pull request Nov 3, 2023
pythongh-109515: When generating deep frozen modules on Windows, use …
af4ffa1
…a list file instead of arguments (pythonGH-109516)

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
pythongh-109515: When generating deep frozen modules on Windows, use …
0cbe9f0
…a list file instead of arguments (pythonGH-109516)

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
pythongh-109515: When generating deep frozen modules on Windows, use …
25d9fa0
…a list file instead of arguments (pythonGH-109516)

	deepfreezemappings.append(f' <FrozenModule Include="$(PySourcePath)\\{header}" FrozenId="{src.frozenid}" />\n')
	deepfreezemappings.append(f' <FrozenModule Include="$(PySourcePath)\\{header}" FrozenId="{src.frozenid}" />\n')

Uh oh!

Uh oh!

Conversation

rghe commented Sep 17, 2023• edited by bedevere-app BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Sep 17, 2023• edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-appBot commented Sep 17, 2023

Uh oh!

bedevere-appBot commented Sep 17, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zooba commented Sep 26, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zooba left a comment

Choose a reason for hiding this comment

Uh oh!

zoobaSep 26, 2023

Choose a reason for hiding this comment

Uh oh!

AA-Turner commented Sep 26, 2023

Uh oh!

AA-TurnerSep 27, 2023

Choose a reason for hiding this comment

Uh oh!

rgheSep 27, 2023

Choose a reason for hiding this comment

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rghe commented Oct 28, 2023

Uh oh!

zooba commented Oct 30, 2023

Uh oh!

ericsnowcurrently commented Oct 30, 2023

Uh oh!

rghe commented Oct 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rghe commented Sep 17, 2023•
edited by bedevere-app Bot
Loading

ghost commented Sep 17, 2023•
edited by ghost
Loading