From ddc72db0de5a707ffafa2fac15a1b887ed903800 Mon Sep 17 00:00:00 2001 From: Yahor Berdnikau Date: Thu, 3 Nov 2022 11:21:52 +0100 Subject: [PATCH] Configure compilation -jvm-target also from Java toolchain spec Correctly wire JVM toolchain spec such way that `-jvm-target` will also be configured from toolchain spec set either via java extension or kotlin extension. ^KT-50115 Fixed --- .../kotlin/gradle/KotlinJavaToolchainTest.kt | 36 +++++- .../kotlin/gradle/dsl/ToolchainDsl.kt | 16 +-- .../internal/kapt/KaptWithoutKotlincTask.kt | 5 +- .../tasks/DefaultKotlinJavaToolchain.kt | 106 +++++++++--------- .../jetbrains/kotlin/gradle/tasks/Tasks.kt | 2 +- 5 files changed, 97 insertions(+), 68 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 2e273911ff5..ac0f1648498 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 @@ -441,7 +441,41 @@ class KotlinJavaToolchainTest : KGPBaseTest() { """.trimIndent() ) - build("build") + build("build") { + assertOutputContains("[KOTLIN] Kotlin compilation 'jdkHome' argument: ${getJdk11Path().replace("\\\\", "\\")}") + } + } + } + + @DisplayName("Setting toolchain via java extension should update jvm-target argument on eager task creation") + @GradleTest + internal fun settingToolchainViaJavaUpdateJvmTarget(gradleVersion: GradleVersion) { + project( + projectName = "kotlinJavaProject".fullProjectName, + gradleVersion = gradleVersion + ) { + //language=groovy + buildGradle.append( + """ + tasks.named("compileKotlin").get() // Trigger task eager creation + + java { + toolchain { + languageVersion.set(JavaLanguageVersion.of(11)) + } + } + + """.trimIndent() + ) + + build("build", buildOptions = defaultBuildOptions.copy(logLevel = LogLevel.DEBUG)) { + val compilerArgs = output.lineSequence() + .filter { it.contains(":compileKotlin Kotlin compiler args:") } + .first() + assert(compilerArgs.contains("-jvm-target 11")) { + "Kotlin compilation jvm-target argument is ${output.substringAfter("-jvm-target ").substringBefore(" ")}" + } + } } } 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 ef273f57c6b..defafd4f544 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 @@ -45,28 +45,24 @@ internal abstract class DefaultToolchainSupport @Inject constructor( .toolchain init { - configureToolchain() + wireToolchainToTasks() } override fun applyToolchain( action: Action ) { action.execute(toolchainSpec) - configureToolchain() } - private fun configureToolchain() { + private fun wireToolchainToTasks() { plugins.withId("org.gradle.java-base") { + val toolchainService = extensions.findByType(JavaToolchainService::class.java) + ?: error("Gradle JavaToolchainService is not available!") + val javaLauncher = toolchainService.launcherFor(toolchainSpec) tasks .withType() .configureEach { - // Only set when toolchain is configured - if (toolchainSpec.languageVersion.isPresent) { - val toolchainService = extensions.findByType(JavaToolchainService::class.java)!! - it.kotlinJavaToolchain.toolchain.use( - toolchainService.launcherFor(toolchainSpec) - ) - } + it.kotlinJavaToolchain.toolchain.use(javaLauncher) } } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt index 8ed7e9c2b58..1c68c8b55ea 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt @@ -25,7 +25,6 @@ import org.jetbrains.kotlin.gradle.internal.kapt.classloaders.rootOrSelf import org.jetbrains.kotlin.gradle.internal.kapt.incremental.KaptIncrementalChanges import org.jetbrains.kotlin.gradle.tasks.Kapt import org.jetbrains.kotlin.gradle.tasks.toSingleCompilerPluginOptions -import org.jetbrains.kotlin.gradle.utils.isGradleVersionAtLeast import org.jetbrains.kotlin.gradle.utils.listPropertyWithConvention import org.jetbrains.kotlin.utils.PathUtil import org.slf4j.LoggerFactory @@ -94,7 +93,7 @@ abstract class KaptWithoutKotlincTask @Inject constructor( if (addJdkClassesToClasspath.get()) { compileClasspath.addAll( 0, - PathUtil.getJdkClassesRoots(defaultKotlinJavaToolchain.get().providedJvm.get().javaHome) + PathUtil.getJdkClassesRoots(defaultKotlinJavaToolchain.get().buildJvm.get().javaHome) ) } @@ -153,7 +152,7 @@ abstract class KaptWithoutKotlincTask @Inject constructor( private fun getWorkerIsolationMode(): IsolationMode { val toolchainProvider = defaultKotlinJavaToolchain.get() - val gradleJvm = toolchainProvider.currentJvm.get() + val gradleJvm = toolchainProvider.gradleJvm.get() // Ensuring Gradle build JDK is set to kotlin toolchain by also comparing javaExecutable paths, // as user may set JDK with same major Java version, but from different vendor val isRunningOnGradleJvm = gradleJvm.javaVersion == toolchainProvider.javaVersion.get() && 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 a63590a2d2f..a2f01aab1eb 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 @@ -16,6 +16,7 @@ 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.utils.* import org.jetbrains.kotlin.gradle.utils.chainedFinalizeValueOnRead import org.jetbrains.kotlin.gradle.utils.property import org.jetbrains.kotlin.gradle.utils.propertyWithConvention @@ -26,36 +27,65 @@ import javax.inject.Inject internal abstract class DefaultKotlinJavaToolchain @Inject constructor( private val objects: ObjectFactory, projectLayout: ProjectLayout, - kotlinCompileTaskProvider: () -> KotlinCompile? + private val kotlinCompileTaskProvider: () -> KotlinCompile? ) : KotlinJavaToolchain { @get:Internal - internal val currentJvm: Provider = objects + internal val gradleJvm: Provider = objects .property(Jvm.current()) + .chainedDisallowChanges() .chainedFinalizeValueOnRead() @get:Internal internal val providedJvm: Property = objects - .propertyWithConvention(currentJvm) + .property() + .chainedFinalizeValueOnRead() + + @get:Internal + internal val buildJvm: Provider = objects + .property(providedJvm.orElse(gradleJvm)) + .chainedDisallowChanges() .chainedFinalizeValueOnRead() final override val javaVersion: Provider = objects .property( - providedJvm.map { jvm -> - jvm.javaVersion - ?: throw GradleException( - "Kotlin could not get java version for the JDK installation: " + - jvm.javaHome?.let { "'$it' " }.orEmpty() - ) - } + buildJvm + .map { jvm -> + jvm.javaVersion + ?: throw GradleException( + "Kotlin could not get java version for the JDK installation: " + + jvm.javaHome?.let { "'$it' " }.orEmpty() + ) + } ) .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) + } + ) + } + } + @get:Internal internal val javaExecutable: RegularFileProperty = objects .fileProperty() .value( - providedJvm.flatMap { jvm -> + buildJvm.flatMap { jvm -> projectLayout.file( objects.property( jvm.javaExecutable @@ -91,12 +121,12 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( } @get:Internal - internal val jdkToolsJar: Provider = getToolsJarFromJvm(providedJvm, javaVersion) + internal val jdkToolsJar: Provider = getToolsJarFromJvm(buildJvm, javaVersion) @get:Internal internal val currentJvmJdkToolsJar: Provider = getToolsJarFromJvm( - currentJvm, - currentJvm.map { + gradleJvm, + gradleJvm.map { // Current JVM should always have java version it.javaVersion!! } @@ -104,42 +134,16 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( final override val jdk: KotlinJavaToolchain.JdkSetter = DefaultJdkSetter( providedJvm, - objects, - kotlinCompileTaskProvider + objects ) final override val toolchain: KotlinJavaToolchain.JavaToolchainSetter = - DefaultJavaToolchainSetter(providedJvm, kotlinCompileTaskProvider) + DefaultJavaToolchainSetter(providedJvm) - private abstract class JvmTargetUpdater( - private val kotlinCompileTaskProvider: () -> KotlinCompile? - ) { - fun updateJvmTarget( - jdkVersion: Provider - ) { - kotlinCompileTaskProvider()?.let { task -> - task.compilerOptions.jvmTarget.convention( - jdkVersion.map { version -> - // For Java 9 and Java 10 JavaVersion returns "1.9" or "1.10" accordingly - // that is not accepted by Kotlin compiler - val normalizedVersion = when (version) { - JavaVersion.VERSION_1_9 -> "9" - JavaVersion.VERSION_1_10 -> "10" - else -> version.toString() - } - JvmTarget.fromTarget(normalizedVersion) - } - ) - } - } - } - - private inner class DefaultJdkSetter( + private class DefaultJdkSetter( private val providedJvm: Property, private val objects: ObjectFactory, - kotlinCompileTaskProvider: () -> KotlinCompile? - ) : JvmTargetUpdater(kotlinCompileTaskProvider), - KotlinJavaToolchain.JdkSetter { + ) : KotlinJavaToolchain.JdkSetter { override fun use( jdkHomeLocation: File, @@ -152,7 +156,6 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( "Supplied jdkHomeLocation does not exist. You supplied: $jdkHomeLocation" } - updateJvmTarget(javaVersion) providedJvm.set( objects.providerWithLazyConvention { Jvm.discovered(jdkHomeLocation, null, jdkVersion) @@ -161,18 +164,15 @@ internal abstract class DefaultKotlinJavaToolchain @Inject constructor( } } - private inner class DefaultJavaToolchainSetter( - private val providedJvm: Property, - kotlinCompileTaskProvider: () -> KotlinCompile? - ) : JvmTargetUpdater(kotlinCompileTaskProvider), - KotlinJavaToolchain.JavaToolchainSetter { + private class DefaultJavaToolchainSetter( + private val providedJvm: Property + ) : KotlinJavaToolchain.JavaToolchainSetter { override fun use( javaLauncher: Provider ) { - updateJvmTarget(javaVersion) providedJvm.set( - javaLauncher.map { - val metadata = javaLauncher.get().metadata + javaLauncher.map { launcher -> + val metadata = launcher.metadata val javaVersion = JavaVersion.toVersion(metadata.languageVersion.asInt()) Jvm.discovered( metadata.installationPath.asFile, diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt index ebb572fcb36..f455deeefd6 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt @@ -773,7 +773,7 @@ abstract class KotlinCompile @Inject constructor( javaPackagePrefix, args, environment, - defaultKotlinJavaToolchain.get().providedJvm.get().javaHome, + defaultKotlinJavaToolchain.get().buildJvm.get().javaHome, taskOutputsBackup ) compilerRunner.errorsFile?.also { gradleMessageCollector.flush(it) }