From 3c4163187dbd0dd1b70a5cbaaf8c989b8aa32973 Mon Sep 17 00:00:00 2001 From: Sebastian Sellmair Date: Tue, 29 Aug 2023 10:23:13 +0200 Subject: [PATCH] [Gradle] K2/KMP: Implement expect/actual discrimination by topologically ordering dependsOn klibs ^KT-61540 Verification Pending --- .../org/jetbrains/kotlin/gradle/K2Tests.kt | 15 ++ .../build.gradle.kts | 29 ++++ .../src/commonMain/kotlin/expects.kt | 3 + .../src/jvmMain/kotlin/actuals.kt | 6 + .../src/linuxMain/kotlin/LinuxMain.kt | 8 + .../src/nativeMain/kotlin/actuals.kt | 8 + .../KotlinMetadataTargetConfigurator.kt | 50 ++++++- .../DependsOnClosureCompilePathTest.kt | 137 ++++++++++++++++++ 8 files changed, 248 insertions(+), 8 deletions(-) create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/build.gradle.kts create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/commonMain/kotlin/expects.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/jvmMain/kotlin/actuals.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/linuxMain/kotlin/LinuxMain.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/nativeMain/kotlin/actuals.kt create mode 100644 libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/DependsOnClosureCompilePathTest.kt diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/K2Tests.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/K2Tests.kt index d24bdf384e8..1ed770cdde7 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/K2Tests.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/K2Tests.kt @@ -186,4 +186,19 @@ class CustomK2Tests : KGPBaseTest() { } } } + + @GradleTest + @DisplayName("Common metadata compilation (expect actual discrimination). KT-61540") + fun kt60438MetadataExpectActualDiscrimination(gradleVersion: GradleVersion) { + project( + "k2-kt-61540-expect-actual-discrimination", gradleVersion, + buildOptions = defaultBuildOptions.copy(languageVersion = "2.0") + ) { + build("assemble") { + assertTasksExecuted(":compileCommonMainKotlinMetadata") + assertTasksExecuted(":compileNativeMainKotlinMetadata") + assertTasksExecuted(":compileLinuxMainKotlinMetadata") + } + } + } } diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/build.gradle.kts b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/build.gradle.kts new file mode 100644 index 00000000000..0364fec6f45 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/build.gradle.kts @@ -0,0 +1,29 @@ +plugins { + kotlin("multiplatform") +} + +repositories { + mavenLocal() + mavenCentral() +} + +kotlin { + jvm() + linuxX64() + linuxArm64() + + + /* + Create custom 'refines' edges. + The goal here is to have refines edges (dependsOn) listed in a way + that linuxMain will see the 'commonMain' edge first + */ + + sourceSets.nativeMain.get().dependsOn(sourceSets.commonMain.get()) + + sourceSets.linuxMain.get().dependsOn(sourceSets.commonMain.get()) + sourceSets.linuxMain.get().dependsOn(sourceSets.nativeMain.get()) + + sourceSets.getByName("linuxX64Main").dependsOn(sourceSets.linuxMain.get()) + sourceSets.getByName("linuxArm64Main").dependsOn(sourceSets.linuxMain.get()) +} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/commonMain/kotlin/expects.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/commonMain/kotlin/expects.kt new file mode 100644 index 00000000000..2c668a012a9 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/commonMain/kotlin/expects.kt @@ -0,0 +1,3 @@ +expect class MyExpectClass { + val myExpectClassProperty: Int +} \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/jvmMain/kotlin/actuals.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/jvmMain/kotlin/actuals.kt new file mode 100644 index 00000000000..fdf9fa3a10e --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/jvmMain/kotlin/actuals.kt @@ -0,0 +1,6 @@ +import java.util.concurrent.locks.ReentrantLock + +actual class MyExpectClass { + actual val myExpectClassProperty: Int = 0 + val myJvmProperty: ReentrantLock = ReentrantLock() +} \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/linuxMain/kotlin/LinuxMain.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/linuxMain/kotlin/LinuxMain.kt new file mode 100644 index 00000000000..a45a3b546e6 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/linuxMain/kotlin/LinuxMain.kt @@ -0,0 +1,8 @@ +@file:OptIn(ExperimentalForeignApi::class) + +import kotlinx.cinterop.ExperimentalForeignApi + +fun linuxMain() { + MyExpectClass().myExpectClassProperty + MyExpectClass().myNativeProperty +} \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/nativeMain/kotlin/actuals.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/nativeMain/kotlin/actuals.kt new file mode 100644 index 00000000000..f8d828824a7 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/k2-kt-61540-expect-actual-discrimination/src/nativeMain/kotlin/actuals.kt @@ -0,0 +1,8 @@ +@file:OptIn(ExperimentalForeignApi::class) + +import kotlinx.cinterop.* + +actual class MyExpectClass { + actual val myExpectClassProperty: Int = 0 + val myNativeProperty: CPointer = TODO() +} \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt index ced3d3e8704..68c7d878e50 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt @@ -346,7 +346,7 @@ class KotlinMetadataTargetConfigurator : val artifacts = sourceSet.internal.resolvableMetadataConfiguration.incoming.artifacts.getResolvedArtifactsCompat(project) // Metadata from visible source sets within dependsOn closure - compilation.compileDependencyFiles += sourceSet.dependsOnClassesDirs + compilation.compileDependencyFiles += sourceSet.dependsOnClosureCompilePath // Requested dependencies that are not Multiplatform Libraries. for example stdlib-common compilation.compileDependencyFiles += project.files(artifacts.map { it.filterNot { it.isMpp }.map { it.file } }) @@ -361,13 +361,6 @@ class KotlinMetadataTargetConfigurator : private val ResolvedArtifactResult.isMpp: Boolean get() = variant.attributes.containsMultiplatformAttributes - private val KotlinSourceSet.dependsOnClassesDirs: FileCollection - get() = project.filesProvider { - internal.dependsOnClosure.mapNotNull { hierarchySourceSet -> - val compilation = project.future { findMetadataCompilation(hierarchySourceSet) }.getOrThrow() ?: return@mapNotNull null - compilation.output.classesDirs - } - } private fun createCommonMainElementsConfiguration(target: KotlinMetadataTarget) { val project = target.project @@ -508,3 +501,44 @@ internal suspend fun Project.findMetadataCompilation(sourceSet: KotlinSourceSet) metadataTarget.awaitMetadataCompilationsCreated() return metadataTarget.compilations.findByName(sourceSet.name) as KotlinMetadataCompilation<*>? } + + +/** + * Contains all 'klibs' produced by compiling 'dependsOn' SourceSet's metadata. + * The compile path can be passed to another metadata compilation as list of dependencies. + * + * Note: The compile path is ordered and will provide klibs containing corresponding actuals before providing + * the klibs defining expects. This ordering is necessary for K2 as the compiler will not implement + * its own 'actual over expect' discrimination anymore. K2 will use the first matching symbol of a given compile path. + * + * e.g. + * When compiling a 'iosMain' source set, using the default hierarchy, we expect the order of the compile path: + * ``` + * appleMain.klib, nativeMain.klib, commonMain.klib + * ``` + * + * Further details: https://youtrack.jetbrains.com/issue/KT-61540 + * + */ +internal val KotlinSourceSet.dependsOnClosureCompilePath: FileCollection + get() = project.filesProvider { + val topologicallySortedDependsOnClosure = internal.dependsOnClosure.sortedWith(Comparator { a, b -> + when { + a in b.internal.dependsOnClosure -> 1 + b in a.internal.dependsOnClosure -> -1 + /* + SourceSet 'a' and SourceSet 'b' are not refining on each other, + therefore no re-ordering is necessary (no requirements in this case). + + The original order of the 'dependsOnClosure' will be preserved, which will depend + on the order of 'KotlinSourceSet.dependsOn' calls + */ + else -> 0 + } + }) + + topologicallySortedDependsOnClosure.mapNotNull { hierarchySourceSet -> + val compilation = project.future { findMetadataCompilation(hierarchySourceSet) }.getOrThrow() ?: return@mapNotNull null + compilation.output.classesDirs + } + } diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/DependsOnClosureCompilePathTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/DependsOnClosureCompilePathTest.kt new file mode 100644 index 00000000000..1baa99293d4 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/DependsOnClosureCompilePathTest.kt @@ -0,0 +1,137 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +@file:Suppress("FunctionName") + +package org.jetbrains.kotlin.gradle.unitTests + +import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformSourceSetConventionsImpl.commonMain +import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformSourceSetConventionsImpl.iosMain +import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformSourceSetConventionsImpl.linuxMain +import org.jetbrains.kotlin.gradle.dsl.multiplatformExtension +import org.jetbrains.kotlin.gradle.targets.metadata.dependsOnClosureCompilePath +import org.jetbrains.kotlin.gradle.util.buildProjectWithMPP +import kotlin.test.Test +import kotlin.test.assertEquals + +class DependsOnClosureCompilePathTest { + private val project = buildProjectWithMPP() + private val kotlin = project.multiplatformExtension + + @Test + fun `test - default hierarchy - iosMain`() { + kotlin.iosArm64() + kotlin.iosX64() + kotlin.iosSimulatorArm64() + project.evaluate() + + assertEquals( + listOf("test_appleMain.klib", "test_nativeMain.klib", "test_commonMain.klib"), + kotlin.sourceSets.iosMain.get().dependsOnClosureCompilePath.toList().map { it.name } + ) + } + + /** + * ``` + * ┌─► commonMain ◄──┐ + * │ ▲ │ + * │ │ │ + * │ ┌──► b ◄────┐ │ + * │ │ │ │ + * │ │ │ │ + * └───c──────────►a──┘ + * ▲ + * │ + * linuxMain + * ▲ + * │ + * ┌─────┴─────┐ + * │ │ + * linuxX64main linuxArm64Main + * ``` + */ + @Test + fun `test - custom dependsOn order`() { + val linuxX64Main = kotlin.linuxX64().compilations.getByName("main").defaultSourceSet + val linuxArm64Main = kotlin.linuxArm64().compilations.getByName("main").defaultSourceSet + + val commonMain = kotlin.sourceSets.commonMain.get() + val a = kotlin.sourceSets.create("a") + val b = kotlin.sourceSets.create("b") + val c = kotlin.sourceSets.create("c") + + a.dependsOn(commonMain) + b.dependsOn(commonMain) + c.dependsOn(commonMain) + + linuxX64Main.dependsOn(c) + linuxArm64Main.dependsOn(c) + + c.dependsOn(b) + c.dependsOn(a) + a.dependsOn(b) + + kotlin.sourceSets.linuxMain.get().dependsOn(c) + + project.evaluate() + + assertEquals( + listOf("test_c.klib", "test_a.klib", "test_b.klib", "test_commonMain.klib"), + kotlin.sourceSets.linuxMain.get().dependsOnClosureCompilePath.toList().map { it.name } + ) + } + + /** + * ``` + * ┌────► commonMain ◄───┐ + * │ │ + * │ │ + * │ │ + * left right + * ▲ ▲ + * │ │ + * │ │ + * └───────bottom────────┘ + * ▲ + * │ + * │ + * │ + * linuxMain + * ``` + */ + @Test + fun `test - diamond`() { + val linuxX64Main = kotlin.linuxX64().compilations.getByName("main").defaultSourceSet + val linuxArm64Main = kotlin.linuxArm64().compilations.getByName("main").defaultSourceSet + + val commonMain = kotlin.sourceSets.commonMain.get() + val left = kotlin.sourceSets.create("left") + val right = kotlin.sourceSets.create("right") + val bottom = kotlin.sourceSets.create("bottom") + val linuxMain = kotlin.sourceSets.linuxMain.get() + + left.dependsOn(commonMain) + right.dependsOn(commonMain) + bottom.dependsOn(left) + bottom.dependsOn(right) + linuxMain.dependsOn(bottom) + + linuxX64Main.dependsOn(linuxMain) + linuxArm64Main.dependsOn(linuxMain) + + project.evaluate() + + /* + ⚠️ We expect 'left' to be listed before 'right' as this reflects + the order of 'dependsOn()' calls. + + If the order changed, please investigate the root cause; Do not update the assertion! + */ + assertEquals( + listOf("test_bottom.klib", "test_left.klib", "test_right.klib", "test_commonMain.klib"), + kotlin.sourceSets.linuxMain.get().dependsOnClosureCompilePath.toList().map { it.name } + ) + } +}