Skip to content

fix: wire merge classes to asm transformer#417

Merged
hurali97 merged 3 commits into
mainfrom
fix/merge-classes
Jul 8, 2026
Merged

fix: wire merge classes to asm transformer#417
hurali97 merged 3 commits into
mainfrom
fix/merge-classes

Conversation

@hurali97

@hurali97 hurali97 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Summary

This PR fixes a bug in which the final merged classes.jar did not contain the third party libraries. This bug was introduced after the BGP optimizations. We did not face any issues until now because running the assemble or bundle command after the first time, gets the final merged classes.jar to have the third party libraries.

In our usual flow, we first run package:aar and then publish:aar, so we did not face this issue, however as part of #413 this issue became evident as Expo SDK 57 clears the native directories and regenerate those on each prebuild invocation.

To fix it, we bring back the code which was responsible to handle this. We create a custom MergeClasses task and wire it to be a dependency of transformClassesWithAsm gradle task.

Test plan

  • Verified locally
  • CI passes

Copilot AI review requested due to automatic review settings July 7, 2026 09:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@hurali97 hurali97 requested a review from artus9033 July 7, 2026 09:02

@artus9033 artus9033 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Awesome work! Left three comments, 2 may be major.

Comment on lines -44 to +23
MergeProcessor.mergeClassesJarIntoClasses(project, aarLibraries, outputDir)
if (isMinifyEnabled) {
MergeProcessor.mergeLibsIntoClasses(project, aarLibraries, jarFiles, outputDir)
if (!isDebuggable) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Previously, we used to check for isMinifyEnabled. I'm wondering: aren't we missing handling of this case? If any consumer enables minify on the brownfield library module, will embedded libs/*.jar from dependencies be merged?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This check was not doing any work as the jarFiles would always contain an empty list. Verified locally by enabling minification for brownfieldlib and we only need to adjust the proguard-rules and add consumer-rules to our packages like brownie, navigation and brownfield, which I will do in a follow-up PR.

@artus9033 artus9033 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@hurali97 hurali97 merged commit 28b534e into main Jul 8, 2026
16 checks passed
@hurali97 hurali97 deleted the fix/merge-classes branch July 8, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants