[FGP to Kotlin conversion] Convert app_plugin_loader Gradle plugin to Kotlin #161352

bartekpacia · 2025-01-09T02:05:48Z

I tried to keep changes in flutter.groovy to a minimum.

flutter-dashboard · 2025-01-09T02:05:52Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

bartekpacia · 2025-01-09T23:16:32Z

Successfully converted app_plugin_loader.groovy to app_plugin_loader.kt (and also: native_plugin_loader.groovy to native_plugin_loader.kt, because calling Groovy from Kotlin seems not easily possible).

My sample app's flutter build apk (--debug/--release) works.

Problem encountered:

include_flutter.groovy (used in hybrid Android app scenarios when building from source, not AAR) applies module_plugin_loader.gradle, which applies native_plugin_loader.groovy.

After my changes in this PR, it's not possible to simply apply native_plugin_loader.kt anymore, because it depends on kotlinx.serialization and in general, it's a .kt file, not a .kts file. This kinda sucks.

The solutions I see are:

keep a copy of groovy native_plugin_loader.groovy, which is only used by module_plugin_loader.gradle. TODO: figure out why module_plugin_loader.gradle actually uses it?
- In the (preferably near) future, think about how we could make module_plugin_loader.gradle a normal/standard/well-behaved Gradle plugin, instead of a Groovy script (similarly to what we did with FlutterAppPluginLoaderPlugin in the past)
don't native_plugin_loader.groovy into Kotlin. Find out a way to call Groovy from Kotlin.
find a way to call into/import native_plugin_loader.kt from module_plugin_loader.groovy (idk how)

cc @gmackall @reidbaker

Edit

My attempt on modifying include_flutter.groovy

More relevant resources:

Kotlin, Groovy and Java Compilation
KT-27692 Kotlin/Groovy interop problem
Make it easier to combining Groovy and Kotlin in one project

gmackall · 2025-01-13T20:09:24Z

The files in the .android directory of the module templates are ephemeral, which is to say they are frequently overwritten from the template values in a given project. So we can make changes to convert module_plugin_loader.gradle into a normal gradle plugin, and then apply it, without going through a long migration period.

But also, I think we can tackle that in a different PR - there is still value in converting the app/native_plugin_loader. Now that #155963 has landed, would it be possible to add tests here for the new Kotlin files?

bartekpacia · 2025-01-17T20:15:44Z

I'll come back to this next week, pretty busy few days ahead.

gmackall · 2025-01-21T19:21:15Z

From Android triage: converting to draft for now just so it doesn't show up in triage, feel free to unmark as draft when it's ready for review

flutter-dashboard · 2025-01-21T19:25:39Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

bartekpacia · 2025-01-23T00:13:34Z

I'll come back to this next week, pretty busy few days ahead.

OK, got my bachelor, studies done 🫡 Coming back to this PR.

…_scripts, outside Kotlin source' They break compilation of proper .kt source in src/main/kotlin

…o compile Error: No signature of method: FlutterPlugin.configurePluginProject() is applicable for argument types: (Plugin) values: [Plugin(name=cloud_firestore, path=/Users/bartek/.cache/pub/hosted/pub.dev/cloud_firestore-5.6.0/, dependencies=[firebase_core], nativeBuild=null, devDependency=null)]

…iserably Cause: Kotlin/kotlinx.serialization#1298

…der`

gmackall · 2025-01-23T18:27:11Z

I'll come back to this next week, pretty busy few days ahead.

OK, got my bachelor, studies done 🫡 Coming back to this PR.

Thats awesome, big congratulations!

bartekpacia · 2025-01-23T23:56:19Z

I spent the whole day on this today, and I'm going to write my findings down.

TL;DR I think the easiest way is to convert the whole flutter.groovy to Kotlin first, and then convert app_plugin_loader.groovy to Kotlin.

Current state of this PR

graph TD;
  flutter.groovy -- import --> native_plugin_loader.groovy;
  flutter.groovy -- import --> BaseApplicationNameHandler.kt;
  module_plugin_loader.groovy -- "ext" --> native_plugin_loader.groovy;
  app_plugin_loader.kt  -- import --> native_plugin_loader.groovy;
  include_flutter.groovy -- "apply from: " --> module_plugin_loader.groovy;

Now, with the dependency graph above, I can't find a way to compile our code:

if we compileGroovy first, then flutter.groovy cannot resolve BaseApplicationNameHandler.kt
if we compileKotlin first, then app_plugin_loader.kt cannot resolve native_plugin_loader.groovy

If flutter.groovy was flutter.kt (tentative name), we could compileGroovy first, and this problem wouldn't occur.

Warning

native_plugin_loader.groovy cannot be easily converted to Kotlin, because this plugin parses JSON. In Groovy, JSON-parsing (JsonSlurper) is in Groovy's stdlib. Once I converted native_plugin_loader to Kotlin, I used kotlinx.serialization, but then I realized we can't longer do apply from: (...)/native_plugin_loader.kt in module_plugin_loader.gradle (because native_plugin_loader.kt file now has some Gradle-provided dependencies, i.e. it's not a simple standalone script). So annoying.

Note

It's probably possible to do some fine-grained compilation order to work around this problem, but it's gonna be some crazy Gradle hacking which I'd prefer to avoid.

Failed workaround idea

One idea I had was to write common code (i.e. code that gets called by both Groovy and Kotlin) in Java (not perfect, but still better than Groovy), so we'd have:

graph TD;
  kt --> java;
  groovy --> java;

But it's not possible because compileJava dependsOn compileKotlin – this I enforced by KGP and cannot be easily changed (see a relevant StackOverflow question) (unless we introduce more Gradle subprojects, which I'd prefer to avoid for now).

Another idea

The files in the .android directory of the module templates are ephemeral, which is to say they are frequently overwritten from the template values in a given project. So we can make changes to convert module_plugin_loader.gradle into a normal gradle plugin, and then apply it, without going through a long migration period.

Investigated here, see notes at the bottom. Looks impossible.

Yet another idea

native_plugin_loader.groovy isn't some mountain of complex code. We could copy it and have it twice.

In the longer term, it'd be nice to make Android add_to_app build system more sane.

Piinks · 2025-03-25T22:51:55Z

Hey @bartekpacia greetings from stale PR triage! Thanks for your work and analysis here. Do you wish to continue working on this?

bartekpacia · 2025-03-26T11:02:28Z

Hi @Piinks 👋🏻 I see that @gmackall has picked this up, and I don't have enough time right now, unfortunately.

I'm gonna close it.

…rReflectionBridge` to expose it in Kotlin (#166027) Graph [stolen from Barteks comment](#161352 (comment)) documenting the existing (pre pr) state: ```mermaid graph TD; flutter.groovy -- import --> native_plugin_loader.groovy; flutter.groovy -- import --> BaseApplicationNameHandler.kt; module_plugin_loader.groovy -- "ext" --> native_plugin_loader.groovy; app_plugin_loader.groovy -- import --> native_plugin_loader.groovy; include_flutter.groovy -- "apply from: " --> module_plugin_loader.groovy; ``` 1. Converts the `app_plugin_loader.groovy` to kotlin source. 2. Converts the `module_plugin_loader.groovy` to kotlin script. This can't be changed to kotlin source yet, as we will need to instruct users to make a change to their host app-level gradle files before we can turn down script application of this separate gradle plugin. This is a breaking change, and will need a quarter at least of notice. 3. Unfortunately, the main Flutter Gradle plugin depends on being able to call methods of the `native_plugin_loader`, which we could do in groovy via wacky dynamic behavior, calling across the compiled plugin->script plugin barrier. We can't do this in Kotlin source, and we also can't fully convert `native_plugin_loader` to kotlin source yet because of (2), so I've added a `NativePluginLoaderReflectionBridge` that allows us to access the methods in the `native_plugin_loader.gradle.kts` from the Kotlin source files, calling across the compiled plugin->script plugin barrier as we were before. The plan here is 1. to follow up by adding a converted `native_plugin_loader.gradle.kts` in Kotlin source, and migrating all paths but the host-app using a module-as-source to use the converted approach (but not deleting the old way) 2. maintaining both ways for a release or two, with the script application printing a message notifying users to update to the non-script based application of the `module_plugin_loader`. 3. Then we can delete the script based apply, and also the reflection bridge. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] 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]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Gray Mackall <[email protected]>

…rReflectionBridge` to expose it in Kotlin (flutter#166027) Graph [stolen from Barteks comment](flutter#161352 (comment)) documenting the existing (pre pr) state: ```mermaid graph TD; flutter.groovy -- import --> native_plugin_loader.groovy; flutter.groovy -- import --> BaseApplicationNameHandler.kt; module_plugin_loader.groovy -- "ext" --> native_plugin_loader.groovy; app_plugin_loader.groovy -- import --> native_plugin_loader.groovy; include_flutter.groovy -- "apply from: " --> module_plugin_loader.groovy; ``` 1. Converts the `app_plugin_loader.groovy` to kotlin source. 2. Converts the `module_plugin_loader.groovy` to kotlin script. This can't be changed to kotlin source yet, as we will need to instruct users to make a change to their host app-level gradle files before we can turn down script application of this separate gradle plugin. This is a breaking change, and will need a quarter at least of notice. 3. Unfortunately, the main Flutter Gradle plugin depends on being able to call methods of the `native_plugin_loader`, which we could do in groovy via wacky dynamic behavior, calling across the compiled plugin->script plugin barrier. We can't do this in Kotlin source, and we also can't fully convert `native_plugin_loader` to kotlin source yet because of (2), so I've added a `NativePluginLoaderReflectionBridge` that allows us to access the methods in the `native_plugin_loader.gradle.kts` from the Kotlin source files, calling across the compiled plugin->script plugin barrier as we were before. The plan here is 1. to follow up by adding a converted `native_plugin_loader.gradle.kts` in Kotlin source, and migrating all paths but the host-app using a module-as-source to use the converted approach (but not deleting the old way) 2. maintaining both ways for a release or two, with the script application printing a message notifying users to update to the non-script based application of the `module_plugin_loader`. 3. Then we can delete the script based apply, and also the reflection bridge. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] 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]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Gray Mackall <[email protected]>

github-actions bot added platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Jan 9, 2025

bartekpacia mentioned this pull request Jan 9, 2025

Convert base application name handling to kotlin source (start of FGP kt conversion) #155963

Merged

9 tasks

gmackall marked this pull request as draft January 21, 2025 19:20

bartekpacia added 8 commits January 23, 2025 19:14

move .kts scripts (to be migrated to proper .kt) to a separate kotlin…

cec575b

…_scripts, outside Kotlin source' They break compilation of proper .kt source in src/main/kotlin

convert app_plugin_loader.groovy into .kt

0d18e9d

fix kotlinx.serialization build failures

8992215

kotlinx.serialization: try to use Map<String, Any> everywhere, fail m…

78d3bce

…iserably Cause: Kotlin/kotlinx.serialization#1298

fix. works with kotlinx.serialization

f473128

clean up debug leftovers

109d844

native_plugin_loader.groovy: change class name to `OldNativePluginLoa…

9a60510

…der`

after-rebase fixes

21250cf

gmackall mentioned this pull request Jan 23, 2025

☂️ [FGP Kotlin conversion] Umbrella bug to track conversion progress #162103

Open

15 tasks

bartekpacia mentioned this pull request Jan 23, 2025

NativePluginLoader: make the class entirely stateless #162110

Closed

9 tasks

bartekpacia mentioned this pull request Jan 23, 2025

Migrate Gradle plugin to Kotlin, away from Groovy #121541

Closed

investigation, can no longer keep track of changes, oh my

18708f5

bartekpacia mentioned this pull request Jan 26, 2025

Fix use of deprecated buildDir in Android templates/tests/examples #157560

Merged

9 tasks

gmackall mentioned this pull request Mar 22, 2025

Try converting the NativePluginLoader and AppPluginLoader, and see what breaks #165698

Closed

9 tasks

bartekpacia closed this Mar 26, 2025

gmackall mentioned this pull request Mar 27, 2025

Convert AppPluginLoaderPlugin to Kotlin, and add NativePluginLoaderReflectionBridge to expose it in Kotlin #166027

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[FGP to Kotlin conversion] Convert app_plugin_loader Gradle plugin to Kotlin #161352

[FGP to Kotlin conversion] Convert app_plugin_loader Gradle plugin to Kotlin #161352

Uh oh!

bartekpacia commented Jan 9, 2025 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Jan 9, 2025

Uh oh!

bartekpacia commented Jan 9, 2025 •

edited

Loading

Uh oh!

gmackall commented Jan 13, 2025

Uh oh!

bartekpacia commented Jan 17, 2025

Uh oh!

gmackall commented Jan 21, 2025 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Jan 21, 2025

Uh oh!

bartekpacia commented Jan 23, 2025

Uh oh!

gmackall commented Jan 23, 2025

Uh oh!

bartekpacia commented Jan 23, 2025 •

edited

Loading

Uh oh!

Piinks commented Mar 25, 2025

Uh oh!

bartekpacia commented Mar 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[FGP to Kotlin conversion] Convert app_plugin_loader Gradle plugin to Kotlin #161352

[FGP to Kotlin conversion] Convert app_plugin_loader Gradle plugin to Kotlin #161352

Uh oh!

Conversation

bartekpacia commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Jan 9, 2025

Uh oh!

bartekpacia commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem encountered:

Edit

Uh oh!

gmackall commented Jan 13, 2025

Uh oh!

bartekpacia commented Jan 17, 2025

Uh oh!

gmackall commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Jan 21, 2025

Uh oh!

bartekpacia commented Jan 23, 2025

Uh oh!

gmackall commented Jan 23, 2025

Uh oh!

bartekpacia commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current state of this PR

Failed workaround idea

Another idea

Yet another idea

Uh oh!

Piinks commented Mar 25, 2025

Uh oh!

bartekpacia commented Mar 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bartekpacia commented Jan 9, 2025 •

edited

Loading

bartekpacia commented Jan 9, 2025 •

edited

Loading

gmackall commented Jan 21, 2025 •

edited

Loading

bartekpacia commented Jan 23, 2025 •

edited

Loading