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
2021-06-21 17:21:53 +02:00
parent 430306b23d
commit af2f716f4d
6 changed files with 198 additions and 32 deletions
@@ -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()
@@ -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<KotlinJavaToolchainProvider> = objectFactory
+    @get:Internal
+    internal val defaultKotlinJavaToolchain: Provider<KotlinJavaToolchainProvider> = objectFactory
        .propertyWithNewInstance(
-            project.gradle
+            project.gradle,
+            { null }
        )

+    final override val kotlinJavaToolchainProvider: Provider<KotlinJavaToolchain> = defaultKotlinJavaToolchain.cast()
+
    @Suppress("unused", "DeprecatedCallableAddReplaceWith")
    @Deprecated(
        message = "Don't use directly. Used only for up-to-date checks",
@@ -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
        )
    }
 }
@@ -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
@@ -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<KotlinJvmOptionsBase>?
 ) : KotlinJavaToolchain {

    private val currentGradleVersion = GradleVersion.version(gradle.gradleVersion)
@@ -98,11 +101,15 @@ abstract class KotlinJavaToolchainProvider @Inject constructor(
    @get:Internal
    internal val currentJvmJdkToolsJar: Provider<File?> = 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<KotlinJvmOptionsBase>?
+    ) {
+        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<Jvm>,
-        private val currentGradleVersion: GradleVersion
-    ) : KotlinJavaToolchain.JdkSetter {
+        private val currentGradleVersion: GradleVersion,
+        kotlinCompileTaskProvider: () -> KotlinCompileTask<KotlinJvmOptionsBase>?
+    ) : 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<Jvm>
-    ) : KotlinJavaToolchain.JavaToolchainSetter {
+        private val providedJvm: Property<Jvm>,
+        kotlinCompileTaskProvider: () -> KotlinCompileTask<KotlinJvmOptionsBase>?
+    ) : JvmTargetUpdater(kotlinCompileTaskProvider),
+        KotlinJavaToolchain.JavaToolchainSetter {
        override fun use(
            javaLauncher: Provider<JavaLauncher>
        ) {
            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
                    )
                }
            )
@@ -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<Boolean>

-    final override val kotlinJavaToolchainProvider: Provider<KotlinJavaToolchainProvider> = objects
+    @get:Internal
+    internal val defaultKotlinJavaToolchain: Provider<KotlinJavaToolchainProvider> = objects
        .propertyWithNewInstance(
-            project.gradle
+            project.gradle,
+            { this }
        )

+    final override val kotlinJavaToolchainProvider: Provider<KotlinJavaToolchain> = defaultKotlinJavaToolchain.cast()
+
    @get:Internal
    override val compilerRunner: Provider<GradleCompilerRunner> = 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
        )
    }