-
Notifications
You must be signed in to change notification settings - Fork 231
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
Workspace broken | UniFFI generates invalid Swift & Python bindings #2153
Comments
I don't really understand the problem you are describing - how is this different from what we do in our tests and examples?
This looks just like https://mozilla.github.io/uniffi-rs/latest/python/configuration.html#external-packages - uniffi doesn't attempt to generate the complete package structure and
Can you please try and clarify exactly what is broken for you, ideally by using our examples and fixtures? |
@mhammond UniFFI does not support Swift bindgen of Cargo Workspace where CrateX depend in CrateY - where both CrateX and CrateY are UniFFI crates. UniFFI generates broken Swift code. |
We have a number of examples where CrateX depends on CrateY (eg, https://github.com/mozilla/uniffi-rs/tree/main/fixtures/ext-types) - I'm trying to understand what's different about your use-case which is broken vs that which apparently is not. |
Next week I can write a detailed report - with minimalistic code examples. |
I did not forget, just have to focus on completing some tasks at work this week before my vacation, will get back to this next week! |
@mhammond @bendk OK so I've created three small demo projects - projects which uses UniFFI with many crates having UniFFI as a dependency in a workspaces setup:
Insights:
Do you want me to create some demo projects using @bendk s fork (source branch of PR #2087 )? |
I would like to see this reproduced in our ext-types fixture against main. There we have many crates, all in the top-level workspace, and all of which depend on each other in various ways, including directly and indirectly. It's also worth noting that what you describe above is exactly how mozilla uses uniffi in production. Assuming Python doesn't compile due to an invalid package import, I believe your problem is that you don't have an appropriate |
@mhammond ok so here is an update:
|
Oops - the docs should mention that, but I think that's going to go away with Ben's work.
While that seems fine, I'm not sure what problems you are actually seeing here. Or to put it another way, what are you trying to do that's different to our fixtures? |
(@bendk asked me to create an issue )
Following up on my comment in 2087, using UniFFI with a Workspace works well in the land of Rust - especially with the new system for remote / custom / external types laid out in #2087.
However, when one generates Swift bindings, no cigar, the Swift code in each file tries to reference
(file)private
symbols such aslift
/lower` on types in the other file - which are not visible. The Kotlin bindings, however, does seem to work. The Kotlin Bindgen test in my small UniFFI Workspace demo project Lemon Meringue Pie passes. I have not tried doing an actual Kotlin bindgen generation besides the bindgen test.The Python bindings seems broken, for the small Python Bindgen tests in Lemon Meringue Pie, I get error:
We cannot simply change the access modifier to
internal
in the generated Swift code since each generated Swift file contains the same types, such asfileprivate protocol FfiConverterRustBuffer
.In this issue I lay out a straightforward idea on how to fix the broken Swift bindings specifically. I do not know enough about Python to lay out a plan for it, but perhaps the say idea is viable also for Python?
Idea of solution for Swift bindings
My idea is to:
generated/
folder (as specified by--out-dir
), and only be generated once, i.e. the contents oftemplate/StringHelper.swift
will be putGenerated/ScaffoldingUniFFI
folderfileprivate struct FfiConverterString: FfiConverter
- gets a changed visibility tointernal struct
instead offileprivate
.RustBufferTemplate
(and similar templates) get changed visibility tointernal
tooGenerated/Foo
,Generated/Bar
,Generated/Buzz
, whereFoo
,Bar
, `Buzz are the names of each of the users UniFFI exported crates.This is a straightforward solution which should not involve too much changes here in UniFFI.
The only downside of this solution I can think of is that some users of UniFFI might need to migrate their build scripts from
mv $out/Foobar.swift
to `mv $out/Generated (or similar).The text was updated successfully, but these errors were encountered: