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

Serious lack of quality in documentation #318

Closed
caesaneer opened this issue Feb 7, 2022 · 59 comments · Fixed by #320
Closed

Serious lack of quality in documentation #318

caesaneer opened this issue Feb 7, 2022 · 59 comments · Fixed by #320
Labels
enhancement New feature or request

Comments

@caesaneer
Copy link

Hello,

I was recently referred to this project by the team at Tokio. It looks very promising, however, the documentation suffers from a serious lack of quality and substance.

There is no "hello world" example and basic steps to get an example working. The tutorial is beyond brief and contains many broken links.

We've attempted to run the example projects by cloning the repo and ensuring all the dependencies are installed as per: https://fzyzcjy.github.io/flutter_rust_bridge/quickstart.html#-show-me-the-code

When executing the project using the command: flutter_rust_bridge_codegen --rust-input frb_example/pure_dart/rust/src/api.rs --dart-output frb_example/pure_dart/lib/bridge_generated.dart --c-output frb_example/pure_dart/ios/Runner/bridge_generated.h

the process panics with:

[SEVERE] : Couldn't find dynamic library in default locations. [SEVERE] : Please supply one or more path/to/llvm in ffigen's config under the key 'llvm-path'.

Please provide a simple step-by-step guide to setting up a hello world application. Please do not simply write refer to this and refer to that, and then see this over here and download this here and hope it works. This is an incredibly unprofessional way to present your tool and makes it very difficult to take your project seriously as a professional tool.

@welcome
Copy link

welcome bot commented Feb 7, 2022

Hi! Thanks for opening your first issue here! 😄

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 8, 2022

I agree. This library, at the beginning, was very small and I never thought it will have the popularity today ;)

And, I usually reply quickly (unless sleeping, surely). So open issues when you have problems as well.

Please provide a simple step-by-step guide to setting up a hello world application.

Could you plz have a look at: https://fzyzcjy.github.io/flutter_rust_bridge/tutorial_with_flutter.html In that example, just clone the code and run it and it is done.

[SEVERE] : Couldn't find dynamic library in default locations. [SEVERE] : Please supply one or more path/to/llvm in ffigen's config under the key 'llvm-path'.

That is ffigen's complain, because it cannot understand where your llvm is. llvm is a dependency of ffigen (not this library)

/cc @Desdaemon Do you have an interest in improving the documentation?

@Desdaemon
Copy link
Contributor

Sure, I can do that.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 8, 2022

@Desdaemon Sounds great!

@caesaneer
Copy link
Author

caesaneer commented Feb 9, 2022

Thank you for the quick reply.

When cloning the repository and executing flutter run from within the frb_example/with_flutter folder, the example works displaying Example 1 and Example 2.

We are able to generate bindings using ffigen without issue when using ffigen on a standalone project by adding a ffigen section to the pubspec.yaml file:

ffigen: name: NativeLibrary description: Bindings to headers/example.h. output: "generated_bindings.dart" llvm-path: - "/usr/lib/llvm-13" headers: entry-points: - "headers/example.h"
The pubspec.yaml files in the examples do not include ffigen, so we have no idea where to even configure llvm for FRB.

Like I said in my first comment, the documentation is incredibly sparse and not user-friendly. I do understand that this is a small project that has started to take off, so hopefully you guys can clean it up. I do believe it looks promising, and we excited to experimented with FRB.

What you need is a simple step-by-step guide that covers creating the basic project structure, installing dependencies, and performing a hello world. You could also walk the user through adding more complex examples once a user is up and running.

Your tutorial basically says: In this tutorial, let us draw a Mandelbrot set, which is plotted in Flutter UI, generated by Rust algorithm, and communicated via this library. Cool.

It then says clone this repo, run this command. This is not a tutorial. It's cloning something and running it. Not useful.

I'm not trying to be overly critical, and appreciate the work you've put into this, and I hope you can come up with more comprehensive documentation - more inline with the quality one expects from Rust projects.

@maranix
Copy link

maranix commented Feb 9, 2022

I agree, took me a whole day to figure out, how to bootstrap a starter flutter project with package. I had to install multiple things at were not even mentioned in the documentation and had to search for the issues separately in order to know what I was missing but It works great when it is configured properly.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 9, 2022

@danielcasler

The pubspec.yaml files in the examples do not include ffigen, so we have no idea where to even configure llvm for FRB.

frb executes ffigen in its flutter_rust_bridge_codegen binary, and you can look at

the documentation is incredibly sparse and not user-friendly. I do understand that this is a small project that has started to take off, so hopefully you guys can clean it up. I do believe it looks promising, and we excited to experimented with FRB.

Sure, @Desdaemon and I will improve the documentation soon, and I will come back to this issue when we are finished.

@ramanverma2k

Sorry to hear that. Personally I have installed everything on my computer and cannot uninstall everything to get a brand new env to work with. But I guess the CI files are a good way to refer to, since that installs everything from the beginning. Anyway we will improve doc.

And thank you for appreciating this lib :)

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 9, 2022

@Desdaemon

Hi how is it going on? Maybe you can make a draft PR so I can cooperate when I have some time.

In addition, shall we (temporarily) pin this issue in the issues tab? Then everyone who is going to use this lib will see that we are improving the doc (instead of leave disappointed without doc)

@fzyzcjy fzyzcjy added the enhancement New feature or request label Feb 9, 2022
@Desdaemon
Copy link
Contributor

@fzyzcjy I was planning to create a template repo then write step-by-step instructions around it, so I took some time setting up auto-build for all the platforms. I will create a draft PR this week.

@Desdaemon Desdaemon mentioned this issue Feb 9, 2022
10 tasks
@maranix
Copy link

maranix commented Feb 9, 2022

I can also contribute to the documentation. I'll wait for the PR and then go from there.

@caesaneer
Copy link
Author

caesaneer commented Feb 9, 2022

I've started putting together an example project with step-by-step instructions. It's very much a WIP.

https://github.com/Neksuhs/rusty_dart/blob/development/STEPS.md

I will likely publish this as a standalone article on Medium - maybe.

EDIT: This is the Dart version.

@Desdaemon
Copy link
Contributor

Desdaemon commented Feb 10, 2022

@danielcasler Looks good! I made a stripped-down version of the with-flutter example and made it into a template, if you want to check it out and see if it works for general usage.

https://github.com/Desdaemon/flutter_rust_bridge_template

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 10, 2022

@Desdaemon

I was planning to create a template repo then write step-by-step instructions around it, so I took some time setting up auto-build for all the platforms. I will create a draft PR this week.

Good idea! GitHub's "template repo" looks friendly for developers

@ramanverma2k @danielcasler Looking forward to it!

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 10, 2022

Cross-linking for who is interested: @Desdaemon has made a wonderful PR - #320

@caesaneer
Copy link
Author

@Desdaemon Nice. The Flutter example will be a good reference. I didn't just want to give people a project to run, I wanted to give them the basics on how to set up a project from scratch. This way people who are new to FFI get a little more hand-holding at the start.

My team started with Go, actually, and we've been using Go for years. We recently started with Rust for a few projects and really like it. Between Rust and Go, there is no way I could convince anyone here to use Dart for any sort of serious business logic layer. UI stuff, sure, but asking them to use Dart is such a downgrade = no one would do it and I can't really blame them. lol

We are hoping that the FFI will allow us to build a nice business logic layer similar to AppFlowy.
https://blog-appflowy.ghost.io/tech-design-flutter-rust/

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 10, 2022

We are hoping that the FFI will allow us to build a nice business logic layer similar to AppFlowy.

Yes, frb currently has the capability.

@caesaneer
Copy link
Author

Is it possible with StreamSink to stream structs to Dart?

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 10, 2022

Yes. Personally I use streams to send logs (you know, a struct with tag, message, time, ...) from rust to dart. It works perfectly in my app in production.

@caesaneer
Copy link
Author

Nice, that sounds close to what we want. Does it work fine on iOS and Android to?

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 10, 2022

@danielcasler Sure. My flutter app works fine on both.

@caesaneer
Copy link
Author

Is there an example of how to use StreamSink with a custom struct?

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 10, 2022

Just do it as normal

pub fn handle_stream(sink: StreamSink<String>, arg: String) -> Result<()> {

@caesaneer
Copy link
Author

caesaneer commented Feb 10, 2022

Yeah, I tried that but it wasn't working. The compiler was complaining that the struct didn't implement IntoDart trait. I think I may have it now though. Just have to do the Dart side...

@caesaneer
Copy link
Author

caesaneer commented Feb 10, 2022

I am really surprised (and impressed) by how performant this is. I have it working, streaming multiple structs back to Dart in sub 500 μs

I don't know why but I was expecting millisecond latency.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 10, 2022

I am really surprised (and impressed) by how performant this is. I have it working, streaming multiple structs back to Dart in sub 500 μs
I don't know why but I was expecting millisecond latency.

Wow that is great! I guess it is because, frb is really a very minimal wrapper (though feature-rich). You can look at the code. It is nothing but a ffi call or some Dart API call.

@caesaneer
Copy link
Author

It's a great little project once you understand it all. Good job!

I noticed it's using @Desdaemon's project allo-isolate, which talks about communicating directly with a Dart isolate using the native port for a send port. Could you tell me what a native port is? The Dart docs don't say much.

Is it an FFI for talking to an isolate directly, or is it like a raw port that can be used to send bits of data? Sorry for the question, this one is a little outside my experience currently.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 10, 2022

I noticed it's using @Desdaemon's project allo-isolate

Well indeed @shekohex . I contribute to that repo as well, since the original code lacks some parts.

Could you tell me what a native port is? The Dart docs don't say much.

Is it an FFI for talking to an isolate directly, or is it like a raw port that can be used to send bits of data? Sorry for the question, this one is a little outside my experience currently.

just like a "handle"?

@caesaneer
Copy link
Author

caesaneer commented Feb 10, 2022

I noticed you added this to the main README.md file: Fast: It is only a thin (though feature-rich) wrapper, no overhead such as protobuf serialization, thus performant (e.g. <500μs when streaming multiple structs back).

My test was very crude just to get a sense as of the latency. It certainly was publishable-ready. :)

What I will do tomorrow is create some actual benchmarks, with code we can publish, so others can run/validate them as well.

fzyzcjy added a commit to Desdaemon/flutter_rust_bridge that referenced this issue Feb 13, 2022
@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 13, 2022

@tmpfs Thanks, doc modified Desdaemon@00b992f (will get merged later).

@ramanverma2k 😄

@tmpfs
Copy link
Contributor

tmpfs commented Feb 13, 2022

Some notes on getting this working in Macos flutter that we can add to the docs later.

Once you have the bindings working add the dynamic library to the Frameworks group in xcode, be sure to use the x86_64-apple-darwin build until flutter has full aarch64 support.

Then ensure it is bundled. Go to Build Phases > Bundle Framework and select the .dylib file you added to Frameworks.

Now you can load the dynamic library for the Macos desktop:

const base = 'rust_flutter';
final path = Platform.isWindows
    ? '$base.dll'
    : Platform.isMacOS
        ? 'lib$base.dylib'
        : 'lib$base.so';
late final dylib = Platform.isIOS ? DynamicLibrary.process() : DynamicLibrary.open(path);
late final api = RustFlutterImpl(dylib);

Replace rust_flutter with the name of your library and change RustFlutterImpl to the implementation in your generated dart bindings.

A lot of this is quite new to me, so hope it helps somebody else out!

@tmpfs
Copy link
Contributor

tmpfs commented Feb 13, 2022

Anyone got any pointers on setting up on IOS correctly? I am stuck on this error:

[VERBOSE-2:ui_dart_state.cc(209)] Unhandled Exception: Invalid argument(s): Failed to lookup symbol 'store_dart_post_cobject': dlsym(RTLD_DEFAULT, store_dart_post_cobject): symbol not found
#0      DynamicLibrary.lookup (dart:ffi-patch/ffi_dynamic_library_patch.dart:34:70)
dart-lang/ffigen#1      RustFlutterWire._store_dart_post_cobjectPtr (package:rust_flutter/bridge_generated.dart:116:76)
dart-lang/ffigen#2      RustFlutterWire._store_dart_post_cobjectPtr (package:rust_flutter/bridge_generated.dart)
dart-lang/ffigen#3      RustFlutterWire._store_dart_post_cobject (package:rust_flutter/bridge_generated.dart:118:41)
dart-lang/ffigen#4      RustFlutterWire._store_dart_post_cobject (package:rust_flutter/bridge_generated.dart)
dart-lang/ffigen#5      RustFlutterWire.store_dart_post_cobject (package:rust_flutter/bridge_generated.dart:110:12)
dart-lang/ffigen#6      FlutterRustBridgeBase._setUpRustToDartComm (package:flutter_rust_bridge/src/basic.dart:35:11)
dart-lang/ffigen#7      new FlutterRustBridgeBase (package:flutter_rust_bridge/src/basic.dart:20:5)
dart-lang/ffigen#8      new RustFlutterImpl.raw (package:rust_flutter/bridg<…>

I have added the C header file to the project and imported it in the primary bridging header, created the universal binary using cargo lipo (in fact I had to ignore the aarch64 architecture cargo lipo --targets x86_64-apple-ios) setup up the excluded architectures to include arm64 so it runs on the simulator.

I have also set Strip Style to Debugging Symbols in case it was getting stripped but it appears to exist when I check with objdump -t librust_flutter.a.

I have configured pods with pod init and pod install and that all seems fine.

What I don't know yet is how to create the Pods_Runner.framework - is that critical?

Code is here: https://github.com/tmpfs/rust-flutter

Thanks for any help 🙏

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 13, 2022

@tmpfs

@Desdaemon is doing an excellent job on documentations, so you may have a look at the book: https://github.com/Desdaemon/flutter_rust_bridge/blob/doc_fix/book/src/SUMMARY.md

@tmpfs
Copy link
Contributor

tmpfs commented Feb 13, 2022

Thanks @fzyzcjy and @Desdaemon those docs look really helpful but sadly the IOS Setup section is empty :(

Any ideas on that error? I believe i am compiling the header file and statically linking the universal library but seem to be stuck on this. How do i create the Pods_Runner.framework that i see in the example project please?

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 13, 2022

@tmpfs what about "[Integrating with existing projects]" and its following several subsection

@Desdaemon
Copy link
Contributor

Desdaemon commented Feb 13, 2022

but sadly the IOS Setup section is empty :(

@tmpfs It's a draft right now, but the only thing I meant to write there is Rust targets specific to iOS development. I'll fill it out soon.

As for the error, try using the dummy function within AppDelegate.swift, that way XCode won't strip the symbols.

@tmpfs
Copy link
Contributor

tmpfs commented Feb 13, 2022

but sadly the IOS Setup section is empty :(

@tmpfs It's a draft right now, but the only thing I meant to write there is Rust targets specific to iOS development. I'll fill it out soon.

As for the error, try using the dummy function within AppDelegate.swift, that way XCode won't strip the symbols.

Thank you so much @Desdaemon! That was the bit I was missing.

I will likely have to go through this process again once I can get it all working so I hope to help you finish those docs as I learn more about how all the pieces of the puzzle fit together 👍

@tmpfs
Copy link
Contributor

tmpfs commented Feb 14, 2022

So android was pretty straightforward, just needed to install the right NDK as per the docs and cargo-ndk. Then I ran:

cargo ndk -t armeabi-v7a -t arm64-v8a -o ./android/app/src/main/jniLibs build --release

I couldn't use the Android Studio snippet for integrating the build as the project is using Kotlin for Gradle to I will need to rewrite that to Kotlin at some point.

However, integrating with Linux is proving to be very painful due to Flutter using an old version of cmake (3.10) and FetchContent not being available until a later version (3.13).

I have installed much newer versions of cmake but getting flutter to use them seems impossible, it always uses the version it installs and I can't find a way to make it use an alternative system version of cmake.

I tried installing Corrosion globally and using it that way but that also fails.

@fzyzcjy and @Desdaemon any ideas how to get flutter to use my newer version of cmake that should allow me to use the rust.cmake include file? I tried using cmake directly but that fails with this error:

Unhandled exception:
RangeError (index): Invalid value: Only valid value is 0: 1
#0      List.[] (dart:core-patch/array.dart:121:36)
dart-lang/ffigen#1      main (file:///home/parallels/snap/flutter/common/flutter/packages/flutter_tools/bin/tool_backend.dart:12:37)
dart-lang/ffigen#2      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:295:32)
dart-lang/ffigen#3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:192:12)
ninja: build stopped: subcommand failed.

FYI I am using Ubuntu 20.04 on ARM64 so maybe I need to try on x86_64.

@Desdaemon
Copy link
Contributor

Desdaemon commented Feb 14, 2022

I tried installing Corrosion globally and using it that way but that also fails.

This is going to cause some issues down the road. I'll try to add more CI setup and investigate this separately.

FYI I am using Ubuntu 20.04 on ARM64 so maybe I need to try on x86_64.

Our CI doesn't (and probably unable to for the time being) cover this host, so we would definitely love to hear more about how to fix this issue.

@tmpfs
Copy link
Contributor

tmpfs commented Feb 14, 2022

I got it working on Linux with ARM64 by ignoring the rust.cmake template and adding this to the bottom of the main cmake file:

set(NATIVE_LIBRARY "${CMAKE_SOURCE_DIR}/../target/aarch64-unknown-linux-gnu/debug/librust_flutter.so")
install(FILES "${NATIVE_LIBRARY}" DESTINATION "${INSTALL_BUNDLE_LIB_DIR}"
  COMPONENT Runtime)

With some conditionals for architectures and debug/release builds it should be good enough for Linux 👍

@caesaneer
Copy link
Author

if a user wanted to have a long-running Rust layer, a good architecture in a Flutter/Rust scenario might be to have a main isolate launch a secondary isolate with a StreamSink that then launches the Rust layer. The Rust layer could then provide the sink to threads or Tokio tasks that need to communicate to the Flutter layer.

Hmm maybe no need? Notice that the rust function calls are async. In other words, say you have a rust function fn compute_heavy_things() that will run for 10 seconds. Then your dart code can simply call it, and happily do everything else, and the dart code will never be blocked.

You're referring to making a call to Rust that starts and stops. I was asking if it is possible to make a call to Rust, start an application layer that runs for the life of the app being open for example and have that layer send messages to Flutter when events take place.

@Desdaemon
Copy link
Contributor

Desdaemon commented Feb 14, 2022

if a user wanted to have a long-running Rust layer, a good architecture in a Flutter/Rust scenario might be to have a main isolate launch a secondary isolate with a StreamSink that then launches the Rust layer. The Rust layer could then provide the sink to threads or Tokio tasks that need to communicate to the Flutter layer.

@danielcasler I believe Rust functions are already called in a separate worker thread, so that calls to Rust never block and removes the need to spawn isolates. Theoretically too many long-running StreamSinks may exhaust the thread pool, but it should be possible to customize the thread pool to accommodate this, please correct me if I'm wrong @fzyzcjy. (Personally, this area is somewhat sparsely documented.)

@caesaneer
Copy link
Author

@Desdaemon I've not looked at the generated code enough to know. I just assumed that when working with a StreamSink via FRB, launching an isolate to handle the StreamSink and then communicate back to the main isolate would make the most sense.

@Desdaemon
Copy link
Contributor

@danielcasler That's fair. The generated code does not make this obvious, because the thread pool implementation of StreamSink lives here in the Rust runtime module.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Feb 16, 2022

@danielcasler I have made the benchmarking into a separate issue dart-lang/native#270 to make the issue tracker clearer ;)

@Desdaemon
Copy link
Contributor

Desdaemon commented Feb 18, 2022

I have installed much newer versions of cmake but getting flutter to use them seems impossible, it always uses the version it installs and I can't find a way to make it use an alternative system version of cmake.

@tmpfs The solution was to bump the CMake version in CMakeLists.txt to 3.11, which is the first version that supported FetchContent. You can check out the diff here.

@tmpfs
Copy link
Contributor

tmpfs commented Feb 19, 2022

I have installed much newer versions of cmake but getting flutter to use them seems impossible, it always uses the version it installs and I can't find a way to make it use an alternative system version of cmake.

@tmpfs The solution was to bump the CMake version in CMakeLists.txt to 3.11, which is the first version that supported FetchContent. You can check out the diff here.

Thanks for the help @Desdaemon I tried that already but whatever I do fails with:

CMake Error at CMakeLists.txt:1 (cmake_minimum_required):
  CMake 3.11 or higher is required.  You are running version 3.10.2

When I run flutter run -d linux.

I have installed a much newer version of cmake:

ubuntu-linux-20-04-desktop% cmake --version
cmake version 3.23.0-rc1

But flutter insists on using it's bundled version. How do I get flutter to use my system cmake?

@tmpfs
Copy link
Contributor

tmpfs commented Feb 19, 2022

Found a related issue: flutter/flutter#75600

@tmpfs
Copy link
Contributor

tmpfs commented Feb 19, 2022

Ok, I found it on Ubuntu when flutter is installed as a snap:

/snap/flutter/current/usr/bin/cmake --version

Is the culprit! So I tried to back it up and symlink but cannot when installed as a snap:

mv: cannot move 'cmake' to 'cmake-3.10': Read-only file system

So I think I need to remove the flutter snap and try to install it another way on Ubuntu. Then I can hack the cmake binary.

Anyone know why flutter is shipping such an out of date version of cmake? I wonder if something else will break once I manage to use a newer version...

Edit: I can't install outside of snap right now as I am running Ubuntu on ARM so installing from the tarball gives me an exec error on the executables as they are pre-compiled for x86_64.

I give up on using Corrosion for now, doesn't seem feasible until Flutter upgrades cmake.

@Desdaemon
Copy link
Contributor

@tmpfs It appears that this issue has already been raised at the official Snap repo to little fanfare. I'll add a new section for Snap users soon.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants