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

External types seem to prevent (easy) distribution #1776

Closed
DrGodCarl opened this issue Oct 3, 2023 · 4 comments · Fixed by #1784
Closed

External types seem to prevent (easy) distribution #1776

DrGodCarl opened this issue Oct 3, 2023 · 4 comments · Fixed by #1784

Comments

@DrGodCarl
Copy link

DrGodCarl commented Oct 3, 2023

Here's the gist of it by example: I have two crates (rust_ffi and tiny_lib). rust_ffi depends on tiny_lib. Both have UDLs. rust_ffi references a type from the tiny_lib. When I generate the Python code for rust_ffi, it has imports pointing to tiny_lib (e.g. from tiny_lib import TinyLibType). I can generate tiny_lib's Python code as well and that seems like it works in general, but after being packaged together in a wheel the import breaks; when the import statement runs in rust_ffi.py it searches for a package named tiny_lib and fails to find it, completely ignoring the adjacent file with the same name.

I have made a small example of this problem. Its README is a bit out of date from what I've described above, but the setup and problem are essentially the same. I have also tried using a megazord and generating using library mode but the Python files generate the same way and exhibit the same behavior. When I modify the rust_ffil.py in the wheel to import from .tiny_lib it works exactly as expected, though that's not really sustainable for a larger project.

It's possible I've missed how I can package these up in the docs somewhere. If that's the case, I'd love to be pointed to those docs and I apologize for opening an issue.

@mhammond
Copy link
Member

mhammond commented Oct 3, 2023

Where are your build scripts for the bindings? None of our examples have good examples of building, but ultimately you probably need to run uniffi_bindgen in "library mode" by pointing it at the final built .so. I'll try and clarify some more tomorrow.

@DrGodCarl
Copy link
Author

I went through a megazord + library mode build just before posting this but didn't get it cleaned up into a build script yet. It had the same issue. I'll try to have my project updated tonight or tomorrow morning with that, plus an example Python project consuming the wheel.

@DrGodCarl
Copy link
Author

DrGodCarl commented Oct 4, 2023

Alright, I updated that project. There's a build/install script, a megazord, and an updated README. Also added a workaround script that does the .tiny_lib replacement, which is less than ideal but I can move forward on the main project for now.

@mhammond
Copy link
Member

mhammond commented Oct 9, 2023

Uniffi is broken for external types and Python :( The specific issue here is that your example fails in the generated code at:

from tiny_lib import TinyLibType

where what it wants is:

from .tiny_lib import TinyLibType

Unfortunately the Python bindings don't understand packages and external types. The UniFFI examples all run in the context where each module is top-level - but that's clearly not a viable strategy.

I think UniFFI should just include that leading period, with a future enhancement being you can override it per external type. This patch is simple, but it will break our tests, so our test infra will need to learn about packages.

mhammond added a commit to mhammond/uniffi-rs that referenced this issue Oct 9, 2023
…be used.

When using external types, the expectation is that most consumers will
want to use a Python package which holds all the modules - therefore,
imports for external types defaults to `from .module import name`.
This behaviour can be changed to support any package name, including
just doing a regular top-level module import - which we leverage for
some tests.

Fixes mozilla#1776
mhammond added a commit to mhammond/uniffi-rs that referenced this issue Oct 10, 2023
…be used.

When using external types, the expectation is that most consumers will
want to use a Python package which holds all the modules - therefore,
imports for external types defaults to `from .module import name`.
This behaviour can be changed to support any package name, including
just doing a regular top-level module import - which we leverage for
some tests.

Fixes mozilla#1776
mhammond added a commit to mhammond/uniffi-rs that referenced this issue Oct 18, 2023
…be used.

When using external types, the expectation is that most consumers will
want to use a Python package which holds all the modules - therefore,
imports for external types defaults to `from .module import name`.
This behaviour can be changed to support any package name, including
just doing a regular top-level module import - which we leverage for
some tests.

Fixes mozilla#1776
mhammond added a commit that referenced this issue Oct 18, 2023
…be used. (#1784)

When using external types, the expectation is that most consumers will
want to use a Python package which holds all the modules - therefore,
imports for external types defaults to `from .module import name`.
This behaviour can be changed to support any package name, including
just doing a regular top-level module import - which we leverage for
some tests.

Fixes #1776
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 a pull request may close this issue.

2 participants