Skip to content

Commit 16306e8

Browse files
committedMar 8, 2023
Read library glide module names from Java indexes
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.
1 parent 462416b commit 16306e8

File tree

3 files changed

+82
-15
lines changed

3 files changed

+82
-15
lines changed
 

‎annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/AppGlideModules.kt

+54-13
Original file line numberDiff line numberDiff line change
@@ -191,23 +191,58 @@ internal class AppGlideModuleParser(
191191
private fun getIndexesAndLibraryGlideModuleNames(): IndexFilesAndLibraryModuleNames {
192192
val allIndexFiles: MutableList<KSDeclaration> = mutableListOf()
193193
val allLibraryGlideModuleNames: MutableList<String> = mutableListOf()
194-
resolver.getDeclarationsFromPackage(GlideSymbolProcessorConstants.PACKAGE_NAME).forEach {
195-
index: KSDeclaration ->
196-
val libraryGlideModuleNames = extractGlideModulesFromIndexAnnotation(index)
197-
if (libraryGlideModuleNames.isNotEmpty()) {
198-
allIndexFiles.add(index)
199-
allLibraryGlideModuleNames.addAll(libraryGlideModuleNames)
200-
}
194+
195+
val allIndexesAndLibraryModules =
196+
getAllLibraryNamesFromJavaIndexes() + getAllLibraryNamesFromKspIndexes()
197+
for ((index, libraryGlideModuleNames) in allIndexesAndLibraryModules) {
198+
allIndexFiles.add(index)
199+
allLibraryGlideModuleNames.addAll(libraryGlideModuleNames)
201200
}
202201

203202
return IndexFilesAndLibraryModuleNames(allIndexFiles, allLibraryGlideModuleNames)
204203
}
205204

206-
private fun extractGlideModulesFromIndexAnnotation(
205+
internal data class IndexAndLibraryModuleNames(
206+
val index: KSDeclaration, val libraryModuleNames: List<String>
207+
)
208+
209+
private fun getAllLibraryNamesFromKspIndexes(): List<IndexAndLibraryModuleNames> =
210+
getAllLibraryNamesFromIndexes(GlideSymbolProcessorConstants.PACKAGE_NAME) { index ->
211+
extractGlideModulesFromKspIndexAnnotation(index)
212+
}
213+
214+
private fun getAllLibraryNamesFromJavaIndexes(): List<IndexAndLibraryModuleNames> =
215+
getAllLibraryNamesFromIndexes(GlideSymbolProcessorConstants.JAVA_ANNOTATION_PACKAGE_NAME) {
216+
index -> extractGlideModulesFromJavaIndexAnnotation(index)
217+
}
218+
219+
@OptIn(KspExperimental::class)
220+
private fun getAllLibraryNamesFromIndexes(
221+
packageName: String, extractLibraryModuleNamesFromIndex: (KSDeclaration) -> List<String>
222+
) = buildList {
223+
resolver.getDeclarationsFromPackage(packageName)
224+
.forEach { index: KSDeclaration ->
225+
val libraryGlideModuleNames = extractLibraryModuleNamesFromIndex(index)
226+
if (libraryGlideModuleNames.isNotEmpty()) {
227+
environment.logger.info(
228+
"Found index annotation: $index with modules: $libraryGlideModuleNames"
229+
)
230+
add(IndexAndLibraryModuleNames(index, libraryGlideModuleNames))
231+
}
232+
}
233+
}
234+
235+
private fun extractGlideModulesFromJavaIndexAnnotation(
236+
index: KSDeclaration,
237+
): List<String> {
238+
val indexAnnotation: KSAnnotation = index.atMostOneJavaIndexAnnotation() ?: return emptyList()
239+
return indexAnnotation.getModuleArgumentValues().toList()
240+
}
241+
242+
private fun extractGlideModulesFromKspIndexAnnotation(
207243
index: KSDeclaration,
208244
): List<String> {
209-
val indexAnnotation: KSAnnotation = index.atMostOneIndexAnnotation() ?: return emptyList()
210-
environment.logger.info("Found index annotation: $indexAnnotation")
245+
val indexAnnotation: KSAnnotation = index.atMostOneKspIndexAnnotation() ?: return emptyList()
211246
return indexAnnotation.getModuleArgumentValues().toList()
212247
}
213248

@@ -220,25 +255,31 @@ internal class AppGlideModuleParser(
220255
throw InvalidGlideSourceException("Found an invalid internal Glide index: $this")
221256
}
222257

223-
private fun KSDeclaration.atMostOneIndexAnnotation() = atMostOneAnnotation(Index::class)
258+
private fun KSDeclaration.atMostOneJavaIndexAnnotation() =
259+
atMostOneAnnotation("com.bumptech.glide.annotation.compiler.Index")
260+
private fun KSDeclaration.atMostOneKspIndexAnnotation() = atMostOneAnnotation(Index::class)
224261

225262
private fun KSDeclaration.atMostOneExcludesAnnotation() = atMostOneAnnotation(Excludes::class)
226263

227264
private fun KSDeclaration.atMostOneAnnotation(
228265
annotation: KClass<out Annotation>,
266+
): KSAnnotation? = atMostOneAnnotation(annotation.qualifiedName)
267+
268+
private fun KSDeclaration.atMostOneAnnotation(
269+
annotationQualifiedName: String?,
229270
): KSAnnotation? {
230271
val matchingAnnotations: List<KSAnnotation> =
231272
annotations
232273
.filter {
233-
annotation.qualifiedName?.equals(
274+
annotationQualifiedName?.equals(
234275
it.annotationType.resolve().declaration.qualifiedName?.asString()
235276
)
236277
?: false
237278
}
238279
.toList()
239280
if (matchingAnnotations.size > 1) {
240281
throw InvalidGlideSourceException(
241-
"""Expected 0 or 1 $annotation annotations on $qualifiedName, but found:
282+
"""Expected 0 or 1 $annotationQualifiedName annotations on $qualifiedName, but found:
242283
${matchingAnnotations.size}"""
243284
)
244285
}

‎annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/GlideSymbolProcessor.kt

+1
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ object GlideSymbolProcessorConstants {
144144
// This variable is visible only for testing
145145
// TODO(b/174783094): Add @VisibleForTesting when internal is supported.
146146
val PACKAGE_NAME: String = GlideSymbolProcessor::class.java.`package`.name
147+
val JAVA_ANNOTATION_PACKAGE_NAME: String = "com.bumptech.glide.annotation.compiler"
147148
const val SINGLE_APP_MODULE_ERROR = "You can have at most one AppGlideModule, but found: %s"
148149
const val DUPLICATE_LIBRARY_MODULE_ERROR =
149150
"LibraryGlideModules %s are included more than once, keeping only one!"

‎glide/build.gradle

+27-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
import com.android.build.gradle.api.LibraryVariant
22

3+
/**
4+
* This module is used for two things:
5+
* <ul>
6+
* <li>Compiling a single unified set of javadocs for Glide
7+
* <li>Providing a jar version of Glide for internal libraries, like
8+
* Glide's annotation processor.
9+
* </ul>
10+
*
11+
* <p>Previously this module was used to produce a release jar for Glide, but
12+
* we've long since stopped releasing the jar. Now all release artifacts come
13+
* from the upload script, which uploads aars for each production submodule
14+
*/
15+
316
apply plugin: 'java'
417

518
// The paths of Android projects that should be included only in Javadoc, not in the jar.
@@ -18,6 +31,10 @@ static def getAndroidPathsForJavadoc() {
1831
]
1932
}
2033

34+
static def getAndroidPathsForJar() {
35+
[':library', ':third_party:disklrucache', ':third_party:gif_decoder']
36+
}
37+
2138
// The paths of Java projects that should be included only in Javadoc, not in the jar.
2239
static def getJavaPathsForJavadoc() {
2340
[':annotation']
@@ -43,8 +60,16 @@ def getAndroidProjectsForJavadoc() {
4360
asProjects(getAndroidPathsForJavadoc())
4461
}
4562

63+
def getAndroidLibraryVariantsForJar() {
64+
getAndroidLibraryVariantsForProjects(asProjects(getAndroidPathsForJar()))
65+
}
66+
4667
def getAndroidLibraryVariantsForJavadoc() {
47-
getAndroidProjectsForJavadoc().collect { project ->
68+
getAndroidLibraryVariantsForProjects(getAndroidProjectsForJavadoc())
69+
}
70+
71+
def getAndroidLibraryVariantsForProjects(projects) {
72+
projects.collect { project ->
4873
project.android.libraryVariants.findAll { type ->
4974
type.buildType.name.equalsIgnoreCase("release")
5075
}
@@ -114,7 +139,7 @@ javadocJarTask.dependsOn(javadocTask)
114139

115140
jar {
116141
from files(
117-
getAndroidLibraryVariantsForJavadoc().collect { LibraryVariant variant ->
142+
getAndroidLibraryVariantsForJar().collect { LibraryVariant variant ->
118143
variant.getJavaCompileProvider().get().destinationDirectory
119144
}
120145
)

0 commit comments

Comments
 (0)
Please sign in to comment.