glide icon indicating copy to clipboard operation
glide copied to clipboard

Support multiple level inheritance for lib and app modules in case of KSP usage

Open 0xnm opened this issue 1 year ago • 3 comments

Description

In Datadog we have instrumentation for Glide by the means of creating DatadogGlideModule which inherits from AppGlideModule. Customers can extend DatadogGlideModule instead of AppGlideModule and get their Glide calls instrumented with Datadog.

This worked fine for KAPT, but doesn't work for KSP, because in case of KSP usage compilation with some class which inherits DatadogGlideModule instead of AppGlideModule directly gives the same error as in https://github.com/bumptech/glide/issues/5328.

This happens because supertypes call here checks only types at declaration site and doesn't go through the whole inheritance chain up.

This PR fixes that issue by traversing the whole inheritance chain, so that functionality is aligned with KAPT compiler (which permits such inheritance chains).

NB: Since we have to traverse the whole inheritance chain and AppGlideModule extends actually LibraryGlideModule, it may be tricky to not register app module as library module as well, but since this PR is using first item in the supertype match, app module will be still registered as app module and not as both.

0xnm avatar Dec 28 '23 15:12 0xnm

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Dec 28 '23 15:12 google-cla[bot]

Thanks for the fix, I really appreciate the thorough tests!

sjudd avatar Jan 26 '24 04:01 sjudd

Hi @sjudd! I had to sync the branch with main for the auto-merge, but it seems workflow needs your approval.

0xnm avatar Feb 12 '24 09:02 0xnm

This is awesome. Do we have any idea when the next version containing this fix will be released?

manishindiainc avatar Mar 21 '24 16:03 manishindiainc