From e7a08b14e38632fa4421e9ef579875ce6cf1a922 Mon Sep 17 00:00:00 2001 From: Yahor Berdnikau Date: Thu, 12 Aug 2021 12:54:45 +0200 Subject: [PATCH] Fix 'jvmTarget' is not updated on reusing configuration cache. Before this change 'kotlinOptions.jvmTarget' was updated on 'providedJvm' property value calculation, but, as configuration cache reuses property value from the cache - 'jvmTarget' update does not happen. This change adds additional call to set 'jvmTarget' on task execution phase to ensure even on configuration cache reuse field is set to correct value. Update is based whether 'providedJvm' property is set or not. ^KT-48226 Fixed --- .../kotlin/gradle/KotlinJavaToolchainTest.kt | 83 +++++++++++++++++++ .../tasks/DefaultKotlinJavaToolchain.kt | 78 +++++++++++++---- .../jetbrains/kotlin/gradle/tasks/Tasks.kt | 5 ++ 3 files changed, 148 insertions(+), 18 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinJavaToolchainTest.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinJavaToolchainTest.kt index e06822807be..5960e5acc8b 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinJavaToolchainTest.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinJavaToolchainTest.kt @@ -101,6 +101,40 @@ class KotlinJavaToolchainTest : KGPBaseTest() { } } + @DisplayName("Kotlin compile task should reuse build cache when toolchain is set and build is happening on different JDKs") + @GradleTestVersions(minVersion = "6.7.1") + @GradleTest + internal fun differentBuildJDKBuildCacheHit(gradleVersion: GradleVersion) { + val buildCache = workingDir.resolve("custom-jdk-build-cache") + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + projectPathAdditionalSuffix = "1/cache-test", + buildOptions = defaultBuildOptions.copy(buildCacheEnabled = true), + buildJdk = getJdk9().javaHome!! + ) { + enableLocalBuildCache(buildCache) + useToolchainExtension(11) + + build("assemble", forceOutput = true, enableBuildCacheDebug = true) + } + + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + projectPathAdditionalSuffix = "2/cache-test", + buildOptions = defaultBuildOptions.copy(buildCacheEnabled = true), + buildJdk = getJdk11().javaHome!! + ) { + enableLocalBuildCache(buildCache) + useToolchainExtension(11) + + build("assemble", enableBuildCacheDebug = true) { + assertTasksFromCache(":compileKotlin") + } + } + } + @GradleTest @DisplayName("Kotlin compile task should not use build cache on using different JDK versions") internal fun differentJdkBuildCacheMiss(gradleVersion: GradleVersion) { @@ -686,6 +720,55 @@ class KotlinJavaToolchainTest : KGPBaseTest() { } } + @DisplayName("Kotlin toolchain should support configuration cache") + @GradleTestVersions(minVersion = "6.7.1") + @GradleTest + internal fun testConfigurationCache(gradleVersion: GradleVersion) { + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + buildOptions = defaultBuildOptions.copy( + configurationCache = true, + configurationCacheProblems = BaseGradleIT.ConfigurationCacheProblems.FAIL + ) + ) { + useToolchainExtension(15) + + //language=properties + gradleProperties.append( + """ + kotlin.jvm.target.validation.mode = error + """.trimIndent() + ) + + build("assemble") + build("assemble") + } + } + + @DisplayName("Should work with configuration cache when toolchain is not configured") + @GradleTest + internal fun testConfigurationCacheNoToolchain(gradleVersion: GradleVersion) { + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + buildOptions = defaultBuildOptions.copy( + configurationCache = true, + configurationCacheProblems = BaseGradleIT.ConfigurationCacheProblems.FAIL + ) + ) { + //language=properties + gradleProperties.append( + """ + kotlin.jvm.target.validation.mode = error + """.trimIndent() + ) + + build("assemble") + build("assemble") + } + } + private fun BuildResult.assertJdkHomeIsUsingJdk( javaexecPath: String ) = assertOutputContains("[KOTLIN] Kotlin compilation 'jdkHome' argument: $javaexecPath") diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/DefaultKotlinJavaToolchain.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/DefaultKotlinJavaToolchain.kt index faaf82c404e..daf58205be3 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/DefaultKotlinJavaToolchain.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/DefaultKotlinJavaToolchain.kt @@ -17,11 +17,13 @@ import org.gradle.api.tasks.Internal import org.gradle.internal.jvm.Jvm import org.gradle.jvm.toolchain.* import org.gradle.util.GradleVersion +import org.jetbrains.kotlin.cli.common.arguments.K2JVMCompilerArguments import org.jetbrains.kotlin.gradle.dsl.KotlinJvmOptionsImpl import org.jetbrains.kotlin.gradle.tasks.KotlinJavaToolchain.Companion.TOOLCHAIN_SUPPORTED_VERSION import org.jetbrains.kotlin.gradle.utils.chainedFinalizeValueOnRead import org.jetbrains.kotlin.gradle.utils.property import org.jetbrains.kotlin.gradle.utils.propertyWithConvention +import org.jetbrains.kotlin.gradle.utils.providerWithLazyConvention import java.io.File import javax.inject.Inject import kotlin.reflect.full.functions @@ -112,6 +114,7 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( final override val jdk: KotlinJavaToolchain.JdkSetter = DefaultJdkSetter( providedJvm, currentGradleVersion, + objects, kotlinCompileTaskProvider ) @@ -127,6 +130,43 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( get() = defaultJavaToolchainSetter ?: throw GradleException("Toolchain support is available from $TOOLCHAIN_SUPPORTED_VERSION") + /** + * Updates [task] 'jvmTarget' if user has configured toolchain and not 'jvmTarget'. + * + * Should be called on execution state to ensure 'jvmTarget' is set to correct value even on + * reusing configuration cache. + * + * Should be called on task execution phase! + */ + internal fun updateJvmTarget( + task: KotlinCompile, + args: K2JVMCompilerArguments + ) { + check(task.state.executing) { + "\"updateJvmTarget()\" method should be called only on task execution!" + } + + if (providedJvm.isPresent) { + val jdkVersion = javaVersion.get() + + // parentKotlinOptionsImpl is set from 'kotlin-android' plugin + val appliedJvmTargets = listOfNotNull(task.kotlinOptions, task.parentKotlinOptionsImpl.orNull) + .mapNotNull { (it as KotlinJvmOptionsImpl).jvmTargetField } + + if (appliedJvmTargets.isEmpty()) { + // For Java 9 and Java 10 JavaVersion returns "1.9" or "1.10" accordingly + // that is not accepted by Kotlin compiler + val toolchainJvmTarget = when (jdkVersion) { + JavaVersion.VERSION_1_9 -> "9" + JavaVersion.VERSION_1_10 -> "10" + else -> jdkVersion.toString() + } + task.kotlinOptions.jvmTarget = toolchainJvmTarget + args.jvmTarget = toolchainJvmTarget + } + } + } + private abstract class JvmTargetUpdater( private val kotlinCompileTaskProvider: () -> KotlinCompile? ) { @@ -139,11 +179,12 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( .mapNotNull { (it as KotlinJvmOptionsImpl).jvmTargetField } if (appliedJvmTargets.isEmpty()) { - // For Java 9 JavaVersion returns "1.9" that is not accepted by Kotlin compiler - task.kotlinOptions.jvmTarget = if (jdkVersion == JavaVersion.VERSION_1_9) { - "9" - } else { - jdkVersion.toString() + // For Java 9 and Java 10 JavaVersion returns "1.9" or "1.10" accordingly + // that is not accepted by Kotlin compiler + task.kotlinOptions.jvmTarget = when (jdkVersion) { + JavaVersion.VERSION_1_9 -> "9" + JavaVersion.VERSION_1_10 -> "10" + else -> jdkVersion.toString() } } } @@ -153,6 +194,7 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( private class DefaultJdkSetter( private val providedJvm: Property, private val currentGradleVersion: GradleVersion, + private val objects: ObjectFactory, kotlinCompileTaskProvider: () -> KotlinCompile? ) : JvmTargetUpdater(kotlinCompileTaskProvider), KotlinJavaToolchain.JdkSetter { @@ -168,19 +210,19 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( "Supplied jdkHomeLocation does not exists. You supplied: $jdkHomeLocation" } - if (currentGradleVersion < GradleVersion.version("6.2.0")) { - // Before Gradle 6.2.0 'Jvm.discovered' does not have 'implementationJavaVersion' parameter - val jvm = Jvm::class.functions - .first { it.name == "discovered" } - .call(jdkHomeLocation, jdkVersion) as Jvm - providedJvm.set(jvm) - } else { - providedJvm.set( - Jvm.discovered(jdkHomeLocation, null, jdkVersion) - ) - } - - updateJvmTarget(jdkVersion) + providedJvm.set( + objects.providerWithLazyConvention { + updateJvmTarget(jdkVersion) + if (currentGradleVersion < GradleVersion.version("6.2.0")) { + // Before Gradle 6.2.0 'Jvm.discovered' does not have 'implementationJavaVersion' parameter + Jvm::class.functions + .first { it.name == "discovered" } + .call(jdkHomeLocation, jdkVersion) as Jvm + } else { + Jvm.discovered(jdkHomeLocation, null, jdkVersion) + } + } + ) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt index 9f04cadadf1..fc01ed202e4 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt @@ -640,6 +640,11 @@ abstract class KotlinCompile @Inject constructor( ignoreClasspathResolutionErrors ) ) + + // This method could be called on configuration phase to calculate `filteredArgumentsMap` property + if (state.executing) { + defaultKotlinJavaToolchain.get().updateJvmTarget(this, args) + } } @get:Internal