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

Missing classes detected while running R8. #167

Open
stranger11 opened this issue Jan 31, 2024 · 15 comments · May be fixed by #177
Open

Missing classes detected while running R8. #167

stranger11 opened this issue Jan 31, 2024 · 15 comments · May be fixed by #177
Assignees
Labels
bug Something isn't working Partner Support

Comments

@stranger11
Copy link

stranger11 commented Jan 31, 2024

Describe the bug

In release version catching:
Missing classes detected while running R8
Missing class java.beans.ConstructorProperties (referenced from: void com.fasterxml.jackson.databind.ext.Java7SupportImpl.())
Missing class java.beans.Transient (referenced from: void com.fasterxml.jackson.databind.ext.Java7SupportImpl.())

Expected behavior

No problems with R8 and shrinking.

Steps to reproduce the bug

  1. implementation 'org.xmtp:android:0.7.6'
  2. Build release
@stranger11 stranger11 added the bug Something isn't working label Jan 31, 2024
@nplasterer
Copy link
Contributor

👋 I recently saw this error when trying to bump us to Java 17 #158
Can you tell me more information about your project are you on AGP 8? Try downgraded to a version of AGP 7 which is what the project is setup with.

@stranger11
Copy link
Author

@nplasterer The project uses quite a few dependencies that require AGP8. What other options are there?

@nplasterer
Copy link
Contributor

Ya it's not very easy to support two AGP versions and it probably makes sense for us to just move to the latest. I'm giving a heads up to any current production integrators and then will merge #158 by Monday.

@nplasterer
Copy link
Contributor

This should be going out in the latest release https://github.com/xmtp/xmtp-android/actions/runs/7792254501

@stranger11
Copy link
Author

@nplasterer last version 0.7.10 didn't resolve problem..

@nplasterer nplasterer reopened this Feb 7, 2024
@nplasterer
Copy link
Contributor

@fabriguespe do you mind looking into this?

@fabriguespe
Copy link

fabriguespe commented Feb 8, 2024

@nplasterer I was able to reproduce the error if the minifyEnabled is set to true setting in the build.gradle file of the example module.

buildTypes {
        release {
            minifyEnabled true
            proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'
        }
    }

When minifyEnabled is set to true in the example module's build.gradle file, the build fails with the "Missing classes detected while running R8" error. However, if minifyEnabled is set to true only in the root build.gradle file, the build completes successfully.

Here's the error log I encountered when minifyEnabled was set to true in the example module:

> Task :example:minifyReleaseWithR8 FAILED
ERROR: Missing classes detected while running R8. Please add the missing classes or apply additional keep rules that are generated in /Users/fabrizioguespe/DevRel/xmtp-android/example/build/outputs/mapping/release/missing_rules.txt.
ERROR: R8: Missing class java.beans.ConstructorProperties (referenced from: void com.fasterxml.jackson.databind.ext.Java7SupportImpl.<init>())
Missing class java.beans.Transient (referenced from: void com.fasterxml.jackson.databind.ext.Java7SupportImpl.<init>())

@stranger11 has reported in Discord that if he sets minifyEnabled to false the build runs successfully.

Versions

  • Gradle version 8.1.1 or 8.0
  • Java version 17.0.9 installed

Run command

./gradlew assembleRelease

@nplasterer
Copy link
Contributor

Awesome thanks for reproducing I'll see what we can do to fix this for minifyed builds

@giovasdistillery
Copy link
Contributor

I fixed the main problem, with that configuration of @fabriguespe is enabling the obfuscation for example module, only the proguard-rules.pro file was changed adding the rules for that warning, but I'm not sure if is correct to not have obfuscation for the library, if yes it will require major work in the rules as is described in this link

@nplasterer
Copy link
Contributor

I think it would be worth spiking on the work there that you linked.

@stranger11
Copy link
Author

@giovasdistillery @nplasterer
I tried this solution even before I opened the issue.
-dontwarn java.beans.ConstructorProperties
-dontwarn java.beans.Transient

It doesn't work because the app crashes with exception:
FATAL EXCEPTION: DefaultDispatcher-worker-1
java.lang.ExceptionInInitializerError
at com.walletconnect.yd8.(SourceFile:1)
at com.walletconnect.f41.(SourceFile:17)
at com.walletconnect.pw0.invokeSuspend(SourceFile:12)
at com.walletconnect.fb0.resumeWith(SourceFile:14)
at kotlinx.coroutines.DispatchedTask.run(Unknown Source:101)

But if you add this rules on SDK level , maybe will another result

@giovasdistillery
Copy link
Contributor

To apply the proper rules for R8 we need the entries and exits of all the modules and classes in the app, for example, the models that are used to parse data from a network connection, there are some classes as com.walletconnect.android.Core, com.walletconnect.wcmodal.client.Modal that are not part of the library but is an external dependency that needs to have some classes without obfuscation, there are other objects that we can add to the proguard rules that allow the partial obfuscation of a class if necessary, some classes as ExampleApp and DappDelegate need that treatment because is using walletconnect, and this is just the tip of the iceberg, if other dependencies like Firebase or Crypto or the library itself need that we will need to do the same.

@stranger11
Copy link
Author

@giovasdistillery @nplasterer you can catch this problem on your xmtp android example https://github.com/xmtp/xmtp-android/tree/main/example with R8.

@stranger11
Copy link
Author

@fabriguespe @nplasterer @giovasdistillery
Hello! I want to share some good news - I have found a solution to the issue we encountered in the library. After a thorough examination of the code and experimentation, I managed to identify the root cause of the problem, and now everything is working as intended.

However, I also want to highlight that this issue remained unresolved for some time. I appreciate your work on the project, but it would have been great if this problem had received attention earlier. It's a serious obstacle for users and could have been avoided with a careful look at this matter.

This rules in proguard solved the problem:

-keepnames class com.fasterxml.jackson.** { ; }
-dontwarn com.fasterxml.jackson.databind.
*

-keep class org.xmtp.** { ; }
-dontwarn java.awt.

-keep class com.sun.jna.** { ; }
-keep class * implements com.sun.jna.
* { *; }

@giovasdistillery
Copy link
Contributor

giovasdistillery commented Feb 20, 2024

@stranger11 I did a commit yesterday with an extended rules approach, I'm checking your rules and I can see that is working at least the first page for me, I had the same problem with my approach, but I'm obfuscated more code, I just sent the message in Slack, it was my bad. Could you please check the latest changes in this PR and tell me if everything is ok?, because with any approach I'm not able to create a Wallet in release mode and I'm not able to see if other parts of the app are ok

Screenshot_20240220_090929

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Partner Support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants