Run Kotlinter in process isolation (#420)

* Run Kotlinter in classloader isolation First in a series of changes leading to a 5.0.0 release. Starting with Kotlin 2.1.0 it's no longer acceptable to run ktlint and the embedded Kotlin compiler in the main Gradle classloader. The solution is to use classloader Gradle worker isolation. Positive consequences: - Users can configure the version of ktlint - Kotlinter is compatible with any version of Kotlin with matching language specification Side effects to watch: - Performance may be slower - Potential [Gradle memory issues](gradle/gradle#18313) * Switch to process isolation and refactor settings for build failure
jeremymailen · Dec 9, 2024 · 19ef1fe · 19ef1fe
1 parent bd47834
commit 19ef1fe
Show file tree

Hide file tree

Showing 17 changed files with 185 additions and 224 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -20,7 +20,7 @@ val githubUrl = "https://github.com/jeremymailen/kotlinter-gradle"
 val webUrl = "https://github.com/jeremymailen/kotlinter-gradle"
 val projectDescription = "Lint and formatting for Kotlin using ktlint with configuration-free setup on JVM and Android projects"
 
-version = "4.5.0"
+version = "5.0.0-M1"
 group = "org.jmailen.gradle"
 description = projectDescription
 
@@ -46,10 +46,13 @@ dependencies {
     compileOnly("org.jetbrains.kotlin:kotlin-gradle-plugin")
     compileOnly(libs.android.tools.gradle)
 
-    implementation(libs.bundles.ktlint.engine)
-    implementation(libs.bundles.ktlint.reporters)
-    implementation(libs.bundles.ktlint.rulesets)
+    compileOnly(libs.bundles.ktlint.engine)
+    compileOnly(libs.bundles.ktlint.reporters)
+    compileOnly(libs.bundles.ktlint.rulesets)
 
+    testImplementation(libs.bundles.ktlint.engine)
+    testImplementation(libs.bundles.ktlint.reporters)
+    testImplementation(libs.bundles.ktlint.rulesets)
     testImplementation(libs.bundles.junit.jupiter)
     testImplementation(libs.commons.io)
     testImplementation(libs.mockito.kotlin)
@@ -67,7 +70,12 @@ tasks {
         outputs.file(propertiesFile)
 
         doLast {
-            propertiesFile.writeText("version = $projectVersion")
+            propertiesFile.writeText(
+                """
+                version = $projectVersion
+                ktlintVersion = ${libs.versions.ktlint.get()}
+                """.trimIndent()
+            )
         }
     }
 

diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/KotlinterExtension.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/KotlinterExtension.kt
@@ -1,15 +1,17 @@
 package org.jmailen.gradle.kotlinter
 
 import org.jmailen.gradle.kotlinter.support.ReporterType
+import org.jmailen.gradle.kotlinter.support.versionProperties
 
 open class KotlinterExtension {
     companion object {
-        const val DEFAULT_FAIL_BUILD_WHEN_CANNOT_AUTO_FORMAT = false
-        const val DEFAULT_IGNORE_FAILURES = false
+        const val DEFAULT_IGNORE_FORMAT_FAILURES = true
+        const val DEFAULT_IGNORE_LINT_FAILURES = false
         val DEFAULT_REPORTER = ReporterType.checkstyle.name
     }
 
-    var failBuildWhenCannotAutoFormat = DEFAULT_FAIL_BUILD_WHEN_CANNOT_AUTO_FORMAT
-    var ignoreFailures = DEFAULT_IGNORE_FAILURES
+    var ktlintVersion = versionProperties.ktlintVersion()
+    var ignoreFormatFailures = DEFAULT_IGNORE_FORMAT_FAILURES
+    var ignoreLintFailures = DEFAULT_IGNORE_LINT_FAILURES
     var reporters = arrayOf(DEFAULT_REPORTER)
 }
diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/KotlinterPlugin.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/KotlinterPlugin.kt
@@ -3,18 +3,22 @@ package org.jmailen.gradle.kotlinter
 import org.gradle.api.Plugin
 import org.gradle.api.Project
 import org.gradle.api.Task
+import org.gradle.api.artifacts.Configuration
 import org.gradle.api.file.RegularFile
 import org.gradle.api.provider.Provider
 import org.gradle.api.tasks.TaskProvider
 import org.jmailen.gradle.kotlinter.pluginapplier.AndroidSourceSetApplier
 import org.jmailen.gradle.kotlinter.pluginapplier.KotlinSourceSetApplier
 import org.jmailen.gradle.kotlinter.support.reporterFileExtension
+import org.jmailen.gradle.kotlinter.tasks.ConfigurableKtLintTask
 import org.jmailen.gradle.kotlinter.tasks.FormatTask
-import org.jmailen.gradle.kotlinter.tasks.InstallPreCommitHookTask
 import org.jmailen.gradle.kotlinter.tasks.InstallPrePushHookTask
 import org.jmailen.gradle.kotlinter.tasks.LintTask
 
 class KotlinterPlugin : Plugin<Project> {
+    companion object {
+        const val KTLINT_CONFIGURATION_NAME = "ktlint"
+    }
 
     private val extendablePlugins = mapOf(
         "org.jetbrains.kotlin.jvm" to KotlinSourceSetApplier,
@@ -35,38 +39,13 @@ class KotlinterPlugin : Plugin<Project> {
             pluginManager.withPlugin(pluginId) {
                 val lintKotlin = registerParentLintTask()
                 val formatKotlin = registerParentFormatTask()
+                val ktlintConfiguration = createKtLintConfiguration(kotlinterExtension)
+
+                registerSourceSetTasks(kotlinterExtension, sourceResolver, lintKotlin, formatKotlin)
 
-                sourceResolver.applyToAll(this) { id, resolvedSources ->
-                    val lintTaskPerSourceSet = tasks.register(
-                        "lintKotlin${id.replaceFirstChar(Char::titlecase)}",
-                        LintTask::class.java,
-                    ) { lintTask ->
-                        lintTask.source(resolvedSources)
-                        lintTask.ignoreFailures.set(provider { kotlinterExtension.ignoreFailures })
-                        lintTask.reports.set(
-                            provider {
-                                kotlinterExtension.reporters.associateWith { reporter ->
-                                    reportFile("$id-lint.${reporterFileExtension(reporter)}").get().asFile
-                                }
-                            },
-                        )
-                    }
-                    lintKotlin.configure { lintTask ->
-                        lintTask.dependsOn(lintTaskPerSourceSet)
-                    }
-
-                    val formatKotlinPerSourceSet = tasks.register(
-                        "formatKotlin${id.replaceFirstChar(Char::titlecase)}",
-                        FormatTask::class.java,
-                    ) { formatTask ->
-                        formatTask.source(resolvedSources)
-                        formatTask.failBuildWhenCannotAutoFormat.set(provider { kotlinterExtension.failBuildWhenCannotAutoFormat })
-                        formatTask.ignoreFailures.set(provider { kotlinterExtension.ignoreFailures })
-                        formatTask.report.set(reportFile("$id-format.txt"))
-                    }
-                    formatKotlin.configure { formatTask ->
-                        formatTask.dependsOn(formatKotlinPerSourceSet)
-                    }
+                // Configure all tasks including custom user tasks
+                tasks.withType(ConfigurableKtLintTask::class.java).configureEach { task ->
+                    task.ktlintClasspath.from(ktlintConfiguration)
                 }
             }
         }
@@ -84,17 +63,67 @@ class KotlinterPlugin : Plugin<Project> {
         it.description = "Formats the Kotlin source files."
     }
 
+    private fun Project.createKtLintConfiguration(kotlinterExtension: KotlinterExtension): Configuration {
+        val configuration = configurations.maybeCreate(KTLINT_CONFIGURATION_NAME).apply {
+            isCanBeResolved = true
+            isCanBeConsumed = false
+            isVisible = false
+
+            val dependencyProvider = provider {
+                // Even though we don't use CLI, it bundles all the runtime dependencies we need.
+                val ktlintVersion = kotlinterExtension.ktlintVersion
+                this@createKtLintConfiguration.dependencies.create("com.pinterest.ktlint:ktlint-cli:$ktlintVersion")
+            }
+            dependencies.addLater(dependencyProvider)
+        }
+        return configuration
+    }
+
+    private fun Project.registerSourceSetTasks(
+        kotlinterExtension: KotlinterExtension,
+        sourceResolver: SourceSetApplier,
+        parentLintTask: TaskProvider<Task>,
+        parentFormatTask: TaskProvider<Task>,
+    ) {
+        sourceResolver.applyToAll(this) { id, resolvedSources ->
+            val lintSourceSetTask = tasks.register(
+                "lintKotlin${id.replaceFirstChar(Char::titlecase)}",
+                LintTask::class.java,
+            ) { lintTask ->
+                lintTask.source(resolvedSources)
+                lintTask.ignoreLintFailures.set(provider { kotlinterExtension.ignoreLintFailures })
+                lintTask.reports.set(
+                    provider {
+                        kotlinterExtension.reporters.associateWith { reporter ->
+                            reportFile("$id-lint.${reporterFileExtension(reporter)}").get().asFile
+                        }
+                    },
+                )
+            }
+            parentLintTask.configure { lintTask ->
+                lintTask.dependsOn(lintSourceSetTask)
+            }
+
+            val formatSourceSetTask = tasks.register(
+                "formatKotlin${id.replaceFirstChar(Char::titlecase)}",
+                FormatTask::class.java,
+            ) { formatTask ->
+                formatTask.source(resolvedSources)
+                formatTask.ignoreFormatFailures.set(provider { kotlinterExtension.ignoreFormatFailures })
+                formatTask.ignoreLintFailures.set(provider { kotlinterExtension.ignoreLintFailures })
+                formatTask.report.set(reportFile("$id-format.txt"))
+            }
+            parentFormatTask.configure { formatTask ->
+                formatTask.dependsOn(formatSourceSetTask)
+            }
+        }
+    }
+
     private fun Project.registerPrePushHookTask(): TaskProvider<InstallPrePushHookTask> =
         tasks.register("installKotlinterPrePushHook", InstallPrePushHookTask::class.java) {
             it.group = "build setup"
             it.description = "Installs Kotlinter Git pre-push hook"
         }
-
-    private fun Project.registerPreCommitHookTask(): TaskProvider<InstallPreCommitHookTask> =
-        tasks.register("installKotlinterPreCommitHook", InstallPreCommitHookTask::class.java) {
-            it.group = "build setup"
-            it.description = "Installs Kotlinter Git pre-commit hook"
-        }
 }
 
 internal val String.id: String

diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/support/VersionProperties.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/support/VersionProperties.kt
@@ -2,10 +2,14 @@ package org.jmailen.gradle.kotlinter.support
 
 import java.util.Properties
 
+val versionProperties by lazy { VersionProperties() }
+
 class VersionProperties : Properties() {
     init {
         load(this.javaClass.getResourceAsStream("/version.properties"))
     }
 
     fun version(): String = getProperty("version")
+
+    fun ktlintVersion(): String = getProperty("ktlintVersion")
 }
diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt
@@ -1,21 +1,23 @@
 package org.jmailen.gradle.kotlinter.tasks
 
+import org.gradle.api.file.ConfigurableFileCollection
 import org.gradle.api.file.FileCollection
 import org.gradle.api.file.ProjectLayout
 import org.gradle.api.model.ObjectFactory
 import org.gradle.api.provider.ListProperty
 import org.gradle.api.provider.MapProperty
 import org.gradle.api.provider.Property
+import org.gradle.api.tasks.Classpath
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.InputFiles
 import org.gradle.api.tasks.PathSensitive
 import org.gradle.api.tasks.PathSensitivity
 import org.gradle.api.tasks.SourceTask
-import org.gradle.internal.exceptions.MultiCauseException
 import org.gradle.work.FileChange
 import org.gradle.work.Incremental
 import org.gradle.work.InputChanges
-import org.jmailen.gradle.kotlinter.KotlinterExtension.Companion.DEFAULT_IGNORE_FAILURES
+import org.gradle.workers.WorkerExecutionException
+import org.jmailen.gradle.kotlinter.KotlinterExtension.Companion.DEFAULT_IGNORE_LINT_FAILURES
 import org.jmailen.gradle.kotlinter.support.findApplicableEditorConfigFiles
 
 abstract class ConfigurableKtLintTask(projectLayout: ProjectLayout, objectFactory: ObjectFactory) : SourceTask() {
@@ -28,7 +30,10 @@ abstract class ConfigurableKtLintTask(projectLayout: ProjectLayout, objectFactor
     }
 
     @Input
-    open val ignoreFailures: Property<Boolean> = objectFactory.property(default = DEFAULT_IGNORE_FAILURES)
+    open val ignoreLintFailures: Property<Boolean> = objectFactory.property(default = DEFAULT_IGNORE_LINT_FAILURES)
+
+    @Classpath
+    val ktlintClasspath: ConfigurableFileCollection = objectFactory.fileCollection()
 
     protected fun getChangedEditorconfigFiles(inputChanges: InputChanges) =
         inputChanges.getFileChanges(editorconfigFiles).map(FileChange::getFile)
@@ -48,10 +53,8 @@ internal inline fun <reified K, reified V> ObjectFactory.mapProperty(default: Ma
         set(default)
     }
 
-inline fun <reified T : Throwable> Throwable.workErrorCauses(): List<Throwable> = when (this) {
-    is MultiCauseException -> this.causes.map { it.cause }
-    else -> listOf(this.cause)
-}.filter {
-    // class instance comparison doesn't work due to different classloaders
-    it?.javaClass?.canonicalName == T::class.java.canonicalName
-}.filterNotNull()
+fun WorkerExecutionException.hasRootCause(type: Class<*>): Boolean {
+    // this is lame, but serialized across worker boundaries exceptions are not comparable
+    // and recursive cause checking runs into serialized placeholder exceptions
+    return this.stackTraceToString().contains(type.canonicalName)
+}
diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt
@@ -1,6 +1,5 @@
 package org.jmailen.gradle.kotlinter.tasks
 
-import org.gradle.api.GradleException
 import org.gradle.api.file.ProjectLayout
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.model.ObjectFactory
@@ -10,10 +9,9 @@ import org.gradle.api.tasks.Optional
 import org.gradle.api.tasks.OutputFile
 import org.gradle.api.tasks.TaskAction
 import org.gradle.work.InputChanges
+import org.gradle.workers.WorkerExecutionException
 import org.gradle.workers.WorkerExecutor
-import org.jetbrains.kotlin.utils.addToStdlib.ifNotEmpty
 import org.jmailen.gradle.kotlinter.KotlinterExtension
-import org.jmailen.gradle.kotlinter.support.KotlinterError
 import org.jmailen.gradle.kotlinter.support.LintFailure
 import org.jmailen.gradle.kotlinter.tasks.format.FormatWorkerAction
 import javax.inject.Inject
@@ -32,8 +30,8 @@ open class FormatTask @Inject constructor(
     val report: RegularFileProperty = objectFactory.fileProperty()
 
     @Input
-    val failBuildWhenCannotAutoFormat: Property<Boolean> = objectFactory.property(
-        default = KotlinterExtension.DEFAULT_FAIL_BUILD_WHEN_CANNOT_AUTO_FORMAT,
+    val ignoreFormatFailures: Property<Boolean> = objectFactory.property(
+        default = KotlinterExtension.DEFAULT_IGNORE_FORMAT_FAILURES,
     )
 
     init {
@@ -42,26 +40,25 @@ open class FormatTask @Inject constructor(
 
     @TaskAction
     fun run(inputChanges: InputChanges) {
-        val result = with(workerExecutor.noIsolation()) {
-            submit(FormatWorkerAction::class.java) { p ->
-                p.name.set(name)
-                p.files.from(source)
-                p.projectDirectory.set(projectLayout.projectDirectory.asFile)
-                p.output.set(report)
-                p.changedEditorConfigFiles.from(getChangedEditorconfigFiles(inputChanges))
-            }
-            runCatching { await() }
+        val workQueue = workerExecutor.processIsolation { config ->
+            config.classpath.setFrom(ktlintClasspath)
         }
-
-        result.exceptionOrNull()?.workErrorCauses<KotlinterError>()?.ifNotEmpty {
-            forEach { logger.error(it.message, it.cause) }
-            throw GradleException("error formatting sources for $name")
+        workQueue.submit(FormatWorkerAction::class.java) { p ->
+            p.name.set(name)
+            p.files.from(source)
+            p.projectDirectory.set(projectLayout.projectDirectory.asFile)
+            p.output.set(report)
+            p.changedEditorConfigFiles.from(getChangedEditorconfigFiles(inputChanges))
         }
-
-        if (failBuildWhenCannotAutoFormat.get()) {
-            val lintFailures = result.exceptionOrNull()?.workErrorCauses<LintFailure>() ?: emptyList()
-            if (lintFailures.isNotEmpty() && !ignoreFailures.get()) {
-                throw GradleException("$name sources failed lint check")
+        try {
+            workQueue.await()
+        } catch (e: WorkerExecutionException) {
+            if (e.hasRootCause(LintFailure::class.java)) {
+                if (!ignoreFormatFailures.get()) {
+                    throw e
+                }
+            } else {
+                throw e
             }
         }
     }

diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/GitHookTasks.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/GitHookTasks.kt
@@ -5,7 +5,7 @@ import org.gradle.api.provider.Property
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.Internal
 import org.gradle.api.tasks.TaskAction
-import org.jmailen.gradle.kotlinter.support.VersionProperties
+import org.jmailen.gradle.kotlinter.support.versionProperties
 import java.io.File
 
 abstract class InstallPreCommitHookTask : InstallHookTask("pre-commit") {
@@ -111,11 +111,9 @@ abstract class InstallHookTask(@get:Internal val hookFileName: String) : Default
     }
 
     companion object {
-        private val version = VersionProperties().version()
-
         internal const val START_HOOK = "\n##### KOTLINTER HOOK START #####"
 
-        internal val hookVersion = "##### KOTLINTER $version #####"
+        internal val hookVersion = "##### KOTLINTER ${versionProperties.version()} #####"
 
         internal const val END_HOOK = "##### KOTLINTER HOOK END #####\n"