Skip to content

Commit 55d23db

Browse files
committed
DPG: Only apply 'Adapter' plugins once, and group PluginIds
The AndroidAdapter, JavaAdapter, and KotlinAdapters apply their configuration multiple times. Currently, this is acceptable because a project cannot have multiple Android/Kotlin/Java plugins applied. But the situation gets more complicated with AGP9 and kotlin-built-in. The kotlin-built-in plugin uses an implementation of `KotlinBasePlugin`, so the Android and Kotlin adapters could get applied multiple times, and some of their operations (registering DokkaSourceSets) aren't idempotent. I also updated the JavaAdapter to use the same approach for consistency (it's not strictly necessary). --- Also, group the plugin IDs into Sets, so they can be accessed at once without needing to remember which IDs should be accessed.
1 parent e78813c commit 55d23db

File tree

6 files changed

+140
-57
lines changed

6 files changed

+140
-57
lines changed

dokka-runners/dokka-gradle-plugin/src/main/kotlin/adapters/AndroidAdapter.kt

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import org.gradle.api.logging.Logging
1616
import org.gradle.api.model.ObjectFactory
1717
import org.gradle.api.provider.Provider
1818
import org.gradle.api.provider.ProviderFactory
19+
import org.gradle.kotlin.dsl.apply
1920
import org.gradle.kotlin.dsl.getByType
2021
import org.gradle.kotlin.dsl.withType
2122
import org.jetbrains.dokka.gradle.DokkaBasePlugin
@@ -41,16 +42,6 @@ abstract class AndroidAdapter @Inject constructor(
4142
override fun apply(project: Project) {
4243
logger.info("applied ${this::class} to ${project.path}")
4344

44-
project.plugins.withType<DokkaBasePlugin>().configureEach {
45-
project.pluginManager.apply {
46-
withPlugin(PluginId.AndroidBase) { configure(project) }
47-
withPlugin(PluginId.AndroidApplication) { configure(project) }
48-
withPlugin(PluginId.AndroidLibrary) { configure(project) }
49-
}
50-
}
51-
}
52-
53-
protected fun configure(project: Project) {
5445
val dokkaExtension = project.extensions.getByType<DokkaExtension>()
5546

5647
val androidExt = AndroidExtensionWrapper(project) ?: return
@@ -79,7 +70,21 @@ abstract class AndroidAdapter @Inject constructor(
7970
}
8071

8172
@InternalDokkaGradlePluginApi
82-
companion object
73+
companion object {
74+
75+
/**
76+
* Apply [AndroidAdapter] a single time to [project], regardless of how many AGP plugins are applied.
77+
*/
78+
internal fun applyTo(project: Project) {
79+
project.plugins.withType<DokkaBasePlugin>().all {
80+
PluginId.androidPlugins.forEach { pluginId ->
81+
project.pluginManager.withPlugin(pluginId) {
82+
project.pluginManager.apply(AndroidAdapter::class)
83+
}
84+
}
85+
}
86+
}
87+
}
8388
}
8489

8590

dokka-runners/dokka-gradle-plugin/src/main/kotlin/adapters/JavaAdapter.kt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import org.gradle.api.tasks.SourceSet
1515
import org.gradle.api.tasks.SourceSet.MAIN_SOURCE_SET_NAME
1616
import org.gradle.api.tasks.SourceSet.TEST_SOURCE_SET_NAME
1717
import org.gradle.api.tasks.SourceSetContainer
18+
import org.gradle.kotlin.dsl.apply
1819
import org.gradle.kotlin.dsl.getByType
1920
import org.gradle.kotlin.dsl.withType
2021
import org.jetbrains.dokka.gradle.DokkaExtension
@@ -118,16 +119,11 @@ abstract class JavaAdapter @Inject constructor(
118119
): Provider<Boolean> {
119120

120121
val projectHasKotlinPlugin = providers.provider {
121-
project.pluginManager.hasPlugin(PluginId.KotlinAndroid)
122-
|| project.pluginManager.hasPlugin(PluginId.KotlinJs)
123-
|| project.pluginManager.hasPlugin(PluginId.KotlinJvm)
124-
|| project.pluginManager.hasPlugin(PluginId.KotlinMultiplatform)
122+
PluginId.kgpPlugins.any { project.pluginManager.hasPlugin(it) }
125123
}
126124

127125
val projectHasAndroidPlugin = providers.provider {
128-
project.pluginManager.hasPlugin(PluginId.AndroidBase)
129-
|| project.pluginManager.hasPlugin(PluginId.AndroidApplication)
130-
|| project.pluginManager.hasPlugin(PluginId.AndroidLibrary)
126+
PluginId.androidPlugins.any { project.pluginManager.hasPlugin(it) }
131127
}
132128

133129
return projectHasKotlinPlugin or projectHasAndroidPlugin
@@ -147,5 +143,9 @@ abstract class JavaAdapter @Inject constructor(
147143
fun SourceSet.isPublished(): Boolean =
148144
name != TEST_SOURCE_SET_NAME
149145
&& name.startsWith(MAIN_SOURCE_SET_NAME)
146+
147+
internal fun applyTo(project: Project) {
148+
project.pluginManager.apply(type = JavaAdapter::class)
149+
}
150150
}
151151
}

dokka-runners/dokka-gradle-plugin/src/main/kotlin/adapters/KotlinAdapter.kt

Lines changed: 76 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@ package org.jetbrains.dokka.gradle.adapters
55

66
import com.android.build.api.variant.AndroidComponentsExtension
77
import com.android.build.api.variant.Variant
8-
import org.gradle.api.Named
9-
import org.gradle.api.NamedDomainObjectContainer
10-
import org.gradle.api.Plugin
11-
import org.gradle.api.Project
8+
import org.gradle.api.*
129
import org.gradle.api.file.ConfigurableFileCollection
1310
import org.gradle.api.file.FileCollection
1411
import org.gradle.api.logging.Logger
@@ -33,12 +30,9 @@ import org.jetbrains.kotlin.commonizer.stdlib
3330
import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension
3431
import org.jetbrains.kotlin.gradle.dsl.KotlinProjectExtension
3532
import org.jetbrains.kotlin.gradle.dsl.KotlinSingleTargetExtension
36-
import org.jetbrains.kotlin.gradle.plugin.KotlinCompilation
33+
import org.jetbrains.kotlin.gradle.plugin.*
3734
import org.jetbrains.kotlin.gradle.plugin.KotlinCompilation.Companion.MAIN_COMPILATION_NAME
38-
import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType
3935
import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType.androidJvm
40-
import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet
41-
import org.jetbrains.kotlin.gradle.plugin.getKotlinPluginVersion
4236
import org.jetbrains.kotlin.gradle.plugin.mpp.AbstractKotlinNativeCompilation
4337
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinJvmAndroidCompilation
4438
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinMetadataCompilation
@@ -62,17 +56,6 @@ abstract class KotlinAdapter @Inject constructor(
6256
override fun apply(project: Project) {
6357
logger.info("Applying $dkaName to ${project.path}")
6458

65-
project.plugins.withType<DokkaBasePlugin>().configureEach {
66-
project.pluginManager.apply {
67-
withPlugin(PluginId.KotlinAndroid) { exec(project) }
68-
withPlugin(PluginId.KotlinJs) { exec(project) }
69-
withPlugin(PluginId.KotlinJvm) { exec(project) }
70-
withPlugin(PluginId.KotlinMultiplatform) { exec(project) }
71-
}
72-
}
73-
}
74-
75-
private fun exec(project: Project) {
7659
val kotlinExtension = project.findKotlinExtension()
7760
if (kotlinExtension == null) {
7861
logger.info("Skipping applying $dkaName in ${project.path} - could not find KotlinProjectExtension")
@@ -210,6 +193,76 @@ abstract class KotlinAdapter @Inject constructor(
210193
val kgpVersion = getKotlinPluginVersion(logger)
211194
KotlinToolingVersion(kgpVersion)
212195
}
196+
197+
/**
198+
* Applies [KotlinAdapter] to the current project when any plugin of type [KotlinBasePlugin]
199+
* is applied.
200+
*
201+
* [KotlinBasePlugin] is the parent type for the Kotlin/JVM, Kotlin/Multiplatform, Kotlin/JS plugins,
202+
* as well as AGP's kotlin-built-in plugin.
203+
*/
204+
internal fun applyTo(project: Project) {
205+
findKotlinBasePlugins(project)?.all {
206+
project.pluginManager.apply(KotlinAdapter::class)
207+
}
208+
}
209+
210+
/**
211+
* Tries fetching all plugins with type [KotlinBasePlugin],
212+
* returning `null` if the class is not available in the current classloader.
213+
*
214+
* (The class might not be available if the current project is a Java or Android project,
215+
* or the buildscripts have an inconsistent classpath https://github.com/gradle/gradle/issues/27218)
216+
*/
217+
private fun findKotlinBasePlugins(project: Project): DomainObjectCollection<KotlinBasePlugin>? {
218+
return try {
219+
project.plugins.withType<KotlinBasePlugin>()
220+
} catch (ex: Throwable) {
221+
when (ex) {
222+
is ClassNotFoundException,
223+
is NoClassDefFoundError -> {
224+
logger.info("Dokka Gradle Plugin could not load KotlinBasePlugin in ${project.displayName} - ${ex::class.qualifiedName} ${ex.message}")
225+
logWarningIfKgpApplied(
226+
project,
227+
kotlinBasePluginNotFoundException = ex,
228+
)
229+
null
230+
}
231+
232+
else -> throw ex
233+
}
234+
}
235+
}
236+
237+
/**
238+
* Check all plugins to see if they are a subtype of [KotlinBasePlugin].
239+
* If any are, log a warning.
240+
*
241+
* ##### Motivation
242+
*
243+
* If the buildscript classpath is inconsistent, it might not be possible for DGP
244+
* to react to KGP because the [KotlinBasePlugin] class can't be loaded.
245+
* If so, DGP will be lenient and not cause errors,
246+
* but it must display a prominent warning to help users find the problem.
247+
*
248+
* @param[kotlinBasePluginNotFoundException] The exception thrown when [KotlinBasePlugin] is not available.
249+
*/
250+
private fun logWarningIfKgpApplied(
251+
project: Project,
252+
kotlinBasePluginNotFoundException: Throwable,
253+
) {
254+
PluginId.kgpPlugins.forEach { pluginId ->
255+
project.pluginManager.withPlugin(pluginId) {
256+
logger.warn(
257+
buildString {
258+
appendLine("Dokka Gradle Plugin could not load KotlinBasePlugin in ${project.displayName}, but plugin $id is applied.")
259+
appendLine("Check the buildscript classpath is consistent (all plugins are ")
260+
},
261+
kotlinBasePluginNotFoundException,
262+
)
263+
}
264+
}
265+
}
213266
}
214267
}
215268

@@ -298,10 +351,10 @@ private class KotlinCompilationDetailsBuilder(
298351
): Provider<Set<AndroidVariantInfo>> {
299352
val androidVariants = objects.setProperty(AndroidVariantInfo::class)
300353

301-
project.pluginManager.apply {
302-
withPlugin(PluginId.AndroidBase) { collectAndroidVariants(project, androidVariants) }
303-
withPlugin(PluginId.AndroidApplication) { collectAndroidVariants(project, androidVariants) }
304-
withPlugin(PluginId.AndroidLibrary) { collectAndroidVariants(project, androidVariants) }
354+
PluginId.androidPlugins.forEach { pluginId ->
355+
project.pluginManager.withPlugin(pluginId) {
356+
collectAndroidVariants(project, androidVariants)
357+
}
305358
}
306359

307360
return androidVariants

dokka-runners/dokka-gradle-plugin/src/main/kotlin/formats/DokkaFormatPlugin.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ abstract class DokkaFormatPlugin(
6565
target.pluginManager.apply(DokkaBasePlugin::class)
6666

6767
// apply the plugin that will autoconfigure Dokka to use the sources of a Kotlin project
68-
target.pluginManager.apply(type = KotlinAdapter::class)
69-
target.pluginManager.apply(type = JavaAdapter::class)
70-
target.pluginManager.apply(type = AndroidAdapter::class)
68+
KotlinAdapter.applyTo(target)
69+
AndroidAdapter.applyTo(target)
70+
JavaAdapter.applyTo(target)
7171

7272
target.plugins.withType<DokkaBasePlugin>().configureEach {
7373
val dokkaExtension = target.extensions.getByType(DokkaExtension::class)

dokka-runners/dokka-gradle-plugin/src/main/kotlin/internal/PluginId.kt

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,23 @@ package org.jetbrains.dokka.gradle.internal
99
@Suppress("ConstPropertyName")
1010
internal object PluginId {
1111

12-
const val KotlinAndroid = "org.jetbrains.kotlin.android"
13-
const val KotlinJs = "org.jetbrains.kotlin.js"
14-
const val KotlinJvm = "org.jetbrains.kotlin.jvm"
15-
const val KotlinMultiplatform = "org.jetbrains.kotlin.multiplatform"
12+
private const val KotlinAndroid = "org.jetbrains.kotlin.android"
13+
private const val KotlinJs = "org.jetbrains.kotlin.js"
14+
private const val KotlinJvm = "org.jetbrains.kotlin.jvm"
15+
private const val KotlinMultiplatform = "org.jetbrains.kotlin.multiplatform"
16+
val kgpPlugins: Set<String> = setOf(
17+
KotlinAndroid,
18+
KotlinJs,
19+
KotlinJvm,
20+
KotlinMultiplatform,
21+
)
1622

17-
const val AndroidBase = "com.android.base"
18-
const val AndroidApplication = "com.android.application"
19-
const val AndroidLibrary = "com.android.library"
23+
private const val AndroidBase = "com.android.base"
24+
private const val AndroidApplication = "com.android.application"
25+
private const val AndroidLibrary = "com.android.library"
26+
val androidPlugins: Set<String> = setOf(
27+
AndroidBase,
28+
AndroidApplication,
29+
AndroidLibrary,
30+
)
2031
}

dokka-runners/dokka-gradle-plugin/src/testFunctional/kotlin/MultiModuleFunctionalTest.kt

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -518,19 +518,33 @@ class MultiModuleFunctionalTest : FunSpec({
518518
}
519519
}
520520

521-
test("expect warning regarding KotlinProjectExtension") {
521+
test("expect KotlinAdapter not applied to root project") {
522522
project.runner
523523
.addArguments(
524524
"clean",
525525
"--stacktrace",
526+
"--info",
526527
)
527-
.forwardOutput()
528528
.build {
529-
// the root project doesn't have the KGP applied, so KotlinProjectExtension shouldn't be applied
530-
output shouldNotContain "Dokka Gradle plugin failed to get extension kotlin org.jetbrains.kotlin.gradle.dsl.KotlinJvmProjectExtension in :\n"
529+
// the root project doesn't have KGP applied, so KotlinAdapter shouldn't be applied
530+
output shouldContain "Dokka Gradle Plugin could not load KotlinBasePlugin in root project 'kpe-warning'"
531+
output shouldNotContain "Applying KotlinAdapter to :\n"
532+
}
533+
}
531534

532-
output shouldContain "Dokka Gradle plugin failed to get extension kotlin org.jetbrains.kotlin.gradle.dsl.KotlinJvmProjectExtension in :subproject-hello\n"
533-
output shouldContain "Dokka Gradle plugin failed to get extension kotlin org.jetbrains.kotlin.gradle.dsl.KotlinJvmProjectExtension in :subproject-goodbye\n"
535+
test("expect KotlinAdapter applied to subprojects, with KotlinProjectExtension warnings") {
536+
project.runner
537+
.addArguments(
538+
"clean",
539+
"--stacktrace",
540+
"--warn",
541+
)
542+
.build {
543+
// the subprojects should have KotlinAdapter applied, but the extension should be unavailable
544+
// because the buildscript classpath is inconsistent.
545+
// (DGP is applied to the root project, but KGP is not.)
546+
output shouldContain "Dokka Gradle Plugin could not load KotlinBasePlugin in project ':subproject-hello', but plugin org.jetbrains.kotlin.jvm is applied"
547+
output shouldContain "Dokka Gradle Plugin could not load KotlinBasePlugin in project ':subproject-goodbye', but plugin org.jetbrains.kotlin.jvm is applied"
534548
}
535549
}
536550
}

0 commit comments

Comments
 (0)