Skip to content

gh-145633: Fix struct.pack('f') on s390x#146422

Merged
vstinner merged 4 commits intopython:mainfrom
vstinner:pack_float
Mar 26, 2026
Merged

gh-145633: Fix struct.pack('f') on s390x#146422
vstinner merged 4 commits intopython:mainfrom
vstinner:pack_float

Conversation

@vstinner
Copy link
Copy Markdown
Member

@vstinner vstinner commented Mar 25, 2026 •
edited by bedevere-app bot
Loading

Use PyFloat_Pack4() to raise OverflowError.

Add more tests on packing/unpacking floats.

@vstinner
pythongh-145633: Fix struct.pack('f') on s390x
eba13d8 
Use PyFloat_Pack4() to raise OverflowError.

Add more tests on packing/unpacking floats.
@vstinner vstinner added skip news needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Mar 25, 2026
@bedevere-app bedevere-app bot mentioned this pull request Mar 25, 2026
Open
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Mar 25, 2026

@skirpichev: Would you mind to review this change?

skirpichev
skirpichev reviewed Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@skirpichev skirpichev left a comment

Choose a reason for hiding this comment

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

(Sorry, I forgot to submit review yesterday.)

Looks ok, but maybe we should rather keep test_705836(), just with a small addition in the end.

@vstinner @skirpichev
Rewrite test_705836()
ce1093b 
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Mar 26, 2026

!buildbot s390x Fedora Stable

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Mar 26, 2026

🤖 New build scheduled with the buildbot fleet by @vstinner for commit ce1093b 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F146422%2Fmerge

The command will test the builders whose names match following regular expression: s390x Fedora Stable

The builders matched are:

  • s390x Fedora Stable Clang Installed PR
  • s390x Fedora Stable Refleaks PR
  • s390x Fedora Stable Clang PR
  • s390x Fedora Stable LTO + PGO PR
  • s390x Fedora Stable LTO PR
  • s390x Fedora Stable PR

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Mar 26, 2026

!buildbot SPARCv9 Oracle Solaris 11.4 PR

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Mar 26, 2026

🤖 New build scheduled with the buildbot fleet by @vstinner for commit ce1093b 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F146422%2Fmerge

The command will test the builders whose names match following regular expression: SPARCv9 Oracle Solaris 11.4 PR

The builders matched are:

  • SPARCv9 Oracle Solaris 11.4 PR

skirpichev
skirpichev reviewed Mar 26, 2026
View reviewed changes
@skirpichev skirpichev self-requested a review March 26, 2026 01:16
@skirpichev
Copy link
Copy Markdown
Member

skirpichev commented Mar 26, 2026 via email

@kulikjak
Copy link
Copy Markdown
Contributor

kulikjak commented Mar 26, 2026 •
edited
Loading

Hi, the SPARC buildbot is unfortunately still not 100% there, but the test_structpasses and I tested it myself and can confirm that the test was failing on SPARC without this change:

======================================================================
FAIL: test_705836 (test.test_struct.StructTest.test_705836)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builds/pythonmain/cpython-main/Lib/test/test_struct.py", line 399, in test_705836
    self.assertRaises(OverflowError, struct.pack, ">f", big)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: OverflowError not raised by pack

and passes with it. Thanks!

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Mar 26, 2026

I added again test_float_round_trip(), now also with tests on half-floats.

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Mar 26, 2026

!buildbot s390x Fedora Stable PR

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Mar 26, 2026

🤖 New build scheduled with the buildbot fleet by @vstinner for commit d4e1edf 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F146422%2Fmerge

The command will test the builders whose names match following regular expression: s390x Fedora Stable PR

The builders matched are:

  • s390x Fedora Stable PR

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Mar 26, 2026

"macOS / build and test (macos-26-intel)": test_tkinter failed with a timeout:

test_wm_iconbitmap (test.test_tkinter.test_misc.WmTest.test_wm_iconbitmap) ... Timeout (0:10:00)!
Thread 0x00007ff84f189c00 (most recent call first):
  File "/Users/runner/work/cpython/cpython/Lib/tkinter/__init__.py", line 2345 in wm_iconbitmap
  File "/Users/runner/work/cpython/cpython/Lib/test/test_tkinter/test_misc.py", line 582 in test_wm_iconbitmap

I re-ran this job.

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Mar 26, 2026

"buildbot/s390x Fedora Stable PR": the new test_struct passed successfully!

@vstinner vstinner enabled auto-merge (squash) March 26, 2026 10:56
@vstinner vstinner merged commit 8de70b3 into python:main Mar 26, 2026
95 of 97 checks passed
@vstinner vstinner deleted the pack_float branch March 26, 2026 11:12
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Mar 26, 2026

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 26, 2026
@vstinner @skirpichev @miss-islington
pythongh-145633: Fix struct.pack('f') on s390x (pythonGH-146422)
21daa5c 
Use PyFloat_Pack4() to raise OverflowError.
Add more tests on packing/unpacking floats.
(cherry picked from commit 8de70b3)

Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Mar 26, 2026

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

cherry_picker 8de70b31c59b1d572d95f8bb471a09cfe4cd2b13 3.13

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Mar 26, 2026

GH-146460 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 Mar 26, 2026
@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Mar 26, 2026

GH-146461 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 Mar 26, 2026
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Mar 26, 2026

I merged my PR. Thanks for the review @skirpichev.

vstinner added a commit that referenced this pull request Mar 26, 2026
@miss-islington @vstinner @skirpichev
[3.14] gh-145633: Fix struct.pack('f') on s390x (GH-146422) (#146460)
8a25840 
gh-145633: Fix struct.pack('f') on s390x (GH-146422)

Use PyFloat_Pack4() to raise OverflowError.
Add more tests on packing/unpacking floats.
(cherry picked from commit 8de70b3)

Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
vstinner added a commit that referenced this pull request Mar 26, 2026
@vstinner @skirpichev
[3.13] gh-145633: Fix struct.pack('f') on s390x (#146422) (#146461)
921515f 
gh-145633: Fix struct.pack('f') on s390x (#146422)

Use PyFloat_Pack4() to raise OverflowError.
Add more tests on packing/unpacking floats.


(cherry picked from commit 8de70b3)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skirpichev skirpichev skirpichev approved these changes

Assignees

@vstinner vstinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner @bedevere-bot @skirpichev @kulikjak