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

Read library glide module names from Java indexes #5052

Merged

Conversation

sjudd
Copy link
Collaborator

@sjudd sjudd commented Mar 8, 2023

Progress for #5043

Annotation processors (including ksp) only run on newly compiled code. Libraries have been previously compiled, so an annotation processor will not be run on them. To find any LibraryGlideModules included in those libraries, we look for specific generated classes in those libraries that we call Indexes. Indexes contain an annotation listing the class names of any LibraryGlideModules. Indexes are generated by Glide's annotation processors for each library and are exported as part of the library.

To preserve the package private visibility of the existing Java Index annotation, I created a new Index annotation for the KSP processor. This lets us reference the KSP Index annotation directly.

Unfortunately it also means that the Java and KSP Index classes are not the same. While it's only a small amount of duplicated code, it's a significant compatibility issue because the KSP and Java processors no longer produce the same Index class.

In particular the KSP processor looks only for its Index class and not for the Java processor's Index class. This is unfortunate because Glide's libraries are always processed by the Java annotation processor, not the KSP processor. In turn this means that Glide's KSP processor effectively ignores any of Glide's integration libraries.

To fix this in the short term I've made the KSP processor look for both its Indexes and the Java annotation processors Indexes. A more robust fix would be to merge the two Index processors so that the annotation processors are mutually compatible. I'll do that in a follow-up.

I've written tests, but they're somewhat involved so I'll send them as a follow-up.

@sjudd sjudd added the import-ready Indicates the PR is ready to be imported to Google. label Mar 8, 2023
Progress for bumptech#5043

Annotation processors (including ksp) only run on newly compiled code.
Libraries have been previously compiled, so an annotation processor will
not be run on them. To find any LibraryGlideModules included in those
libraries, we look for specific generated classes in those libraries
that we call Indexes. Indexes contain an annotation listing the class
names of any LibraryGlideModules. Indexes are generated by Glide's
annotation processors for each library and are exported as part of the
library.

To preserve the package private visibility of the existing Java Index
annotation, I created a new Index annotation for the KSP processor. This
lets us reference the KSP Index annotation directly.

Unfortunately it also means that the Java and KSP Index classes are not
the same. While it's only a small amount of duplicated code, it's a
significant compatibility issue because the KSP and Java processors no
longer produce the same Index class.

In particular the KSP processor looks only for its Index class and not
for the Java processor's Index class. This is unfortunate because
Glide's libraries are always processed by the Java annotation processor,
not the KSP processor. In turn this means that Glide's KSP processor
effectively ignores any of Glide's integration libraries.

To fix this in the short term I've made the KSP processor look for both
its Indexes and the Java annotation processors Indexes. A more robust
fix would be to merge the two Index processors so that the annotation
processors are mutually compatible. I'll do that in a follow-up.

I've written tests, but they're somewhat involved so I'll send them as
a follow-up.
@sjudd sjudd force-pushed the handle_java_indexes_in_ksp_processor branch from c600e4b to 16306e8 Compare March 8, 2023 19:46
@copybara-service copybara-service bot merged commit 902bbd4 into bumptech:master Mar 8, 2023
nikclayton referenced this pull request in pachli/pachli-android Oct 13, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[com.github.bumptech.glide:okhttp3-integration](https://togithub.com/bumptech/glide)
| `4.15.1` -> `4.16.0` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/com.github.bumptech.glide:okhttp3-integration/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.github.bumptech.glide:okhttp3-integration/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.github.bumptech.glide:okhttp3-integration/4.15.1/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.github.bumptech.glide:okhttp3-integration/4.15.1/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [com.github.bumptech.glide:glide](https://togithub.com/bumptech/glide)
| `4.15.1` -> `4.16.0` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/com.github.bumptech.glide:glide/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.github.bumptech.glide:glide/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.github.bumptech.glide:glide/4.15.1/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.github.bumptech.glide:glide/4.15.1/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [com.github.bumptech.glide:ksp](https://togithub.com/bumptech/glide) |
`4.15.1` -> `4.16.0` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/com.github.bumptech.glide:ksp/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.github.bumptech.glide:ksp/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.github.bumptech.glide:ksp/4.15.1/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.github.bumptech.glide:ksp/4.15.1/4.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>bumptech/glide
(com.github.bumptech.glide:okhttp3-integration)</summary>

###
[`v4.16.0`](https://togithub.com/bumptech/glide/releases/tag/v4.16.0):
Glide 4.16.0

This release focuses on some build improvements and Compose. The two
major Compose improvements are adding support for Compose specific
transitions (e.g. cross fade) and supporting recomposition based on
request state using `GlideSubcomposition`. There's also been a bunch of
internal refactoring to move away from Painters to Modifier nodes based
on feedback from the Compose team. This is still an alpha release of
Compose, but barring unexpectedly negative feedback, the next release
should be beta.

This should be the last release of Glide that targets Java 7. That
probably means our next release will be a major version change.

##### Features

- Allow passing an executor into ChromiumRequestSerializer in
[https://github.com/bumptech/glide/pull/5077](https://togithub.com/bumptech/glide/pull/5077)
- Allow host app to provide a way to clear all resources onStop() by
[@&#8203;osamaaftab](https://togithub.com/osamaaftab) in
[https://github.com/bumptech/glide/pull/5145](https://togithub.com/bumptech/glide/pull/5145)

##### Compose

- Add a Transition API and a CrossFade Transition for Compose by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5235](https://togithub.com/bumptech/glide/pull/5235)

- Influence layout using intrinsics in GlideNode by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5240](https://togithub.com/bumptech/glide/pull/5240)
\* Log instead of throwing parsing manifests to fix compose previews by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5167](https://togithub.com/bumptech/glide/pull/5167)

- Launch no more than one request per onRemembered by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5062](https://togithub.com/bumptech/glide/pull/5062)

- Remove GlidePainter in favor of Modifier nodes / Flows by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5230](https://togithub.com/bumptech/glide/pull/5230)

- Replace flows in GlideSubcomposition with a listener on GlideNode by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5238](https://togithub.com/bumptech/glide/pull/5238)

##### Bugs

- Read library glide module names from Java indexes by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5052](https://togithub.com/bumptech/glide/pull/5052)
- Fix typo. anay -> any in GlideSymbolProcessor.kt. by
[@&#8203;trevorhackman](https://togithub.com/trevorhackman) in
[https://github.com/bumptech/glide/pull/5029](https://togithub.com/bumptech/glide/pull/5029)
- Include URL in error log by
[@&#8203;paulsowden](https://togithub.com/paulsowden) in
[https://github.com/bumptech/glide/pull/5164](https://togithub.com/bumptech/glide/pull/5164)
- Add `isInitialized` visible for testing method by
[@&#8203;paulsowden](https://togithub.com/paulsowden) in
[https://github.com/bumptech/glide/pull/5163](https://togithub.com/bumptech/glide/pull/5163)
- Use onIdle to avoid a race in FlowTests by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5202](https://togithub.com/bumptech/glide/pull/5202)
- Add a isEquivalentTo method to correctly check equality by
[@&#8203;mori-atsushi](https://togithub.com/mori-atsushi) in
[https://github.com/bumptech/glide/pull/5232](https://togithub.com/bumptech/glide/pull/5232)
- Add
[@&#8203;RequiresPermission](https://togithub.com/RequiresPermission) to
NotificationTarget by
[@&#8203;TWiStErRob](https://togithub.com/TWiStErRob) in
[https://github.com/bumptech/glide/pull/5220](https://togithub.com/bumptech/glide/pull/5220)

##### Deprecations

- `placeholderOf(@&#8203;Composable)` in `GlideImage` is deprecated, use
`GlideSubcomposition` instead. Keep in mind that using either forces a
recomposition each time the state of the image load changes.
Recomposition will have a significant performance penalty in scrolling
lists and should be avoided.

##### Behavior Changes

- Hard code disabling hardware bitmaps on O/OMR1. by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5115](https://togithub.com/bumptech/glide/pull/5115)
- Do not set requireOriginal on Android photo picker uris. by
[@&#8203;phoenixli](https://togithub.com/phoenixli) in
[https://github.com/bumptech/glide/pull/5162](https://togithub.com/bumptech/glide/pull/5162)

##### Breaking Changes

##### Build Changes

- Add integration tests for ksp library modules. by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5054](https://togithub.com/bumptech/glide/pull/5054)
- Update README.md to use https by
[@&#8203;simoarpe](https://togithub.com/simoarpe) in
[https://github.com/bumptech/glide/pull/5058](https://togithub.com/bumptech/glide/pull/5058)
- Use dokka to build scripts/update_javadocs.sh by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5104](https://togithub.com/bumptech/glide/pull/5104)
- avif integration: Update libavif dependency by
[@&#8203;vigneshvg](https://togithub.com/vigneshvg) in
[https://github.com/bumptech/glide/pull/5128](https://togithub.com/bumptech/glide/pull/5128)
- Disable java 7 source obsolete warning. by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5168](https://togithub.com/bumptech/glide/pull/5168)
- Update mockito version to fix j16 compilation. by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5169](https://togithub.com/bumptech/glide/pull/5169)
- Switch Glide's dependencies to a version catalog. by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5183](https://togithub.com/bumptech/glide/pull/5183)
- Remove jetifier by [@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5184](https://togithub.com/bumptech/glide/pull/5184)
- Add an updated proguard plugin to compile on Java 17. by
[@&#8203;sjudd](https://togithub.com/sjudd) in
[https://github.com/bumptech/glide/pull/5185](https://togithub.com/bumptech/glide/pull/5185)
- Configure Renovate in
[https://github.com/bumptech/glide/pull/5186](https://togithub.com/bumptech/glide/pull/5186)
- Increment ROBOLECTRIC_SDK to 19 from 18. by
[@&#8203;brettchabot](https://togithub.com/brettchabot) in
[https://github.com/bumptech/glide/pull/5208](https://togithub.com/bumptech/glide/pull/5208)
and
[https://github.com/bumptech/glide/pull/5207](https://togithub.com/bumptech/glide/pull/5207)
- AGP: Upgrade AndroidManifest.xml's package to build.gradle's
namespace. by [@&#8203;TWiStErRob](https://togithub.com/TWiStErRob) in
[https://github.com/bumptech/glide/pull/5221](https://togithub.com/bumptech/glide/pull/5221)

##### New Contributors

- [@&#8203;trevorhackman](https://togithub.com/trevorhackman) made their
first contribution in
[https://github.com/bumptech/glide/pull/5029](https://togithub.com/bumptech/glide/pull/5029)
- [@&#8203;simoarpe](https://togithub.com/simoarpe) made their first
contribution in
[https://github.com/bumptech/glide/pull/5058](https://togithub.com/bumptech/glide/pull/5058)
- [@&#8203;paulsowden](https://togithub.com/paulsowden) made their first
contribution in
[https://github.com/bumptech/glide/pull/5164](https://togithub.com/bumptech/glide/pull/5164)
- [@&#8203;phoenixli](https://togithub.com/phoenixli) made their first
contribution in
[https://github.com/bumptech/glide/pull/5162](https://togithub.com/bumptech/glide/pull/5162)
- [@&#8203;osamaaftab](https://togithub.com/osamaaftab) made their first
contribution in
[https://github.com/bumptech/glide/pull/5145](https://togithub.com/bumptech/glide/pull/5145)
- [@&#8203;brettchabot](https://togithub.com/brettchabot) made their
first contribution in
[https://github.com/bumptech/glide/pull/5207](https://togithub.com/bumptech/glide/pull/5207)
- [@&#8203;mori-atsushi](https://togithub.com/mori-atsushi) made their
first contribution in
[https://github.com/bumptech/glide/pull/5232](https://togithub.com/bumptech/glide/pull/5232)

**Full Changelog**:
bumptech/glide@v4.15.0...v4.16.0

Note - there's been a change in the gpg key used to sign these releases.
The new public key is attached

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/pachli/pachli-android).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDcuMiIsInVwZGF0ZWRJblZlciI6IjM3LjguMSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Nik Clayton <[email protected]>
@renovate renovate bot mentioned this pull request Dec 7, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
import-ready Indicates the PR is ready to be imported to Google.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant