Skip to content
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

gh-109740: Use 't' in --disable-gil SOABI #109922

Merged
merged 8 commits into from
Sep 27, 2023
Merged

gh-109740: Use 't' in --disable-gil SOABI #109922

merged 8 commits into from
Sep 27, 2023

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Sep 26, 2023 •
edited by bedevere-app bot
Loading

Shared libraries for CPython 3.13 --disable-gil builds are now marked with a 't' for threading. For example, binascii.cpython-313t-darwin.so.

@colesbury
pythongh-109740: Use 't' in --disable-gil SOABI
e745f32 
Shared libraries for CPython 3.13 are now marked with a 't' for
threading. For example, `binascii.cpython-313t-darwin.so`.
@bedevere-app bedevere-app bot mentioned this pull request Sep 26, 2023
Closed
@colesbury
Copy link
Contributor Author

colesbury commented Sep 26, 2023 •
edited
Loading

Notes for reviewers:

  • I moved the --disable-gil flag up in configure so that it comes before the debug check so that the optional "d" ABI flag appears on the outside (e.g., package-cpython-313td-darwin.so for --disable-gil --with-pydebug). This is purely aesthetic and can be changed, if you prefer.
  • Contrary to what I originally thought, pypa/packaging will need to be updated to compute CPython tags for --disable-gil builds.

@colesbury @erlend-aasland
Update Misc/NEWS.d/next/Build/2023-09-26-16-00-50.gh-issue-109740.wbo…
44f0e17 
…WdQ.rst

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
vstinner
vstinner reviewed Sep 26, 2023
View reviewed changes
Lib/test/test_sys.py Show resolved Hide resolved
configure.ac Show resolved Hide resolved
vstinner
vstinner approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM, but i left a few minor suggestions.

Python/dynload_win.c Outdated Show resolved Hide resolved
configure.ac Outdated Show resolved Hide resolved
corona10
corona10 approved these changes Sep 27, 2023
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

lgtm!

@vstinner vstinner enabled auto-merge (squash) September 27, 2023 08:07
@vstinner
Copy link
Member

I included my suggested changes to be able to merge your PR more quickly ;-)

@erlend-aasland
Copy link
Contributor

I included my suggested changes to be able to merge your PR more quickly ;-)

You need to regen configure.

@vstinner vstinner enabled auto-merge (squash) September 27, 2023 08:39
@vstinner
Copy link
Member

You need to regen configure.

Oops. Otherwise, it would be too easy! Done.

@colesbury
Copy link
Contributor Author

Thanks!

@vstinner
Copy link
Member

Windows x64 unstable tests:

FAIL: test_future_times_out (test.test_concurrent_futures.test_as_completed.ThreadPoolAsCompletedTest.test_future_times_out) [0.01]

Should be already fixed by PR #109949.

FAIL: test_set_and_clear (test.test_threading.EventTests.test_set_and_clear)

I created issue #109974 for this failure.

@vstinner vstinner merged commit 773614e into python:main Sep 27, 2023
23 checks passed
@vstinner
Copy link
Member

Good, it got merged after a few CI hiccups :-)

@colesbury colesbury deleted the abi branch September 27, 2023 15:29
csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 28, 2023
@colesbury @csm10495
pythongh-109740: Use 't' in --disable-gil SOABI (python#109922)
9e0dbb9 
Shared libraries for CPython 3.13 are now marked with a 't' for
threading. For example, `binascii.cpython-313t-darwin.so`.
@vstinner
Copy link
Member

vstinner commented Oct 4, 2023

The test broke test_importlib on Windows: see issue gh-110359.

FAIL: test_tagged_suffix (test.test_importlib.test_windows.Source_WindowsExtensionSuffixTests.test_tagged_suffix)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Administrator\buildarea\3.x.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\test\test_importlib\test_windows.py", line 122, in test_tagged_suffix
    self.assertIn(expected_tag, suffixes)
AssertionError: '_d.cp313-win32.pyd' not found in ['_d.cp313t-win_amd64.pyd', '_d.pyd']

@radarhere radarhere mentioned this pull request Feb 16, 2024
Merged
@hugovk hugovk mentioned this pull request Feb 20, 2024
Merged
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@colesbury @Glyphack
pythongh-109740: Use 't' in --disable-gil SOABI (python#109922)
292ac4f 
Shared libraries for CPython 3.13 are now marked with a 't' for
threading. For example, `binascii.cpython-313t-darwin.so`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

@corona10 corona10 corona10 approved these changes

@erlend-aasland erlend-aasland erlend-aasland approved these changes

@brettcannon brettcannon Awaiting requested review from brettcannon

@gpshead gpshead Awaiting requested review from gpshead

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@colesbury @vstinner @erlend-aasland @corona10