gh-151422: Don't link libffi into _ctypes_test.so by hoodmane · Pull Request #151423 · python/cpython · GitHub
Skip to content

gh-151422: Don't link libffi into _ctypes_test.so - #151423

Merged
freakboy3742 merged 3 commits into
python:mainfrom
hoodmane:dont-link-libffi-into-ctypes-test
Jun 15, 2026
Merged

gh-151422: Don't link libffi into _ctypes_test.so#151423
freakboy3742 merged 3 commits into
python:mainfrom
hoodmane:dont-link-libffi-into-ctypes-test

Conversation

@hoodmane

@hoodmane hoodmane commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Minor build fix. This makes the ctypes tests run on Emscripten.

There is a bug in the Emscripten dynamic loader that caused any dynamic library that links libffi to fail to load. _ctypes_test.so unnecessarily links libffi so it would fail to load and tests that needed it were skipped.

There are two test failures behind that: one involving stack overflows which we have to skip as usual, and one that assumes that the abi for a function that takes a single struct with two doubles is the same as the abi for a function that takes two double arguments. This is not true in webassembly so we skip the test.

@bedevere-app bedevere-app Bot mentioned this pull request Jun 12, 2026
Closed
@hoodmane
hoodmane requested a review from freakboy3742 June 13, 2026 15:19
@hoodmane
hoodmane force-pushed the dont-link-libffi-into-ctypes-test branch from 295fabb to 4fb91c5 Compare June 13, 2026 15:22
@hoodmane hoodmane mentioned this pull request Jun 14, 2026
Closed
@hoodmane hoodmane added the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 14, 2026
freakboy3742
freakboy3742 requested changes Jun 15, 2026
View reviewed changes

@freakboy3742 freakboy3742 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes sense; one question inline about the skip strategy.

The only question is about why libffi was linked in the first place. It appears to have been introduced in #120894, but I can't see any reason why it would have been (other than oversight/overenthusiasm that didn't hurt anything at the time).

That patch was included in the development of 3.14; you've flagged this for 3.15 back porting - can you think of any reason this shouldn't be backported to 3.14 as well?

Comment thread Lib/test/test_ctypes/test_structures.py Outdated
@bedevere-app

bedevere-app Bot commented Jun 15, 2026

Copy link
Copy Markdown

@hoodmane

hoodmane commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

can you think of any reason this shouldn't be backported to 3.14 as well?

No, I'll add that.

@hoodmane hoodmane added the needs backport to 3.14 bugs and security fixes label Jun 15, 2026
@hoodmane

This comment was marked as resolved.

Sign in to view
@hoodmane

This comment was marked as resolved.

Sign in to view
freakboy3742
freakboy3742 approved these changes Jun 15, 2026
View reviewed changes

@freakboy3742 freakboy3742 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@freakboy3742
freakboy3742 merged commit 8646385 into python:main Jun 15, 2026
55 of 56 checks passed
@miss-islington-app

miss-islington-app Bot commented Jun 15, 2026

Copy link
Copy Markdown

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

@bedevere-app

bedevere-app Bot commented Jun 15, 2026

Copy link
Copy Markdown

GH-151516 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 15, 2026
@bedevere-app

bedevere-app Bot commented Jun 15, 2026

Copy link
Copy Markdown

GH-151517 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 Jun 15, 2026
freakboy3742 pushed a commit that referenced this pull request Jun 15, 2026
@miss-islington @hoodmane
[3.14] gh-151422: Don't link libffi into _ctypes_test.so (GH-151423) (#…
aa48555 
…151517)

_ctypes_test doesn't use libffi directly, and linking it into the module
causes emscripten tests to fail.
(cherry picked from commit 8646385)

Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
freakboy3742 pushed a commit that referenced this pull request Jun 15, 2026
@miss-islington @hoodmane
[3.15] gh-151422: Don't link libffi into _ctypes_test.so (GH-151423) (#…
e2c7fa7 
…151516)

_ctypes_test doesn't use libffi directly, and linking it into the module
causes emscripten tests to fail.
(cherry picked from commit 8646385)

Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
@bedevere-bot

bedevere-bot commented Jun 15, 2026

Copy link
Copy Markdown

philthompson10 pushed a commit to philthompson10/cpython that referenced this pull request Jun 17, 2026
@hoodmane @philthompson10
pythongh-151422: Don't link libffi into _ctypes_test.so (python#151423)
0140462 
_ctypes_test doesn't use libffi directly, and linking it into the module 
causes emscripten tests to fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@freakboy3742 freakboy3742 freakboy3742 approved these changes
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@corona10 corona10 Awaiting requested review from corona10 corona10 is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
@emmatyping emmatyping Awaiting requested review from emmatyping emmatyping is a code owner
@itamaro itamaro Awaiting requested review from itamaro itamaro is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hoodmane @bedevere-bot @freakboy3742