Skip to content

Remove default for stripped option in engine/src/flutter/tools/gn, don't strip by default on android #161546

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

Merged

Conversation

gmackall
Copy link
Member

@gmackall gmackall commented Jan 13, 2025

Remake of flutter/engine#52852.

Makes it so that stripped defaults to false for android in gn arguments, i.e. we don't strip the Android engine builds. AGP does this by default when the NDK is installed (and we download it automatically now after #159756).

In testing, the step that AGP does to strip symbols adds ~1-2 seconds to the build.

Adds a tool verification for release app bundle builds that checks to make sure we have stripped the debug symbols and placed them in the BUNDLE-METADATA directory. The check is done by invoking apkanalyzer, which takes ~0.5 seconds, which is why the check is only enabled for release builds.

BEFORE LANDING: I need to follow up on flutter/engine#50443 (comment), to ensure we start stripping symbols internally as well.

Pre-launch Checklist

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

@github-actions github-actions bot added the engine flutter/engine repository. See also e: labels. label Jan 13, 2025
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jan 29, 2025
@gmackall gmackall marked this pull request as ready for review January 30, 2025 01:22
@gmackall gmackall requested review from a team and bkonyi January 30, 2025 15:34
@gmackall gmackall changed the title [remake] Remove default for stripped option, don't strip by default on android Remove default for stripped option in engine/src/flutter/tools/gn, don't strip by default on android Jan 30, 2025
processManager.addCommand(
const FakeCommand(
command: <String>[
'gradlew',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a lot of flags here that probably came from the flutter tool. Later when the tool is changed and we need to update this test it will be harder to figure out why.

link where you came up with these flags and their values.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulled out to a constant which will make updating easy. For where they came from, I just copied them from the existing tests. I can go find where all the flags come from if you think it is worthwhile

Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can share a lot of the logic between tests, so it's probably worth taking another look and refactoring them.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine flutter/engine repository. See also e: labels. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants