From 350a5dfa6c53263ef1e18690f28bbef7b11754d4 Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov <654232+AlexeyTsvetkov@users.noreply.github.com> Date: Thu, 17 Aug 2023 18:45:01 +0200 Subject: [PATCH] Fix cache kind management with nested subprojects (#3519) * Fix cache kind management with nested subprojects Previously, cache kind property management worked incorrectly when Compose Gradle plugin was applied to both parent and child subprojects, e.g. :compose-subproject-1:compose-subproject-2. With this example the plugin would successfully set the property for compose-subproject-1, but then for compose-subproject-2 the following snippet would fail: ``` if (project.hasProperty(targetCacheKindPropertyName)) { project.setProperty(targetCacheKindPropertyName, NONE_VALUE) } ``` because project.hasProperty would have return true (because it checks parent subproject properties too), but project.setProperty would fail, because parent project's properties are read only. Warnings were also handled incorrectly in this case, because during the configuration of compose-subproject-1 we might set `kotlin.native.cacheKind.ios*=none`, which would then cause a warning during the configuration of compose-subproject-2. To avoid incorrect warnings, we now record the snapshot of relevant properties during Compose Multiplatform build service initialization Resolves #3515 * Fix issues from code review --- .../ComposeMultiplatformBuildService.kt | 33 ++++++++-- .../configureNativeCompilerCaching.kt | 52 ++++++++++----- .../internal/utils/KGPPropertyProvider.kt | 10 +-- .../compose/internal/utils/fileUtils.kt | 7 ++- .../tests/integration/GradlePluginTest.kt | 63 ++++++++++++++----- .../misc/nativeCacheKind/settings.gradle | 3 +- .../nativeCacheKind/subproject/build.gradle | 27 ++++++++ .../subproject/src/commonMain/kotlin/App.kt | 10 +++ .../nativeCacheKindWarning/settings.gradle | 3 +- .../subproject/build.gradle | 20 ++++++ .../subproject/src/commonMain/kotlin/App.kt | 10 +++ 11 files changed, 194 insertions(+), 44 deletions(-) create mode 100644 gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/subproject/build.gradle create mode 100644 gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/subproject/src/commonMain/kotlin/App.kt create mode 100644 gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/subproject/build.gradle create mode 100644 gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/subproject/src/commonMain/kotlin/App.kt diff --git a/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/ComposeMultiplatformBuildService.kt b/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/ComposeMultiplatformBuildService.kt index e8da1a73c4..85bf9b673b 100644 --- a/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/ComposeMultiplatformBuildService.kt +++ b/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/ComposeMultiplatformBuildService.kt @@ -2,24 +2,35 @@ package org.jetbrains.compose import org.gradle.api.Project import org.gradle.api.logging.Logging +import org.gradle.api.provider.MapProperty import org.gradle.api.provider.Provider import org.gradle.api.provider.SetProperty import org.gradle.api.services.BuildService import org.gradle.api.services.BuildServiceParameters import org.gradle.tooling.events.FinishEvent import org.gradle.tooling.events.OperationCompletionListener +import org.jetbrains.compose.experimental.internal.SUPPORTED_NATIVE_CACHE_KIND_PROPERTIES import org.jetbrains.compose.internal.utils.BuildEventsListenerRegistryProvider +import org.jetbrains.compose.internal.utils.loadProperties +import org.jetbrains.compose.internal.utils.localPropertiesFile import org.jetbrains.kotlin.gradle.plugin.SubpluginArtifact // The service implements OperationCompletionListener just so Gradle would use the service // even if the service is not used by any task or transformation -abstract class ComposeMultiplatformBuildService : BuildService, +abstract class ComposeMultiplatformBuildService : BuildService, OperationCompletionListener, AutoCloseable { + abstract class Parameters : BuildServiceParameters { + abstract val gradlePropertiesCacheKindSnapshot: MapProperty + abstract val localPropertiesCacheKindSnapshot: MapProperty + } + private val log = Logging.getLogger(this.javaClass) internal abstract val unsupportedCompilerPlugins: SetProperty> internal abstract val delayedWarnings: SetProperty + internal val gradlePropertiesSnapshot: Map = parameters.gradlePropertiesCacheKindSnapshot.get() + internal val localPropertiesSnapshot: Map = parameters.localPropertiesCacheKindSnapshot.get() fun warnOnceAfterBuild(message: String) { delayedWarnings.add(message) @@ -74,6 +85,9 @@ abstract class ComposeMultiplatformBuildService : BuildService + gradlePropertiesCacheKindSnapshot.put(cacheKindProperty, value) + } + localProperties[cacheKindProperty]?.toString()?.let { value -> + localPropertiesCacheKindSnapshot.put(cacheKindProperty, value) + } + } + } } } - diff --git a/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/experimental/internal/configureNativeCompilerCaching.kt b/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/experimental/internal/configureNativeCompilerCaching.kt index bc0b7b2296..b7c279d417 100644 --- a/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/experimental/internal/configureNativeCompilerCaching.kt +++ b/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/experimental/internal/configureNativeCompilerCaching.kt @@ -12,24 +12,40 @@ import org.jetbrains.compose.internal.mppExt import org.jetbrains.compose.internal.utils.KGPPropertyProvider import org.jetbrains.compose.internal.utils.configureEachWithType import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget +import org.jetbrains.kotlin.konan.target.KonanTarget import org.jetbrains.kotlin.konan.target.presetName private const val PROJECT_CACHE_KIND_PROPERTY_NAME = "kotlin.native.cacheKind" private const val COMPOSE_NATIVE_MANAGE_CACHE_KIND = "compose.kotlin.native.manageCacheKind" private const val NONE_VALUE = "none" +private val SUPPORTED_NATIVE_TARGETS = setOf( + KonanTarget.IOS_ARM32, + KonanTarget.IOS_X64, + KonanTarget.IOS_ARM64, + KonanTarget.IOS_SIMULATOR_ARM64, + KonanTarget.MACOS_X64, + KonanTarget.MACOS_ARM64, +) + +internal val SUPPORTED_NATIVE_CACHE_KIND_PROPERTIES = + SUPPORTED_NATIVE_TARGETS.map { it.targetCacheKindPropertyName } + + PROJECT_CACHE_KIND_PROPERTY_NAME + internal fun Project.configureNativeCompilerCaching() { if (findProperty(COMPOSE_NATIVE_MANAGE_CACHE_KIND) == "false") return plugins.withId(KOTLIN_MPP_PLUGIN_ID) { mppExt.targets.configureEachWithType { - checkCacheKindUserValueIsNotNone() - disableKotlinNativeCache() + if (konanTarget in SUPPORTED_NATIVE_TARGETS) { + checkExplicitCacheKind() + disableKotlinNativeCache() + } } } } -private fun KotlinNativeTarget.checkCacheKindUserValueIsNotNone() { +private fun KotlinNativeTarget.checkExplicitCacheKind() { // To determine cache kind KGP checks kotlin.native.cacheKind. first, then kotlin.native.cacheKind // For each property it tries to read Project.property, then checks local.properties // See https://github.com/JetBrains/kotlin/blob/d4d30dcfcf1afb083f09279c6f1ba05031efeabb/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesProvider.kt#L416 @@ -43,35 +59,41 @@ private fun KotlinNativeTarget.checkCacheKindUserValueIsNotNone() { for (provider in propertyProviders) { val value = provider.valueOrNull(cacheKindProperty) if (value != null) { - if (value.equals(NONE_VALUE, ignoreCase = true)) { - ComposeMultiplatformBuildService - .getInstance(project) - .warnOnceAfterBuild(cacheKindPropertyWarningMessage(cacheKindProperty, provider)) - } + ComposeMultiplatformBuildService + .getInstance(project) + .warnOnceAfterBuild( + explicitCacheKindWarningMessage(cacheKindProperty, value, provider) + ) return } } } } -private fun cacheKindPropertyWarningMessage( +private fun explicitCacheKindWarningMessage( cacheKindProperty: String, + value: String, provider: KGPPropertyProvider ) = """ - |Warning: '$cacheKindProperty' is explicitly set to `none`. - |Compose Multiplatform Gradle plugin can manage this property automatically - |based on a Kotlin compiler version being used. + |Warning: '$cacheKindProperty' is explicitly set to '$value'. + |Compose Multiplatform Gradle plugin manages this property automatically based on a Kotlin compiler version being used. |In future versions of Compose Multiplatform this warning will become an error. - | * Recommended action: remove explicit '$cacheKindProperty=$NONE_VALUE' from ${provider.location}. + | * Recommended action: remove explicit '$cacheKindProperty=$value' from ${provider.location}. | * Alternative action: disable cache kind management by adding '$COMPOSE_NATIVE_MANAGE_CACHE_KIND=false' to your 'gradle.properties'. """.trimMargin() private val KotlinNativeTarget.targetCacheKindPropertyName: String - get() = "$PROJECT_CACHE_KIND_PROPERTY_NAME.${konanTarget.presetName}" + get() = konanTarget.targetCacheKindPropertyName + +private val KonanTarget.targetCacheKindPropertyName: String + get() = "$PROJECT_CACHE_KIND_PROPERTY_NAME.${presetName}" private fun KotlinNativeTarget.disableKotlinNativeCache() { - if (project.hasProperty(targetCacheKindPropertyName)) { + val existingValue = project.findProperty(targetCacheKindPropertyName)?.toString() + if (NONE_VALUE.equals(existingValue, ignoreCase = true)) return + + if (targetCacheKindPropertyName in project.properties) { project.setProperty(targetCacheKindPropertyName, NONE_VALUE) } else { project.extensions.extraProperties.set(targetCacheKindPropertyName, NONE_VALUE) diff --git a/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/internal/utils/KGPPropertyProvider.kt b/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/internal/utils/KGPPropertyProvider.kt index bef38b4a25..ffff63ce16 100644 --- a/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/internal/utils/KGPPropertyProvider.kt +++ b/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/internal/utils/KGPPropertyProvider.kt @@ -6,6 +6,7 @@ package org.jetbrains.compose.internal.utils import org.gradle.api.Project +import org.jetbrains.compose.ComposeMultiplatformBuildService import java.util.* /** @@ -23,13 +24,14 @@ internal abstract class KGPPropertyProvider { abstract val location: String class GradleProperties(private val project: Project) : KGPPropertyProvider() { - override fun valueOrNull(propertyName: String): String? = project.findProperty(propertyName)?.toString() + override fun valueOrNull(propertyName: String): String? = + ComposeMultiplatformBuildService.getInstance(project).gradlePropertiesSnapshot[propertyName] override val location: String = "gradle.properties" } - class LocalProperties(project: Project) : KGPPropertyProvider() { - private val localProperties: Properties by lazyLoadProperties(project.localPropertiesFile) - override fun valueOrNull(propertyName: String): String? = localProperties.getProperty(propertyName) + class LocalProperties(private val project: Project) : KGPPropertyProvider() { + override fun valueOrNull(propertyName: String): String? = + ComposeMultiplatformBuildService.getInstance(project).localPropertiesSnapshot[propertyName] override val location: String = "local.properties" } } \ No newline at end of file diff --git a/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/internal/utils/fileUtils.kt b/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/internal/utils/fileUtils.kt index cb1cc91f4b..46c3a2a1bd 100644 --- a/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/internal/utils/fileUtils.kt +++ b/gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/internal/utils/fileUtils.kt @@ -59,11 +59,14 @@ private fun Array>.ioFiles(): Array = let { providers -> Array(size) { i -> providers[i].ioFile } } internal fun lazyLoadProperties(propertiesFile: File): Lazy = lazy { + loadProperties(propertiesFile) +} + +internal fun loadProperties(propertiesFile: File): Properties = Properties().apply { if (propertiesFile.isFile) { propertiesFile.inputStream().use { load(it) } } - } -} \ No newline at end of file + } \ No newline at end of file diff --git a/gradle-plugins/compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/GradlePluginTest.kt b/gradle-plugins/compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/GradlePluginTest.kt index e47b9ca371..2e47c3946a 100644 --- a/gradle-plugins/compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/GradlePluginTest.kt +++ b/gradle-plugins/compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/GradlePluginTest.kt @@ -116,7 +116,7 @@ class GradlePluginTest : GradlePluginTestBase() { defaultTestEnvironment.copy(kotlinVersion = kotlinVersion, useGradleConfigurationCache = false) ) - val task = ":linkDebugFrameworkIosX64" + val task = ":subproject:linkDebugFrameworkIosX64" with(nativeCacheKindProject(kotlinVersion = TestKotlinVersions.v1_8_20)) { gradle(task, "--info").checks { check.taskSuccessful(task) @@ -136,26 +136,55 @@ class GradlePluginTest : GradlePluginTestBase() { @Test fun nativeCacheKindWarning() { Assumptions.assumeTrue(currentOS == OS.MacOS) - fun nativeCacheKindWarningProject(kotlinVersion: String) = testProject( - TestProjects.nativeCacheKindWarning, - defaultTestEnvironment.copy(kotlinVersion = kotlinVersion) - ) - - val cacheKindWarning = "'kotlin.native.cacheKind' is explicitly set to `none`" - - val args = arrayOf("build", "--dry-run", "-Pkotlin.native.cacheKind=none") - with(nativeCacheKindWarningProject(kotlinVersion = TestKotlinVersions.v1_8_20)) { - gradle(*args).checks { - check.logContains(cacheKindWarning) + fun withNativeCacheKindWarningProject(kotlinVersion: String, fn: TestProject.() -> Unit) { + with(testProject( + TestProjects.nativeCacheKindWarning, + defaultTestEnvironment.copy(kotlinVersion = kotlinVersion) + )) { + fn() + testWorkDir.deleteRecursively() + testWorkDir.mkdirs() } } - testWorkDir.deleteRecursively() - testWorkDir.mkdirs() - with(nativeCacheKindWarningProject(kotlinVersion = TestKotlinVersions.v1_9_0) ) { - gradle(*args).checks { - check.logContains(cacheKindWarning) + + fun testKotlinVersion(kotlinVersion: String) { + val args = arrayOf("build", "--dry-run") + val commonPartOfWarning = "Compose Multiplatform Gradle plugin manages this property automatically" + withNativeCacheKindWarningProject(kotlinVersion = kotlinVersion) { + gradle(*args).checks { + check.logDoesntContain("Warning: 'kotlin.native.cacheKind") + check.logDoesntContain(commonPartOfWarning) + } + } + withNativeCacheKindWarningProject(kotlinVersion = kotlinVersion) { + gradle(*args, "-Pkotlin.native.cacheKind=none").checks { + check.logContainsOnce("Warning: 'kotlin.native.cacheKind' is explicitly set to 'none'") + check.logContainsOnce(commonPartOfWarning) + } + } + withNativeCacheKindWarningProject(kotlinVersion = kotlinVersion) { + gradle(*args, "-Pkotlin.native.cacheKind=static").checks { + check.logContainsOnce("Warning: 'kotlin.native.cacheKind' is explicitly set to 'static'") + check.logContainsOnce(commonPartOfWarning) + } + } + withNativeCacheKindWarningProject(kotlinVersion = kotlinVersion) { + gradle(*args, "-Pkotlin.native.cacheKind.iosX64=none").checks { + check.logContainsOnce("Warning: 'kotlin.native.cacheKind.iosX64' is explicitly set to 'none'") + check.logContainsOnce(commonPartOfWarning) + } + } + withNativeCacheKindWarningProject(kotlinVersion = kotlinVersion) { + gradle(*args, "-Pkotlin.native.cacheKind.iosX64=static").checks { + check.logContainsOnce("Warning: 'kotlin.native.cacheKind.iosX64' is explicitly set to 'static'") + check.logContainsOnce(commonPartOfWarning) + } } } + + + testKotlinVersion(TestKotlinVersions.v1_8_20) + testKotlinVersion(TestKotlinVersions.v1_9_0) } @Test diff --git a/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/settings.gradle b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/settings.gradle index a270b9409b..6155b39d24 100644 --- a/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/settings.gradle +++ b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/settings.gradle @@ -9,4 +9,5 @@ pluginManagement { maven { url "https://maven.pkg.jetbrains.space/kotlin/p/kotlin/dev" } } } -rootProject.name = "nativeCacheKind" \ No newline at end of file +rootProject.name = "nativeCacheKind" +include(":subproject") \ No newline at end of file diff --git a/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/subproject/build.gradle b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/subproject/build.gradle new file mode 100644 index 0000000000..1e43a147b8 --- /dev/null +++ b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/subproject/build.gradle @@ -0,0 +1,27 @@ +plugins { + id "org.jetbrains.kotlin.multiplatform" + id "org.jetbrains.compose" +} + +kotlin { + iosX64 { + binaries.framework { + isStatic = true + baseName = "shared" + } + } + iosArm64 { + binaries.framework { + isStatic = true + baseName = "shared" + } + } + + sourceSets { + commonMain { + dependencies { + implementation(compose.runtime) + } + } + } +} \ No newline at end of file diff --git a/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/subproject/src/commonMain/kotlin/App.kt b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/subproject/src/commonMain/kotlin/App.kt new file mode 100644 index 0000000000..2fe6d0719f --- /dev/null +++ b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKind/subproject/src/commonMain/kotlin/App.kt @@ -0,0 +1,10 @@ +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue + +@Composable +fun App() { + var text by remember { mutableStateOf("Hello, World!") } +} diff --git a/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/settings.gradle b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/settings.gradle index a270b9409b..6155b39d24 100644 --- a/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/settings.gradle +++ b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/settings.gradle @@ -9,4 +9,5 @@ pluginManagement { maven { url "https://maven.pkg.jetbrains.space/kotlin/p/kotlin/dev" } } } -rootProject.name = "nativeCacheKind" \ No newline at end of file +rootProject.name = "nativeCacheKind" +include(":subproject") \ No newline at end of file diff --git a/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/subproject/build.gradle b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/subproject/build.gradle new file mode 100644 index 0000000000..40170b68cb --- /dev/null +++ b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/subproject/build.gradle @@ -0,0 +1,20 @@ +plugins { + id "org.jetbrains.kotlin.multiplatform" + id "org.jetbrains.compose" +} + +kotlin { + iosX64() + iosArm64() + iosSimulatorArm64() + macosX64() + macosArm64() + + sourceSets { + commonMain { + dependencies { + implementation(compose.runtime) + } + } + } +} \ No newline at end of file diff --git a/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/subproject/src/commonMain/kotlin/App.kt b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/subproject/src/commonMain/kotlin/App.kt new file mode 100644 index 0000000000..2fe6d0719f --- /dev/null +++ b/gradle-plugins/compose/src/test/test-projects/misc/nativeCacheKindWarning/subproject/src/commonMain/kotlin/App.kt @@ -0,0 +1,10 @@ +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue + +@Composable +fun App() { + var text by remember { mutableStateOf("Hello, World!") } +}