Skip to content

gh-142454: Make the JIT digest more deterministic by sorting the files in Tools/jit #142455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
savannahostrowski merged 4 commits into from
Dec 9, 2025
Merged

gh-142454: Make the JIT digest more deterministic by sorting the files in Tools/jit #142455

savannahostrowski merged 4 commits into from
Dec 9, 2025
+5 −1

Conversation

@hroncok
Copy link
Contributor

@hroncok hroncok commented Dec 9, 2025 •
edited by bedevere-app bot
Loading

The order of filenames provided by os.walk is filesystem-dependent.

Different filesystems return the files in a different order, hence invalidating the digest.
Keeping everything sorted makes the digest deterministic.

Fixes #142454

@hroncok
pythongh-142454: Make the JIT digest more deterministic by sorting th…
38eff7b 
…e files in Tools/jit

The order of filenames provided by os.walk is filesystem-dependent.

Different filesystems return the files in a different order,
hence invalidating the digest.
Keeping everything sorted makes the digest deterministic.

Fixes python#142454
@bedevere-app bedevere-app bot mentioned this pull request Dec 9, 2025
Closed
@hroncok @Fidget-Spinner
Fix typo in Misc/NEWS.d/next/Build/2025-12-09-13-33-46.gh-issue-14245…
6115fef 
…4.cqUxzQ.rst

Co-authored-by: Ken Jin <kenjin4096@gmail.com>
diegorusso
diegorusso approved these changes Dec 9, 2025
View reviewed changes
Copy link
Contributor

@diegorusso diegorusso left a comment

Choose a reason for hiding this comment

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

The change makes sense, LGTM. Thanks a lot!

hroncok
hroncok commented Dec 9, 2025
View reviewed changes
savannahostrowski
savannahostrowski reviewed Dec 9, 2025
View reviewed changes
Copy link
Member

@savannahostrowski savannahostrowski left a comment

Choose a reason for hiding this comment

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

I think this change makes sense but it sounded like you only started seeing this issue in 3.15.0a2 in the issue. @hroncok Can you confirm that this worked as expected in 3.15.0a1?

@diegorusso
Copy link
Contributor

diegorusso commented Dec 9, 2025

I guess this needs to be back ported to 3.14 as well (as specified in the original issue)

@savannahostrowski
Copy link
Member

savannahostrowski commented Dec 9, 2025

Ah, I missed that and was confused by the wording "we have found out that the digest in JIT stencils in 3.15.0a2 differs." Sounded like this was a new issue. The reason I was asking is because I think this bug existing from the very beginning, in 3.13 as well. I bisected and this appears to have always been the case.

@savannahostrowski savannahostrowski added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Dec 9, 2025
@hroncok
Copy link
Contributor Author

hroncok commented Dec 9, 2025

Indeed, this is not new in 3.15.0a2. Sorry for the confusing wording. 3.15.0a2 only happens to be the first version we have attempted to build in this build system while asserting the prebuilt stencils match the just built ones. This bug appears to have existed from the very beginning of the _compute_digest function.

@savannahostrowski savannahostrowski enabled auto-merge (squash) December 9, 2025 17:05
@savannahostrowski
Copy link
Member

savannahostrowski commented Dec 9, 2025

Thanks for catching this issue and opening the fix, @hroncok!

@savannahostrowski savannahostrowski merged commit bcf90de into python:main Dec 9, 2025
72 checks passed
@miss-islington-app
Copy link

miss-islington-app bot commented Dec 9, 2025

Thanks @hroncok for the PR, and @savannahostrowski for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link

miss-islington-app bot commented Dec 9, 2025

Sorry, @hroncok and @savannahostrowski, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker bcf90de8ba2ea087540a5f632656ef880ee46b5c 3.14

@miss-islington-app
Copy link

miss-islington-app bot commented Dec 9, 2025

Sorry, @hroncok and @savannahostrowski, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker bcf90de8ba2ea087540a5f632656ef880ee46b5c 3.13

@hroncok
Copy link
Contributor Author

hroncok commented Dec 9, 2025

Should I open a backport PR?

@savannahostrowski
Copy link
Member

savannahostrowski commented Dec 9, 2025

If you'd like to do it, feel free. Otherwise, I'm happy to handle it!

@hroncok
Copy link
Contributor Author

hroncok commented Dec 9, 2025

I am happy to do it but the conflict is because of #138131, which seems safe to backport as well. WDYT?

@savannahostrowski
Copy link
Member

savannahostrowski commented Dec 9, 2025 •
edited
Loading

I think so. I'm kind of surprised that wasn't backported originally, given that the original bug report was for 3.14. I think we could also backport that fix first and then retrigger the backport for this PR, if we want clearer separation of concerns.

@hroncok
Copy link
Contributor Author

hroncok commented Dec 9, 2025

I don't have the right to request backports via the labels, but if done in order, it should no longer conflict.

@hroncok hroncok deleted the sort-jit-tools branch December 9, 2025 17:53
@savannahostrowski savannahostrowski added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes and removed needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Dec 9, 2025
@miss-islington-app
Copy link

miss-islington-app bot commented Dec 9, 2025

Thanks @hroncok for the PR, and @savannahostrowski for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

miss-islington-app bot commented Dec 9, 2025

Thanks @hroncok for the PR, and @savannahostrowski for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 9, 2025
@hroncok @Fidget-Spinner @miss-islington
pythongh-142454: Make the JIT digest more deterministic by sorting th…
e38a8ba 
…e files in Tools/jit (pythonGH-142455)

(cherry picked from commit bcf90de)

Co-authored-by: Miro Hrončok <miro@hroncok.cz>
Co-authored-by: Ken Jin <kenjin4096@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 9, 2025
@hroncok @Fidget-Spinner @miss-islington
pythongh-142454: Make the JIT digest more deterministic by sorting th…
a6e0316 
…e files in Tools/jit (pythonGH-142455)

(cherry picked from commit bcf90de)

Co-authored-by: Miro Hrončok <miro@hroncok.cz>
Co-authored-by: Ken Jin <kenjin4096@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Dec 9, 2025

GH-142484 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Dec 9, 2025
@bedevere-app
Copy link

bedevere-app bot commented Dec 9, 2025

GH-142485 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Dec 9, 2025
@hroncok
Copy link
Contributor Author

hroncok commented Dec 9, 2025

Thanks.

savannahostrowski pushed a commit that referenced this pull request Dec 9, 2025
@miss-islington @hroncok @Fidget-Spinner
[3.13] gh-142454: Make the JIT digest more deterministic by sorting t…
e96367d 
…he files in Tools/jit (GH-142455) (#142484)

gh-142454: Make the JIT digest more deterministic by sorting the files in Tools/jit (GH-142455)
(cherry picked from commit bcf90de)

Co-authored-by: Miro Hrončok <miro@hroncok.cz>
Co-authored-by: Ken Jin <kenjin4096@gmail.com>
savannahostrowski pushed a commit that referenced this pull request Dec 9, 2025
@miss-islington @hroncok @Fidget-Spinner
[3.14] gh-142454: Make the JIT digest more deterministic by sorting t…
7621bbd 
…he files in Tools/jit (GH-142455) (#142485)

gh-142454: Make the JIT digest more deterministic by sorting the files in Tools/jit (GH-142455)
(cherry picked from commit bcf90de)

Co-authored-by: Miro Hrončok <miro@hroncok.cz>
Co-authored-by: Ken Jin <kenjin4096@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diegorusso diegorusso diegorusso approved these changes

@savannahostrowski savannahostrowski savannahostrowski approved these changes

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

@brandtbucher brandtbucher Awaiting requested review from brandtbucher brandtbucher is a code owner

Assignees

@savannahostrowski savannahostrowski

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

JIT digest differs based on filesystem order

4 participants

@hroncok @diegorusso @savannahostrowski @Fidget-Spinner