Fix plugin java class desugar #73758
Conversation
flutter-dashboard
bot
added
c: contributor-productivity
dev/prod_builders.json
Outdated
| { | ||
| "name": "Linux gradle_desugar_classes_test", | ||
| "repo": "flutter", | ||
| "task_name": "linux_gradle_desugar_classes_test", |
There was a problem hiding this comment.
@godofredoc I was going over https://github.com/flutter/flutter/blob/master/dev/README.md, is there anything else required to configure the prod/try builders?
There was a problem hiding this comment.
Yes if this is a new tasks then the builders should exist before adding these configurations. My recommendation is to land the code changes then create the configs in infra and the final step would be updating the prod|try_builders.json.
We just added some docs: https://g3doc.corp.google.com/company/teams/flutter/luci/builders.md?cl=head
| // Not doing so, causes transitive dependency resolution conflicts. | ||
| // That is, the embedding dependencies resolved in the plugin are | ||
| // different than the ones resolved in the app. | ||
| if (isGradleVersionGraterOrEqualThan('6.0.0')) { |
There was a problem hiding this comment.
nit: just noticed this but that method should be Greater, not Grater.
There was a problem hiding this comment.
ahh. I removed this method as it's no longer used.
|
You are right that this has broken flavors. I'm fairly certain that there are people using this. I'm not sure I grasp the full scope of the problem here - I suspect it's some confluence of flavors being something hacky that was never fully baked/thought through/owned and gradle madness. /cc @xster for thoughts about killing support for flavors on Android. It might also be worth consulting with the Gradle/AGP team to see if there's some way we can still make this worth. |
|
I don't think we added specific analytics but based on github issues, flavors and build configs on iOS are definitely used. Can you describe the root issue a bit and how this solution was chosen? |
|
I left a comment in the Gradle script. Basically, after https://issuetracker.google.com/139821726, AGP has a new optimization that configures the build such as Java 8+ APIs are desugared if an explicit API dependency is set on such code. Here the API is the Android embedding, which is used by a plugin. When two Gradle projects have the same API dependency, you get duplicated symbols https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8858351162389675568/+/steps/run_gradle_plugin_light_apk_test/0/stdout?format=raw I will ask the AGP team to confirm that my understanding is correct. |
|
Alright. The good news is that I was able to fix the issue following a suggestion from the AGP team. The new solution is: If an app has plugin, the app doesn't depend on the embedding directly. Instead, the app sets an API dependency on the plugins, and the plugins set an API dependency on the embedding. Let me know if that makes sense. ptal |
|
Will that work out ok if there are multiple plugins? |
|
Yes, it will |
| } | ||
| // The embedding is set as an API dependency in a Flutter plugin. | ||
| // Therefore, don't make the app project depend on the embedding if there are Flutter | ||
| // plugins since it causes duplicated classes. |
There was a problem hiding this comment.
nit: maybe mention the interplay with the flavors implementation here.
5 participants
Fixes #72185.
I think this change will cause an issue if flavors are used. Unfortunately, if that's the case, we will have no choice, but to remove the support.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.