Remove support for imperative apply of Flutter Gradle Plugin #160947
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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. |
github-actions
bot
added
platform-android
bartekpacia
force-pushed
the
bartekpacia/chore/remove_imperative_gradle_apply
branch
from
3fa3f1d to
11db045
Compare
Contributor
|
Change LGTM at a high-level, you have the right reviewers already to do a formal review. SIde-note: the tests you are deleting in this change are a constant headache for reasons we've yet to understand (a recent example being #157640), so my 2c is that we should be extremely/highly motivated to help Bartek finalize and merge some version of this PR. /cc @bkonyi @andrewkolos for tool visibility (related to #157640). |
bartekpacia
force-pushed
the
bartekpacia/chore/remove_imperative_gradle_apply
branch
from
5b2c28d to
93d9570
Compare
gmackall
reviewed
Jan 7, 2025
Member
gmackall
left a comment
gmackall
left a comment
There was a problem hiding this comment.
Mostly LGTM, just one question!
| release. Migrate to applying Gradle plugins with the declarative plugins \ | ||
| block: https://flutter.dev/to/flutter-gradle-plugin-apply\n") | ||
|
|
||
| def pathToThisDirectory = buildscript.sourceFile.parentFile |
Member
There was a problem hiding this comment.
Would it be possible to here (and in packages/flutter_tools/gradle/app_plugin_loader.gradle) keep the file around and just use it to throw an explicit exception with the upgrade instructions? Or would we hit other errors first, preventing the useful error from being thrown?
People have had a long time to upgrade so I think if it isn't possible that is fine, but if it is easy it would be good to keep the helpful messaging around for a release.
Member
Author
There was a problem hiding this comment.
This is a good idea, thanks! I think it's possible to keep this file (as along as we won't manually apply FGP any more).
Member
Author
There was a problem hiding this comment.
Done – instead of logger.error()-ing the message, we now throw new GradleException with the same message. I tested it with flutter build apk and it works as expected.
gmackall
added
the
autosubmit
auto-submit
bot
deleted the
bartekpacia/chore/remove_imperative_gradle_apply
branch
flutter-dashboard
bot
removed
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 8, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 8, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 8, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 8, 2025
srujzs
pushed a commit
to srujzs/flutter
that referenced
this pull request
Jan 12, 2025
…#160947) imperative apply has been deprecated since flutter#139690 part of flutter#121541 Doing this will make things easier for us during the conversion of `flutter.groovy` into Kotlin.
9 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 14, 2025
…om lockfile exclusion yaml (#161622) The test was intentionally broken by #160947. That PR removed the CI configuration to run the test, but the autoroller (which was previously turned off) runs a gradle command in all android subdirectories, so the fact that the directory wasn't deleted is blocking turning the autoroller back on: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8725731612391969937/+/u/run_roll-packages_script/stdout ``` [stderr] * What went wrong: [stderr] A problem occurred evaluating script. [stderr] > You are applying Flutter's main Gradle plugin imperatively using the apply script method, which is not possible anymore. Migrate to applying Gradle plugins with the declarative plugins block: https://flutter.dev/to/flutter-gradle-plugin-apply ``` ## 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]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] 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. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [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]>
maheshj01
pushed a commit
to maheshj01/flutter
that referenced
this pull request
Jan 15, 2025
…om lockfile exclusion yaml (flutter#161622) The test was intentionally broken by flutter#160947. That PR removed the CI configuration to run the test, but the autoroller (which was previously turned off) runs a gradle command in all android subdirectories, so the fact that the directory wasn't deleted is blocking turning the autoroller back on: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8725731612391969937/+/u/run_roll-packages_script/stdout ``` [stderr] * What went wrong: [stderr] A problem occurred evaluating script. [stderr] > You are applying Flutter's main Gradle plugin imperatively using the apply script method, which is not possible anymore. Migrate to applying Gradle plugins with the declarative plugins block: https://flutter.dev/to/flutter-gradle-plugin-apply ``` ## 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]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] 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. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [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]>
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 12, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 13, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 13, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 6, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 7, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
imperative apply has been deprecated since #139690
part of #121541
Doing this will make things easier for us during the conversion of
flutter.groovyinto Kotlin.