bpo-45188: Windows now regenerates frozen modules at the start of build instead of late by zooba · Pull Request #28322

bpo-45188: Windows now regenerates frozen modules at the start of build instead of late by zooba · Pull Request #28322 · python/cpython

I haven't looked at how much slower this makes the build, but it ought to be a minute or so. It'd be possible to make the freeze_module project share a temp directory with pythoncore, which would speed it up, but that's going to hurt reliability so I'd rather not.

Choose a reason for hiding this comment

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

LGTM

Thanks for doing this!

I think you can also drop the special-case in regen_pcbuild() in Tools/scripts/freeze_modules.py, where we skip the module if it isn't in ESSENTIAL.

🤖 New build scheduled with the buildbot fleet by @zooba for commit 4250dc7 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

I think you can also drop the special-case in regen_pcbuild() in Tools/scripts/freeze_modules.py, where we skip the module if it isn't in ESSENTIAL.

Potentially, but I'll leave that one to you ;) In case it breaks other things

		pyfile = os.path.relpath(src.pyfile, ROOT_DIR).replace('/', '\\')
		header = os.path.relpath(src.frozenfile, ROOT_DIR).replace('/', '\\')
		intfile = header.split('\\')[-1].strip('.h') + '.g.h'
		intfile = os.path.splitext(os.path.basename(header))[0] + '.g.h'

Choose a reason for hiding this comment

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

FYI @ericsnowcurrently I fixed a bug in your script here (should've been removesuffix('.h') rather than strip, but better to stick with os.path when dealing with paths anyway).

Choose a reason for hiding this comment

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

Thanks for fixing this. I was going to suggest using ntpath explicitly here, but in this case it really should be os.path. 🙂

Thanks again for doing this, @zooba!