Resolve android_build_all_packages Warnings
#9643
Conversation
There was a problem hiding this comment.
Code Review
This PR addresses a flaky Android build by updating several Gradle-related versions, which is a good approach. The change to create_all_packages_app_command.dart correctly moves a directory copy operation out of a loop, improving efficiency. However, I've pointed out a logical flaw in the new file/directory deletion logic that could lead to incorrect behavior and suggested a more robust implementation.
| @@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME | |||
| distributionPath=wrapper/dists | |||
| zipStoreBase=GRADLE_USER_HOME | |||
| zipStorePath=wrapper/dists | |||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-all.zip | |||
There was a problem hiding this comment.
Are there flutter tool warnings associated with all of these changes? I know I've seen one for the Kotlin version, but I'm not sure what the current state of the warnings on these other two changes is.
There was a problem hiding this comment.
Yes Flutter tooling errors appear for all the values I changed--KGP, AGP, and Gradle.
android_build_all_packages flakeandroid_build_all_packages Warnings
@stuartmorgan-g Instead of changing the tooling, I paired wtih Reid to resolve my local In the meantime, I updated this PR to only address the versioning warnings. That is still a necessary change. |
There was a problem hiding this comment.
LGTM with one nit.
Instead of changing the tooling, I paired wtih Reid to resolve my local
.DS_Storeissue . I reverted the changes to this command. I had thought this could potentially be the source of flaking because I could not build the app locally, but it was due to.DS_Storeissue.
Please feel free to send me a PR that has just those tool changes; there's no reason not to have the tooling be robust against local automatic .DS_Store creation to spare future developers the debugging you went through.
| one that we support. | ||
| - Modifies `settings.gradle` to upgrade the Android Gradle Plugin (AGP) | ||
| from version 8.3.0 to 8.6.0. If a user runs into an error with the AGP | ||
| version, the warning is clear on how to upgrade the version to one that we support. |
There was a problem hiding this comment.
These should be folded into the bullet points above, just updating the final version. E.g., "... upgrade the Gradle version from 6.7 to 8.7. ...". The goal here isn't to have a history of the specific incremental changes, but a catalog of what is different between a legacy app and the current state.
Addressed versioning warnings by updating the following dependencies to their minimum supported versions:
Gradle: 8.7
AGP: 8.6.0
KGP: 2.1.0
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3