Skip to content

Conversation

@dcharkes
Copy link
Contributor

@dcharkes dcharkes commented Sep 20, 2023

Support for FFI calls with @Native external functions through Native assets on Android. This enables bundling native code without any build-system boilerplate code.

For more info see:

Implementation details for Android.

Mainly follows the design of the previous PRs.

For Android, we detect the compilers inside the NDK inside SDK.

And bundling of the assets is done by the flutter.groovy file.

The minSdkVersion is propagated from the flutter.groovy file as well.

The NDK is not part of flutter doctor, and users can omit it if no native assets have to be build.
However, if any native assets must be built, flutter throws a tool exit if the NDK is not installed.

Add 2 app is not part of this PR yet, instead flutter build aar will tool exit if there are any native assets.

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.

@github-actions github-actions bot added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Sep 20, 2023
@dcharkes
Copy link
Contributor Author

$ flutter build aar
AARs can only be built for plugin or module projects.

@stuartmorgan What is the add2app story for standalone Dart packages for Android? Is it not supported? (Packages with native assets are plain old Dart packages with a build.dart.)

And, who should review this PR besides you.

Note: for now this is just a draft. Linux need to be relanded, and also Windows needs to be landed first. I'm making a draft so I can run tests on the CI and have a place to ask questions.

@stuartmorgan-g
Copy link
Contributor

@stuartmorgan What is the add2app story for standalone Dart packages for Android?

I don't have any familiarity with Android add-to-app, so I don't know the answer to this.

And, who should review this PR besides you.

@reidbaker probably

@dcharkes dcharkes force-pushed the native-assets-android branch from 63dbab2 to c59cf7b Compare September 22, 2023 06:11
@dcharkes
Copy link
Contributor Author

[ +1 ms] [ +2 ms] Target native_assets failed: Exception: No NDK found in LocalDirectory: '/b/s/w/ir/cache/android/sdk'

[ +1 ms] [ +3 ms] Target native_assets failed: Exception: No NDK found in LocalDirectory: 'C:\b\s\w\ir\cache\android\sdk'

It looks like we have no NDK on the Windows and Linux bots, we do have an NDK on the MacOS bots.

I don't see a mention of the NDK .ci.yaml, so I assume it's embedded inside the CIPD package for the android_sdk?

(The failing tests succeed locally on my Linux and Windows machines where the NDK is installed.)

@stuartmorgan How do we get the NDK on the bots?

I was thinking we should have had it already, because the FFI plugins for Android use it. But it likely gets auto-downloaded there by Gradle. With the FFI packages, we don't have a gradle file that does that. Maybe we could consider generating such a file if we notice there is at least one package with a build.dart?

@stuartmorgan-g
Copy link
Contributor

@stuartmorgan How do we get the NDK on the bots?

That's probably a question for @reidbaker.

@reidbaker reidbaker self-requested a review September 26, 2023 14:47
@dcharkes
Copy link
Contributor Author

@johnmccutchan As Reid is pretty busy, might you be able to take a look at the questions above and the general approach in this PR?

@dcharkes dcharkes force-pushed the native-assets-android branch from 67db6dd to 36eec66 Compare September 28, 2023 06:57
@dcharkes dcharkes marked this pull request as ready for review September 28, 2023 06:59
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

Some code looks like it was formatted by dartfmt. Could you please follow https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo ?

@zanderso
Copy link
Member

@stuartmorgan IIRC you reviewed some of the other PRs on this project. Would you be able to review/confirm that this PR comports with the other changes that have been landed, at least at a surface level?

@christopherfujino for flutter_tool review.

@reidbaker reidbaker removed their request for review September 29, 2023 19:43
@stuartmorgan-g stuartmorgan-g added platform-android Android applications specifically and removed platform-ios iOS applications specifically labels Oct 9, 2023
@dcharkes dcharkes requested a review from reidbaker October 11, 2023 12:41
@dcharkes
Copy link
Contributor Author

How do we get the NDK on the bots?

@godofredoc Can we get the NDK pre-installed on the Windows and Linux bots? "Linux tool_integration_tests_3_4" and "Windows tool_integration_tests_5_6" fail because there is no Android NDK inside the used Android SDK on those bots.

@dcharkes dcharkes added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 7, 2023
@auto-submit auto-submit bot merged commit 6ad7555 into master Dec 7, 2023
@auto-submit auto-submit bot deleted the native-assets-android branch December 7, 2023 16:29
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-android Android applications specifically team-android Owned by Android platform team tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants