Skip to content
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

Fix #3962: Embed proguard.map in optimized AAB builds #3973

Merged

Conversation

BenHenning
Copy link
Sponsor Member

@BenHenning BenHenning commented Oct 27, 2021

Explanation

Fix #3962

This PR updates the oppia_android_application macro to support embedding the generated proguard.map file in optimized builds within the AAB. This is something that Gradle does, and it's especially convenient since it'll make both archiving & uploading the build to Play Store overall nicer. To verify that it works, download the alpha build from the CI artifacts and open it in a zip/archive viewer to see the proguard.map file under the top-level BUNDLE-METADATA directory.

APK and non-optimized AAB builds should be uanffected.

Note that the specific solution here came after multiple iterations of trying to determine how Gradle went about embedding the file since bundletool wasn't making it simple to add the metadata directory. The final solution was to add a new stage at the end of the pipeline to update the generated AAB to include the directory, though this needed to be done such that no directories were added themselves as zip entries (since this breaks bundletool). See google/bundletool#251 for a more detailed description of the issues discovered as part of this PR.

No tests are added since this is a Bazel infrastructure-only change, and we don't currently have tests for our Starlark rules. Manual verification should suffice.

Finally, I updated the other rules to accept single inputs since none of them actually pass multiple inputs. And, I'm planning to cherry-pick this into the release-0.6 branch to make the release archiving & upload easier, even though #3962 is not actually a release blocker.

Essential Checklist

  • The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

N/A -- infrastructural change that doesn't affect any user flows

@BenHenning BenHenning added the PR: Cherrypick requested Indicates that a PR is being requested for being cherrypicked into the ongoing release branch. label Oct 27, 2021
@BenHenning BenHenning marked this pull request as ready for review October 27, 2021 06:58
@BenHenning
Copy link
Sponsor Member Author

@anandwana001 PTAL as a full reviewer.

Copy link
Contributor

@anandwana001 anandwana001 left a comment

Choose a reason for hiding this comment

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

LGTM,

@BenHenning Just to confirm, am I looking at the correct file here?
Screenshot 2021-10-27 at 17 30 01

@oppiabot
Copy link

oppiabot bot commented Oct 27, 2021

Hi @BenHenning, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

@BenHenning
Copy link
Sponsor Member Author

LGTM,

@BenHenning Just to confirm, am I looking at the correct file here? Screenshot 2021-10-27 at 17 30 01

That's correct @anandwana001 -- thanks for checking & for the review!

@BenHenning
Copy link
Sponsor Member Author

Merging since it seems that there are no outstanding items.

@BenHenning BenHenning merged commit 550eecb into develop Oct 27, 2021
@BenHenning BenHenning deleted the add-support-for-embedding-proguard-map-file-in-aab branch October 27, 2021 21:07
@BenHenning BenHenning added this to the Alpha MR3 milestone Oct 27, 2021
BenHenning added a commit that referenced this pull request Oct 29, 2021
#3975)

* Fix #3946: Fix solution automatically revealing after first hint reveal (#3955)

* Fix solution auto showing after hint reveal.

See #3946 & PR for specifics.

* Add translations for new hint/solution.

* Lint fixes.

* Add TODO.

* Fix Gradle-variant tests.

* Fix #3937: Ensure ViewEventLogsViewModel builds for alpha builds (#3957)

* Ensure ViewEventLogsViewModel builds for alpha.

* BUILD file lint fix.

* Fix #3939 & #3938: Fix KitKat crash & SVG rendering issues (#3963)

* Fix KitKat crash when opening Help menu.

* Fix SVG rendering on SDKs 19-23 (incl).

* Add regex check to prohibit Delegates.

* Lint fixes.

* Add exemption for regex script test.

* Update file_content_validation_checks.textproto

Grammar fix in error.

* Update RegexPatternValidationCheckTest.kt

Copy grammar fix to test copy of error.

* Update version.bzl (#3964)

Bump version codes for another RC of release-0.6.

* Embed proguard.map in optimized AAB builds. (#3973)
@BenHenning BenHenning added the PR: Cherrypick completed Indicates a cherrypick request was approved & completed for a PR. label Oct 30, 2021
@BenHenning
Copy link
Sponsor Member Author

This is verified as working on the release-0.6 build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Cherrypick completed Indicates a cherrypick request was approved & completed for a PR. PR: Cherrypick requested Indicates that a PR is being requested for being cherrypicked into the ongoing release branch. PR: LGTM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Proguard map file to AABs
2 participants