From ac2d00916c0e3373e5303dddffc389c347463298 Mon Sep 17 00:00:00 2001 From: Yahor Berdnikau Date: Mon, 8 May 2023 23:23:23 +0200 Subject: [PATCH] Fix toolchain does not configure project level jvm target convention Now JDK toolchain configures project level compiler options jvm target convention and does not override convention for task. For use case when toolchain is configured via task DSL jvm target convention overrides project level convention. ^KT-57393 Fixed --- .../kotlin/gradle/KotlinJavaToolchainTest.kt | 56 ++++++++++++ .../gradle/dsl/KotlinProjectExtension.kt | 2 +- .../kotlin/gradle/dsl/ToolchainDsl.kt | 26 +++++- .../kotlin/gradle/plugin/KotlinJvmPlugin.kt | 4 + .../tasks/DefaultKotlinJavaToolchain.kt | 90 +++++++++++++------ .../kotlin/gradle/tasks/KotlinCompile.kt | 2 +- 6 files changed, 150 insertions(+), 30 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 b108929505c..b8d342fe2a2 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 @@ -12,6 +12,7 @@ import org.gradle.internal.jvm.Jvm import org.gradle.testkit.runner.BuildResult import org.gradle.util.GradleVersion import org.jetbrains.kotlin.gradle.testbase.* +import org.jetbrains.kotlin.test.TestMetadata import org.junit.jupiter.api.DisplayName import java.io.File import kotlin.io.path.appendText @@ -702,6 +703,61 @@ class KotlinJavaToolchainTest : KGPBaseTest() { } } + @DisplayName("Toolchain should not override Jvm target configured in project level DSL") + @TestMetadata("kotlin-java-toolchain/simple") + @GradleTest + fun toolchainNotOverrideProjectJvmTarget(gradleVersion: GradleVersion) { + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + buildOptions = defaultBuildOptions.copy(logLevel = LogLevel.DEBUG) + ) { + buildGradle.appendText( + //language=groovy + """ + | + |kotlin.compilerOptions.jvmTarget.set(org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_11) + | + """.trimMargin() + ) + + build(":compileKotlin") { + assertTasksExecuted(":compileKotlin") + assertCompilerArgument(":compileKotlin", "-jvm-target 11") + } + } + } + + @AndroidGradlePluginTests + @DisplayName("Toolchain should not override Jvm taget configured via kotlinOptions in android project") + @AndroidTestVersions(minVersion = TestVersions.AGP.AGP_42) + @GradleAndroidTest + internal fun kotlinOptionsAndroidAndToolchainNotOverrideJvmTarget( + gradleVersion: GradleVersion, + agpVersion: String, + providedJdk: JdkVersions.ProvidedJdk + ) { + project( + "android".fullProjectName, + gradleVersion, + buildOptions = defaultBuildOptions.copy(androidVersion = agpVersion, logLevel = LogLevel.DEBUG), + buildJdk = providedJdk.location + ) { + buildGradle.appendText( + //language=groovy + """ + | + |android.kotlinOptions.jvmTarget = "11" + """.trimMargin() + ) + + build(":compileDebugKotlin") { + assertTasksExecuted(":compileDebugKotlin") + assertCompilerArgument(":compileDebugKotlin", "-jvm-target 11") + } + } + } + private fun BuildResult.assertJdkHomeIsUsingJdk( javaexecPath: String ) = assertOutputContains("[KOTLIN] Kotlin compilation 'jdkHome' argument: $javaexecPath") diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/KotlinProjectExtension.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/KotlinProjectExtension.kt index e84818d0b28..45beeed67d2 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/KotlinProjectExtension.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/KotlinProjectExtension.kt @@ -75,7 +75,7 @@ abstract class KotlinTopLevelExtension(internal val project: Project) : KotlinTo override lateinit var coreLibrariesVersion: String - private val toolchainSupport = ToolchainSupport.createToolchain(project) + private val toolchainSupport = ToolchainSupport.createToolchain(project, this) /** * Configures [Java toolchain](https://docs.gradle.org/current/userguide/toolchains.html) both for Kotlin JVM and Java tasks. diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/ToolchainDsl.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/ToolchainDsl.kt index 47f75336df5..25aa249ee50 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/ToolchainDsl.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/ToolchainDsl.kt @@ -14,6 +14,7 @@ import org.gradle.api.tasks.TaskContainer import org.gradle.jvm.toolchain.JavaToolchainService import org.gradle.jvm.toolchain.JavaToolchainSpec import org.jetbrains.kotlin.gradle.tasks.DefaultKotlinJavaToolchain +import org.jetbrains.kotlin.gradle.tasks.KotlinJvmCompile import org.jetbrains.kotlin.gradle.tasks.UsesKotlinJavaToolchain import org.jetbrains.kotlin.gradle.tasks.withType import org.jetbrains.kotlin.gradle.utils.newInstance @@ -24,12 +25,14 @@ internal interface ToolchainSupport { companion object { internal fun createToolchain( - project: Project + project: Project, + kotlinExtension: KotlinTopLevelExtensionConfig ): ToolchainSupport { return project.objects.newInstance( project.extensions, project.tasks, - project.plugins + project.plugins, + kotlinExtension ) } } @@ -38,7 +41,8 @@ internal interface ToolchainSupport { internal abstract class DefaultToolchainSupport @Inject constructor( private val extensions: ExtensionContainer, private val tasks: TaskContainer, - private val plugins: PluginContainer + private val plugins: PluginContainer, + private val kotlinExtension: KotlinTopLevelExtensionConfig ) : ToolchainSupport { private val toolchainSpec: JavaToolchainSpec get() = extensions @@ -60,12 +64,28 @@ internal abstract class DefaultToolchainSupport @Inject constructor( val toolchainService = extensions.findByType(JavaToolchainService::class.java) ?: error("Gradle JavaToolchainService is not available!") val javaLauncher = toolchainService.launcherFor(toolchainSpec) + tasks .withType() .configureEach { (it.kotlinJavaToolchain.toolchain as DefaultKotlinJavaToolchain.DefaultJavaToolchainSetter) .useAsConvention(javaLauncher) } + + if (kotlinExtension !is KotlinJvmProjectExtension && + kotlinExtension !is KotlinAndroidProjectExtension + ) { + tasks + .withType() + .configureEach { task -> + if (task is KotlinJvmCompile) { + DefaultKotlinJavaToolchain.wireJvmTargetToJvm( + task.compilerOptions, + (task.kotlinJavaToolchain as DefaultKotlinJavaToolchain).providedJvm + ) + } + } + } } } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinJvmPlugin.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinJvmPlugin.kt index 6f3ed79a8fd..aa004949603 100755 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinJvmPlugin.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinJvmPlugin.kt @@ -33,6 +33,10 @@ internal open class KotlinJvmPlugin( ) { extensionCompilerOptions.verbose.convention(logger.isDebugEnabled) extensionCompilerOptions.moduleName.convention(baseModuleName()) + DefaultKotlinJavaToolchain.wireJvmTargetToToolchain( + extensionCompilerOptions, + project + ) compilationsContainer.configureEach { val jvmCompilerOptions = it.compilerOptions.options as KotlinJvmCompilerOptions KotlinJvmCompilerOptionsHelper.syncOptionsAsConvention( diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/DefaultKotlinJavaToolchain.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/DefaultKotlinJavaToolchain.kt index a278cd8a590..63359e58bbb 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/DefaultKotlinJavaToolchain.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/DefaultKotlinJavaToolchain.kt @@ -7,15 +7,18 @@ package org.jetbrains.kotlin.gradle.tasks import org.gradle.api.GradleException import org.gradle.api.JavaVersion +import org.gradle.api.Project import org.gradle.api.file.ProjectLayout import org.gradle.api.file.RegularFileProperty import org.gradle.api.model.ObjectFactory +import org.gradle.api.plugins.JavaPluginExtension import org.gradle.api.provider.Property import org.gradle.api.provider.Provider import org.gradle.api.tasks.Internal import org.gradle.internal.jvm.Jvm import org.gradle.jvm.toolchain.* import org.jetbrains.kotlin.gradle.dsl.JvmTarget +import org.jetbrains.kotlin.gradle.dsl.KotlinJvmCompilerOptions import org.jetbrains.kotlin.gradle.utils.* import org.jetbrains.kotlin.gradle.utils.chainedFinalizeValueOnRead import org.jetbrains.kotlin.gradle.utils.property @@ -27,7 +30,7 @@ import javax.inject.Inject internal abstract class DefaultKotlinJavaToolchain @Inject constructor( private val objects: ObjectFactory, projectLayout: ProjectLayout, - private val kotlinCompileTaskProvider: () -> KotlinCompile? + jvmCompilerOptions: () -> KotlinJvmCompilerOptions? ) : KotlinJavaToolchain { @get:Internal @@ -60,27 +63,6 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( ) .chainedFinalizeValueOnRead() - init { - wireJvmTargetToToolchain() - } - - private fun wireJvmTargetToToolchain() { - kotlinCompileTaskProvider()?.let { task -> - task.compilerOptions.jvmTarget.convention( - providedJvm.map { jvm -> - // For Java 9 and Java 10 JavaVersion returns "1.9" or "1.10" accordingly - // that is not accepted by Kotlin compiler - val normalizedVersion = when (jvm.javaVersion) { - JavaVersion.VERSION_1_9 -> "9" - JavaVersion.VERSION_1_10 -> "10" - else -> jvm.javaVersion.toString() - } - JvmTarget.fromTarget(normalizedVersion) - }.orElse(JvmTarget.DEFAULT) - ) - } - } - @get:Internal internal val javaExecutable: RegularFileProperty = objects .fileProperty() @@ -134,15 +116,20 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( final override val jdk: KotlinJavaToolchain.JdkSetter = DefaultJdkSetter( providedJvm, - objects + objects, + jvmCompilerOptions ) final override val toolchain: KotlinJavaToolchain.JavaToolchainSetter = - DefaultJavaToolchainSetter(providedJvm) + DefaultJavaToolchainSetter( + providedJvm, + jvmCompilerOptions + ) private class DefaultJdkSetter( private val providedJvm: Property, private val objects: ObjectFactory, + private val jvmCompilerOptions: () -> KotlinJvmCompilerOptions? ) : KotlinJavaToolchain.JdkSetter { override fun use( @@ -161,11 +148,16 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( Jvm.discovered(jdkHomeLocation, null, jdkVersion) } ) + + jvmCompilerOptions()?.let { + wireJvmTargetToJvm(it, providedJvm) + } } } internal class DefaultJavaToolchainSetter( - private val providedJvm: Property + private val providedJvm: Property, + private val jvmCompilerOptions: () -> KotlinJvmCompilerOptions? ) : KotlinJavaToolchain.JavaToolchainSetter { internal fun useAsConvention( @@ -178,6 +170,54 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( javaLauncher: Provider ) { providedJvm.set(javaLauncher.map(::mapToJvm)) + + jvmCompilerOptions()?.let { + wireJvmTargetToJvm(it, providedJvm) + } + } + } + + companion object { + + internal fun wireJvmTargetToJvm( + jvmCompilerOptions: KotlinJvmCompilerOptions, + toolchainJvm: Provider + ) { + jvmCompilerOptions.jvmTarget.convention( + toolchainJvm.map { jvm -> + // For Java 9 and Java 10 JavaVersion returns "1.9" or "1.10" accordingly + // that is not accepted by Kotlin compiler + val normalizedVersion = when (jvm.javaVersion) { + JavaVersion.VERSION_1_9 -> "9" + JavaVersion.VERSION_1_10 -> "10" + else -> jvm.javaVersion.toString() + } + JvmTarget.fromTarget(normalizedVersion) + }.orElse(JvmTarget.DEFAULT) + ) + } + + private fun wireJvmTargetToToolchain( + jvmCompilerOptions: KotlinJvmCompilerOptions, + javaLauncher: Provider + ): Unit = wireJvmTargetToJvm( + jvmCompilerOptions, + javaLauncher.map(::mapToJvm) + ) + + internal fun wireJvmTargetToToolchain( + compilerOptions: KotlinJvmCompilerOptions, + project: Project, + ) { + project.plugins.withId("org.gradle.java-base") { + val toolchainService = project.extensions.findByType(JavaToolchainService::class.java) + ?: error("Gradle JavaToolchainService is not available!") + val toolchainSpec = project.extensions + .getByType(JavaPluginExtension::class.java) + .toolchain + val javaLauncher = toolchainService.launcherFor(toolchainSpec) + wireJvmTargetToToolchain(compilerOptions, javaLauncher) + } } private fun mapToJvm(javaLauncher: JavaLauncher): Jvm { diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/KotlinCompile.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/KotlinCompile.kt index 6eead75819e..e266085f964 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/KotlinCompile.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/KotlinCompile.kt @@ -144,7 +144,7 @@ abstract class KotlinCompile @Inject constructor( @get:Internal final override val defaultKotlinJavaToolchain: Provider = objectFactory - .propertyWithNewInstance({ this }) + .propertyWithNewInstance({ compilerOptions }) final override val kotlinJavaToolchainProvider: Provider = defaultKotlinJavaToolchain.cast()