-
Notifications
You must be signed in to change notification settings - Fork 6
Fix generated module configuration files being obfuscated #82
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
Fix generated module configuration files being obfuscated #82
Conversation
buildSrc/src/main/kotlin/Project.kt
Outdated
| const val kotlin = "1.4.10" | ||
| const val androidX = "1.1.0" | ||
| const val license = "0.8.5" | ||
| const val license = "0.9.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have aligned this version with the mapbox-maps-android project as 0.8.5 version is no longer available
| // if configuration is enabled, generate module instance provider field that has to be passed by the user | ||
| val providerInterface = | ||
| TypeSpec.interfaceBuilder(MODULE_CONFIGURATION_PROVIDER_CLASS_NAME) | ||
| .addAnnotation(Keep::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you add some code snippets showcasing the generated code before the fix vs after the fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only change is at
@Keep
interface ModuleProvider
Before
package com.mapbox.module
import androidx.annotation.Keep
import com.mapbox.maps.module.MapTelemetry
import kotlin.Boolean
import kotlin.jvm.JvmStatic
/**
* Configuration provider for MapTelemetry module.
*/
@Keep
object Mapbox_MapTelemetryModuleConfiguration {
@JvmStatic
val enableConfiguration: Boolean = true
/**
* Set this dependency provider before initializing any components of the modularized library.
*
* When you're not using the library anymore, you should pass `null` to clean up the provider
* reference and prevent memory leaks.
*/
@JvmStatic
var moduleProvider: ModuleProvider? = null
interface ModuleProvider {
fun createMapTelemetry(): MapTelemetry
}
}
After
package com.mapbox.module
import androidx.annotation.Keep
import com.mapbox.maps.module.MapTelemetry
import kotlin.Boolean
import kotlin.jvm.JvmStatic
/**
* Configuration provider for MapTelemetry module.
*/
@Keep
object Mapbox_MapTelemetryModuleConfiguration {
@JvmStatic
val enableConfiguration: Boolean = true
/**
* Set this dependency provider before initializing any components of the modularized library.
*
* When you're not using the library anymore, you should pass `null` to clean up the provider
* reference and prevent memory leaks.
*/
@JvmStatic
var moduleProvider: ModuleProvider? = null
@Keep
interface ModuleProvider {
fun createMapTelemetry(): MapTelemetry
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Did you also validate the fix in maps SDK?
|
@kediarov please also update the title of the PR with descriptions |
| tools = "4.1.3" | ||
| kotlin = "1.4.10" | ||
| androidX = "1.1.0" | ||
| license = "0.9.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Older version Is not available
Aligned with mapbox-maps-android
|
@kediarov it's also good to update the snapshot version in https://github.com/mapbox/mapbox-base-android/blob/master/gradle.properties to 1.12.0-SNAPSHOT since you are going to publish 1.12.0 release |
pengdev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the fix!
| enableFeaturePreview("VERSION_CATALOGS") | ||
| dependencyResolutionManagement { | ||
| versionCatalogs { | ||
| create("baseLibs") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity, why rename it to baseLibs instead of the its default name?
| // if configuration is enabled, generate module instance provider field that has to be passed by the user | ||
| val providerInterface = | ||
| TypeSpec.interfaceBuilder(MODULE_CONFIGURATION_PROVIDER_CLASS_NAME) | ||
| .addAnnotation(Keep::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch! 💎
https://mapbox.atlassian.net/browse/MAPSAND-2026
First things first: why the
@Keepannotation is not working?The crash is happening when trying to call
getDeclaredMethod("create...")ofModuleProviderwhich is being shrunk because theModuleProvideris an interface inside, but not a member ofMapbox_module so it is not affected by the outer@Keepannotation.ModuleProvideris generated whenenableConfiguration = true.Even though the Keep annotation docs say
Do not use this within library codetheMapbox_module is generated by annotation processor and is being added to the module with where the target class annotated with@MapboxModuleis located.The rule
-keep @androidx.annotation.Keep class * {*;}to keep annotated targets is added toproguard-androidfile which is usually always added to every project.The direct solution is to add the
@Keepannotation to theModuleProvider. The other solution is to add-keep class com.mapbox.module.** {*;}but this rule is less accurate and more vague and might affect other classes in the package.