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

Allow different crate types when using library mode #1671

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

bendk
Copy link
Contributor

@bendk bendk commented Jul 25, 2023

This makes library mode work when the dependent crate doesn't include "lib" in their crate-type list. I think that check was leftover from older code and isn't needed anymore.

This makes library mode work when the dependent crate doesn't include
"lib" in their `crate-type` list.  I think that check was leftover from
older code and isn't needed anymore.
@bendk bendk requested a review from a team as a code owner July 25, 2023 23:36
@bendk bendk requested review from badboy and removed request for a team July 25, 2023 23:36
@bendk
Copy link
Contributor Author

bendk commented Jul 25, 2023

@jplatte I think this should fix your issue with library mode, can you test it against your branch?

As a workaround in the meantime, I think you should be able to use a regular release if you add lib to the crate-type list for matrix-sdk-ffi

@jplatte
Copy link
Collaborator

jplatte commented Jul 31, 2023

I'm getting a new, somewhat cryptic, error message with this:

     Running `target/debug/uniffi-bindgen generate --language kotlin --language swift --library target/debug/libmatrix_sdk_ffi.a --out-dir target/generated-bindings`
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Generate bindings for kotlin requires a cdylib, but target/debug/libmatrix_sdk_ffi.a was given', /home/jplatte/.cargo/git/checkouts/uniffi-rs-6f89edd2a1ffa4bd/088e565/uniffi/src/lib.rs:38:21

@bendk
Copy link
Contributor Author

bendk commented Aug 2, 2023

@jplatte Could use use target/debug/libmatrix_sdk_ffi.so instead?

Maybe that check should be removed, but the idea was to ensure that you use the same dylib that the bindings are eventually going to open.

@jplatte
Copy link
Collaborator

jplatte commented Aug 2, 2023

Huh? Consumers could use either, the static or dynamic library. Anyways, I totally missed that I had given the static lib, so the error message actually was pretty clear 🤦🏼

@jplatte
Copy link
Collaborator

jplatte commented Aug 2, 2023

Anyways, it works when passing the .so file! 🎉

@bendk bendk merged commit 02ada51 into mozilla:main Aug 7, 2023
@aringenbach
Copy link

aringenbach commented Aug 17, 2023

@jplatte I think this should fix your issue with library mode, can you test it against your branch?

As a workaround in the meantime, I think you should be able to use a regular release if you add lib to the crate-type list for matrix-sdk-ffi

Just an FIY for people who might use this workaround, for some reason adding the lib type to the ffi bindings crate did increase the size of static libraries by quite a bunch (40MB to 80MB on one of my projects), so using the actual fix as soon as possible is definitely better :).

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.

4 participants