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

Enable wasm32 compilation by wrapping libc #1362

Closed
wants to merge 4 commits into from
Closed

Enable wasm32 compilation by wrapping libc #1362

wants to merge 4 commits into from

Conversation

dalcde
Copy link
Contributor

@dalcde dalcde commented Jan 5, 2021

My plan is to use this with pyodide.

It is unclear how to test this at the moment. I think a reasonable solution is to make sure it compiles with --target wasm32-unknown-unknown after supplying suitable header files, possibly copying from pyodide, and run no tests. When/if things get ready with pyodide, we can consider running integration tests with it, but that would require a very different setup. Due to the nature of the builds, I also think it is unlikely that it can be usable without nightly in the near future.

I'm also happy to leave this as a PR until then. See also #1221

@dalcde dalcde changed the title Eanble wasm32 compilation by wrapping libc Enable wasm32 compilation by wrapping libc Jan 5, 2021
@davidhewitt
Copy link
Member

... so as I understand it, libc doesn't provide any exports on wasm32? Should this be fixed upstream?

@dalcde
Copy link
Contributor Author

dalcde commented Jan 6, 2021 via email

@davidhewitt
Copy link
Member

I see, thanks. In which case to have wasm32 support we're definitely going to need something like this.

To keep the "wrapper" as small as possible, for all types we use which are exported in std::os::raw can you please change this PR to use std::os::raw in the usage site and remove them from the wrapper?

Let's see what's left after they're gone - I think except for size_t, they probably account for most of the usages.

@dalcde dalcde marked this pull request as draft January 6, 2021 13:53
@@ -357,7 +357,7 @@ pub(crate) fn impl_wrap_setter(
#[allow(unused_mut)]
unsafe extern "C" fn __wrap(
_slf: *mut pyo3::ffi::PyObject,
_value: *mut pyo3::ffi::PyObject, _: *mut ::std::os::raw::c_void) -> pyo3::libc::c_int
_value: *mut pyo3::ffi::PyObject, _: *mut ::std::os::raw::c_void) -> ::std::os::raw::c_int
Copy link
Member

Choose a reason for hiding this comment

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

BTW, #1367 may conflict with this line (I'm sorry!) and you will need to merge that manually.

@kngwyu
Copy link
Member

kngwyu commented Jan 7, 2021

I think we can remove atexit but are still discussing at #1355

@dalcde
Copy link
Contributor Author

dalcde commented Jan 7, 2021 via email

@davidhewitt
Copy link
Member

we can merge the
changes that convert libc to std::os::raw when applicable,

Agreed - are you willing to factor those changes out into a separate PR?

@dalcde
Copy link
Contributor Author

dalcde commented Jan 7, 2021 via email

@dalcde
Copy link
Contributor Author

dalcde commented Jan 7, 2021

Closing in favor of #1368. May be reopened if this turns out to be useful/needed

@dalcde dalcde closed this Jan 7, 2021
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.

3 participants