Don't strip debug symbols by default from android builds

[gmackall]

See flutter/flutter#98773 (comment) and https://discord.com/channels/608014603317936148/608021010377080866/1202326905294954577. Basically, AGP strips symbols by default from the app that gets delivered to users when building app bundles, but includes them to debug crashes in the play store (edit for clarity: in app stores). Including the symbols in the artifacts that get used to build the flutter app allows this process to work by default.

See also https://support.google.com/googleplay/android-developer/answer/9848633#upload_file

WIP because (but also if any reviewer who comes across this happens to know, please tell me!) I don't actually know how these artifacts get delivered to flutter developers during the build process? Do they get downloaded? And if so where?

Also wip because the flag also changes the output directory of the build, so I need to figure out how to plumb those changes through where needed Test failures fixed

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[jonahwilliams]

What about non-play store or APKs?

[gmackall]

What about non-play store

In particular, what this changes in the unzipped appbundle is the inclusion of a libflutter.so.sym symbol file at BUNDLE-METADATA/com.android.tools.build.debugsymbols/arm64-v8a/libflutter.so.sym (20Mb unzipped). The libflutter.so files in the app apks are still almost the same size (10Mb vs 11Mb), though I'm not sure why they aren't exactly the same.

According to the BUNDLE-METADATA description from the app bundle format,

This directory includes metadata files that contain information useful for tools or app stores. Such metadata files may include ProGuard mappings and the complete list of your app's DEX files. Files in this directory are not packaged into your app's APKs.

It looks like it is up to app stores to make use of (or not make use of) the data in this directory. But this is not play store specific, it is outlined in the app bundle format.

or APKs?

From what I can tell they are also getting stripped by default. For an individual abi, from within the unzipped apks
current master: du -sh lib/arm64-v8a/ - 13Mb
with a no-stripped engine du -sh lib/arm64-v8a/ - 14Mb

I don't know why this is. Listing the gradle tasks enabled, there seems to be a task 'stripReleaseDebugSymbols' that is enabled by default, which is a potential culprit.

[jonahwilliams]

Thanks! I would suspect some local size differences are just due to LTO.

I wasn't familiar with bundle metadata stuff, I will look into it more. This will be a great change that will help our developers with crashes and also CPU profile the engine, every supportive 👍

[gmackall]

Looks like tests are failing, mostly with an error of the form

ANGLE Display::initialize error 0: Internal Vulkan error (-3): Initialization of an object could not be completed for implementation-specific reasons, in ../../third_party/angle/src/libANGLE/renderer/vulkan/RendererVk.cpp, initialize:1728.

Need to figure out if there is a way to determine that "implementation-specific reason"

[gmackall]

Looks like tests are failing, mostly with an error of the form

ANGLE Display::initialize error 0: Internal Vulkan error (-3): Initialization of an object could not be completed for implementation-specific reasons, in ../../third_party/angle/src/libANGLE/renderer/vulkan/RendererVk.cpp, initialize:1728.

Need to figure out if there is a way to determine that "implementation-specific reason"

This was fixed by #50629

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

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.

[jiahaog]

It seems like this is going to change the libflutter.so that is in flutter.jar (which is part of the artifacts.zip), unless the builders explicitly specify --stripped? Just doing a quick size check, lib/arm64-v8a/libflutter.so within the jar is going to increase in uncompressed size from 11 MB to 148 MB.

In Google3, I don't think there are any intermediate stripping steps before this file is used in APKs. Need to double check on this, will report back.

[gmackall]

It seems like this is going to change the libflutter.so that is in flutter.jar (which is part of the artifacts.zip), unless the builders explicitly specify --stripped? Just doing a quick size check, lib/arm64-v8a/libflutter.so within the jar is going to increase in uncompressed size from 11 MB to 148 MB.

That is correct

In Google3, I don't think there are any intermediate stripping steps before this file is used in APKs. Need to double check on this, will report back.

Please do! I've asked internally as well and mostly gotten the reply that it "should" be fine, but nothing concrete either way

[jiahaog]

I sent cl/607168388 to perform stripping as a build action internally.

The changes here seem to be universal - would this affect other platforms too? I vaguely remember that on iOS artifacts are stripped separately by luci, but not sure if we considered them in this change.

[gmackall]

I sent cl/607168388 to perform stripping as a build action internally.

The changes here seem to be universal - would this affect other platforms too? I vaguely remember that on iOS artifacts are stripped separately by luci, but not sure if we considered them in this change.

The original PR seemed to indicate that it only affected Android. cc @jason-simmons , do you know if this is still the case?

[jason-simmons]

The --no-stripped flag is also being applied to the Linux embedder libraries:
flutter/buildroot@ff1975d

I think this PR should change the build recipes for Linux desktop targets to explicitly pass --stripped.

@cbracken

[johnmccutchan]

Why do we want to strip our .sos by default? It seems useful for everyone to have the debug symbols. Stripping should be a downstream decision not an upstream decision.

[jason-simmons]

My main concern is binary size for developers building apps with the default tooling configuration.

[johnmccutchan]

But why does binary size during development matter? I've never even looked at how big my APKs are during development.

[jason-simmons]

AFAICT flutter_tools does not strip the engine library when building and packaging Linux desktop apps in release mode.

One option would be distributing unstripped builds of binaries like libflutter_linux_gtk.so for debug/profile mode and stripped builds for release mode.

[dnfield]

Can we scope this only to Android?

Right now this is applying to all builds on all platforms.

[dnfield]

If we want this for Android, we should scope this to only Android, not to all builds.

If we do this for Android, gradle will strip the libflutter.so anyway when it builds the APK by default, unless we update the build rules to tell gradle to not strip the binary.

I'm fine with having something that makes it easier to get symbols when building/debugging a Fluter app, but this change affects more platforms/workflows than I think it intends to.

[johnmccutchan]

I think we should be shipping unstripped binaries for all platforms and then allow our downstream users to decide to strip or not. Benefits include:

1. Greatly simplifies the symbolization problem for Android apps as the APK uploaded to the Play Store will contain the symbol information and the Play Store extracts this automatically. @gmackall please let me know if I've misunderstood.
2. Makes debugging easier.

The drawbacks seem to be:

1. Larger download size.
2. Larger built apps (until/unless they are stripped).
3. Communicate this change to developers.

[jason-simmons]

Another disadvantage will be increased time to install each build of an app on a device during development.

This is especially noticeable on older/slower devices.

I tried building a default template app with a local android_debug_unopt build with and without stripping.

The APK was 173MB with the unstripped engine library and 62MB with a stripped engine.

On a Moto G4, running adb install for the unstripped APK took 32 seconds, versus 11 seconds for stripped.

[dnfield]

1. Greatly simplifies the symbolization problem for Android apps as the APK uploaded to the Play Store will contain the symbol information and the Play Store extracts this automatically. @gmackall please let me know if I've misunderstood.

This is incorrect - gradle will strip the native library during the local build process before the APK is uploaded to the store.

[gmackall]

1. Greatly simplifies the symbolization problem for Android apps as the APK uploaded to the Play Store will contain the symbol information and the Play Store extracts this automatically. @gmackall please let me know if I've misunderstood.

This is incorrect - gradle will strip the native library during the local build process before the APK is uploaded to the store.

Neither the original issue (flutter/flutter#60240) nor the more recent duplicate (flutter/flutter#98773) are concerning uploading APKs to the play store.

They are concerning uploading appbundles to the play store (the reason that the main issue has so many likes is that the Play Store has an "Appbundle Explorer tool" which is providing this warning).

Appbundles have a BUNDLE-METADATA subdirectory which contains information that is not included in the APKs that get delivered to users, but is useful to the app store itself. The debug symbols are automatically included in this directory while building an appbundle, if they are available.

Comparing my branch to main, building the exact same flutter app (brand new app with only the changes from https://docs.flutter.dev/deployment/android regarding signing, and changes so the play store doesn't complain about the namespace):

(Main) flutter build appbundle creates an appbundle with a BUNDLE-METADATA with subdirectories:

com.android.tools.build.gradle		com.android.tools.build.libraries	com.android.tools.build.obfuscation

(My branch) flutter build appbundle --local-engine=android_release_arm64 --local-engine-host=host_release_arm64 --local-engine-src-path /Users/mackall/development/engine/src creates an appbundle with BUNDLE-METADATA with subdirectories

com.android.tools.build.debugsymbols	com.android.tools.build.gradle		com.android.tools.build.libraries	com.android.tools.build.obfuscation

And indeed, after uploading to the play store
main:

my branch:

It simply works by default, with no Gradle changes

[gmackall]

Another disadvantage will be increased time to install each build of an app on a device during development.

This is especially noticeable on older/slower devices.

I tried building a default template app with a local android_debug_unopt build with and without stripping.

The APK was 173MB with the unstripped engine library and 62MB with a stripped engine.

On a Moto G4, running adb install for the unstripped APK took 32 seconds, versus 11 seconds for stripped.

I need to rebuild my debug version to verify, but I believe that APKs will continue to be stripped of symbols by default, and presumably flutter run is building an apk and then installing that? So I would assume this would have no impact, but I will check!

[gmackall]

Also yes I did not intend for this to affect non-android platforms. I will change the scope to fix that, though @johnmccutchan makes good points that it might be something for other platforms to consider as well

[cbracken]

Agreed with limiting this patch to Android. We haven't had any reports of issues on Linux so I'd prefer to avoid modifying the behaviour (or to do so only in a dedicated patch that can be discussed separately).

[dnfield]

Is there some other way to add the symbols to the app bundle? E.g. downloading them from cloud storage as an artifact and putting them in a place where gradle recognizes them?

[gmackall]

Another disadvantage will be increased time to install each build of an app on a device during development.

This is especially noticeable on older/slower devices.

I tried building a default template app with a local android_debug_unopt build with and without stripping.

The APK was 173MB with the unstripped engine library and 62MB with a stripped engine.

On a Moto G4, running adb install for the unstripped APK took 32 seconds, versus 11 seconds for stripped.

When you tried this, did you modify any Gradle files to explicitly not strip debug symbols? Or did you find that they were simply not stripped when building the same app with and without a stripped version of the engine?

[jason-simmons]

The measurement was done using local android_debug_unopt builds of the engine and flutter --local-engine android_debug_unopt run.

I did not modify any Gradle scripts.

[gmackall]

Is there some other way to add the symbols to the app bundle? E.g. downloading them from cloud storage as an artifact and putting them in a place where gradle recognizes them?

I'm not aware of any way to accomplish this short of modifying the output appbundle itself, but if anyone who comes across this knows better suggestions are welcome
(the only answer I could find was someone trying this and giving up)

[gmackall]

The measurement was done using local android_debug_unopt builds of the engine and flutter --local-engine android_debug_unopt run.

I did not modify any Gradle scripts.

Interesting, I expect it could be possible to solve this by modifying flutter.groovy to make sure we strip the symbols by default, so we don't unintentionally increase apk install time for folks (at least in that way, hopefully the actual process of stripping is fast). I can try this and report back

[reidbaker]

Is there some other way to add the symbols to the app bundle? E.g. downloading them from cloud storage as an artifact and putting them in a place where gradle recognizes them?

I'm not aware of any way to accomplish this short of modifying the output appbundle itself, but if anyone who comes across this knows better suggestions are welcome (the only answer I could find was someone trying this and giving up)

This is actually interesting because if we could make the .so.dbg files available then our own tooling could add them when we build an appbundle. I am not sure what the scope of that work would be though.

[dnfield]

It's step 2 here:

https://support.google.com/googleplay/android-developer/answer/9848633?hl=en#zippy=%2Cupload-files-using-play-console

[dnfield]

There is an API for it as well: https://developers.google.com/android-publisher/api-ref/rest/v3/edits.deobfuscationfiles/upload

[gmackall]

It's step 2 here:

https://support.google.com/googleplay/android-developer/answer/9848633?hl=en#zippy=%2Cupload-files-using-play-console

Yes, this is what some users have been doing manually, although I believe they've been zipping files with no debug symbols in them (as they can't have the engine debug symbols, because they aren't provided in the pre-built engine artifacts), but for some reason it still makes the warning go away (ex https://stackoverflow.com/a/68778908, flutter/flutter#60240 (comment))

If we don't want to include the debug symbols in the actual engine artifacts by default and make it work as the linked step 2 indicates,

Important: This step is only required for developers using APKs. If you’re using an app bundle and Android Gradle plugin version 4.1 or later, then there's nothing you need to do.

, we could alternatively grab them for release builds to put beside the appbundle, and include in our documentation where to find them and upload them to the play store. But I don't know of a way to actually make them be included in the appbundle itself as part of the build process, besides not stripping them in the first place.

[dnfield]

Users can definitely get the symbols today, we document how to do it at https://github.com/flutter/flutter/wiki/Crashes#get-the-symbols

Including them in release builds should not regress debug build times, and should have a smaller (although still significant) impact on release size. You'll have to update a few different CI build jsons to achieve it though.

Are we sure these errors are not about missing symbols for the app.so that contains the Dart code though?

[dnfield]

I'm going to close this to get it off review queues - it's coming up in our weekly triage and there aren't any new changes. I'm happy to talk more about what would make this something we could land (e.g. scoping it only to the build configuration for Android release, or at least just Android).

I'm also still not sure why we can't have the tool distribute the symbol files we already upload, as per the crash wiki I linked last week, although if we need to make changes to how we build the "default" libflutter.so for this let's scope it to just that.

[gmackall]

Keeping the PR updated that we will be meeting this week to discuss next steps