Re-land "Ensure flutter build apk --release optimizes+shrinks platform code"#153868
Conversation
github-actions
bot
added
the
tool
gmackall
changed the title
mkustermann
left a comment
mkustermann
left a comment
There was a problem hiding this comment.
LGTM, thanks for working on this, @gmackall !
|
|
||
|
|
||
| String _getAgpLocation(FlutterProject project) { | ||
| return ' The version of AGP that your project uses is likely' |
There was a problem hiding this comment.
nit: You could consider multi-line strings:
return '''
The version of AGP that your project uses is likely
${project.android.settingsGradleFile.path}
in the 'plugins' closure.
Alternatively, ...
''';(also other places)
There was a problem hiding this comment.
Changed to multi line strings
|
|
||
| @visibleForTesting | ||
| final GradleHandledError r8DexingBugInAgp73Handler = GradleHandledError( | ||
| test: (String line) => line.contains('com.android.tools.r8.internal.Y10: Unused argument with users'), |
There was a problem hiding this comment.
You may not want to match Y10 as that identifier could be different between minor versions of R8 (it's very likely a obfuscated class name)
There was a problem hiding this comment.
Changed to just test for the parts around it
…y for special case kotlin handler
|
Updated for review, and to make sure that Summary of why this is necessary is that we test each log line as it comes in, and the lines that trigger Priority of handlers is determined by order of log lines, with only ties being broken by the order of the list of handlers. |
gmackall
added
the
autosubmit
…m code" (flutter#153868) Re-lands flutter#136880, fixes flutter#136879. Additions to/things that are different from the original PR: - Adds an entry to `gradle_errors.dart` that tells people when they run into the R8 bug because of using AGP 7.3.0 (https://issuetracker.google.com/issues/242308990). - Previous PR moved templates off of AGP 7.3.0. - Packages repo has been moved off AGP 7.3.0 (flutter/packages#7432). Also, unrelatedly: - Deletes an entry in `gradle_errors.dart` that informed people to build with `--no-shrink`. This flag [doesn't do anything](flutter/website#11022 (comment)), so it can't be the solution to any error. - Uniquely lowers the priority of the `incompatibleKotlinVersionHandler`. This is necessary because the ordering of the errors doesn't fully determine the priority of which handler we decide to use, but also the order of the log lines. The kotlin error lines often print before the other error lines, so putting it last in the list of handlers isn't sufficient to lower it to be the lowest priority handler.
|
FYI: This change regressed release build times on Windows hosts by ~30 seconds: https://flutter-flutter-perf.skia.org/e/?begin=1722984359&end=1725384267&keys=X3ba6023ddb62c0aa3cb1d6d43943ab9a&requestType=0&selected=commit%3D42295%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253Dmokey%252Cdevice_version%253Dnone%252Chost_type%253Dwin%252Csub_result%253Drelease_full_compile_millis%252Ctest%253Dflutter_gallery_win__compile%252C&xbaroffset=42295. The regression on more modern M1 mac minis was less severe in absolute terms. No regression was detected on Linux. Just flagging this to ensure folks are aware of this trade-off. |
|
We expect an increase in build time when it comes to optimizing code size. The benefits and costs grow as the size/complexity of your code grows. |
4 participants
Re-lands #136880, fixes #136879.
Additions to/things that are different from the original PR:
gradle_errors.dartthat tells people when they run into the R8 bug because of using AGP 7.3.0 (https://issuetracker.google.com/issues/242308990).Also, unrelatedly:
gradle_errors.dartthat informed people to build with--no-shrink. This flag doesn't do anything, so it can't be the solution to any error.incompatibleKotlinVersionHandler. This is necessary because the ordering of the errors doesn't fully determine the priority of which handler we decide to use, but also the order of the log lines. The kotlin error lines often print before the other error lines, so putting it last in the list of handlers isn't sufficient to lower it to be the lowest priority handler.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.