From 97d71dc160d45d6cb7a191d896883805a86e8cb7 Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Thu, 2 Sep 2021 08:54:19 +0200 Subject: [PATCH] [Gradle] Ensure enableGranularSourceSetsMetadata property being populated Previously it was only populated when KGP was applied to the root project. CLI compilation would still work, since it also checks for the new hmpp properties, whereas the IDE import is still 'just' checking on the old property (while defining its own defaults) ^KT-48513 Verification Pending --- .../IdeImportPropertiesConsistencyTest.kt | 98 +++++++++++++++++++ .../gradle/MultiplatformExtensionTest.kt | 29 +++++- .../kotlin/gradle/plugin/KotlinProperties.kt | 15 +-- .../hierarchicalStructureMigrationHandling.kt | 9 ++ .../KotlinNativePlatformDependencies.kt | 4 +- 5 files changed, 141 insertions(+), 14 deletions(-) create mode 100644 libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/IdeImportPropertiesConsistencyTest.kt diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/IdeImportPropertiesConsistencyTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/IdeImportPropertiesConsistencyTest.kt new file mode 100644 index 00000000000..6018bbb19eb --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/IdeImportPropertiesConsistencyTest.kt @@ -0,0 +1,98 @@ +/* + * Copyright 2010-2021 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. + */ +/* Associate compilations are not yet supported by the IDE. KT-34102 */ +@file:Suppress("invisible_reference", "invisible_member", "FunctionName") + +package org.jetbrains.kotlin.gradle + +import org.gradle.api.Project +import org.gradle.testfixtures.ProjectBuilder +import org.jetbrains.kotlin.gradle.targets.metadata.isKotlinGranularMetadataEnabled +import org.jetbrains.kotlin.gradle.targets.native.internal.isNativeDependencyPropagationEnabled +import kotlin.test.Test +import kotlin.test.assertEquals + +class IdeImportPropertiesConsistencyTest { + + /* + IDE IMPORT SOURCE CODE: + https://youtrack.jetbrains.com/issue/KTIJ-19551 + + As described in the ticket above, we have a separate implementation running in the IDE import that + will resolve properties on the project. Unfortunately, this implementation will choose its own defaults. + Changing defaults for such properties therefore can't be done easily in the KGP, without taking + extra precaution ensuring that the IDE plugin would still receive the correct property. + + Until the implementation is changed and IDEs with old implementation are out of support, + this code is copied into this test here and checked for consistency! + */ + private fun Project.getProperty(property: GradleImportProperties): Boolean { + val explicitValueIfAny = try { + (findProperty(property.id) as? String)?.toBoolean() + } catch (e: Exception) { + logger.error("Error while trying to read property $property from project $project", e) + null + } + + return explicitValueIfAny ?: property.defaultValue + } + + private enum class GradleImportProperties(val id: String, val defaultValue: Boolean) { + IS_HMPP_ENABLED("kotlin.mpp.enableGranularSourceSetsMetadata", false), + ENABLE_NATIVE_DEPENDENCY_PROPAGATION("kotlin.native.enableDependencyPropagation", true), + /* IDE only relevant cases omitted */ + ; + } + + /* + END OF IDE IMPORT SOURCE CODE! + */ + + @Test + fun `test simple project`() { + val project = ProjectBuilder.builder().build() + project.applyMultiplatformPlugin() + project.assertPropertiesMatch() + } + + @Test + fun `test simple project with new hmpp flag`() { + val project = ProjectBuilder.builder().build() + project.enableHierarchicalStructureByDefault() + project.applyMultiplatformPlugin() + project.assertPropertiesMatch() + } + + @Test + fun `test sub project with new hmpp flag`() { + val rootProject = ProjectBuilder.builder().build() + val project = ProjectBuilder.builder().withParent(rootProject).build() + rootProject.enableHierarchicalStructureByDefault() + project.applyMultiplatformPlugin() + project.assertPropertiesMatch() + } + + private fun Project.assertPropertiesMatch() { + val isHmppValueUsedInCli = project.isKotlinGranularMetadataEnabled + val isHmppValueUsedInIde = project.getProperty(GradleImportProperties.IS_HMPP_ENABLED) + assertEquals( + isHmppValueUsedInCli, isHmppValueUsedInIde, + """ + project.isKotlinGranularMetadataEnabled: $isHmppValueUsedInCli + GradleImportProperties.IS_HMPP_ENABLED: ${isHmppValueUsedInIde} + """.trimIndent() + ) + + val dependencyPropagationEnabledInCli = project.isNativeDependencyPropagationEnabled + val dependencyPropagationEnabledInIde = project.getProperty(GradleImportProperties.ENABLE_NATIVE_DEPENDENCY_PROPAGATION) + assertEquals( + dependencyPropagationEnabledInCli, dependencyPropagationEnabledInIde, + """ + propertiesProvider.nativeDependencyPropagation: $dependencyPropagationEnabledInCli + GradleImportProperties.ENABLE_NATIVE_DEPENDENCY_PROPAGATION: $dependencyPropagationEnabledInIde + """.trimIndent() + ) + } +} \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/MultiplatformExtensionTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/MultiplatformExtensionTest.kt index 0eeeeacb04e..ce8e4869d30 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/MultiplatformExtensionTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/MultiplatformExtensionTest.kt @@ -8,11 +8,13 @@ package org.jetbrains.kotlin.gradle +import org.gradle.api.Project import org.gradle.api.internal.project.ProjectInternal import org.gradle.api.plugins.ExtraPropertiesExtension import org.gradle.testfixtures.ProjectBuilder import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet +import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider.PropertyNames.KOTLIN_MPP_HIERARCHICAL_STRUCTURE_BY_DEFAULT import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinSharedNativeCompilation import org.jetbrains.kotlin.gradle.plugin.mpp.pm20.KotlinCompilationData @@ -28,16 +30,15 @@ abstract class MultiplatformExtensionTest { @BeforeTest open fun setup() { - project.plugins.apply("kotlin-multiplatform") - kotlin = project.extensions.getByName("kotlin") as KotlinMultiplatformExtension + kotlin = project.applyMultiplatformPlugin() } protected fun enableGranularSourceSetsMetadata() { - project.extensions.getByType(ExtraPropertiesExtension::class.java).set("kotlin.mpp.enableGranularSourceSetsMetadata", "true") + project.enableGranularSourceSetsMetadata() } protected fun enableCInteropCommonization() { - project.extensions.getByType(ExtraPropertiesExtension::class.java).set("kotlin.mpp.enableCInteropCommonization", "true") + project.enableCInteropCommonization() } internal fun expectCInteropCommonizerDependent(compilation: KotlinSharedNativeCompilation): CInteropCommonizerDependent { @@ -76,4 +77,24 @@ abstract class MultiplatformExtensionTest { internal fun KotlinCompilationData<*>.cinteropIdentifier(name: String): CInteropIdentifier { return CInteropIdentifier(CInteropIdentifier.Scope.create(this), name) } +} + +fun Project.applyMultiplatformPlugin(): KotlinMultiplatformExtension { + plugins.apply("kotlin-multiplatform") + return extensions.getByName("kotlin") as KotlinMultiplatformExtension +} + +val Project.propertiesExtension: ExtraPropertiesExtension + get() = extensions.getByType(ExtraPropertiesExtension::class.java) + +fun Project.enableGranularSourceSetsMetadata() { + propertiesExtension.set("kotlin.mpp.enableGranularSourceSetsMetadata", "true") +} + +fun Project.enableCInteropCommonization() { + propertiesExtension.set("kotlin.mpp.enableCInteropCommonization", "true") +} + +fun Project.enableHierarchicalStructureByDefault() { + propertiesExtension.set(KOTLIN_MPP_HIERARCHICAL_STRUCTURE_BY_DEFAULT, "true") } \ No newline at end of file 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 62f1aad4784..2030e0b9de2 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 @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.gradle.internal.testing.TCServiceMessageOutputStream import org.jetbrains.kotlin.gradle.plugin.Kotlin2JsPlugin.Companion.NOWARN_2JS_FLAG import org.jetbrains.kotlin.gradle.plugin.KotlinJsCompilerType.Companion.jsCompilerProperty import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider.PropertyNames.KOTLIN_MPP_ENABLE_GRANULAR_SOURCE_SETS_METADATA +import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider.PropertyNames.KOTLIN_MPP_HIERARCHICAL_STRUCTURE_BY_DEFAULT import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider.PropertyNames.KOTLIN_MPP_HIERARCHICAL_STRUCTURE_SUPPORT import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider.PropertyNames.KOTLIN_NATIVE_DEPENDENCY_PROPAGATION import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinMultiplatformPlugin @@ -27,7 +28,6 @@ import org.jetbrains.kotlin.gradle.tasks.CompileUsingKotlinDaemon import org.jetbrains.kotlin.gradle.tasks.Kotlin2JsCompile import org.jetbrains.kotlin.gradle.tasks.KotlinCompile import org.jetbrains.kotlin.gradle.utils.SingleWarningPerBuild -import org.jetbrains.kotlin.gradle.utils.getSystemProperty import org.jetbrains.kotlin.konan.target.KonanTarget import org.jetbrains.kotlin.konan.target.presetName import org.jetbrains.kotlin.statistics.metrics.StringMetrics @@ -61,7 +61,7 @@ internal fun PropertiesProvider.mapKotlinDaemonProperties(task: CompileUsingKotl } } -internal class PropertiesProvider private constructor(private val project: Project) { + internal class PropertiesProvider private constructor(private val project: Project) { private val localProperties: Properties by lazy { Properties().apply { val localPropertiesFile = project.rootProject.file("local.properties") @@ -148,7 +148,7 @@ internal class PropertiesProvider private constructor(private val project: Proje set(value) { project.extensions.extraProperties.set("kotlin.internal.mpp.13X.flags.setByPlugin") { "$value" } } val mppHierarchicalStructureByDefault: Boolean - get() = booleanProperty("kotlin.internal.mpp.hierarchicalStructureByDefault") ?: false + get() = booleanProperty(KOTLIN_MPP_HIERARCHICAL_STRUCTURE_BY_DEFAULT) ?: false val enableCompatibilityMetadataVariant: Boolean get() = booleanProperty("kotlin.mpp.enableCompatibilityMetadataVariant") ?: !mppHierarchicalStructureByDefault @@ -391,10 +391,11 @@ internal class PropertiesProvider private constructor(private val project: Proje localProperties.getProperty(propName) } - internal object PropertyNames { - internal const val KOTLIN_MPP_ENABLE_GRANULAR_SOURCE_SETS_METADATA = "kotlin.mpp.enableGranularSourceSetsMetadata" - internal const val KOTLIN_MPP_HIERARCHICAL_STRUCTURE_SUPPORT = "kotlin.mpp.hierarchicalStructureSupport" - internal const val KOTLIN_NATIVE_DEPENDENCY_PROPAGATION = "kotlin.native.enableDependencyPropagation" + object PropertyNames { + const val KOTLIN_MPP_ENABLE_GRANULAR_SOURCE_SETS_METADATA = "kotlin.mpp.enableGranularSourceSetsMetadata" + const val KOTLIN_MPP_HIERARCHICAL_STRUCTURE_BY_DEFAULT = "kotlin.internal.mpp.hierarchicalStructureByDefault" + const val KOTLIN_MPP_HIERARCHICAL_STRUCTURE_SUPPORT = "kotlin.mpp.hierarchicalStructureSupport" + const val KOTLIN_NATIVE_DEPENDENCY_PROPAGATION = "kotlin.native.enableDependencyPropagation" } companion object { diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/internal/hierarchicalStructureMigrationHandling.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/internal/hierarchicalStructureMigrationHandling.kt index 52942545b0c..9cc02c2e5fb 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/internal/hierarchicalStructureMigrationHandling.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/internal/hierarchicalStructureMigrationHandling.kt @@ -9,10 +9,19 @@ import org.gradle.api.GradleException import org.gradle.api.Project import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider.PropertyNames +import org.jetbrains.kotlin.gradle.utils.SingleActionPerProject import org.jetbrains.kotlin.gradle.utils.SingleWarningPerBuild import org.jetbrains.kotlin.gradle.utils.getOrPut internal fun handleHierarchicalStructureFlagsMigration(project: Project) { + SingleActionPerProject.run(project.rootProject, "handleHierarchicalStructureFlagsMigration - rootProject") { + doHandleHierarchicalStructureFlagsMigration(project.rootProject) + } + + doHandleHierarchicalStructureFlagsMigration(project) +} + +private fun doHandleHierarchicalStructureFlagsMigration(project: Project) { with(PropertiesProvider(project)) { checkHmppFeatureFlagsForConsistency(project) 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 0bcd6ea7f44..0477d2183b5 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 @@ -12,8 +12,6 @@ import org.jetbrains.kotlin.compilerRunner.konanHome import org.jetbrains.kotlin.gradle.dsl.multiplatformExtension import org.jetbrains.kotlin.gradle.dsl.multiplatformExtensionOrNull import org.jetbrains.kotlin.gradle.plugin.* -import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider -import org.jetbrains.kotlin.gradle.plugin.compareVersionNumbers import org.jetbrains.kotlin.gradle.plugin.sources.DefaultKotlinSourceSet import org.jetbrains.kotlin.gradle.targets.metadata.getMetadataCompilationForSourceSet import org.jetbrains.kotlin.gradle.targets.metadata.isKotlinGranularMetadataEnabled @@ -74,7 +72,7 @@ private fun File.listLibraryFiles(): List = listFiles().orEmpty() .filter { it.isDirectory || it.extension == "klib" } -private val Project.isNativeDependencyPropagationEnabled: Boolean +internal val Project.isNativeDependencyPropagationEnabled: Boolean get() = PropertiesProvider(this).nativeDependencyPropagation ?: true //for reflection call from KotlinCommonizerModelBuilder