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

Make coreLibraryDesugaring optional (when minSdk >= 26) #682

Open
hannesbolmstedt opened this issue Nov 5, 2024 · 4 comments
Open

Make coreLibraryDesugaring optional (when minSdk >= 26) #682

hannesbolmstedt opened this issue Nov 5, 2024 · 4 comments

Comments

@hannesbolmstedt
Copy link

coreLibraryDesugaring should only be required for Android API levels lower than 26 (this is stated both in the README.md and VERSIONING.md).

However, isCoreLibraryDesugaringEnabled = true is currently set in both otel.android-app-conventions.gradle.kts and otel.android-library-conventions.gradle.kts making it impossible to import the project without enabling coreLibraryDesugaring, even when using a minSdk >= 26 (correct me if I am wrong).

My suggestion is to remove these two lines in order to make coreLibraryDesugaring optional.

@LikeTheSalad
Copy link
Contributor

LikeTheSalad commented Nov 6, 2024

I just checked and our demo app does raise a compilation issue when isCoreLibraryDesugaringEnabled = false even if the minSdk version is set to 26.

I would think that the AGP should verify the minSdk version before raising the compilation issue, though that doesn't seem to be the case. One alternative might be to use the same animalsniffer check as the upstream Java repo does, to ensure we don't add code that only works on > 26 versions and remove the AGP check from our android lib conventions. Wdyt? @breedx-splk @bidetofevil @marandaneto

@marandaneto
Copy link
Member

I use ru.vyarus.animalsniffer as well so it is def. a good thing, another option is to have a sample project (just a template) with low minSdk that compiles on CI, so if compilation breaks, you know you have used something you should not, it's not guaranteed that it won't break at runtime though.

@bidetofevil
Copy link
Contributor

Yeah I don't think it's necessary - the app is responsible for deciding whether desugaring is necessary, so the SDK doing it is a bit of an over reach. I'd remove even the library dependency and let the app take care of that. A warning or failure during build time or in CI would be nice, but not strictly necessary IMO if we've documented this.

@breedx-splk
Copy link
Contributor

I use ru.vyarus.animalsniffer as well so it is def. a good thing, another option is to have a sample project (just a template) with low minSdk that compiles on CI, so if compilation breaks, you know you have used something you should not, it's not guaranteed that it won't break at runtime though.

Yeah I also see the value in this. Maybe if it's slow we just do it nightly, but it seems like catching the obvious/easy stuff automatically is helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants