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

Excluding the gms play-service-location from maplibre for fdroid builds #6136

Merged
merged 1 commit into from
May 25, 2022

Conversation

ouchadam
Copy link
Contributor

@ouchadam ouchadam commented May 24, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

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
+--- org.maplibre.gl:android-sdk:9.5.2
|    +--- org.maplibre.gl:android-sdk-geojson:5.9.0
|    |    \--- com.google.code.gson:gson:2.8.6
|    +--- com.mapbox.mapboxsdk:mapbox-android-gestures:0.7.0
|    |    +--- androidx.core:core:1.0.0 -> 1.7.0 (*)
|    |    \--- androidx.annotation:annotation:1.0.0 -> 1.3.0
|    +--- org.maplibre.gl:android-sdk-turf:5.9.0
|    |    \--- org.maplibre.gl:android-sdk-geojson:5.9.0 (*)
|    +--- androidx.annotation:annotation:1.0.0 -> 1.3.0
|    +--- androidx.fragment:fragment:1.0.0 -> 1.4.1 (*)
|    +--- com.squareup.okhttp3:okhttp:3.12.3 -> 4.9.3
|    |    +--- com.squareup.okio:okio:2.8.0 -> 2.10.0
|    |    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.20 -> 1.6.21 (*)
|    |    |    \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.4.20 -> 1.6.21
|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.4.10 -> 1.6.21 (*)
|    \--- com.google.android.gms:play-services-location:16.0.0
|         +--- com.google.android.gms:play-services-base:16.0.1 -> 17.0.0 (*)
|         +--- com.google.android.gms:play-services-basement:16.0.1 -> 17.0.0 (*)
|         +--- com.google.android.gms:play-services-places-placereport:16.0.0
|         |    \--- com.google.android.gms:play-services-basement:16.0.1 -> 17.0.0 (*)
|         \--- com.google.android.gms:play-services-tasks:16.0.1 -> 17.0.0 (*)
+--- org.maplibre.gl:android-plugin-annotation-v9:1.0.0
|    +--- androidx.appcompat:appcompat:1.0.0 -> 1.4.1 (*)
|    \--- org.maplibre.gl:android-sdk:9.4.2 -> 9.5.2 (*)
fdroid After
+--- org.maplibre.gl:android-sdk:9.5.2
|    +--- org.maplibre.gl:android-sdk-geojson:5.9.0
|    |    \--- com.google.code.gson:gson:2.8.6
|    +--- com.mapbox.mapboxsdk:mapbox-android-gestures:0.7.0
|    |    +--- androidx.core:core:1.0.0 -> 1.7.0 (*)
|    |    \--- androidx.annotation:annotation:1.0.0 -> 1.3.0
|    +--- org.maplibre.gl:android-sdk-turf:5.9.0
|    |    \--- org.maplibre.gl:android-sdk-geojson:5.9.0 (*)
|    +--- androidx.annotation:annotation:1.0.0 -> 1.3.0
|    +--- androidx.fragment:fragment:1.0.0 -> 1.4.1 (*)
|    \--- com.squareup.okhttp3:okhttp:3.12.3 -> 4.9.3 (*)
+--- org.maplibre.gl:android-plugin-annotation-v9:1.0.0
|    +--- androidx.appcompat:appcompat:1.0.0 -> 1.4.1 (*)
|    \--- org.maplibre.gl:android-sdk:9.4.2 -> 9.5.2 (*)
+--- io.jsonwebtoken:jjwt-impl:0.11.5
|    \--- io.jsonwebtoken:jjwt-api:0.11.5
\--- io.jsonwebtoken:jjwt-orgjson:0.11.5
     \--- io.jsonwebtoken:jjwt-api:0.11.5
FDROID LOCATION
Screenshot_20220524_100030

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

  • Physical
  • Emulator
  • OS version(s): 28

…cy for the fdroid variant

- fixes fdroid being unable to compile the project due to a non foss dependency
@ouchadam ouchadam force-pushed the feature/adm/foss-avoid-gms-location-provider branch from a1bbdd3 to bec7226 Compare May 24, 2022 09:10
@ouchadam ouchadam marked this pull request as ready for review May 24, 2022 09:10
@ouchadam ouchadam requested review from a team, bmarty and Claire1817 and removed request for a team May 24, 2022 09:10
@ouchadam ouchadam added A-Location PR-Small PR with less than 20 updated lines labels May 24, 2022
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'
Copy link
Contributor Author

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

Copy link
Member

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?

Copy link
Contributor Author

@ouchadam ouchadam May 24, 2022

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

Copy link
Member

Choose a reason for hiding this comment

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

@github-actions
Copy link

Unit Test Results

124 files  ±0  124 suites  ±0   2m 32s ⏱️ +16s
220 tests ±0  220 ✔️ ±0  0 💤 ±0  0 ±0 
726 runs  ±0  726 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit bec7226. ± Comparison against base commit a59b8bf.

Copy link
Member

@bmarty bmarty left a 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'
Copy link
Member

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'
Copy link
Member

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?

Copy link
Contributor Author

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 🤔

Copy link
Member

Choose a reason for hiding this comment

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

OK, thanks!

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Can be merged!

@ouchadam ouchadam merged commit ae9922a into develop May 25, 2022
@ouchadam ouchadam deleted the feature/adm/foss-avoid-gms-location-provider branch May 25, 2022 07:59
@github-actions
Copy link

Matrix SDK

Integration Tests Results:

  • [org.matrix.android.sdk.session]
    = passed=20 failures=0 errors=0 skipped=3
  • [org.matrix.android.sdk.account]
    = passed=3 failures=0 errors=0 skipped=2
  • [org.matrix.android.sdk.internal]
    = passed=53 failures=2 errors=0 skipped=1
  • [org.matrix.android.sdk.ordering]
    = passed=16 failures=0 errors=0 skipped=0
  • [org.matrix.android.sdk.PermalinkParserTest]
    = passed=2 failures=0 errors=0 skipped=0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Location PR-Small PR with less than 20 updated lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

F-Droid can't build - org.maplibre.gl:android-sdk pulls in non-FOSS Google Services
3 participants