Installing NDK automatically

[knopp]

Right now when appropriate NDK is not installed the build will fail with

Target native_assets failed: Error: Android NDK Clang could not be found.

IIRC gradle will install ndk automatically for externalNativeBuild projects. We do the same thing in cargokit (glue used to build rust code for Flutter plugins). That results in a smoother experience compared to failing the build. I'm currently looking into reimplementing cargokit in terms of native assets and this would be a bit of a downgrade.

This does raise question whether running build should be allowed to install NDK (or other toolchains for that matter). At least for gradle/NDK nobody seems to object.

Longer term it would be nice to have an option to install toolchains as part of build. When user has rustup installed, cargokit will currently install missing toolchains and targets. But this means that build is modifying files outside of build folder (or pub cache). Which does feels a little bit iffy. It would be great if a hypothetical native_toolchain_rust could install and manage rust toolchain, possibly stored in .pub_cache so that it can get reused between project. Or maybe this could be done during pub get through a hook, but at that point we don't know the target architecture so for rust for example we'd need to install all targets, which might be an overkill.

[dcharkes]

I think the best solution would be to introduce a package native_toolchain_ndk, that in it's build.dart does the following:

1. Installs the NDK if it has not been installed.
2. Adds the install location of the NDK to its BuildOutput as metadata.

And then the package has some code in lib/

3. with an extension method on BuildConfig to get the NDK path from the dependency metadata

Other packages can then depend on that package, which will cause the build.dart of package:native_toolchain_ndk to be run first.

This leads to some open questions:

• What version of the NDK should this package be preferring?
  • The latest?
  • Should it try to stay in sync with the latest version from Flutter so that it avoids multiple installations of the NDK?
• Should it be auto-updating? Probably no, but it should probably pick the newest installation it finds on disk if there is more than one installation.
• Where should the package live. Happy to you own it if you want to maintain it. So it doesn't have to be on this repo.

Target native_assets failed: Error: Android NDK Clang could not be found.

We'd need to check if that error message comes from Flutter or from the build.dart invocation.
If that error message comes from flutter, then the NDK auto-install would need to be implemented in flutter_tools instead. Let me file an issue on the flutter repo.

All of the same applies to a package native_toolchain_rust. Note that that package name is reserved currently on pub.dev, but I'm happy to give it to someone willing to own and maintain such package. 🚀

[dcharkes]

We'd need to check if that error message comes from Flutter or from the build.dart invocation.
If that error message comes from flutter, then the NDK auto-install would need to be implemented in flutter_tools instead. Let me file an issue on the flutter repo.

Okay, the error is coming from the flutter_tools, and flutter_tools policy is to not auto-install native tooling. flutter/flutter#144530

[knopp]

@dcharkes, thanks for following that up. Flutter doctor currently doesn't seem to complain if NDK is not installed, possibly something to add. I generally agree with the sentiment of not installing things silently during build. Maybe something like this would make sense down the line, which could be suggested by the error message:

dart pub activate global dart_native_asset_install
dart_native_asset_install install_ndk

Or perhaps a hook on pub get so that package could verify that all native dependencies are there and if not suggest installing them.

I did some work on a proof of concept implementation of native_toolchain_rust. It's only a rough prototype right now and needs quite a bit of love, but it can build macOS, iOS and android dylibs (I only tested it on macOS for now, but I wouldn't be too surprised if it worked on other platforms as well).

  final rustup = Rustup.systemRustup()!;
  final toolchain = rustup.installedToolchains().first;

  final builder = RustBuilder(
    assetId: 'package:$packageName/${packageName}_hello_rust_ffi',
    toolchain: toolchain,
    manifestPath: 'rust/Cargo.toml',
    buildConfig: buildConfig,
  );
  builder.run(output: buildOutput);

I noticed couple of things:

• On android the build mode is always debug, even in release configurations
• On iOS the framework name seems to be trimmed (from hello_ffi_plugin.framework to hello_ffi_plugi.framework)
• On macOS the framework is added to bundle twice (as hello_ffi_pl1.framework and hello_ffi_plugi.framework).

If these are not known issues I can file them separately.

Overall this seems like a well thought approach and so far I like it a lot. cargokit holds together with a lot of ducktape and this removes pretty much all of it. I'll work on this some more and if the feedback is good I'd be interested in maintaining the package.

[dcharkes]

Thanks for giving it a spin! 🎸

I did some work on a proof of concept implementation of native_toolchain_rust.

❤️ !

• On android the build mode is always debug, even in release configurations

Good catch!

The error is here:

https://github.com/flutter/flutter/blob/d3c274b25540de40e17693ce6800454b5f32ecb8/packages/flutter_tools/lib/src/build_system/targets/native_assets.dart#L318

Should be a simple fix, would you mind making a PR?
Otherwise, please file an issue on the flutter repo and cc me.

• On iOS the framework name seems to be trimmed (from hello_ffi_plugin.framework to hello_ffi_plugi.framework)

This is a feature, not a bug. https://github.com/flutter/flutter/blob/d3c274b25540de40e17693ce6800454b5f32ecb8/packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets_host.dart#L152-L208

• On macOS the framework is added to bundle twice (as hello_ffi_pl1.framework and hello_ffi_plugi.framework).

That sounds like a bug. Please file a bug with a minimal repro on the flutter repo and cc me.

Overall this seems like a well thought approach and so far I like it a lot.

Thanks! 😄

I'll work on this some more and if the feedback is good I'd be interested in maintaining the package.

Awesome! 🚀

[knopp]

https://github.com/flutter/flutter/blob/d3c274b25540de40e17693ce6800454b5f32ecb8/packages/flutter_tools/lib/src/build_system/targets/native_assets.dart#L318

Sure, I'll take a look.

• On iOS the framework name seems to be trimmed (from hello_ffi_plugin.framework to hello_ffi_plugi.framework)

This is a feature, not a bug. https://github.com/flutter/flutter/blob/d3c274b25540de40e17693ce6800454b5f32ecb8/packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets_host.dart#L152-L208

I don't think this is enforced in any way. Many first party flutter plugins have frameworks with names longer than 15 characters, i.e.

• GoogleDataTransport.framework (19 characters)
• FirebaseAppCheckInterop.framework (23 characters)
• shared_preferences_foundation.framework (29 characters)

These are just random picks from app successfully deployed to AppStore. EDIT: Also looking at Apple frameworks they routinely have names longer than 15 characters.

[dcharkes]

These are just random picks from app successfully deployed to AppStore. EDIT: Also looking at Apple frameworks they routinely have names longer than 15 characters.

cc @jmagman should we consider removing the 15 char limit?

[knopp]

Some additional information: As far as I can tell the 15 character limit of CFBundleName may be applied when displaying bundle name in places with limited space (i.e. menubar). But event that only applies for app bundles, it doesn't seem to be relevant for frameworks.

[jmagman]

cc @jmagman should we consider removing the 15 char limit?

Thanks for the research, @knopp. Let's take the char limit out. If there's actually a problem with App Store submission I'm sure we'll hear about it.

[dcharkes]

@knopp If you would like to make a PR, the corresponding tests are in packages/flutter_tools/test/integration.shard/native_assets_test.dart. Otherwise, I can take a look tomorrow.