-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Start writing migration guide for Gradle plugin apply #9857
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
Conversation
Visit the preview URL for this PR (updated for commit 101445a): https://flutter-docs-prod--pr9857-migration-guide-gradle-plugi-vyllekcb.web.app |
merged |
…laratively (#139006) This PR updates the app templates generated by `flutter create` to use declarative `plugins {}` syntax for applying the Kotlin Gradle Plugin. I realized this is missing while writing [#9857.](flutter/website#9857)
ef91242
to
ca7c290
Compare
TODO (and probably discuss): Add |
|
||
To build a Flutter app for Android, the Flutter Gradle plugin must be applied. | ||
Historically, this was done imperatively with Gradle's | ||
[legacy, imperative apply method][]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider linking the code in flutter that does this from 3.13 (last stable branch to release to users using the old system by default).
https://github.com/flutter/flutter/blob/flutter-3.13-candidate.0/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean flutter.gradle
? Or maybe the android/app/build.gradle
of a real example app? If so, which one?
|
||
In Flutter 3.16, support has been added for applying these plugins with Gradle's | ||
[declarative plugins {} block][], and it is the recommended approach. Since | ||
Flutter 3.16, projects generated with `flutter create` use the new apply method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion
new apply method -> new declarative binary plugin method.
We need a way of referencing the old method without saying old since the definition of old changes over time. I think saying the "apply method" or "apply script method" seems like a reasonable definition to use internally. If you agree then let us avoid saying "apply method" unless it is in reference to the older method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
Gradle itself refers to plugins {}
as "plugins {}
block", so my proposal is:
- imperative apply script method is the old way of applying Gradle plugins
- declarative plugins {} block is the new way of applying Gradle plugins
I don't want to bikeshed over this too much so I'll let you make the final call and I'm fine with whatever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the delta between the 2 you listed. I agree on wanting to avoid bikeshedding.
- imperative apply script method is the old way of applying Gradle plugins
- declarative plugins {} block is the new way of applying Gradle plugins
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in a7b28e3
In Flutter 3.16, support has been added for applying these plugins with Gradle's | ||
[declarative plugins {} block][], and it is the recommended approach. Since | ||
Flutter 3.16, projects generated with `flutter create` use the new apply method | ||
of applying Gradle plugins. Older projects, however, should be migrated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion
Older Projects, however, should be ... -> Projects created with versions of flutter prior to 3.16, for now, should be migrated manually.
Then it might be good to help set the users expectation of effort/time/risk the migration will take. Perhaps something like
After updating to at least flutter 3.16 and confirming your app works the build migration (link to steps) should take one engineer less than a day. The new mechanism of linking flutter executes the same code and should produce equivalent binarys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, I mostly agree, done in 1350769. I went with:
Projects created with versions of Flutter prior to 3.16 need to be migrated manually.
because I'm not aware of any work happening on flutter migrate
, which is the only potential solution to automate this migration.
Also I understand your good intentions with this:
the build migration (link to steps) should take one engineer less than a day.
but we don't have data to back that up (except personal experience). I've seen big Flutter projects that have a lot of custom Gradle conifg and I just wouldn't be comfortable with saying this (haha, I'm referring to personal experience now, but you get me). I think it might sometimes take more than 1 day, esp. if the engineer is very new to Gradle. I'd not include it, but of course it's your call and I don't have strong feeling about it.
I included this:
Applying Gradle plugins using the
plugins {}
executes the same code as before
and should produce equivalent app binaries.
in 3308f5a
To learn about advantages the new declarative apply syntax has over the old | ||
imperative apply syntax, see [Gradle docs about plugins {} block][plugins block]. | ||
|
||
Migrating the app ecosystem to use the new approach will also make it easier for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the advantages stated in this documentation should be advantages for the developers doing the work not for the maintainers of the project.
Migrating the app ecosystem to use this approach will eventually be required, see flutter deprecation timeline (with link), It also is a prerequisite for flutter migrating to kotlin and off of groovy which we believe will lead to greater build stability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migrating the app ecosystem to use this approach will eventually be required, see flutter deprecation timeline (with link), It also is a prerequisite for flutter migrating to kotlin and off of groovy which we believe will lead to greater build stability.
Should this be included in this doc?
|
||
plugins { | ||
id "dev.flutter.flutter-plugin-loader" version "1.0.0" | ||
id "com.android.application" version "7.3.0" apply false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should avoid giving a version here and below for kotlin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure – would this:
id "com.android.application" version "{{agpVersion}}" apply false
id "org.jetbrains.kotlin.android" version "{{kotlinVersion}}" apply false
be ok?
PS I was also thinking about including the newest possible versions of AGP and Kotlin here, because I think many people are not aware these things actually get updates and are stuck on AGP v4, haha.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a quick thought, don't know if it's practical and can be ignored: can't we make a test, if the docs code is equivalent to the template, so it would replicate the up to date version of this one. But it does of course not work with more complicated code snippets like below or at least only partial.
I always find it helpful to know what is the Flutter supported version or at least, have a link to the updated template file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd be great – but I think it's out of scope of this PR (it deserves a separate issue imho)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this version before merging.
"{{agpVersion}} works for me so does to mirror how you document
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 35e5815
|
||
### android/build.gradle | ||
|
||
Replace contents with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about replace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
|
||
-apply plugin: 'com.android.application' | ||
-apply plugin: 'kotlin-android' | ||
-apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should highlight this change more. Perhaps calling out that this is the root or core of the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 93a4b2a!
-apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle" | ||
|
||
android { | ||
// ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider "// Your android block unmodified."
id "dev.flutter.flutter-plugin-loader" version "1.0.0" | ||
id "com.android.application" version "7.3.0" apply false | ||
id "org.jetbrains.kotlin.android" version "1.7.10" apply false | ||
+ id "com.google.gms.google-services" version "4.4.0" apply false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do users have to move all their dependencies to the declarative plugins block? If not then maybe we should focus on how to migrate just flutter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh I'm not sure if it's required. But it's a good practice and I think it's worth demonstrating it on a common example.
In Flutter 3.16, support has been added for applying these plugins with Gradle's | ||
[declarative plugins {} block][], and it is the recommended approach. Since | ||
Flutter 3.16, projects generated with `flutter create` use the new apply method | ||
of applying Gradle plugins. Older projects, however, should be migrated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/should/need to ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed
done in 1350769
manually. | ||
|
||
To learn about advantages the new declarative apply syntax has over the old | ||
imperative apply syntax, see [Gradle docs about plugins {} block][plugins block]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be an actual link to the Gradle docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is. At the bottom of the page there's:
[plugins block]: https://docs.gradle.org/current/userguide/plugins.html#plugins_dsl_limitations
|
||
### android/settings.gradle | ||
|
||
Replace contents with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be more explicit:
Replace the contents of android/settings.gradle
with:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but also, what if people have made additions / changes to this file? Perhaps the "replace" should be more specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I rephreased this in 8205446
But making changes to settings.gradle
is rare (at least according to my experience as Flutter developer), changing build.gradle
and app/build.gradle
is done much more often.
|
||
### android/build.gradle | ||
|
||
Replace contents with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be more explicit, Replace the contents of android/build.gradel
with:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but also, what if people have made additions / changes to this file? Perhaps the "replace" should be more specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, assuming the version for AGP in the example is removed.
|
||
plugins { | ||
id "dev.flutter.flutter-plugin-loader" version "1.0.0" | ||
id "com.android.application" version "7.3.0" apply false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this version before merging.
"{{agpVersion}} works for me so does to mirror how you document
+} | ||
``` | ||
|
||
## Examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before the example I think we should have a section that gives a user a way to validate they migrated successfully.
Perhaps something like
### Validation
Execute `flutter run apk` and confirm your app builds and launches.
## Examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in b7f022f
Thanks @reidbaker – I applied your suggestions. I have merge access but am not comfortable clicking it since this is my first ever contribution this repo. I'll defer to somebody more experienced. |
Congrats on your first PR, @bartekpacia! |
This PR fixes errors in the migration guide: - missing entry for this guide from _index_ ([link](https://docs.flutter.dev/release/breaking-changes#not-yet-released-to-stable)) - {agpVersion} and {kotlinVersion} not displaying ([link](https://docs.flutter.dev/release/breaking-changes/flutter-gradle-plugin-apply)) Original PR #9857 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. ## Preview [here](https://flutter-docs-prod--pr9907-fix-migration-guide-gradle-p-f53g480f.web.app/release/breaking-changes/flutter-gradle-plugin-apply#google-mobile-services-and-crashlytics)
This PR adds a guide explaining how to migrate from _imperative apply_ of Flutter's Gradle plugins to _declarative apply_. Resolves flutter/flutter#135392 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. ## Preview [here](https://flutter-docs-prod--pr9857-migration-guide-gradle-plugi-vyllekcb.web.app/release/breaking-changes/flutter-gradle-plugin-apply) --------- Co-authored-by: Anthony Sansone <[email protected]> Co-authored-by: Gustl22 <[email protected]>
This PR fixes errors in the migration guide: - missing entry for this guide from _index_ ([link](https://docs.flutter.dev/release/breaking-changes#not-yet-released-to-stable)) - {agpVersion} and {kotlinVersion} not displaying ([link](https://docs.flutter.dev/release/breaking-changes/flutter-gradle-plugin-apply)) Original PR flutter#9857 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. ## Preview [here](https://flutter-docs-prod--pr9907-fix-migration-guide-gradle-p-f53g480f.web.app/release/breaking-changes/flutter-gradle-plugin-apply#google-mobile-services-and-crashlytics)
…ethod" way (#139690) This PR adds a deprecation message when Android build is using the legacy "apply script method" way of applying Flutter's Gradle plugins (that is: [`flutter.gradle`](https://github.com/flutter/flutter/blob/3.16.0/packages/flutter_tools/gradle/flutter.gradle) and [`app_plugin_loader.gradle`](https://github.com/flutter/flutter/blob/3.16.0/packages/flutter_tools/gradle/app_plugin_loader.gradle)). See also: - #121541 - in particular #121541 (comment) - #135392 - and PR that add the migration guide: [#9857](flutter/website#9857) - I think either `logger.error` or `logger.quiet` must be used, because all other error levels are not shown during `flutter build apk` (and that's what most people use).
… script method" way" (#140102) Reverts #139690 Initiated by: hellohuanlin This change reverts the following previous change: Original Description: This PR adds a deprecation message when Android build is using the legacy "apply script method" way of applying Flutter's Gradle plugins (that is: [`flutter.gradle`](https://github.com/flutter/flutter/blob/3.16.0/packages/flutter_tools/gradle/flutter.gradle) and [`app_plugin_loader.gradle`](https://github.com/flutter/flutter/blob/3.16.0/packages/flutter_tools/gradle/app_plugin_loader.gradle)). See also: - #121541 - in particular #121541 (comment) - #135392 - and PR that add the migration guide: [#9857](flutter/website#9857) - I think either `logger.error` or `logger.quiet` must be used, because all other error levels are not shown during `flutter build apk` (and that's what most people use).
…cript method (#140103) > This PR relands #139690 which was reverted in #140102 This PR adds a deprecation message when Android build is using the legacy "apply script method" way of applying Flutter's Gradle plugins (that is: [`flutter.gradle`](https://github.com/flutter/flutter/blob/3.16.0/packages/flutter_tools/gradle/flutter.gradle) and [`app_plugin_loader.gradle`](https://github.com/flutter/flutter/blob/3.16.0/packages/flutter_tools/gradle/app_plugin_loader.gradle)). See also: - #121541 - in particular #121541 (comment) - #135392 - and PR that add the migration guide: [#9857](flutter/website#9857) - I think either `logger.error` or `logger.quiet` must be used, because all other error levels are not shown during `flutter build apk` (and that's what most people use).
…cript method (flutter#140103) > This PR relands flutter#139690 which was reverted in flutter#140102 This PR adds a deprecation message when Android build is using the legacy "apply script method" way of applying Flutter's Gradle plugins (that is: [`flutter.gradle`](https://github.com/flutter/flutter/blob/3.16.0/packages/flutter_tools/gradle/flutter.gradle) and [`app_plugin_loader.gradle`](https://github.com/flutter/flutter/blob/3.16.0/packages/flutter_tools/gradle/app_plugin_loader.gradle)). See also: - flutter#121541 - in particular flutter#121541 (comment) - flutter#135392 - and PR that add the migration guide: [flutter#9857](flutter/website#9857) - I think either `logger.error` or `logger.quiet` must be used, because all other error levels are not shown during `flutter build apk` (and that's what most people use).
…laratively (flutter#139006) This PR updates the app templates generated by `flutter create` to use declarative `plugins {}` syntax for applying the Kotlin Gradle Plugin. I realized this is missing while writing [flutter#9857.](flutter/website#9857)
Inconsistent syntax used in the guide "Most changes have to be made in the /android/app/build.gradle. " ... "Now apply the plugins again, but this time using the Plugin DSL syntax. At the very top of your file, add:"
|
It would be helpful to mention that if the following subtraction (from /android/build.gradle) is also present in /android/app/build.gradle, it should be removed from that file as well.
|
In reply to this comment: You cannot use "kotlin-android" when specifying plugin coordinates in the And it |
This PR adds a guide explaining how to migrate from imperative apply of Flutter's Gradle plugins to declarative apply.
Resolves flutter/flutter#135392
Presubmit checklist
Preview
here