gh-151422: Don't link libffi into _ctypes_test.so#151423
Conversation
hoodmane
requested review from
AA-Turner,
corona10,
emmatyping,
erlend-aasland and
itamaro
as code owners
hoodmane
force-pushed
the
dont-link-libffi-into-ctypes-test
branch
from
295fabb to
4fb91c5
Compare
hoodmane
added
the
needs backport to 3.15
freakboy3742
left a comment
freakboy3742
left a comment
There was a problem hiding this comment.
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?
| if support.is_wasm32: | ||
| self.skipTest( | ||
| "wasm ABI is incompatible with test expectations") |
There was a problem hiding this comment.
Any reason this is done as in-test logic, rather than a @skipif(support.is_wasm32, ...)?
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
2 participants
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.