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

FFI plugins #96225

Merged
merged 1 commit into from
Jan 26, 2022
Merged

FFI plugins #96225

merged 1 commit into from
Jan 26, 2022

Conversation

dcharkes
Copy link
Contributor

@dcharkes dcharkes commented Jan 6, 2022

Add support for flutter create --template=plugin_ffi and flutter run support for ffiPlugin: true in pubspec.yaml.

This addresses 2 issues:

  1. Currently, the way to do FFI with Flutter is to use flutter create --template=plugin. However, this generates boilerplate for plugin registration and example code for method channels which go unused.
  2. We've see dozens of issues with building, linking, and bundling native code. The FFI plugin template should be the sample that works for all platforms for users to reference or use as a starting point.

Design doc.
NDK versioning doc.

Some relevant issues:

Included in this PR:

  • Building shared C source code as part of the native build for platforms Android, iOS, Linux desktop, MacOS desktop, and Windows desktop.
  • Sample code doing a synchronous FFI call.
  • Sample code doing a long running synchronous FFI call on a helper isolate.
  • Use of package:ffigen to generate the bindings.

Not included in this PR:

  • Bundling closed source libraries. (MacOS, iOS, and Android require copying to the right place. Linux and Windows require referencing from CMake files.)
  • Building closed source libraries with the right compiler options (most notably rpath and include path).
  • Support for flutter test. This requires a bin/setup.dart to invoke a native build outside flutter run or flutter build.
  • Support for using the generated package as Dart standalone. This requires a bin/setup.dart to invoke a native build.
  • Linking against dart_api_dl.h to be able to invoke functions from dart_api.h.
  • Sample code doing an asynchronous callback through native ports form dart_api.h.
  • Compatibility between the plugin and plugin_ffi templates. The example apps (example/lib/main.dart) are different.

Test this PR locally without building Flutter by running the Flutter commands from source (replace platforms for your host OS).

$ dart <PATH_TO_FLUTTER_CHECKOUT>/packages/flutter_tools/bin/flutter_tools.dart create -t plugin_ffi --platforms=macos,ios,android my_ffi_plugin
$ cd my_ffi_plugin/example/
$ dart <PATH_TO_FLUTTER_CHECKOUT>/packages/flutter_tools/bin/flutter_tools.dart run -d macos

For context, previous PR (reverted):

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels. labels Jan 6, 2022
@dcharkes dcharkes force-pushed the ffi-plugins branch 2 times, most recently from d14b977 to de71c8d Compare January 7, 2022 14:21
@simc
Copy link

simc commented Jan 11, 2022

Will support for prebuilt libraries be added in the future? It's not only important for closed-source libraries but also if the compiler is too difficult to setup on the users machine (for example for Rust libraries)

@dcharkes
Copy link
Contributor Author

Will support for prebuilt libraries be added in the future? It's not only important for closed-source libraries but also if the compiler is too difficult to setup on the users machine (for example for Rust libraries)

It is the intention to support that use case as well. At this point we're not entirely sure whether that warrants a new template, or whether a sample and documentation is enough. (Some pointers based on the source template: add the precompiled dl path to Windows Linux CMakeLists.txt in _bundled_libraries, copy the dl to MacOS iOS Frameworks folder, copy the dl to jniLibs.)

Also, it would require package authors to upload the binaries for all (18) target platforms (hardware * OS combination) to pub. We're exploring if there are any obstacles for doing that.

@simc
Copy link

simc commented Jan 11, 2022

Building the binaries for all platforms is no problem but more documentation would be very helpful. Especially how to:

  • put shared libraries in the correct place to be found at runtime
  • add multiple binaries for the same OS (ios_arm64, ios_arm64_sim etc.)
  • make sure the binaries don't get stripped
  • verify that the binary is added at compile time


Future<TaskResult> call() async {
final Directory tempDir =
Directory.systemTemp.createTempSync('flutter_devicelab_plugin_test.');
// FFI plugins do not have support for `flutter test`.
// `flutter test` does not do a native build.
Copy link
Member

Choose a reason for hiding this comment

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

unrelated to this test, what is the user experience if I run flutter test on an app with an FFI plugin? Does it just fail to lookup the symbol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The FFI plugin template itself doesn't include any unit tests:

Test directory "test" not found.

When adding a test, opening the dynamic library fails:

Shell: Invalid argument(s): Failed to load dynamic library '<...>':

I created a tracking issue:

'Try creating a fresh project and migrating your existing code to '
'the new project manually.');
}
}
Copy link
Member

Choose a reason for hiding this comment

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

this could still be within the else {, right? since if we've set template on line 165 then the value of detectedProjectType won't be used anyway, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is in the else, the following code is dead code:

    if (detectedProjectType != null && template != detectedProjectType && metadataExists) {
      // We can only be definitive that this is the wrong type if the .metadata file
      // exists and contains a type that doesn't match.
      throwToolExit("The requested template type '${flutterProjectTypeToString(template)}' doesn't match the "
          "existing template type of '${flutterProjectTypeToString(detectedProjectType)}'.");
    }

template != detectedProjectType will never be true.

That means we do not detect inconsistent project types. For example having method channel plugin in the .metadata file and plugin_ffi as an argument. Not detecting inconsistent project types results in a garbled project state with files from two different templates mixed together.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

Lots of small comments, but overall this is looking really good!

@dcharkes
Copy link
Contributor Author

Lots of small comments, but overall this is looking really good!

@stuartmorgan I've addressed all of them. PTAL.

@christopherfujino I've replied to your comment above. Please let me know if you have any further comments.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for all the work here :)

packages/flutter_tools/gradle/flutter.gradle Outdated Show resolved Hide resolved
packages/flutter_tools/lib/src/commands/create_base.dart Outdated Show resolved Hide resolved
packages/flutter_tools/lib/src/flutter_plugins.dart Outdated Show resolved Hide resolved
packages/flutter_tools/lib/src/platform_plugins.dart Outdated Show resolved Hide resolved
@dcharkes
Copy link
Contributor Author

Thanks Stuart!

Thanks for all the work here :)

Let's make our users happy!

@christopherfujino waiting on your approval (or delegation) as flutter_tools owner.

Copy link
Member

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for landing this!

@dcharkes dcharkes merged commit 0e2f51d into flutter:master Jan 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2022
@GregoryConrad
Copy link
Contributor

GregoryConrad commented Nov 17, 2022

I have actually just ran into the issue @leisim prophesied; I am trying to build a Flutter library wrapping around a Rust library. Seeing as I cannot expect users of the library to have rustup installed, I plan on distributing precompiled versions of the library for all the targets.

For macOS/iOS, my best attempt was to create an XCFramework from the compiled rust static libraries (quite a few .a files!). After lipo-ing a the simulator and macOS static libs together, I was able to create the full XCFramework. For future reference, perhaps if someone is making some Rust/Flutter library documentation, I have this process thrown together in a build script. This takes care of all Apple platforms in one framework, which is very handy. However, when I went to include this Framework in my ffi plugin, I ran into linker errors as seen in this SO post.

@dcharkes mentions:

(Some pointers based on the source template: add the precompiled dl path to Windows Linux CMakeLists.txt in _bundled_libraries, copy the dl to MacOS iOS Frameworks folder, copy the dl to jniLibs.)

I'm most interested in the macOS/iOS part here (for now). Could you please provide a brief working example of how to go about this for macOS/iOS? I have tried for hours and cannot get the XCFramework & podspec file to work properly.

Edit: just realized pub.dev has a 100 MB package limit, which is an issue. I guess users will need to install rustup in the meantime. However, what I outlined above is still an issue.

@dcharkes
Copy link
Contributor Author

100 MB package version limit

@jonasfj We have our first user hit the 100 MB limit.

Precompiled static libraries on MacOS/iOS in a xcframework

I'm most interested in the macOS/iOS part here (for now). Could you please provide a brief working example of how to go about this for macOS/iOS? I have tried for hours and cannot get the XCFramework & podspec file to work properly.

I've got something slightly working for precompiled static libraries in an xcframework:

mylib_staticlib/macos/mylib_staticlib.podspec

#
# To learn more about a Podspec see http://guides.cocoapods.org/syntax/podspec.html.
# Run `pod lib lint mylib_staticlib.podspec` to validate before publishing.
#
Pod::Spec.new do |s|
  s.name             = 'mylib_staticlib'
  s.version          = '0.0.1'
  s.summary          = 'A new flutter plugin project.'
  s.description      = <<-DESC
A new flutter plugin project.
                       DESC
  s.homepage         = 'http://example.com'
  s.license          = { :file => '../LICENSE' }
  s.author           = { 'Your Company' => 'email@example.com' }
  s.source           = { :path => '.' }
  s.source_files     = 'Classes/**/*'
  s.dependency 'FlutterMacOS'

  s.platform = :osx, '10.11'
  s.pod_target_xcconfig = { 'DEFINES_MODULE' => 'YES' }
  s.vendored_libraries = 'Frameworks/libmylib_staticlib.a'
  s.pod_target_xcconfig = { "OTHER_LDFLAGS" => "-force_load $(PODS_TARGET_SRCROOT)/Frameworks/mylib_staticlib.xcframework/macos-arm64_x86_64/libmylib_staticlib.a" }
  s.swift_version = '5.0'
end

mylib_staticlib/macos/Classes/dummy_file.c

// A dummy file so that the mylib_staticlib.framework is created.

Path of the framework with static libraries: mylib_staticlib/ios/Frameworks/mylib_staticlib.xcframework/ios-arm64_x86_64-simulator/libmylib_staticlib.a

One issue with doing it like this is that the ios_arm64 is compiled either against the iossimulator or against ios, and I wasn't able to figure out how to make that both work.

@simc
Copy link

simc commented Nov 17, 2022

Check out the Isar podspec and the build script for Rust.

It works really well even without -force_load and supports all cpu/simulator configurations. That being said it is still a huge pain to ship native libraries for all platforms and I really hope there will be a nice solution by Flutter. Ideally allowing static compilation into the Flutter native lib.

@dcharkes
Copy link
Contributor Author

dcharkes commented Nov 17, 2022

Check out the Isar podspec and the build script for Rust.

This looks like a better solution than mine, @GregoryConrad please take this as example instead. 😄

It works really well even without -force_load and supports all cpu/simulator configurations. That being said it is still a huge pain to ship native libraries for all platforms and I really hope there will be a nice solution by Flutter. Ideally allowing static compilation into the Flutter native lib.

As the end goal, we would like static compilation into the compiled Dart code, that means we can use the native linker to get rid off all native code not referenced. Tracked in:

@GregoryConrad
Copy link
Contributor

GregoryConrad commented Nov 17, 2022

@leisim Thanks for that! I didn't think of using a regular plugin package instead of an FFI plugin to actually integrate everything but it looks like it works smoothly. Also, thank you both for the speedy response!

Static Linking w/ Dart

This would be absolutely fantastic, as the generated static libraries I am dealing with are quite large (~100-200 mb per platform & architecture) and I am sure they have plenty of dead code not needed by my Dart library in them (caused by a large number of dependencies in the Rust library I am wrapping).

100 MB Limit

As I mentioned in the previous paragraph, the static libraries I am dealing with are exceedingly large, and the entire upload could easily be above 1 GB. If you do consider changing the pub limit, perhaps it would be worth considering having a separate upload for static/native libraries and then link to them in the main upload (or perhaps a different way to handle them altogether)? Related (above comment):

Also, it would require package authors to upload the binaries for all (18) target platforms (hardware * OS combination) to pub. We're exploring if there are any obstacles for doing that.

Maybe, if Dart does get static linking, the theoretical separate upload I mentioned above could just be for native libraries that are linked automatically by Flutter/Dart tooling? That could be a really elegant solution that is probably easier for package maintainers and the tooling alike, i.e.:

  • macOS/iOS: 1 XCFramework, as it seems to be the recommended approach nowadays and handles iOS/macOS/iOS Simulator. Or, just for the .a files for each architecture/platform? Not sure what would be best here.
  • Android/Linux: some .sos
  • Windows: a .dll

@simc
Copy link

simc commented Nov 17, 2022

@GregoryConrad 100-200MB sounds incredibly large. Did you check the steps to reduce Rust binary size? It's quite expensive to load such a large binary at runtime.

@GregoryConrad
Copy link
Contributor

I was actually going to look into that later today since it did seem rather large; thanks for the link.

@GregoryConrad
Copy link
Contributor

In case anyone else stumbles into this, I got FFI working with my cross-platform XCFramework using ffiPlugin: true (thanks to @leisim's great example with Isar!). I can't believe I didn't realize this, but my XCFramework was getting tree-shaken. Here's how I fixed it (same exact code for macOS and iOS, but cannot be symbolically linked unfortunately):

  • my_package.podspec
Pod::Spec.new do |spec|
  spec.name          = 'my_package'
  spec.version       = '0.0.1'
  spec.license       = { :file => '../LICENSE' }
  spec.homepage      = ''
  spec.authors       = { 'Your Name' => 'blabla@example.com' }
  spec.summary       = 'Some summary'

  spec.source              = { :path => '.' }
  spec.source_files        = 'Classes/**/*'
  spec.public_header_files = 'Classes/**/*.h'
  spec.vendored_frameworks = 'Frameworks/MyPackage.xcframework'

  spec.ios.deployment_target = '11.0'
  spec.osx.deployment_target = '10.11'
end
  • Classes/binding.h
void enforce_binding();
  • Classes/EnforceBinding.swift
public func dummyMethodToEnforceBundling() {
    enforce_binding() // disable tree shaking
}
  • Frameworks/MyPackage.xcframework needs to have a void enforce_binding() {} function. I did this in Rust with:
/// Enforce the binding for this library (to prevent tree-shaking)
#[no_mangle]
pub extern "C" fn enforce_binding() {}

@jonasfj
Copy link
Member

jonasfj commented Nov 18, 2022

I'm not sure we should tweak the 100MB limit.

If you're things are really this big, then maybe:

  • You should download platform specific binary blobs in the build step (@dcharkes this certainly makes sense if binary blobs are 200mb per platform).
  • Work to make your package smaller,
  • Split the package into multiple packages, if this makes sense, so that users don't have to download all at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants