From af2f716f4d0abf668042be0b01019ebefd3bdfd8 Mon Sep 17 00:00:00 2001 From: Yahor Berdnikau Date: Mon, 21 Jun 2021 17:21:53 +0200 Subject: [PATCH] Set 'jvmTarget' value on toolchain configuration. Toolchain will set 'kotlinOptions.jvmTarget' value to the toolchain jdk version, if 'jvmTarget' is not explicitly set by user. If toolchain is not configured, 'jvmTarget' field uses default value, which is now - "1.8". ^KT-45611 In Progress ^KT-43095 In Progress --- .../kotlin/gradle/KotlinJavaToolchainTest.kt | 145 ++++++++++++++++-- .../kotlin/gradle/internal/kapt/KaptTask.kt | 9 +- .../internal/kapt/KaptWithKotlincTask.kt | 4 +- .../internal/kapt/KaptWithoutKotlincTask.kt | 6 +- .../tasks/KotlinJavaToolchainProvider.kt | 53 +++++-- .../jetbrains/kotlin/gradle/tasks/Tasks.kt | 13 +- 6 files changed, 198 insertions(+), 32 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 ce7008e700e..02d5b08ab53 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 @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.gradle +import org.gradle.api.JavaVersion import org.gradle.api.logging.LogLevel import org.gradle.internal.jvm.JavaInfo import org.gradle.internal.jvm.Jvm @@ -62,7 +63,10 @@ class KotlinJavaToolchainTest : KGPBaseTest() { projectName = "simple".fullProjectName, gradleVersion = gradleVersion, ) { - useJdk9ToCompile() + useJdkToCompile( + getJdk9Path(), + JavaVersion.VERSION_1_9 + ) build("assemble") { assertJdkHomeIsUsingJdk(getJdk9().javaHomeRealPath) @@ -84,7 +88,10 @@ class KotlinJavaToolchainTest : KGPBaseTest() { if (shouldUseToolchain(gradleVersion)) { useToolchainExtension(11) } else { - useJdk9ToCompile() + useJdkToCompile( + getJdk9Path(), + JavaVersion.VERSION_1_9 + ) } build("assemble") @@ -100,7 +107,10 @@ class KotlinJavaToolchainTest : KGPBaseTest() { if (shouldUseToolchain(gradleVersion)) { useToolchainExtension(11) } else { - useJdk9ToCompile() + useJdkToCompile( + getJdk9Path(), + JavaVersion.VERSION_1_9 + ) } build("assemble") { @@ -124,7 +134,10 @@ class KotlinJavaToolchainTest : KGPBaseTest() { if (shouldUseToolchain(gradleVersion)) { useToolchainExtension(11) } else { - useJdk9ToCompile() + useJdkToCompile( + getJdk9Path(), + JavaVersion.VERSION_1_9 + ) } build("assemble") } @@ -153,7 +166,10 @@ class KotlinJavaToolchainTest : KGPBaseTest() { if (shouldUseToolchain(gradleVersion)) { useToolchainExtension(11) } else { - useJdk9ToCompile() + useJdkToCompile( + getJdk9Path(), + JavaVersion.VERSION_1_9 + ) } gradleProperties.append( "kapt.workers.isolation = none" @@ -202,7 +218,10 @@ class KotlinJavaToolchainTest : KGPBaseTest() { if (shouldUseToolchain(gradleVersion)) { useToolchainExtension(11) } else { - useJdk9ToCompile() + useJdkToCompile( + getJdk9Path(), + JavaVersion.VERSION_1_9 + ) } build("assemble") @@ -218,7 +237,10 @@ class KotlinJavaToolchainTest : KGPBaseTest() { if (shouldUseToolchain(gradleVersion)) { useToolchainExtension(11) } else { - useJdk9ToCompile() + useJdkToCompile( + getJdk9Path(), + JavaVersion.VERSION_1_9 + ) } build("assemble") { @@ -272,8 +294,7 @@ class KotlinJavaToolchainTest : KGPBaseTest() { internal fun jdkHomeIsDeprecated(gradleVersion: GradleVersion) { project( projectName = "simple".fullProjectName, - gradleVersion = gradleVersion, - buildOptions = defaultBuildOptions.copy(logLevel = LogLevel.DEBUG) + gradleVersion = gradleVersion ) { //language=Groovy rootBuildGradle.append( @@ -340,14 +361,95 @@ class KotlinJavaToolchainTest : KGPBaseTest() { } } + @DisplayName("Should set 'jvmTarget' option if user does not specify it explicitly via jdk setter") + @GradleTest + internal fun shouldSetJvmTargetNonSpecifiedByUserViaSetJdk(gradleVersion: GradleVersion) { + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + buildOptions = defaultBuildOptions.copy(logLevel = LogLevel.DEBUG) + ) { + useJdkToCompile( + getJdk11Path(), + JavaVersion.VERSION_11 + ) + + build("build") { + assertOutputContains("-jvm-target 11") + assertOutputDoesNotContain("-jvm-target 1.8") + } + } + } + + @DisplayName("Should not override user specified 'jvmTarget' option via jdk setter") + @GradleTest + internal fun shouldNotOverrideUserJvmTargetViaSetJDK(gradleVersion: GradleVersion) { + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + buildOptions = defaultBuildOptions.copy(logLevel = LogLevel.DEBUG) + ) { + setJvmTarget("1.8") + + useJdkToCompile( + getJdk11Path(), + JavaVersion.VERSION_11 + ) + + build("build") { + assertOutputContains("-jvm-target 1.8") + assertOutputDoesNotContain("-jvm-target 11") + } + } + } + + @DisplayName("Should set 'jvmTarget' option if user does not specify it explicitly via toolchain setter") + @GradleTestVersions(minVersion = "6.7.1") + @GradleTest + internal fun shouldSetJvmTargetNonSpecifiedByUserViaToolchain(gradleVersion: GradleVersion) { + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + buildOptions = defaultBuildOptions.copy(logLevel = LogLevel.DEBUG) + ) { + useToolchainToCompile(11) + + build("build") { + assertOutputContains("-jvm-target 11") + assertOutputDoesNotContain("-jvm-target 1.8") + } + } + } + + @DisplayName("Should not override user specified 'jvmTarget' option via toolchain setter") + @GradleTestVersions(minVersion = "6.7.1") + @GradleTest + internal fun shouldNotOverrideUserSpecifiedJvmTargetViaToolchain(gradleVersion: GradleVersion) { + project( + projectName = "simple".fullProjectName, + gradleVersion = gradleVersion, + buildOptions = defaultBuildOptions.copy(logLevel = LogLevel.DEBUG) + ) { + setJvmTarget("1.8") + useToolchainToCompile(11) + + build("build") { + assertOutputContains("-jvm-target 1.8") + assertOutputDoesNotContain("-jvm-target 11") + } + } + } + private fun BuildResult.assertJdkHomeIsUsingJdk( javaexecPath: String ) = assertOutputContains("[KOTLIN] Kotlin compilation 'jdkHome' argument: $javaexecPath") private fun getUserJdk(): JavaInfo = Jvm.forHome(File(System.getenv("JAVA_HOME"))) private fun getJdk9(): JavaInfo = Jvm.forHome(File(System.getenv("JDK_9"))) + private fun getJdk11(): JavaInfo = Jvm.forHome(File(System.getenv("JDK_11"))) // replace required for windows paths so Groovy will not complain about unexpected char '\' private fun getJdk9Path(): String = getJdk9().javaHome.absolutePath.replace("\\", "\\\\") + private fun getJdk11Path(): String = getJdk11().javaHome.absolutePath.replace("\\", "\\\\") private val JavaInfo.javaHomeRealPath get() = javaHome .toPath() @@ -357,7 +459,26 @@ class KotlinJavaToolchainTest : KGPBaseTest() { private val String.fullProjectName get() = "kotlin-java-toolchain/$this" - private fun TestProject.useJdk9ToCompile() { + private fun TestProject.setJvmTarget( + jvmTarget: String + ) { + //language=Groovy + rootBuildGradle.append( + """ + + tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).configureEach { + kotlinOptions { + jvmTarget = "$jvmTarget" + } + } + """.trimIndent() + ) + } + + private fun TestProject.useJdkToCompile( + jdkPath: String, + jdkVersion: JavaVersion + ) { //language=Groovy rootBuildGradle.append( """ @@ -368,8 +489,8 @@ class KotlinJavaToolchainTest : KGPBaseTest() { .withType(UsesKotlinJavaToolchain.class) .configureEach { it.kotlinJavaToolchain.jdk.use( - "${getJdk9Path()}", - JavaVersion.VERSION_1_9 + "$jdkPath", + JavaVersion.${jdkVersion.name} ) } """.trimIndent() diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt index 692adcaa107..da1f89a5f61 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt @@ -26,6 +26,7 @@ import org.jetbrains.kotlin.gradle.utils.getValue import org.jetbrains.kotlin.gradle.utils.isConfigurationCacheAvailable import org.jetbrains.kotlin.gradle.utils.property import org.jetbrains.kotlin.gradle.utils.propertyWithNewInstance +import org.jetbrains.kotlin.utils.addToStdlib.cast import java.io.File import java.util.concurrent.Callable import java.util.jar.JarFile @@ -124,11 +125,15 @@ abstract class KaptTask @Inject constructor( @get:Internal abstract val classpath: ConfigurableFileCollection - final override val kotlinJavaToolchainProvider: Provider = objectFactory + @get:Internal + internal val defaultKotlinJavaToolchain: Provider = objectFactory .propertyWithNewInstance( - project.gradle + project.gradle, + { null } ) + final override val kotlinJavaToolchainProvider: Provider = defaultKotlinJavaToolchain.cast() + @Suppress("unused", "DeprecatedCallableAddReplaceWith") @Deprecated( message = "Don't use directly. Used only for up-to-date checks", diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt index d4b3e808844..ff5c7ee0203 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt @@ -117,7 +117,7 @@ abstract class KaptWithKotlincTask @Inject constructor( val compilerRunner = GradleCompilerRunner( taskProvider.get(), - kotlinJavaToolchainProvider.get().currentJvmJdkToolsJar.orNull + defaultKotlinJavaToolchain.get().currentJvmJdkToolsJar.orNull ) compilerRunner.runJvmCompilerAsync( sourcesToCompile = emptyList(), @@ -126,7 +126,7 @@ abstract class KaptWithKotlincTask @Inject constructor( javaPackagePrefix = javaPackagePrefix.orNull, args = args, environment = environment, - jdkHome = kotlinJavaToolchainProvider.get().providedJvm.get().javaHome + jdkHome = defaultKotlinJavaToolchain.get().providedJvm.get().javaHome ) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt index e4c4de6385e..67edbd5eef4 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt @@ -157,7 +157,7 @@ abstract class KaptWithoutKotlincTask @Inject constructor( val kaptClasspath = kaptJars.toList() + kotlinStdlibClasspath val isolationMode = getWorkerIsolationMode() logger.info("Using workers $isolationMode isolation mode to run kapt") - val toolsJarURLSpec = kotlinJavaToolchainProvider.get() + val toolsJarURLSpec = defaultKotlinJavaToolchain.get() .jdkToolsJar.orNull?.toURI()?.toURL()?.toString().orEmpty() submitWork( @@ -169,7 +169,7 @@ abstract class KaptWithoutKotlincTask @Inject constructor( } private fun getWorkerIsolationMode(): IsolationMode { - val toolchainProvider = kotlinJavaToolchainProvider.get() + val toolchainProvider = defaultKotlinJavaToolchain.get() val gradleJvm = toolchainProvider.currentJvm.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 @@ -199,7 +199,7 @@ abstract class KaptWithoutKotlincTask @Inject constructor( // for tests it.forkOptions.jvmArgs("-verbose:class") } - it.forkOptions.executable = kotlinJavaToolchainProvider.get() + it.forkOptions.executable = defaultKotlinJavaToolchain.get() .javaExecutable .asFile.get() .absolutePath diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/KotlinJavaToolchainProvider.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/KotlinJavaToolchainProvider.kt index 8ae1a10ec3a..dfcfcec3a73 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/KotlinJavaToolchainProvider.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/KotlinJavaToolchainProvider.kt @@ -17,6 +17,8 @@ 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.gradle.dsl.KotlinCompile as KotlinCompileTask +import org.jetbrains.kotlin.gradle.dsl.KotlinJvmOptionsBase 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 @@ -25,10 +27,11 @@ import java.io.File import javax.inject.Inject import kotlin.reflect.full.functions -abstract class KotlinJavaToolchainProvider @Inject constructor( +internal abstract class KotlinJavaToolchainProvider @Inject constructor( private val objects: ObjectFactory, projectLayout: ProjectLayout, - gradle: Gradle + gradle: Gradle, + kotlinCompileTaskProvider: () -> KotlinCompileTask? ) : KotlinJavaToolchain { private val currentGradleVersion = GradleVersion.version(gradle.gradleVersion) @@ -98,11 +101,15 @@ abstract class KotlinJavaToolchainProvider @Inject constructor( @get:Internal internal val currentJvmJdkToolsJar: Provider = getToolsJarFromJvm(currentJvm) - final override val jdk: KotlinJavaToolchain.JdkSetter = DefaultJdkSetter(providedJvm, currentGradleVersion) + final override val jdk: KotlinJavaToolchain.JdkSetter = DefaultJdkSetter( + providedJvm, + currentGradleVersion, + kotlinCompileTaskProvider + ) private val defaultJavaToolchainSetter by lazy(LazyThreadSafetyMode.NONE) { if (currentGradleVersion >= TOOLCHAIN_SUPPORTED_VERSION) { - DefaultJavaToolchainSetter(providedJvm) + DefaultJavaToolchainSetter(providedJvm, kotlinCompileTaskProvider) } else { null } @@ -112,10 +119,32 @@ abstract class KotlinJavaToolchainProvider @Inject constructor( get() = defaultJavaToolchainSetter ?: throw GradleException("Toolchain support is available from $TOOLCHAIN_SUPPORTED_VERSION") + + private abstract class JvmTargetUpdater( + private val kotlinCompileTaskProvider: () -> KotlinCompileTask? + ) { + fun updateJvmTarget( + jdkVersion: JavaVersion + ) { + kotlinCompileTaskProvider()?.kotlinOptions { + if (jvmTargetField == null) { + // For Java 9 JavaVersion returns "1.9" that is not accepted by Kotlin compiler + jvmTarget = if (jdkVersion == JavaVersion.VERSION_1_9) { + "9" + } else { + jdkVersion.toString() + } + } + } + } + } + private class DefaultJdkSetter( private val providedJvm: Property, - private val currentGradleVersion: GradleVersion - ) : KotlinJavaToolchain.JdkSetter { + private val currentGradleVersion: GradleVersion, + kotlinCompileTaskProvider: () -> KotlinCompileTask? + ) : JvmTargetUpdater(kotlinCompileTaskProvider), + KotlinJavaToolchain.JdkSetter { override fun use( jdkHomeLocation: File, @@ -139,22 +168,28 @@ abstract class KotlinJavaToolchainProvider @Inject constructor( Jvm.discovered(jdkHomeLocation, null, jdkVersion) ) } + + updateJvmTarget(jdkVersion) } } private inner class DefaultJavaToolchainSetter( - private val providedJvm: Property - ) : KotlinJavaToolchain.JavaToolchainSetter { + private val providedJvm: Property, + kotlinCompileTaskProvider: () -> KotlinCompileTask? + ) : JvmTargetUpdater(kotlinCompileTaskProvider), + KotlinJavaToolchain.JavaToolchainSetter { override fun use( javaLauncher: Provider ) { providedJvm.set( javaLauncher.map { val metadata = javaLauncher.get().metadata + val javaVersion = JavaVersion.toVersion(metadata.languageVersion.asInt()) + updateJvmTarget(javaVersion) Jvm.discovered( metadata.installationPath.asFile, null, - JavaVersion.toVersion(metadata.languageVersion.asInt()) + javaVersion ) } ) 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 8d8c503c8cc..c5f00cd7f6d 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 @@ -57,6 +57,7 @@ import org.jetbrains.kotlin.incremental.ChangedFiles import org.jetbrains.kotlin.incremental.IncrementalCompilerRunner import org.jetbrains.kotlin.library.impl.isKotlinLibrary import org.jetbrains.kotlin.utils.JsLibraryUtils +import org.jetbrains.kotlin.utils.addToStdlib.cast import java.io.File import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject @@ -459,15 +460,19 @@ abstract class KotlinCompile @Inject constructor( @get:Input abstract val useClasspathSnapshot: Property - final override val kotlinJavaToolchainProvider: Provider = objects + @get:Internal + internal val defaultKotlinJavaToolchain: Provider = objects .propertyWithNewInstance( - project.gradle + project.gradle, + { this } ) + final override val kotlinJavaToolchainProvider: Provider = defaultKotlinJavaToolchain.cast() + @get:Internal override val compilerRunner: Provider = objects.propertyWithConvention( // From Gradle 6.6 better to replace flatMap with provider.zip() - kotlinJavaToolchainProvider.flatMap { toolchain -> + defaultKotlinJavaToolchain.flatMap { toolchain -> objects.property(gradleCompileTaskProvider.map { GradleCompilerRunner( it, @@ -535,7 +540,7 @@ abstract class KotlinCompile @Inject constructor( javaPackagePrefix, args, environment, - kotlinJavaToolchainProvider.get().providedJvm.get().javaHome + defaultKotlinJavaToolchain.get().providedJvm.get().javaHome ) }