Proposal/Attempt to remove library desugaring

snowplow / snowplow-android-tracker

Snowplow event tracker for Android. Add analytics to your Android apps and games

http://snowplowanalytics.com

Apache License 2.0

109 stars 63 forks source link

Proposal/Attempt to remove library desugaring #675

Open dpgmedia-gbero opened 7 months ago

dpgmedia-gbero commented 7 months ago

We observed crashes with devices running Android API 23. Initialization of FocalMeterConfiguration would crash.

A thread (https://github.com/snowplow/snowplow-android-tracker/issues/601) in the issue tracker suggested to use of coreLibraryDesugaring but we believe there might be a different option : Remove usage of java.util.function.* in order to get rid of library desugaring.

I'm not sure how to entirely test this but can this be looked at ?

Many thanks,

snowplowcla commented 7 months ago

Thanks for your pull request. Is this your first contribution to a Snowplow open source project? Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://docs.snowplowanalytics.com/docs/contributing/contributor-license-agreement/ to learn more and sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

matus-tomlein commented 7 months ago

Thanks for the proposal, @dpgmedia-gbero! This makes a lot of sense and I do like it better than our current approach.

But my concern is that this is a breaking change and we would need to wait for another major version of the tracker in order to release this (we have just published version 6 and don't plan to publish version 7 any time soon).

In particular, moving from Consumer<InspectableEvent> to (InspectableEvent) -> Unit? or (InspectableEvent) -> Void? is not the same because the Consumer doesn't care about the return type, but the latter representation does. We can also see in our tests that they can't be compiled using the new API.

Do you see any way that we could make this into a non-breaking change so that we can release this in a v6 update to the tracker?

dpgmedia-gbero commented 7 months ago

I understand your concern. We decided to disable FocalMeterConfiguration on Android 6 and lower instead to circumvent this issue. You can close this or keep it for reference if you'd foresee to get rid of core library desugaring

plastiv commented 6 months ago

Hi @matus-tomlein !

Could you consider to evaluate if desugar_jdk_libs_minimal can be used? Based on description it includes function package snowplow sdk has used. And if you agree to not extend the scope of java standard lib usage that may minimize the bundled dependency.

https://github.com/google/desugar_jdk_libs/blob/master/CHANGELOG.md#version-200-2022-09-16

matus-tomlein commented 5 months ago

Thank you for the suggestion @plastiv! That makes sense, I'll make sure we evaluate this (contributions are of course welcome too).