Skip to content

gh-133059: fix Tools/build/deepfreeze.py nsmallposints - #139906

Merged
Eclips4 merged 5 commits into
python:mainfrom
albertedwardson:nsmallposints
Oct 17, 2025
Merged

gh-133059: fix Tools/build/deepfreeze.py nsmallposints#139906
Eclips4 merged 5 commits into
python:mainfrom
albertedwardson:nsmallposints

Conversation

@albertedwardson

@albertedwardsonalbertedwardson commented Oct 10, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

In Tools/build/deepfreeze.py nsmallposints was still 256.

Although deepfreeze is no longer used (#116919), this script still needs to be kept and be actual until it is completely removed.

This PR steals the parser of those numbers from Tools/build/generate_global_objects.py and puts it in a new script, so getting constants can be unified.

@bedevere-app

bedevere-appBot commented Oct 10, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@Eclips4Eclips4 changed the title gh-133160: fix Tools/build/deepfreeze.py nsmallposintsgh-133059: fix Tools/build/deepfreeze.py nsmallposintsOct 10, 2025
@bedevere-appbedevere-appBot mentioned this pull request Oct 10, 2025
Closed
@Eclips4
Eclips4 self-requested a review October 10, 2025 15:28
@albertedwardson
albertedwardson marked this pull request as ready for review October 12, 2025 12:33
sobolevn
sobolevn reviewed Oct 13, 2025
View reviewed changes
Comment threadTools/build/consts_getter.py Outdated
import os

SCRIPT_NAME = 'Tools/build/consts_getter.py'
__file__ = os.path.abspath(__file__)

@sobolevnsobolevnOct 13, 2025

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.

Why do you redefine __file__?

Comment threadTools/build/consts_getter.py Outdated

SCRIPT_NAME = 'Tools/build/consts_getter.py'
__file__ = os.path.abspath(__file__)
ROOT = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))

@sobolevnsobolevnOct 13, 2025

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.

What about builds that are performed in a different folder? It is possible to do that.

@albertedwardsonalbertedwardsonOct 13, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

these constants were stolen from Tools/build/generate_global_objects.py, so in this case I think it will be more than enough not to override __file__ and just specify a valid path to Include's

sobolevn
sobolevn reviewed Oct 13, 2025
View reviewed changes
Comment threadTools/build/consts_getter.py Outdated
@@ -0,0 +1,22 @@
import os

SCRIPT_NAME = 'Tools/build/consts_getter.py'

@sobolevnsobolevnOct 13, 2025

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.

Is it needed?

@albertedwardsonalbertedwardsonOct 13, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

in this case, I think it's not. it's only used in scripts that generate .c files to put a comment on top to indicate that those file were generated by this particular script

will remove

Eclips4
Eclips4 reviewed Oct 16, 2025
View reviewed changes

@Eclips4Eclips4 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 have one minor comment, otherwise LGTM.

Comment threadTools/build/consts_getter.py Outdated
@albertedwardson@Eclips4
Update consts_getter.py
55b55ce
Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
efimov-mikhail
efimov-mikhail reviewed Oct 16, 2025
View reviewed changes
Comment threadTools/build/consts_getter.py Outdated
efimov-mikhail
efimov-mikhail approved these changes Oct 16, 2025
View reviewed changes

@efimov-mikhailefimov-mikhail 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

@Eclips4
Eclips4 merged commit 999ab89 into python:mainOct 17, 2025
55 checks passed
@Eclips4

Eclips4 commented Oct 17, 2025

Copy link
Copy Markdown
Member

Thank you Albert!

StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
@albertedwardson@StanFromIreland
pythongh-133059: FixTools/build/deepfreeze.pyfor new nsmallposints (
6849326
python#139906)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Eclips4Eclips4Eclips4 left review comments
@efimov-mikhailefimov-mikhailefimov-mikhail approved these changes
@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner
@ezio-melottiezio-melottiAwaiting requested review from ezio-melottiezio-melotti is a code owner
@hugovkhugovkAwaiting requested review from hugovkhugovk is a code owner
@AA-TurnerAA-TurnerAwaiting requested review from AA-TurnerAA-Turner is a code owner
@sobolevnsobolevnAwaiting requested review from sobolevn

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@albertedwardson@Eclips4@sobolevn@efimov-mikhail