Navigation Menu

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.

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

ENH Add Ctypes support #1656

Merged
merged 32 commits into from Jun 26, 2021
Merged

ENH Add Ctypes support #1656

merged 32 commits into from Jun 26, 2021

Conversation

hoodmane
Copy link
Member

@hoodmane hoodmane commented Jun 23, 2021

Resolves #728.

I took the libffi port started by @Brion and added support for structs and closures (and fixed some issues with long double). I have that code here:
https://github.com/hoodmane/libffi-emscripten

It currently passes 56/56 of the "call" tests and 84/94 of the "closures" tests. It would be good to set up continuous integration for the library and to upstream it into the emscripten patches.

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Very excited about the progress you have been making on this @hoodmane !

Makefile.envs Outdated Show resolved Hide resolved
@hoodmane
Copy link
Member Author

It built!! Finally!

@hoodmane
Copy link
Member Author

Uh-oh now it's failing with ModuleNotFoundError: No module named '_socket'.

@hoodmane
Copy link
Member Author

Indeed for some reason it didn't build a bunch of built in modules.

@hoodmane
Copy link
Member Author

hoodmane commented Jun 24, 2021

Okay the build in 27df231 actually worked and a simple ctypes use works:

>>> import ctypes
>>> tuple_size = ctypes.CFUNCTYPE(ctypes.c_long, ctypes.py_object).in_dll(ctypes.pythonapi, "PyTuple_Size")
>>> tuple_size((1,2,3))
3

https://6515-322074228-gh.circle-artifacts.com/0/root/repo/build/console.html

@hoodmane
Copy link
Member Author

The CoExtra test fails because PyCode_SetExtra doesn't seem to work at all. Using it corrupts memory. Not ctype's fault. cpython_core[test_ctypes] passes on both Firefox and Chrome. Still have to investigate why CLAPACK build is failing...

@hoodmane
Copy link
Member Author

Okay so cpython_core[test_unicode-chrome] is failing for some reason and CLAPACK build is failing for some reason but other packages build and pass tests, even with ctypes patches removed. So we're in pretty good shape here.

@hoodmane
Copy link
Member Author

Right so test_unicode uses ctypes to do a bunch of tests for C APIs when ctypes is available. Probably it's again an issue with one of the C string APIs just like the code test was failing because PyCode_SetExtra is bad.

@hoodmane
Copy link
Member Author

I think the problem with test_unicode is that _PyUnicode_FromFormat is variadic. I'm not sure how to make call variadic functions from Javascript.

@hoodmane
Copy link
Member Author

hoodmane commented Jun 25, 2021

@rth Any clue what could be causing the CLAPACK build failure on 3f2baf4? The error message is pretty not helpful. I guess CLAPACK doesn't link Python and it doesn't take that long to fail so it should be easy to bisect.

@hoodmane
Copy link
Member Author

Okay it broke between 66400d0 and 426f5b2. I guess that emsdk patch causes trouble -- those optimization passes I dummied out are probably necessary for the CLAPACK build to succeed. Luckily I can remove the patch because I found out it is caused by the 123n syntax for a BigInt so I can remove uses of that from the libffi implementation and we should be good to go.
emscripten-core/emscripten/issues/14525

@hoodmane
Copy link
Member Author

Well removing that patch definitely fixes the CLAPACK build. But removing the bigint n's didn't fix the linking.

@rth
Copy link
Member

rth commented Jun 25, 2021

Looks like you found the solution, so I'm not going to be of much help :)

@hoodmane
Copy link
Member Author

Scipy imports are failing. I guess I'll restore the patch.

@hoodmane
Copy link
Member Author

Okay on 281c721 all tests passed except for two typical chrome-packages timeouts.

@hoodmane hoodmane requested a review from rth June 26, 2021 00:37
Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Great work @hoodmane !

@@ -56,6 +56,9 @@ substitutions:
- The standard library module `audioop` is now included, making the `wave`,
`sndhdr`, `aifc`, and `sunau` modules usable. {pr}`1623`

- Added support for `ctypes`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Added support for `ctypes`.
- Added support for the [`ctypes`](https://docs.python.org/3/library/ctypes.html)
stdlib module.

@rth rth changed the title Ctypes support ENH Add Ctypes support Jun 26, 2021
@rth rth merged commit 653891b into pyodide:main Jun 26, 2021
@rth rth deleted the ctypes branch June 26, 2021 08:34
hamlet4401 pushed a commit to tytgatlieven/pyodide that referenced this pull request Jul 3, 2021
@hoodmane hoodmane mentioned this pull request Jul 29, 2021
@mattficke mattficke mentioned this pull request Oct 18, 2021
@lsb lsb mentioned this pull request Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ctypes support
2 participants