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

Add configuration property to to select the gradle worker isolation strategy #205

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
25 changes: 20 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ Please note that `ktfmt-gradle` relies on `ktfmt` hence the minimum supported JD

Please also note the following requirements:

* **Kotlin 1.4+**. In order to reformat Kotlin 1.4 code, you need run on **Gradle to 6.8+** (This is due to Gradle 6.7 embedding Kotlin 1.3.x - See [#12660](https://github.com/gradle/gradle/issues/12660)).
* **Kotlin 1.4+**. In order to reformat Kotlin 1.4 code, you need run on **Gradle to 6.8+** (This is due to Gradle 6.7
embedding Kotlin 1.3.x - See [#12660](https://github.com/gradle/gradle/issues/12660)).

* **Android**. `ktfmt-gradle` relies on features from **Android Gradle Plugin 4.1+**. So make sure you bump AGP before applying this plugin.
* **Android**. `ktfmt-gradle` relies on features from **Android Gradle Plugin 4.1+**. So make sure you bump AGP before
applying this plugin.

### Task

Expand Down Expand Up @@ -91,10 +93,10 @@ To enable different styles you can simply:
ktfmt {
// Dropbox style - 4 space indentation
dropboxStyle()

// Google style - 2 space indentation
googleStyle()

// KotlinLang style - 4 space indentation - From kotlinlang.org/docs/coding-conventions.html
kotlinLangStyle()
}
Expand All @@ -115,11 +117,24 @@ ktfmt {
}
```

The Ktfmt formatter can be invoked from gradle with different dependency isolation strategies. By default, there is no
isolation between ktfmt and other build dependencies. This should be sufficient for most projects. When you encounter
errors related to dependency conflicts, you can try to invoke ktfmt in a separate process with its own classpath.

```kotlin
import com.ncorti.ktfmt.gradle.GradleWorkerIsolationStrategy

ktfmt {
gradleWorkerIsolationStrategy.set(GradleWorkerIsolationStrategy.NO_ISOLATION)
Copy link
Owner

Choose a reason for hiding this comment

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

Can we add an example for PROCESS_ISOLATION also here?

}
```

## Using with a pre-commit hook 🎣

You can leverage the `--include-only` to let ktfmt-gradle run only on a specific subset of files.

To this you can register a simple task of type `KtfmtCheckTask` or `KtfmtFormatTask` in your `build.gradle.kts` as follows:
To this you can register a simple task of type `KtfmtCheckTask` or `KtfmtFormatTask` in your `build.gradle.kts` as
follows:

```kotlin
import com.ncorti.ktfmt.gradle.tasks.*
Expand Down
3 changes: 3 additions & 0 deletions example/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import com.ncorti.ktfmt.gradle.GradleWorkerIsolationStrategy

plugins {
kotlin("jvm")
id("com.ncorti.ktfmt.gradle")
Expand All @@ -7,6 +9,7 @@ plugins {

ktfmt {
kotlinLangStyle()
gradleWorkerIsolationStrategy.set(GradleWorkerIsolationStrategy.NO_ISOLATION)
}

dependencies {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.ncorti.ktfmt.gradle

import com.ncorti.ktfmt.gradle.GradleWorkerIsolationStrategy.*
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.workers.WorkQueue
import org.gradle.workers.WorkerExecutor

enum class GradleWorkerIsolationStrategy {
NO_ISOLATION,
PROCESS_ISOLATION
}

fun WorkerExecutor.getWorkQueue(
isolationStrategy: GradleWorkerIsolationStrategy,
classPath: ConfigurableFileCollection
): WorkQueue {
return when (isolationStrategy) {
NO_ISOLATION -> this.noIsolation()
PROCESS_ISOLATION -> this.processIsolation { it.classpath.from(classPath) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ abstract class KtfmtExtension {
continuationIndent.convention(DEFAULT_CONTINUATION_INDENT)
removeUnusedImports.convention(DEFAULT_REMOVE_UNUSED_IMPORTS)
debuggingPrintOpsAfterFormatting.convention(DEFAULT_DEBUGGING_PRINT_OPTS)
gradleWorkerIsolationStrategy.convention(DEFAULT_GRADLE_WORKER_ISOLATION_STRATEGY)
}

internal var ktfmtStyle = FACEBOOK
Expand Down Expand Up @@ -54,6 +55,9 @@ abstract class KtfmtExtension {
*/
abstract val debuggingPrintOpsAfterFormatting: Property<Boolean>

/** Defines the isolation strategy between the ktfmt classpath and the project classpath. */
abstract val gradleWorkerIsolationStrategy: Property<GradleWorkerIsolationStrategy>

/** Enables --dropbox-style (equivalent to set blockIndent to 4 and continuationIndent to 4). */
@Suppress("MagicNumber")
fun dropboxStyle() {
Expand Down Expand Up @@ -97,5 +101,7 @@ abstract class KtfmtExtension {
internal const val DEFAULT_CONTINUATION_INDENT: Int = 4
internal const val DEFAULT_REMOVE_UNUSED_IMPORTS: Boolean = true
internal const val DEFAULT_DEBUGGING_PRINT_OPTS: Boolean = false
internal val DEFAULT_GRADLE_WORKER_ISOLATION_STRATEGY: GradleWorkerIsolationStrategy =
GradleWorkerIsolationStrategy.NO_ISOLATION
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ abstract class KtfmtPlugin : Plugin<Project> {

project.tasks.withType(KtfmtBaseTask::class.java).configureEach {
it.ktfmtClasspath.from(ktFmt)
it.isolationStrategy.set(ktfmtExtension.gradleWorkerIsolationStrategy)
}

topLevelFormat = createTopLevelFormatTask(project)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package com.ncorti.ktfmt.gradle.tasks

import com.ncorti.ktfmt.gradle.FormattingOptionsBean
import com.ncorti.ktfmt.gradle.KtfmtExtension
import com.ncorti.ktfmt.gradle.KtfmtPlugin
import com.ncorti.ktfmt.gradle.*
import com.ncorti.ktfmt.gradle.tasks.worker.KtfmtWorkAction
import com.ncorti.ktfmt.gradle.tasks.worker.Result
import com.ncorti.ktfmt.gradle.util.d
Expand Down Expand Up @@ -56,6 +54,8 @@ internal constructor(
@get:Input
abstract val includeOnly: Property<String>

@get:Input abstract val isolationStrategy: Property<GradleWorkerIsolationStrategy>

@get:PathSensitive(PathSensitivity.RELATIVE)
@get:InputFiles
@get:IgnoreEmptyDirectories
Expand All @@ -64,8 +64,8 @@ internal constructor(

@TaskAction
internal fun taskAction() {
val workQueue =
workerExecutor.classLoaderIsolation { spec -> spec.classpath.from(ktfmtClasspath) }
val workQueue = workerExecutor.getWorkQueue(isolationStrategy.get(), ktfmtClasspath)

execute(workQueue)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import org.intellij.lang.annotations.Language
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource

internal class KtfmtCheckTaskIntegrationTest {

Expand Down Expand Up @@ -157,6 +159,45 @@ internal class KtfmtCheckTaskIntegrationTest {
assertThat(result.output).contains("Reusing configuration cache.")
}

@ParameterizedTest
@ValueSource(ints = [10, 15, 30, 50, 100, 1000])
fun `check task can check the formatting of multiple files using with default no-isolation strategy`(
n: Int
) {
repeat(n) { index ->
createTempFile(content = "val answer${index} = 42\n", fileName = "TestFile$index.kt")
}
val result =
GradleRunner.create()
.withProjectDir(tempDir)
.withPluginClasspath()
.withArguments("ktfmtCheckMain", "--info")
.forwardOutput()
.build()

assertThat(result.task(":ktfmtCheckMain")?.outcome).isEqualTo(SUCCESS)
}

@ParameterizedTest
@ValueSource(ints = [10, 15, 30, 50, 100, 1000])
fun `check task can check the formatting of multiple files using process isolation strategy`(
n: Int
) {
configureProcessIsolationStrategy()
repeat(n) { index ->
createTempFile(content = "val answer${index} = 42\n", fileName = "TestFile$index.kt")
}
val result =
GradleRunner.create()
.withProjectDir(tempDir)
.withPluginClasspath()
.withArguments("ktfmtCheckMain", "--info")
.forwardOutput()
.build()

assertThat(result.task(":ktfmtCheckMain")?.outcome).isEqualTo(SUCCESS)
}

@Test
fun `check task validates all the file with a failure`() {
createTempFile(content = "val answer = `\n", fileName = "File1.kt")
Expand Down Expand Up @@ -196,6 +237,11 @@ internal class KtfmtCheckTaskIntegrationTest {
assertThat(result.output).contains("Valid formatting for:")
}

private fun configureProcessIsolationStrategy() {
File("src/test/resources/jvmProjectProcessIsolation")
.copyRecursively(tempDir, overwrite = true)
}

private fun createTempFile(
@Language("kotlin") content: String,
fileName: String = "TestFile.kt",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import org.intellij.lang.annotations.Language
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource

internal class KtfmtFormatTaskIntegrationTest {

Expand Down Expand Up @@ -182,6 +184,43 @@ internal class KtfmtFormatTaskIntegrationTest {
assertThat(result.output).contains("Reusing configuration cache.")
}

@ParameterizedTest
@ValueSource(ints = [10, 15, 30, 50, 100, 1000])
fun `format task can format multiple files using with default no-isolation workerQueue`(
n: Int
) {
repeat(n) { index ->
createTempFile(content = "val answer${index}=42\n", fileName = "TestFile$index.kt")
}
val result =
GradleRunner.create()
.withProjectDir(tempDir)
.withPluginClasspath()
.withArguments("ktfmtFormatMain", "--info")
.forwardOutput()
.build()

assertThat(result.task(":ktfmtFormatMain")?.outcome).isEqualTo(SUCCESS)
}

@ParameterizedTest
@ValueSource(ints = [10, 15, 30, 50, 100, 1000])
fun `format task can format multiple files using process isolation strategy`(n: Int) {
configureProcessIsolationStrategy()
repeat(n) { index ->
createTempFile(content = "val answer${index}=42\n", fileName = "TestFile$index.kt")
}
val result =
GradleRunner.create()
.withProjectDir(tempDir)
.withPluginClasspath()
.withArguments("ktfmtFormatMain", "--info")
.forwardOutput()
.build()

assertThat(result.task(":ktfmtFormatMain")?.outcome).isEqualTo(SUCCESS)
}

@Test
fun `format task reformats all the file even with a failure`() {
val file1 = createTempFile(content = "val answer = `", fileName = "File1.kt")
Expand Down Expand Up @@ -242,6 +281,11 @@ internal class KtfmtFormatTaskIntegrationTest {
assertThat(result.output).contains("[ktfmt] Successfully reformatted 1 files with Ktfmt")
}

private fun configureProcessIsolationStrategy() {
File("src/test/resources/jvmProjectProcessIsolation")
.copyRecursively(tempDir, overwrite = true)
}

private fun createTempFile(
@Language("kotlin") content: String,
fileName: String = "TestFile.kt",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import com.ncorti.ktfmt.gradle.GradleWorkerIsolationStrategy

plugins {
kotlin("jvm") version "1.7.20"
id("com.ncorti.ktfmt.gradle")
}

repositories { mavenCentral() }

ktfmt {
kotlinLangStyle()
gradleWorkerIsolationStrategy.set(GradleWorkerIsolationStrategy.NO_ISOLATION)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package jvmProjectProcessIsolation

rootProject.name = ("test-fixtures")
Loading