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.

Sign up for GitHub

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

Jump to bottom

Fix compatibility with Gradle's configuration cache #2499

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Fix compatibility with Gradle's configuration cache #2499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/AbstractDokkaTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import groovy.lang.Closure
import org.gradle.api.Action
import org.gradle.api.DefaultTask
import org.gradle.api.Task
import org.gradle.api.artifacts.Configuration
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.plugins.JavaBasePlugin
import org.gradle.api.provider.ListProperty
import org.gradle.api.provider.MapProperty
Expand Down Expand Up @@ -76,10 +76,10 @@ abstract class AbstractDokkaTask : DefaultTask() {
}

@Classpath
val plugins: Configuration = project.maybeCreateDokkaPluginConfiguration(name)
val plugins: ConfigurableFileCollection = project.objects.fileCollection()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can drop explicit initialization, just make this
abstract val plugins: ConfigurableFileCollection and Gradle will initialize this for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'm aware, I just left it as it currently is to align with the rest of the parameters which still use manual initialisation.


@Classpath
val runtime: Configuration = project.maybeCreateDokkaRuntimeConfiguration(name)
val runtime: ConfigurableFileCollection = project.objects.fileCollection()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above


final override fun doFirst(action: Action<in Task>): Task = super.doFirst(action)

Expand Down
2 changes: 1 addition & 1 deletion runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/DokkaCollectorTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ abstract class DokkaCollectorTask : AbstractDokkaParentTask() {
cacheRoot = cacheRoot.getSafe(),
failOnWarning = failOnWarning.getSafe(),
offlineMode = offlineMode.getSafe(),
pluginsClasspath = plugins.resolve().toList(),
pluginsClasspath = plugins.toList(),
pluginsConfiguration = buildPluginsConfiguration(),
suppressObviousFunctions = suppressObviousFunctions.getSafe(),
suppressInheritedMembers = suppressInheritedMembers.getSafe(),
Expand Down
2 changes: 1 addition & 1 deletion runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/DokkaMultiModuleTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ abstract class DokkaMultiModuleTask : AbstractDokkaParentTask() {
pluginsConfiguration = buildPluginsConfiguration(),
failOnWarning = failOnWarning.getSafe(),
offlineMode = offlineMode.getSafe(),
pluginsClasspath = plugins.resolve().toList(),
pluginsClasspath = plugins.toList(),
modules = childDokkaTasks.map { dokkaTask ->
DokkaModuleDescriptionImpl(
name = dokkaTask.moduleName.getSafe(),
Expand Down
2 changes: 1 addition & 1 deletion runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/DokkaTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ abstract class DokkaTask : AbstractDokkaLeafTask() {
failOnWarning = failOnWarning.getSafe(),
sourceSets = unsuppressedSourceSets.build(),
pluginsConfiguration = buildPluginsConfiguration(),
pluginsClasspath = plugins.resolve().toList(),
pluginsClasspath = plugins.toList(),
suppressObviousFunctions = suppressObviousFunctions.getSafe(),
suppressInheritedMembers = suppressInheritedMembers.getSafe(),
)
Expand Down
2 changes: 1 addition & 1 deletion runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/DokkaTaskPartial.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ abstract class DokkaTaskPartial : AbstractDokkaLeafTask() {
failOnWarning = failOnWarning.getSafe(),
sourceSets = unsuppressedSourceSets.build(),
pluginsConfiguration = buildPluginsConfiguration(),
pluginsClasspath = plugins.resolve().toList(),
pluginsClasspath = plugins.toList(),
delayTemplateSubstitution = true,
suppressObviousFunctions = suppressObviousFunctions.getSafe(),
suppressInheritedMembers = suppressInheritedMembers.getSafe(),
Expand Down
5 changes: 2 additions & 3 deletions runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/dokkaBootstrapFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@

package org.jetbrains.dokka.gradle

import org.gradle.api.artifacts.Configuration
import org.gradle.api.file.FileCollection
import org.jetbrains.dokka.DokkaBootstrap
import java.net.URLClassLoader
import kotlin.reflect.KClass

fun DokkaBootstrap(configuration: Configuration, bootstrapClass: KClass<out DokkaBootstrap>): DokkaBootstrap {
val runtimeJars = configuration.resolve()
fun DokkaBootstrap(runtimeJars: FileCollection, bootstrapClass: KClass<out DokkaBootstrap>): DokkaBootstrap {
val runtimeClassLoader = URLClassLoader(
runtimeJars.map { it.toURI().toURL() }.toTypedArray(),
ClassLoader.getSystemClassLoader().parent
Expand Down
21 changes: 18 additions & 3 deletions runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/main.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import org.gradle.api.DefaultTask
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.artifacts.Dependency
import org.gradle.api.file.FileCollection
import org.gradle.kotlin.dsl.register
import org.gradle.util.GradleVersion

Expand All @@ -14,24 +15,38 @@ open class DokkaPlugin : Plugin<Project> {
}

project.setupDokkaTasks("dokkaHtml") {
runtime.from(project.configurations.getByName("dokkaHtmlRuntime"))
plugins.from(project.configurations.getByName("dokkaHtmlPlugin"))
description = "Generates documentation in 'html' format"
}

project.setupDokkaTasks(
name = "dokkaJavadoc",
multiModuleTaskSupported = false
) {
plugins.dependencies.add(project.dokkaArtifacts.javadocPlugin)
project.configurations.getByName("dokkaJavadocPlugin") {
it.dependencies.add(project.dokkaArtifacts.javadocPlugin)
}
runtime.from(project.configurations.getByName("dokkaJavadocRuntime"))
plugins.from(project.configurations.getByName("dokkaJavadocPlugin"))
description = "Generates documentation in 'javadoc' format"
}

project.setupDokkaTasks("dokkaGfm", allModulesPageAndTemplateProcessing = project.dokkaArtifacts.gfmTemplateProcessing) {
plugins.dependencies.add(project.dokkaArtifacts.gfmPlugin)
project.configurations.getByName("dokkaGfmPlugin") {
it.dependencies.add(project.dokkaArtifacts.gfmPlugin)
}
runtime.from(project.configurations.getByName("dokkaGfmRuntime"))
plugins.from(project.configurations.getByName("dokkaGfmPlugin"))
description = "Generates documentation in GitHub flavored markdown format"
}

project.setupDokkaTasks("dokkaJekyll", allModulesPageAndTemplateProcessing = project.dokkaArtifacts.jekyllTemplateProcessing) {
plugins.dependencies.add(project.dokkaArtifacts.jekyllPlugin)
project.configurations.getByName("dokkaJekyllPlugin") {
it.dependencies.add(project.dokkaArtifacts.jekyllPlugin)
}
runtime.from(project.configurations.getByName("dokkaJekyllRuntime"))
plugins.from(project.configurations.getByName("dokkaJekyllPlugin"))
description = "Generates documentation in Jekyll flavored markdown format"
}
}
Expand Down
8 changes: 5 additions & 3 deletions runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaCollectorTaskTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jetbrains.dokka.gradle

import org.gradle.kotlin.dsl.create
import org.gradle.kotlin.dsl.repositories
import org.gradle.kotlin.dsl.withType
import org.gradle.testfixtures.ProjectBuilder
import org.jetbrains.dokka.DokkaConfigurationImpl
Expand All @@ -21,8 +22,9 @@ class DokkaCollectorTaskTest {

rootProject.allprojects { project ->
project.plugins.apply("org.jetbrains.dokka")
project.tasks.withType<AbstractDokkaTask>().configureEach { task ->
task.plugins.withDependencies { dependencies -> dependencies.clear() }
project.repositories {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the context for this change? I'd leave a comment why we are adding mavenLocal here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a comment once I have guidance on what's preferred in terms of setting up the test environment with access to the required dokka dependencies and put that in place - I tried to hint at this in the PR description:

The changes are fairly simple, but because the configurations are now resolved when executing certain tests, those tests fail if the dependencies aren't available (this includes project dependencies as well as external dependencies).

The way to properly achieve this would be to publish the required artifacts to either mavenLocal() or a custom local file repository and adding that repository so they're made available to the tests.

I'm not aware of any standard pattern for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of any standard pattern for this.

Maven Local can work, but isn't great because it's possible for other dependencies to get picked up as well, and it's not easy to clean out old dependencies.

A custom local file repository has significant drawback: when publishing a SNAPSHOT version, Gradle will always publish a new artifact. This is slow, and causes the local dir to grow in size when working locally, which is annoying.

Anyway, I'm working on a Gradle Plugin for solving this problem, if you'd like to take a look (though it's still awaiting approval in the Plugin Portal). It will publish to a local directory, but I added some caching to prevent SNAPSHOT spam, and utilities for depending on other subprojects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aSemy what if we start by publishing the artifacts to MavenLocal to unblock this PR and release it in 1.8.20? Once your plugin is ready, would you be able to substitute the MavenLocal publishing with it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally fine with publishing it to Maven Local or to a custom local file repository, doesn't seem like we have a better alternative anyway, and having the configuration cache support is I think more important

So I think we can take this way for now 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah for sure, Maven Local will work, just be aware that it might cause some unexpected problems and the solution is usually 'clear Dokka artifacts from Maven Local'

mavenLocal()
mavenCentral()
}
project.tasks.withType<DokkaTask>().configureEach { task ->
task.dokkaSourceSets.configureEach { sourceSet ->
Expand Down Expand Up @@ -56,7 +58,7 @@ class DokkaCollectorTaskTest {
.map { it.sourceSets }
.reduce { acc, list -> acc + list },
pluginsClasspath = task.childDokkaTasks
.map { it.plugins.resolve().toList() }
.map { it.plugins.toList() }
.reduce { acc, mutableSet -> acc + mutableSet }
),
dokkaConfiguration
Expand Down
6 changes: 4 additions & 2 deletions ...rs/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaConfigurationJsonTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

package org.jetbrains.dokka.gradle

import org.gradle.kotlin.dsl.repositories
import org.gradle.kotlin.dsl.withType
import org.gradle.testfixtures.ProjectBuilder
import org.jetbrains.dokka.*
Expand All @@ -17,8 +18,9 @@ class DokkaConfigurationJsonTest {
val project = ProjectBuilder.builder().build()
project.plugins.apply("org.jetbrains.dokka")
val dokkaTask = project.tasks.withType<DokkaTask>().first()
dokkaTask.plugins.withDependencies { dependencies ->
dependencies.clear()
project.repositories {
mavenLocal()
mavenCentral()
}
dokkaTask.apply {
this.failOnWarning by true
Expand Down
6 changes: 4 additions & 2 deletions ...e-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaConfigurationSerializableTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.dokka.gradle

import org.gradle.kotlin.dsl.repositories
import org.gradle.kotlin.dsl.withType
import org.gradle.testfixtures.ProjectBuilder
import org.jetbrains.dokka.DokkaConfiguration
Expand All @@ -25,8 +26,9 @@ class DokkaConfigurationSerializableTest {
val project = ProjectBuilder.builder().build()
project.plugins.apply("org.jetbrains.dokka")
val dokkaTask = project.tasks.withType<DokkaTask>().first()
dokkaTask.plugins.withDependencies { dependencies ->
dependencies.clear()
project.repositories {
mavenLocal()
mavenCentral()
}
dokkaTask.apply {
this.failOnWarning by true
Expand Down
8 changes: 0 additions & 8 deletions runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaMultiModuleTaskTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ class DokkaMultiModuleTaskTest {
addChildTask(childDokkaTask)
}

init {
rootProject.allprojects { project ->
project.tasks.withType<AbstractDokkaTask>().configureEach { task ->
task.plugins.withDependencies { dependencies -> dependencies.clear() }
}
}
}

@Test
fun `child project is withing root project`() {
assertEquals(
Expand Down
10 changes: 7 additions & 3 deletions runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaPluginApplyTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package org.jetbrains.dokka.gradle

import org.gradle.api.plugins.JavaBasePlugin
import org.gradle.kotlin.dsl.repositories
import org.gradle.kotlin.dsl.withType
import org.gradle.testfixtures.ProjectBuilder
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertSame
import kotlin.test.assertTrue

class DokkaPluginApplyTest {
Expand Down Expand Up @@ -40,12 +40,16 @@ class DokkaPluginApplyTest {
fun `dokka plugin configurations extend dokkaPlugin`() {
val project = ProjectBuilder.builder().build()
project.plugins.apply("org.jetbrains.dokka")
project.repositories {
mavenLocal()
mavenCentral()
}

val dokkaPluginsConfiguration = project.maybeCreateDokkaDefaultPluginConfiguration()

project.tasks.withType<DokkaTask>().forEach { dokkaTask ->
assertSame(
dokkaTask.plugins.extendsFrom.single(), dokkaPluginsConfiguration,
assertTrue(
dokkaTask.plugins.files.containsAll(dokkaPluginsConfiguration.files),
"Expected dokka plugins configuration to extend default ${dokkaPluginsConfiguration.name} configuration"
)
}
Expand Down