From d9eb130f8244a4ffe1ba5efccebf512ce17834db Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Tue, 6 Apr 2021 15:52:28 +0200 Subject: [PATCH] [Gradle] Enable hierarchical commonization by default --- .../gradle.properties | 1 + .../gradle/CInteropCommonizerTaskTest.kt | 17 +---------- .../kotlin/gradle/plugin/KotlinProperties.kt | 2 +- .../native/internal/CInteropCommonizerTask.kt | 3 +- .../native/internal/CommonizerTasks.kt | 28 +++++++++++-------- ...rchicalKotlinNativePlatformDependencies.kt | 23 +++++++++------ .../KotlinNativePlatformDependencies.kt | 2 +- 7 files changed, 36 insertions(+), 40 deletions(-) diff --git a/idea/testData/gradle/importAndCheckNavigation/navigationToCommonizedLibrary/gradle.properties b/idea/testData/gradle/importAndCheckNavigation/navigationToCommonizedLibrary/gradle.properties index 1d0d44f1d8e..04763deac75 100644 --- a/idea/testData/gradle/importAndCheckNavigation/navigationToCommonizedLibrary/gradle.properties +++ b/idea/testData/gradle/importAndCheckNavigation/navigationToCommonizedLibrary/gradle.properties @@ -1,2 +1,3 @@ kotlin.mpp.enableGranularSourceSetsMetadata=true kotlin.native.enableDependencyPropagation=false +kotlin.mpp.enableHierarchicalCommonization=false diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/CInteropCommonizerTaskTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/CInteropCommonizerTaskTest.kt index 6c2bc49e677..6324cd232f5 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/CInteropCommonizerTaskTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/CInteropCommonizerTaskTest.kt @@ -12,6 +12,7 @@ import org.gradle.api.internal.project.ProjectInternal import org.gradle.api.plugins.ExtraPropertiesExtension import org.gradle.testfixtures.ProjectBuilder import org.jetbrains.kotlin.commonizer.CommonizerTarget +import org.jetbrains.kotlin.commonizer.SharedCommonizerTarget import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinSharedNativeCompilation @@ -114,41 +115,25 @@ class CInteropCommonizerTaskTest { project.evaluate() - /* - //https://youtrack.jetbrains.com/issue/KT-39324 assertEquals( CInteropCommonizationParameters( SharedCommonizerTarget(CommonizerTarget(IOS_X64, IOS_ARM64), CommonizerTarget(MACOS_X64), CommonizerTarget(LINUX_X64)), setOf(linuxInterop, macosInterop, iosX64Interop, iosArm64Interop) ), task.getCommonizationParameters(sharedNativeCompilation(nativeMain)) ) - */ - assertNull( - task.getCommonizationParameters(sharedNativeCompilation(nativeMain)) - ) - /* - //https://youtrack.jetbrains.com/issue/KT-39324 assertEquals( CInteropCommonizationParameters( SharedCommonizerTarget(CommonizerTarget(IOS_X64, IOS_ARM64), CommonizerTarget(MACOS_X64), CommonizerTarget(LINUX_X64)), setOf(linuxInterop, macosInterop, iosX64Interop, iosArm64Interop) ), task.getCommonizationParameters(sharedNativeCompilation(iosMain)) ) - */ - assertEquals( - CInteropCommonizationParameters(CommonizerTarget(IOS_X64, IOS_ARM64), setOf(iosX64Interop, iosArm64Interop)), - task.getCommonizationParameters(sharedNativeCompilation(iosMain)) - ) - /* - //https://youtrack.jetbrains.com/issue/KT-39324 assertTrue( task.getCommonizationParameters(sharedNativeCompilation(iosMain))!! in task.getCommonizationParameters(sharedNativeCompilation(nativeMain))!!, "Expected CInteropCommonizerTarget of iosMain to be fully contained in nativeMain" ) - */ } private fun sharedNativeCompilation(sourceSet: KotlinSourceSet): KotlinSharedNativeCompilation { diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinProperties.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinProperties.kt index fc868145653..9a3b1c25f46 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinProperties.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinProperties.kt @@ -228,7 +228,7 @@ internal class PropertiesProvider private constructor(private val project: Proje * Enables experimental commonization of 'higher level' shared native source sets */ val enableHierarchicalCommonization: Boolean - get() = booleanProperty("kotlin.mpp.enableHierarchicalCommonization") ?: false + get() = booleanProperty("kotlin.mpp.enableHierarchicalCommonization") ?: true /** * Dependencies caching strategy for all targets that support caches. diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CInteropCommonizerTask.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CInteropCommonizerTask.kt index d60909e3682..4b9606741af 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CInteropCommonizerTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CInteropCommonizerTask.kt @@ -48,7 +48,6 @@ internal open class CInteropCommonizerTask : AbstractCInteropCommonizerTask() { internal var cinterops = setOf() private set - @OutputDirectories fun getAllOutputDirectories(): Set { return getCommonizationParameters().map { outputDirectory(it) }.toSet() @@ -101,7 +100,7 @@ internal open class CInteropCommonizerTask : AbstractCInteropCommonizerTask() { } private fun nativeDistributionDependencies(parameters: CInteropCommonizationParameters): Set { - val task = project.commonizeNativeDistributionHierarchicalTask?.get() ?: return emptySet() + val task = project.commonizeNativeDistributionHierarchicallyTask?.get() ?: return emptySet() val rootTarget = task.rootCommonizerTargets .firstOrNull { rootTarget -> parameters.commonizerTarget in rootTarget } ?: return emptySet() diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CommonizerTasks.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CommonizerTasks.kt index 0e8abc1ca89..4f6b0f937ca 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CommonizerTasks.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CommonizerTasks.kt @@ -49,7 +49,8 @@ internal val Project.commonizeCInteropTask: TaskProvider "commonizeCInterop", invokeWhenRegistered = { commonizeTask.dependsOn(this) - commonizeNativeDistributionHierarchicalTask?.let(this::dependsOn) ?: dependsOn(commonizeNativeDistributionTask) + commonizeNativeDistributionHierarchicallyTask?.let(this::dependsOn) + commonizeNativeDistributionTask?.let(this::dependsOn) }, configureTask = { group = "interop" @@ -76,21 +77,24 @@ internal val Project.copyCommonizeCInteropForIdeTask: TaskProvider - get() = locateOrRegisterTask( - "commonizeNativeDistribution", - invokeWhenRegistered = { commonizeTask.dependsOn(this) }, - configureTask = { - group = "interop" - description = "Invokes the commonizer on the platform libraries provided by the Kotlin/Native distribution" - } - ) +internal val Project.commonizeNativeDistributionTask: TaskProvider? + get() { + if (isHierarchicalCommonizationEnabled) return null + return locateOrRegisterTask( + "commonizeNativeDistribution", + invokeWhenRegistered = { commonizeTask.dependsOn(this) }, + configureTask = { + group = "interop" + description = "Invokes the commonizer on the platform libraries provided by the Kotlin/Native distribution" + } + ) + } -internal val Project.commonizeNativeDistributionHierarchicalTask: TaskProvider? +internal val Project.commonizeNativeDistributionHierarchicallyTask: TaskProvider? get() { if (!isHierarchicalCommonizationEnabled) return null return locateOrRegisterTask( - "commonizeNativeDistributionHierarchically", + "commonizeNativeDistribution", invokeWhenRegistered = { commonizeTask.dependsOn(this) }, configureTask = { group = "interop" diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/HierarchicalKotlinNativePlatformDependencies.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/HierarchicalKotlinNativePlatformDependencies.kt index 9820570240b..0d201915a47 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/HierarchicalKotlinNativePlatformDependencies.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/HierarchicalKotlinNativePlatformDependencies.kt @@ -15,7 +15,7 @@ import org.jetbrains.kotlin.gradle.targets.metadata.getMetadataCompilationForSou import org.jetbrains.kotlin.gradle.utils.filesProvider internal fun Project.setUpHierarchicalKotlinNativePlatformDependencies() { - val task = commonizeNativeDistributionHierarchicalTask?.get() ?: return + val task = commonizeNativeDistributionHierarchicallyTask?.get() ?: return val kotlin = multiplatformExtensionOrNull ?: return kotlin.sourceSets.forEach { sourceSet -> val target = getCommonizerTarget(sourceSet) ?: return@forEach @@ -31,15 +31,22 @@ private fun HierarchicalNativeDistributionCommonizerTask.dependenciesFor(target: val targetOutputDirectory = HierarchicalCommonizerOutputLayout.getTargetDirectory(getRootOutputDirectory(rootTarget), target) val targetDependencies = project.filesProvider { targetOutputDirectory.listFiles().orEmpty().toList() }.builtBy(this) - if (target is LeafCommonizerTarget) { - val expectTarget = rootTarget.withAllAncestors() - .firstOrNull { (it as? SharedCommonizerTarget)?.targets?.contains(target) == true } - ?: return targetDependencies + /* + LeafCommonizerTargets will still analyze against 'new' 'commonized' platform dependencies. + This means, that some parts of the API might get lifted to the direct parent dependency. + */ + val necessaryParentTargetDependencies: FileCollection = rootTarget.takeIf { target is LeafCommonizerTarget } + ?.findParentOf(target) + ?.let { parentTarget -> dependenciesFor(parentTarget) } + ?: project.files() - return targetDependencies + dependenciesFor(expectTarget) - } + return targetDependencies + necessaryParentTargetDependencies +} - return targetDependencies +private fun CommonizerTarget.findParentOf(target: CommonizerTarget): CommonizerTarget? { + return withAllAncestors() + .filterIsInstance() + .firstOrNull { target in it.targets } } private fun Project.addDependencies(sourceSet: KotlinSourceSet, libraries: FileCollection) { diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/KotlinNativePlatformDependencies.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/KotlinNativePlatformDependencies.kt index 653e4ee8c72..5ac9f1a0e71 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/KotlinNativePlatformDependencies.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/KotlinNativePlatformDependencies.kt @@ -101,7 +101,7 @@ private class NativePlatformDependencyResolver(val project: Project, val kotlinV check(!alreadyResolved) alreadyResolved = true - project.commonizeNativeDistributionTask.configure { commonizerTask -> + project.commonizeNativeDistributionTask?.configure { commonizerTask -> commonizerTask.targetGroups = dependencies.keys.filterIsInstance().map { it.targets }.toSet() }