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

Add a relatively simple way to compile and generate Swift bindings. #1647

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

gezihuzi
Copy link

@gezihuzi gezihuzi commented Jul 9, 2023

As mentioned in issue #1623 , by using the cargo-xcode crate, Rust crates can be quickly compiled into static (or dynamic) libraries. Then, by wrapping the generated code with UniFFI into a Framework, the process of integrating Rust crates can be simplified.

Add quick integration of Rust crate using Xcode+Framework, and include relevant documentation and usage examples.
@gezihuzi gezihuzi requested a review from a team as a code owner July 9, 2023 14:14
@gezihuzi gezihuzi requested review from mhammond and removed request for a team July 9, 2023 14:14
@mhammond
Copy link
Member

This looks great at a quick glance, thanks! I'll look more in a few days, but in the meantime: I don't know much about xcode, but how difficult would it be to describe how to create the project in xcode rather than adding an entire project to the repo? It seems a shame cargo-xcode can't help :)

@gezihuzi
Copy link
Author

This looks great at a quick glance, thanks! I'll look more in a few days, but in the meantime: I don't know much about xcode, but how difficult would it be to describe how to create the project in xcode rather than adding an entire project to the repo? It seems a shame cargo-xcode can't help :)

Thank you for reviewing this PR. Let me explain my thoughts. Developers can certainly continue to use the method mentioned in Integrating with Xcode (implementation in examples/app/ios) to compile Rust crate dependencies. However, cargo-xcode only requires one command to quickly generate an Xcode project that can be directly used and imported as a target dependency into other Xcode projects. It is simply a simpler and faster alternative solution. Regardless of which approach is chosen, the ultimate goal is to use Xcode for related development work, so I hope that integrating Rust crates can be as simple, clear, and fast as possible.

@mhammond
Copy link
Member

However, cargo-xcode only requires one command to quickly generate an Xcode project that can be directly used and imported as a target dependency into other Xcode projects.

That's why I was a little surprised to find what appears to be an xcode project as part of the PR - I don't think it captures exactly how it makes life easier. Isn't there some way to demonstrate it without so many new files added to the repo?

@gezihuzi
Copy link
Author

However, cargo-xcode only requires one command to quickly generate an Xcode project that can be directly used and imported as a target dependency into other Xcode projects.

That's why I was a little surprised to find what appears to be an xcode project as part of the PR - I don't think it captures exactly how it makes life easier. Isn't there some way to demonstrate it without so many new files added to the repo?

I provided an example application project that has the same content as the document. This Xcode sample application is optional.

@theolampert
Copy link

theolampert commented Jul 13, 2023

I think it's a great idea to add this to the documentation, I was looking for something like this. But I feel relying on another tool is quite opinionated, I think it would make more sense if the UniFFI docs just provided a minimal, working example as a starting point. Generating and configuring an Xcode project should be left up to the end user.

Maybe something like the bash script the y-crdt people are using here could be helpful? I was able to modify that to create an xcframework which I could drag into Xcode along with the generated Swift file or package as an SPM module.

@mhammond
Copy link
Member

I think it's a great idea to add this to the documentation, I was looking for something like this.

Yeah, I'm still trying to find the correct balance here.

But I feel relying on another tool is quite opinionated, ...
Maybe something like the bash script the y-crdt people are using here could be helpful?

But that's another tool, right? That one is written in bash while the other is in Rust, but I don't see a difference. And the Rust version is more likely to work on Windows :)

I think it would make more sense if the UniFFI docs just provided a minimal, working example as a starting point. Generating and configuring an Xcode project should be left up to the end user.

I'm inclined to agree with that - if we end up where we don't actually check any xcode projects in, but instead just allow a some relatively modest instructions, is there any reason we can't allow many tools to be so documented?

@theolampert
Copy link

But that's another tool, right? That one is written in bash while the other is in Rust, but I don't see a difference. And the Rust version is more likely to work on Windows :)

Yeah, that's definitely fair. It was clearer to me to look over a bash script and see what the exact steps were to get something similar working in my app and how I'd use it in a CI pipeline for example. I have to admit, I forgot Windows support. The cargo-xcode library lists Xcode as a requirement, however, so it might not work either. I also stumbled on this https://github.com/antoniusnaumann/cargo-swift - maybe it's somehow useful for reference too.

Copy link
Member

@badboy badboy left a comment

Choose a reason for hiding this comment

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

Did a first pass on this, see inline comments mostly


First, we need to install `cargo-xcode`. This tool can help us generate Xcode project files and compile them into static libraries.

Run the command `cargo install cargo-xcode` to install.
Copy link
Member

Choose a reason for hiding this comment

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

Better to make it a code block (easier to copy and more obvious)

Suggested change
Run the command `cargo install cargo-xcode` to install.
```
cargo install cargo-xcode
```


Run the command `cargo install cargo-xcode` to install.

We need to modify the `Cargo.toml` file and add crate-type = ["lib", "staticlib"] in the [lib] section. Here you can add other types according to your needs, but only `staticlib` and `cdylib` can be recognized by `cargo-xcode`.
Copy link
Member

@badboy badboy Jul 11, 2023

Choose a reason for hiding this comment

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

Suggested change
We need to modify the `Cargo.toml` file and add crate-type = ["lib", "staticlib"] in the [lib] section. Here you can add other types according to your needs, but only `staticlib` and `cdylib` can be recognized by `cargo-xcode`.
Ensure your crate builds as a `cdylib` by adding the following to your `Cargo.toml` file:
```toml
[lib]
crate-type = ["lib", "staticlib"]
name = "<library name>"
```

Copy link
Author

@gezihuzi gezihuzi Jul 22, 2023

Choose a reason for hiding this comment

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

Thank you for your suggestion, I am reviewing the relevant changes. I have found some issues with the description here. staticlib and cdylib correspond to *.a and *.dylib libraries respectively. The staticlib in the crate-type section does not match the described cdylib on documentation, so it needs to be modified.

Details:

Ensure your crate builds as a `staticlib` by adding the following to your `Cargo.toml` file: 

Copy link
Member

Choose a reason for hiding this comment

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

Woops, yes. That should of course be the same in text and code.

* Add a build rule processing files with names matching *.udl.

* Use something like the following as the custom script:
* `$HOME/.cargo/bin/uniffi-bindgen-cli generate $INPUT_FILE_PATH --language swift --out-dir $INPUT_FILE_DIR/Generated`
Copy link
Member

Choose a reason for hiding this comment

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

We don't ship this CLI anymore. See https://mozilla.github.io/uniffi-rs/tutorial/foreign_language_bindings.html
So we need to rephrase that in a way to use a bundled CLI.

Copy link
Author

Choose a reason for hiding this comment

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

The uniffi-bindgen-cli here is actually a binary file generated by me based on the content in https://mozilla.github.io/uniffi-rs/tutorial/foreign_language_bindings.html. The code is provided by the documentation, with just the addition of "-cli" to its name. It is used for conveniently calling the uniffi-bindgen generation command with a fixed path in Xcode.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but this binary won't be installed into $HOME/.cargo/bin/uniffi-bindgen-cli by default anymore, so we shouldn't recommend that in official documentation.

docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
@badboy
Copy link
Member

badboy commented Jul 17, 2023

I think it's a great idea to add this to the documentation, I was looking for something like this.

Yeah, I'm still trying to find the correct balance here.

But I feel relying on another tool is quite opinionated, ...
Maybe something like the bash script the y-crdt people are using here could be helpful?

But that's another tool, right? That one is written in bash while the other is in Rust, but I don't see a difference. And the Rust version is more likely to work on Windows :)

I don't think Windows support really plays a role for any Xcode-related project. You will need a Mac anyway.


As seen above I gave this a first look, but I'll take a detailed look tomorrow again.

docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
docs/manual/src/swift/framework.md Outdated Show resolved Hide resolved
Comment on lines +77 to +82
It also provides an [ios-with-framework](examples/app/ios-with-framework/) that you can check out under examples/app/ios-with-framework/.

* `ios-with-framework`: Root directory of the sample project

* `iOS-UniFFI-Framework`: Includes handling of compiling Rust crates into static libraries and generating bindings.
* `iOS-UniFFI-App`: Includes the use of the generated framework.
Copy link
Member

Choose a reason for hiding this comment

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

The project does not build as is for me.
Let's rip that out for now and see if maybe we can add it as an example later. That way we can at least land the initial docs now.

Copy link
Author

@gezihuzi gezihuzi Jul 22, 2023

Choose a reason for hiding this comment

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

I guess it's caused by the inability to invoke uniffi-bindgen-cli

Copy link
Member

Choose a reason for hiding this comment

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

No, I actually fixed that.
I'd still say let's not overiterate on that, we can add the project in a followup PR

Comment on lines +84 to +86
## Known issues

* If you encounter an error when generating bindings, please check if `uniffi-bindgen-cli` is installed. If the path is incorrect, please modify the script path in `Build Rules`.
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned above we don't ship that CLI anymore as is.

gezihuzi and others added 11 commits July 22, 2023 12:48
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
Co-authored-by: Jan-Erik Rediger <janerik@fnordig.de>
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.

None yet

4 participants