fix: Update ProGuard keep rules for Breadcrumb class #114
Conversation
This file contains 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
the NDK requires access to the Breadcrumb class as of 1.2.0, which changed location. this fix ensures the class is kept so that it can be accessed via the JNI
fractalwrench
changed the title
Pezzah
approved these changes
Jun 21, 2018
snmaynard
reviewed
Jun 21, 2018
| @@ -17,6 +17,7 @@ class BugsnagProguardConfigTask extends DefaultTask { | |||
| static final String PROGUARD_CONFIG_SETTINGS = """\ | |||
| -keepattributes LineNumberTable,SourceFile | |||
| -keep class com.bugsnag.android.NativeInterface { *; } | |||
| -keep class com.bugsnag.android.Breadcrumb { *; } | |||
| -keep class com.bugsnag.android.Breadcrumbs { *; } | |||
| -keep class com.bugsnag.android.Breadcrumbs\$Breadcrumb { *; } | |||
There was a problem hiding this comment.
I couldnt see com.bugsnag.android.Breadcrumbs\$Breadcrumb but maybe i'm missing something?
There was a problem hiding this comment.
Looks like it was removed in bugsnag/bugsnag-android@45b4544#diff-326654b6e94ba89f06579d82d3119049
There was a problem hiding this comment.
Yep. We need to keep it in the ProGuard rules for backwards compatibility with older notifiers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Goal
The NDK requires access to the
Breadcrumbclass as of 1.2.0, which changed location in the bugsnag-android notifier from an inner to root class. This fixensures that our added ProGuard rules keep the class, making it accessible via the JNI.
Changeset
Added proguard entry to keep new class location
Tests
Tested against NDK example project with bugsnag-android-ndk 1.2.0, a local maven installation of the plugin, and ProGuard enabled. With the old plugin version, the notifier crashes on startup, with this fix it launches and is capable of notifying. Adding the keep rule to the ProGuard file manually also works.
Review
For the submitter, initial self-review:
For the pull request reviewer(s), this changeset has been reviewed for: