-
Notifications
You must be signed in to change notification settings - Fork 739
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
Excluding the gms play-service-location from maplibre for fdroid builds #6136
Excluding the gms play-service-location from maplibre for fdroid builds #6136
Conversation
…cy for the fdroid variant - fixes fdroid being unable to compile the project due to a non foss dependency
a1bbdd3
to
bec7226
Compare
implementation 'org.maplibre.gl:android-plugin-annotation-v9:1.0.0' | ||
|
||
fdroidImplementation(libs.maplibre.androidSdk) { | ||
exclude group: 'com.google.android.gms', module: 'play-services-location' |
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.
we should be safe to exclude this due to maplibre doing a runtime check for play services location
classes when attempting to create a location provider https://github.com/maplibre/maplibre-gl-native/blob/ea234edf67bb3aec75f077e15c1c30c99756b926/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/engine/LocationEngineProvider.java#L59
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 for checking. For my understanding, Is it mandatory to specify the module here?
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.
not mandatory, I can reduce the explicitness if wanted
I chose to target only the location module as that's the module maplibre is doing the runtime check against
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.
OK, also this seems to be the only imported module, see:
- https://github.com/maplibre/maplibre-gl-native/blob/ea234edf67bb3aec75f077e15c1c30c99756b926/platform/android/MapboxGLAndroidSDK/build.gradle#L16
- https://github.com/maplibre/maplibre-gl-native/blob/ea234edf67bb3aec75f077e15c1c30c99756b926/platform/android/gradle/dependencies.gradle#L68
No need to reduce the explicitness.
Thanks!
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.
Perfect, thanks!
implementation 'org.maplibre.gl:android-plugin-annotation-v9:1.0.0' | ||
|
||
fdroidImplementation(libs.maplibre.androidSdk) { | ||
exclude group: 'com.google.android.gms', module: 'play-services-location' |
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 for checking. For my understanding, Is it mandatory to specify the module here?
exclude group: 'com.google.android.gms', module: 'play-services-location' | ||
} | ||
fdroidImplementation(libs.maplibre.pluginAnnotation) { | ||
exclude group: 'com.google.android.gms', module: 'play-services-location' |
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 am pretty sure you have checked that, but does libs.maplibre.pluginAnnotation
also bring this module?
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.
it depends on maplibre-android-sdk
which I was then seeing pull back in the transitive dependency 🤔
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.
OK, thanks!
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.
Can be merged!
Matrix SDKIntegration Tests Results:
|
Type of change
Content
Fixes #6100
Excludes the non foss
play services location
optional transitive dependency from maplibre on fdroid builds.Motivation and context
To allow the fdroid variant to contain only FOSS code.
Screenshots / GIFs
fdroid Before
fdroid After
Tests
print out dependency graph with
./gradlew vector:dependencies
Notice fdroid variants exclude play-services-location from maplibre's transitive dependencies
Launch the fdroid app
Enable location sharing via settings -> preferences
Share a location
Tested devices