commons-app / apps-android-commons

The Wikimedia Commons Android app allows users to upload pictures from their Android phone/tablet to Wikimedia Commons
https://commons-app.github.io/
Apache License 2.0
998 stars 1.18k forks source link

[Bug]: F-Droid: GMS libs in the final apk #4994

Closed linsui closed 1 year ago

linsui commented 2 years ago

Summary

F-Droid deploys a new binary scanner which finds GMS in the apk. See https://gitlab.com/linsui/fdroiddata/-/jobs/2577487197 I don't know where it comes from.

Steps to reproduce

You can use dexdump and grep to check the apk.

Expected behaviour

There is no nonfree libs.

Actual behaviour

GMS sleaks in.

Device name

No response

Android version

_

Commons app version

Since 2.6.5

Device logs

No response

Screen-shots

No response

Would you like to work on the issue?

No response

nicolas-raoul commented 2 years ago

Thanks for letting us know about this!

licaon-kter commented 2 years ago

It's via mapbox, there are references in com/mapbox/android/core/location/

Any chance to update to https://github.com/maplibre/maplibre-gl-native instead of the old lib?

Ref: https://github.com/maplibre/maplibre-gl-native/pull/313

/LE: I tried to replace it locally (kept telemetry plugin since there's no -libre equivalent), and exclude gms, but it fails to build: Unresolved reference: Timber in some kotlin files

nicolas-raoul commented 1 year ago

The Timber error sounds like something that could be solved with some Gradle wizardry maybe?

bhavanagarlapati commented 1 year ago

@nicolas-raoul Hi, can I take this up?

nicolas-raoul commented 1 year ago

@bhavanagarlapati Yes, please let us know about your progress every two or three days, thanks! :-)

bhavanagarlapati commented 1 year ago

Hi @nicolas-raoul I started working on the issue. I will keep you posted

kartikaykaushik14 commented 1 year ago

Hi @nicolas-raoul , is the issue fixed or I can take this up?

nicolas-raoul commented 1 year ago

@kartikaykaushik14 You are very welcome to take it up, thank you! Please let us know about your progress every two or three days.

@bhavanagarlapati I assume you have moved on, but if you have found anything please feel free to post your findings or ideas, thanks! :-)

kartikaykaushik14 commented 1 year ago

Update: I first tried following @licaon-kter 's suggestion and resolved all build-related issues but even maplibre-gl uses GMS libraries internally. Trying to look for other alternatives.

licaon-kter commented 1 year ago

Try to look at Delta Chat or Element recipes, where it's cleaned up and rebuilt before the app is built.

It sounds straight forward to me but you might need to try it first to confirm.

nicolas-raoul commented 1 year ago

_(Just in case MapBox and maplibre are not usable, I want to mention that other options seem to be available: https://wiki.openstreetmap.org/wiki/Software_libraries#Native_widgets)_

linsui commented 1 year ago

Maplibre will remove the dep of GMS soon. See https://github.com/maplibre/maplibre-gl-native/pull/801

kartikaykaushik14 commented 1 year ago

I have a probable solution and just wanted to confirm if it's feasible.

As @licaon-kter rightly pointed out, there are references in com/mapbox/android/core/location/. This is currently being used in only one class fr.free.nrw.commons.LocationPicker.LocationPickerActiviy.java. However, we can avoid using the libraries altogether by getting the last known location from locationComponent instead of using the location engine to get the last location of the user. In addition to it we can exclude gms from mapbox-android-sdk as suggested in this comment in the MapLibre repo

Please refer to 116ff70

licaon-kter commented 1 year ago

@kartikaykaushik14 unfortunately the exclusion is not enough anymore, see Linsui's link above for the path forward.

kartikaykaushik14 commented 1 year ago

I was going through Delta Chat as a reference and they seemed to have removed telemetry as in this line after they made changes to move to maplibre.

Do we retain telemetry or remove it?

I'm facing a build issue with com.mapbox.mapboxsdk.maps.TelemetryDefinition as it seems to be derived from the mapbox-android-sdk dependency which we are trying to eliminate. The telemetry dependency that we had earlier - "mapbox-android-telemetry:7.0.0" does not contain the TelemetryDefinition class and can in fact be removed since maplibre already provides the required support.

licaon-kter commented 1 year ago

@kartikaykaushik14 when I said "deltachat recipe" I meant https://gitlab.com/fdroid/fdroiddata/-/blob/master/metadata/com.b44t.messenger.yml#L1855-L1880

kartikaykaushik14 commented 1 year ago

@licaon-kter so if my understanding is correct, we do not have to implement the pre-build steps in the deltachat recipe once MapLibre removes the dependency of GMS right? I see that the changes were merged just yesterday and they have removed gms dependencies in the files that were previously being done through the pre-build steps. So can we probably wait for the official release in such a case?

kartikaykaushik14 commented 1 year ago

Also, please advise regarding Telemetry if I should retain it or remove it like in the case of deltachat?

licaon-kter commented 1 year ago

@kartikaykaushik14 yup, I guess you'd need a new maven release

nicolas-raoul commented 1 year ago

@kartikaykaushik14 We need to remove telemetry, due to our app's privacy policy. Thanks a lot!

kartikaykaushik14 commented 1 year ago

MapLibre has released a new version 10.0.0 that contains the fixes to remove gms dependencies. But as a result, there is a heavy backward compatibility break. For example, It requires a Kotlin version 1.7.20 (we currently use 1.5.10). And the minSdkVersion is 21 (currently 19, compile and target Android SDK Versions have to be modified to 31). The changes are not straightforward. Is there an open issue where we are handling any such upgrades?

kartikaykaushik14 commented 1 year ago

Also, can we have a separate issue to track removing telemetry and I can be assigned to it? We have to remove telemetry first, make the required upgrades, and then we can proceed to move to MapLibre.

nicolas-raoul commented 1 year ago

@kartikaykaushik14 Good idea, feel free to create such issues as you see fit. Thank you!

nicolas-raoul commented 1 year ago

@kartikaykaushik14 Would you mind creating an issue about switching to MapLibre? You can be assigned to it if you want (the general rule is one assigned issue at a time, but since you are very knowledgeable on the topic and involved already, an exception would be justified).

kartikaykaushik14 commented 1 year ago

@nicolas-raoul Could we close this issue too as we closed #5154, #5158, #5175 and #5182?

nicolas-raoul commented 1 year ago

Yes, sure.

Thanks a lot @kartikaykaushik14 for brilliantly solving this very complex issue!

licaon-kter commented 1 year ago

Ping us when ready to release.

nicolas-raoul commented 1 year ago

@sivaraam How about pushing a beta release as soon as you can? 🙂

licaon-kter commented 1 year ago

Which commit you want as beta?

nicolas-raoul commented 1 year ago

@licaon-kter Commit cf35307d9ace28157263342ddf85e46f454cd4a8 (latest commit on master) would be great for beta, it is very reliable already.

licaon-kter commented 1 year ago

Pushed as beta: https://gitlab.com/fdroid/fdroiddata/-/commit/ecd41b9d6db090c64d1eb5ddc50a3ed163904fe8

nicolas-raoul commented 1 year ago

@licaon-kter Thanks a lot! Will it appear at https://f-droid.org/en/packages/fr.free.nrw.commons/? I do not see any beta-related setting in the F-Droid app, is there a way for F-Droid users get it? :-)

licaon-kter commented 1 year ago

F-Droid Client, your app, either manually expand Versions and install or go to upper right menu and enable "Betas".

This will be built in the next cycle.

sivaraam commented 1 year ago

@sivaraam How about pushing a beta release as soon as you can? slightly_smiling_face

Sure. We certainly need to do that. Will initiate the process soon 🙂

mnalis commented 1 year ago

Is this still blocking f-droid release? I'd love to see latest bugfixed version 4.1.0 published on f-droid

sivaraam commented 1 year ago

Is this still blocking f-droid release? I'd love to see latest bugfixed version 4.1.0 published on f-droid

Thanks for nudging about this in the corresponding F-droid issue @mnalis ! I too hope to see 4.1.0 on F-droid soon 🙂

@licaon-kter I believe F-droid has an option to automatically recognize when new version of the app are released. Kindly let me know if anything is to be done for it in our end. That would help automate this stuff.

licaon-kter commented 1 year ago

@sivaraam if it works once tested we can re-enable the disabled autoupdate

licaon-kter commented 1 year ago

Done :tada: https://gitlab.com/fdroid/fdroiddata/-/commit/32e5bd97aa49be4b8d1228e1ae7413cc81d5ffff