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

Pick up consumer proguard rules correctly #62

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Conversation

marandaneto
Copy link
Member

@marandaneto marandaneto commented Nov 13, 2023

💡 Motivation and Context

java.lang.RuntimeException: Failed to invoke constructor 'com.posthog.PostHogEvent()' with no args.

If Proguard/R8 removed the classes/methods/ctor not referenced directly.
https://github.com/google/gson/blob/2658aca66a1d0f097794a5ad5eb33c095a8f8d5d/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java#L273C39-L273C67

  • Consumer file was not picked up correctly since it was set twice/and a typo.
  • Issue reported by a user.

💚 How did you test it?

Running the sample with R8 enabled.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

@@ -17,15 +17,9 @@ org.gradle.jvmargs=-Xmx2048m -Dfile.encoding=UTF-8
android.useAndroidX=true
# Kotlin code style for this project: "official" or "obsolete":
kotlin.code.style=official
# Enables namespacing of each library's R class so that its R class includes only the
Copy link
Member Author

Choose a reason for hiding this comment

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

Those are default values from >= AGP 8, so removing them

@@ -32,7 +31,7 @@ android {

buildTypes {
release {
consumerProguardFiles("proguard-rules.pro")
consumerProguardFiles("consumer-rules.pro")
Copy link
Member Author

Choose a reason for hiding this comment

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

The file name didn't match, probably a typo since its correct in L24.

Copy link

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

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

Nothing looks wrong so assuming you know what you're doing 😅

@marandaneto
Copy link
Member Author

Nothing looks wrong so assuming you know what you're doing 😅

Hopefully :D
I could not reproduce the issue locally but there was room for improvement anyway.

@marandaneto marandaneto merged commit 0e1fb69 into main Nov 14, 2023
5 checks passed
@marandaneto marandaneto deleted the fix/proguard-consumer branch November 14, 2023 08:38
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.

2 participants