[FSSDK-11888] fix: update ProGuard rules for AGP 8+ and R8 compatibility#86
Closed
muzahidul-opti wants to merge 9 commits intomasterfrom
Closed
[FSSDK-11888] fix: update ProGuard rules for AGP 8+ and R8 compatibility#86muzahidul-opti wants to merge 9 commits intomasterfrom
muzahidul-opti wants to merge 9 commits intomasterfrom
Conversation
- Update proguard configuration in build.gradle - Add missing dependencies for ProGuard rules - Update Android multidex support version in dependencies in app/build.gradle
muzahidul-opti
changed the title
- Add workflow for testing minification compatibility - Set up Flutter and Java environments - Install dependencies and create ProGuard rules - Test with minification enabled and disabled - Run unit tests and verify APK artifacts - Check for ProGuard/R8 artifacts and upload APKs - Report test results and clean up temporary files
- Update 'actions/upload-artifact' action version from v3 to v4 for workflow optimization and compatibility.
- Implement a CI workflow for testing minification compatibility - Include job for testing with minification enabled and disabled - Add steps for verifying APK artifacts and ProGuard files - Include uploading APK artifacts and reporting test results
muzahidul-opti
changed the title
- Update Gradle JVM and Android build options - Ensure minification is enabled - Build for single architecture to optimize memory - Refactor script for APK artifact verification - Simplify test execution line - Improve output messages for various tasks
- Update the Java version in the setup to '11' in minification workflow
…iguration - Update release build configuration in build.gradle - Remove minifyEnabled and shrinkResources settings from release build configuration
- Update Java version in the minification workflow from 11 to 17
muzahidul-opti
changed the title
muzahidul-opti
changed the title
jaeopt
requested changes
Oct 7, 2025
Contributor
jaeopt
left a comment
jaeopt
left a comment
There was a problem hiding this comment.
It's a good idea to have minify tests! A few more clarifications.
Comment on lines
+64
to
+73
| # Keep example app classes | ||
| -keep class com.optimizely.optimizely_flutter_sdk_example.** { *; } | ||
|
|
||
| # Keep Flutter classes | ||
| -keep class io.flutter.app.** { *; } | ||
| -keep class io.flutter.plugins.GeneratedPluginRegistrant { *; } | ||
|
|
||
| # Google Play Core (for Flutter engine) | ||
| -keep class com.google.android.play.core.** { *; } | ||
| -dontwarn com.google.android.play.core.** |
Contributor
There was a problem hiding this comment.
Can't we put all these in example-app proguard-rules?
Contributor
Author
There was a problem hiding this comment.
Yes, it is added into example app proguard rules.
Contributor
There was a problem hiding this comment.
Once added into example proguard, we can remove these from here to simplify?
| minifyEnabled true | ||
| proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' | ||
| minifyEnabled true | ||
| proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.txt', 'proguard-rules.pro' |
Contributor
There was a problem hiding this comment.
why this change? I understand proguard-rules.pro is new.
Contributor
Author
There was a problem hiding this comment.
We added 'proguard-rules.txt' here to apply in the release build.
Contributor
There was a problem hiding this comment.
Then we can drop "proguard-rules.pro" here?
Comment on lines
+15
to
+33
| # Keep Jackson classes for JSON parsing | ||
| -keep class com.fasterxml.jackson.** { *; } | ||
| -dontwarn com.fasterxml.jackson.** | ||
|
|
||
| # Keep Guava classes | ||
| -keep class com.google.common.** { *; } | ||
| -dontwarn com.google.common.** | ||
| -dontwarn com.google.android.play.core.** | ||
|
|
||
| # Keep SLF4J and Logback classes | ||
| -keep class org.slf4j.** { *; } | ||
| -keep class ch.qos.logback.** { *; } | ||
| -dontwarn org.slf4j.** | ||
| -dontwarn ch.qos.logback.** | ||
|
|
||
| # Missing Dependencies (Android doesn't have these) | ||
| -dontwarn javax.mail.** | ||
| -dontwarn javax.activation.** | ||
| -dontwarn javax.servlet.** |
Contributor
There was a problem hiding this comment.
Do we need these - not used in our flutter sdk?
Contributor
Author
There was a problem hiding this comment.
Updated the list.
| echo "ℹ️ No mapping file found" | ||
| fi | ||
|
|
||
| - name: Upload APK artifacts |
Contributor
There was a problem hiding this comment.
Obfuscation can cause app crash in runtime, even if building is fine. We may need to run tests after buidling.
No need to include in this fix, but we can consider it later.
- Remove additional safety rules for Optimizely - Update APK artifacts upload path for minification test - Remove unnecessary Guava classes and warnings - Adjust SLF4J and Logback rules for ProGuard - Remove warnings for missing dependencies in ProGuard configuration
jaeopt
requested changes
Oct 8, 2025
| architecture: x64 | ||
| - run: flutter pub get | ||
| - run: flutter test | ||
| - run: flutter test No newline at end of file |
Comment on lines
+64
to
+73
| # Keep example app classes | ||
| -keep class com.optimizely.optimizely_flutter_sdk_example.** { *; } | ||
|
|
||
| # Keep Flutter classes | ||
| -keep class io.flutter.app.** { *; } | ||
| -keep class io.flutter.plugins.GeneratedPluginRegistrant { *; } | ||
|
|
||
| # Google Play Core (for Flutter engine) | ||
| -keep class com.google.android.play.core.** { *; } | ||
| -dontwarn com.google.android.play.core.** |
Contributor
There was a problem hiding this comment.
Once added into example proguard, we can remove these from here to simplify?
| if [ -f "example/android/app/build.gradle.backup" ]; then | ||
| mv example/android/app/build.gradle.backup example/android/app/build.gradle | ||
| echo "✅ Restored original build.gradle" | ||
| fi No newline at end of file |
| minifyEnabled true | ||
| proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' | ||
| minifyEnabled true | ||
| proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.txt', 'proguard-rules.pro' |
Contributor
There was a problem hiding this comment.
Then we can drop "proguard-rules.pro" here?
Comment on lines
+13
to
+15
| # Keep Jackson classes for JSON parsing | ||
| -keep class com.fasterxml.jackson.** { *; } | ||
| -dontwarn com.fasterxml.jackson.** |
Contributor
There was a problem hiding this comment.
why Jackson here? Do we use Jackson in our sdk plugin?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Issues