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

Hack: Copy metadata module to avoid uniffi_core dependency #1665

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

badboy
Copy link
Member

@badboy badboy commented Jul 25, 2023

In m-c we're running into linking issues in uniffi_macros, where it's missing symbols coming from uniffi_core:

/bin/ld: uniffi_foreign_executor_callback_set: undefined version:
/bin/ld: failed to set dynamic section sizes: bad value

That's most likely this issue on Rust:
rust-lang/rust#111888
It's called out that this likely actually broke because of another PR: rust-lang/rust#99944

Despite this bug there's a bit of an issue in UniFFI to begin with: We're exporting extern "C" functions from a crate that is a dependency of some other crates, including uniffi_macros, and thus the symbols land in a part where they are not needed.

However for uniffi_meta in particular we don't need much from uniffi_core, so for now we just copy the necessary bits to get it all working.


This is tackling the issues uncovered in https://bugzilla.mozilla.org/show_bug.cgi?id=1840044
It's a short-term fix that I want to release as a patch version 0.24.2 without anything else from main (I have a separate patch for only the 0.24.2 fix then

@badboy badboy requested a review from travis79 July 25, 2023 11:53
@badboy badboy requested a review from a team as a code owner July 25, 2023 11:53
@badboy badboy force-pushed the copied-metadata-module-main branch 2 times, most recently from 5c0deeb to 124959b Compare July 25, 2023 12:03
Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

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

Please file a bug to follow up and remove this once the Rust bug is fixed, aside from that, LGTM!

@mhammond
Copy link
Member

mhammond commented Jul 25, 2023

I wonder if things have changed enough that could move uniffi_core/src/ffi_converter_*.rs into uniffi_meta, which might make this go away.

In m-c we're running into linking issues in `uniffi_macros`,
where it's missing symbols coming from `uniffi_core`:

    /bin/ld: uniffi_foreign_executor_callback_set: undefined version:
    /bin/ld: failed to set dynamic section sizes: bad value

That's most likely this issue on Rust:
rust-lang/rust#111888
It's called out that this likely actually broke because of another PR:
rust-lang/rust#99944

Despite this bug there's a bit of an issue in UniFFI to begin with:
We're exporting `extern "C"` functions from a crate that is a dependency
of some other crates, including `uniffi_macros`, and thus the symbols
land in a part where they are not needed.

However for `uniffi_meta` in particular we don't need much from
`uniffi_core`, so for now we just copy the necessary bits to get it all
working.
@badboy badboy force-pushed the copied-metadata-module-main branch from 124959b to d574d18 Compare July 25, 2023 13:52
@badboy badboy merged commit e78018b into main Jul 26, 2023
@badboy badboy deleted the copied-metadata-module-main branch July 26, 2023 08:30
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