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

[WIP] 🙋FFI bindings #39

Closed
wants to merge 1 commit into from
Closed

Conversation

luizirber
Copy link
Contributor

(I'm opening this PR quite early to get more feedback and fix it)

Checklist

  • tests pass
  • tests and/or benchmarks are included
  • documentation is changed or added

Context

As discussed in #36, this sets up the FFI infrastructure with cbindgen. It is using the build.rs way, but many projects define a Makefile or some other external way to generate the bindings via the cbindgen command line. The reason is that expanding macros might take some time (I even disabled it for now), and this ends up being executed every time the crate is built (but the C header rarely changes).

I'm keeping the FFI module inside the hypercore repo. Some projects create another crate for the FFI, but I think this is easier to keep in sync.

I set up a Python project using it to help guide what APIs to expose thru the FFI. I was thinking about developing this by:

  • Creating a test function in Python, with a Pythonic API
  • Implementing the Python side of the FFI and figure out what needs to be called on the Rust FFI
  • Implementing the Rust FFI function.

There is only one function for now (and it's not even very useful), but it already works to create a feed on the Python side

Semver Changes

No API changes on the Rust side.

@luizirber
Copy link
Contributor Author

And with the Android bindings by @bltavares I think we have a good way to drive the 'Implementing the Rust FFI function' step, because we need to agree on what's useful for both Python and Java/Kotlin =]

@luizirber
Copy link
Contributor Author

Hmm, turns out #![forbid(unsafe_code)] will be an issue for FFI. While it's mostly avoidable, there are things that need unsafe (like freeing memory with Box::from_raw).

The solution for now was to remove the crate-level attribute and put it on every mod line in lib.rs, which is a bit ugly and a lot of repetition...

@luizirber luizirber force-pushed the feature/ffi branch 2 times, most recently from b899c10 to 6569cdf Compare September 17, 2018 23:10
@yoshuawuyts
Copy link
Contributor

yoshuawuyts commented Oct 18, 2018

@luizirber o/ have you had the time to look at this more? Is there any way I could help to unblock you?

@luizirber
Copy link
Contributor Author

Hi @yoshuawuyts! I wasn't sure how to start the hypercore-py API (what to expose and test there to guide the FFI bindings here). At first I was thinking about using the tests here as templates, but maybe following the original API is a better idea?

@yoshuawuyts
Copy link
Contributor

At first I was thinking about using the tests here as templates, but maybe following the original API is a better idea?

Yeah, that sounds very reasonable! I'm not sure how far we could take this entirely because it makes use of some class overloading, and inherits from an emitter -- but trying to approximate it seems very reasonable!

@luizirber luizirber closed this Feb 12, 2023
@luizirber luizirber deleted the feature/ffi branch February 12, 2023 23:35
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.

2 participants