gh-109515: Allow a large number of frozen modules on Windows #109516

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

cpython-cla-bot · 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.

PCbuild/_freeze_module.vcxproj

Tools/build/deepfreeze.py

Misc/NEWS.d/next/Build/2023-09-17-15-17-53.gh-issue-109515.zpEECA.rst

Tools/build/freeze_modules.py

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>

Tools/build/deepfreeze.py

Tools/build/freeze_modules.py

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

PCbuild/_freeze_module.vcxproj

+        <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

Tools/build/freeze_modules.py

@@ -659,8 +662,7 @@ def regen_pcbuild(modules):
        filterlines.append(f'    <None Include="..\\{pyfile}">')
        filterlines.append('      <Filter>Python Files</Filter>')
        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!

Tools/build/deepfreeze.py

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)

rghe and 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-app bot added the awaiting review label Sep 17, 2023

bedevere-app bot 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-it bot and 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 Windows Sep 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

rghe and 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

Tools/build/freeze_modules.py Outdated Show resolved Hide resolved

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

Tools/build/deepfreeze.py Outdated Show resolved Hide resolved

Tools/build/deepfreeze.py Outdated Show resolved Hide resolved

Tools/build/deepfreeze.py Outdated Show resolved Hide resolved

Tools/build/freeze_modules.py Outdated Show resolved Hide resolved

rghe and 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-app bot 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

Tools/build/deepfreeze.py Outdated Show resolved Hide resolved

Update Tools/build/deepfreeze.py

5620f94

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

zooba merged commit 8eaa206 into python:main Oct 30, 2023

bedevere-app bot 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)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-109515: Allow a large number of frozen modules on Windows #109516

gh-109515: Allow a large number of frozen modules on Windows #109516

rghe commented Sep 17, 2023 •

edited by bedevere-app bot

Loading

cpython-cla-bot bot commented Sep 17, 2023 •

edited

Loading

bedevere-app bot commented Sep 17, 2023

bedevere-app bot commented Sep 17, 2023

zooba commented Sep 26, 2023

zooba left a comment

zooba Sep 26, 2023

AA-Turner commented Sep 26, 2023

AA-Turner Sep 27, 2023

rghe Sep 27, 2023

gvanrossum left a comment

rghe commented Oct 28, 2023

zooba commented Oct 30, 2023

ericsnowcurrently commented Oct 30, 2023

rghe commented Oct 30, 2023

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

gh-109515: Allow a large number of frozen modules on Windows #109516

gh-109515: Allow a large number of frozen modules on Windows #109516

Conversation

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

cpython-cla-bot bot commented Sep 17, 2023 • edited Loading

bedevere-app bot commented Sep 17, 2023

bedevere-app bot commented Sep 17, 2023

zooba commented Sep 26, 2023

zooba left a comment

Choose a reason for hiding this comment

zooba Sep 26, 2023

Choose a reason for hiding this comment

AA-Turner commented Sep 26, 2023

AA-Turner Sep 27, 2023

Choose a reason for hiding this comment

rghe Sep 27, 2023

Choose a reason for hiding this comment

gvanrossum left a comment

Choose a reason for hiding this comment

rghe commented Oct 28, 2023

zooba commented Oct 30, 2023

ericsnowcurrently commented Oct 30, 2023

rghe commented Oct 30, 2023

rghe commented Sep 17, 2023 •

edited by bedevere-app bot

Loading

cpython-cla-bot bot commented Sep 17, 2023 •

edited

Loading