Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[gradle] A flag to disable multimodule resources. #4771

Merged
merged 4 commits into from
May 8, 2024
Merged

[gradle] A flag to disable multimodule resources. #4771

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
33a5b20
[gradle] Hide multimodule resources under a flag.
terrakok May 7, 2024
9a4990b
[gradle] Rename 'compose.resources.multimodule.enable, to 'org.jetbra…
terrakok May 8, 2024
98c3f76
[gradle] Change default of the multimodule resources to "enabled" and…
terrakok May 8, 2024
cf86438
Update gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/r…
terrakok May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...ain/kotlin/org/jetbrains/compose/desktop/application/internal/ComposeProjectProperties.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal object ComposeProperties {
internal const val MAC_NOTARIZATION_PASSWORD = "compose.desktop.mac.notarization.password"
internal const val MAC_NOTARIZATION_TEAM_ID_PROVIDER = "compose.desktop.mac.notarization.teamID"
internal const val CHECK_JDK_VENDOR = "compose.desktop.packaging.checkJdkVendor"
internal const val ALWAYS_GENERATE_RESOURCE_ACCESSORS = "compose.resources.always.generate.accessors"
MatkovIvan marked this conversation as resolved.
Show resolved Hide resolved
internal const val ENABLE_MULTIMODULE_RESOURCES = "compose.resources.multimodule.enable"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it about publication and depending on resource library, or only about publication?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is about a publication and a multimoduleness (very close things in the gradle)

Copy link
Collaborator

@igordmn igordmn May 7, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happen if it is false, and users add a dependency on a library with resources? Will it work without this property?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. It's false by default and all tests are green at least)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, you meant a library with resources (not the resource's library!). No, it requires the flag

internal const val SYNC_RESOURCES_PROPERTY = "compose.ios.resources.sync"

fun isVerbose(providers: ProviderFactory): Provider<Boolean> =
Expand Down Expand Up @@ -55,6 +55,9 @@ internal object ComposeProperties {
fun checkJdkVendor(providers: ProviderFactory): Provider<Boolean> =
providers.valueOrNull(CHECK_JDK_VENDOR).toBooleanProvider(true)

fun enableMultimoduleResources(providers: ProviderFactory): Provider<Boolean> =
providers.valueOrNull(ENABLE_MULTIMODULE_RESOURCES).toBooleanProvider(false)

//providers.valueOrNull works only with root gradle.properties
fun dontSyncResources(project: Project): Provider<Boolean> = project.provider {
project.findProperty(SYNC_RESOURCES_PROPERTY)?.toString().equals("false", true)
Expand Down
37 changes: 24 additions & 13 deletions gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/resources/ComposeResources.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.gradle.api.tasks.SourceSet
import org.gradle.api.tasks.TaskProvider
import org.gradle.util.GradleVersion
import org.jetbrains.compose.ComposePlugin
import org.jetbrains.compose.desktop.application.internal.ComposeProperties
import org.jetbrains.compose.internal.KOTLIN_JVM_PLUGIN_ID
import org.jetbrains.compose.internal.KOTLIN_MPP_PLUGIN_ID
import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension
Expand Down Expand Up @@ -41,23 +42,33 @@ private fun Project.onKgpApplied(config: Provider<ResourcesExtension>, kgp: Kotl
val hasKmpResources = extraProperties.has(KMP_RES_EXT)
val currentGradleVersion = GradleVersion.current()
val minGradleVersion = GradleVersion.version(MIN_GRADLE_VERSION_FOR_KMP_RESOURCES)
val kmpResourcesAreAvailable = hasKmpResources && currentGradleVersion >= minGradleVersion
val enableMultimoduleResources = ComposeProperties.enableMultimoduleResources(providers).get()
val kmpResourcesAreAvailable = enableMultimoduleResources && hasKmpResources && currentGradleVersion >= minGradleVersion

if (kmpResourcesAreAvailable) {
configureKmpResources(kotlinExtension, extraProperties.get(KMP_RES_EXT)!!, config)
} else {
if (!hasKmpResources) logger.info(
"""
Compose resources publication requires Kotlin Gradle Plugin >= 2.0
Current Kotlin Gradle Plugin is ${kgp.pluginVersion}
""".trimIndent()
)
if (currentGradleVersion < minGradleVersion) logger.info(
"""
Compose resources publication requires Gradle >= $MIN_GRADLE_VERSION_FOR_KMP_RESOURCES
Current Gradle is ${currentGradleVersion.version}
""".trimIndent()
)
if (!enableMultimoduleResources) {
logger.info(
"""
Multimodule Compose Resources are disabled by default.
To enable it add 'compose.resources.multimodule.enable=true' int the root gradle.properties file.
terrakok marked this conversation as resolved.
Show resolved Hide resolved
""".trimIndent()
)
} else {
if (!hasKmpResources) logger.info(
"""
Compose resources publication requires Kotlin Gradle Plugin >= 2.0
Current Kotlin Gradle Plugin is ${kgp.pluginVersion}
""".trimIndent()
)
if (currentGradleVersion < minGradleVersion) logger.info(
"""
Compose resources publication requires Gradle >= $MIN_GRADLE_VERSION_FOR_KMP_RESOURCES
Current Gradle is ${currentGradleVersion.version}
""".trimIndent()
)
}

val commonMain = KotlinSourceSet.COMMON_MAIN_SOURCE_SET_NAME
configureComposeResources(kotlinExtension, commonMain, config)
Expand Down
45 changes: 36 additions & 9 deletions ...ins/compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/ResourcesTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,42 +158,50 @@ class ResourcesTest : GradlePluginTestBase() {
check.logContains("${testXml.name} is not valid. Check the file content.")
}

testXml.writeText("""
testXml.writeText(
"""
<resources>
<aaa name="v">aaa</aaa>
</resources>
""".trimIndent())
""".trimIndent()
)
gradleFailure("prepareKotlinIdeaImport").checks {
check.logContains("${testXml.name} is not valid. Unknown resource type: 'aaa'.")
}

testXml.writeText("""
testXml.writeText(
"""
<resources>
<drawable name="v">aaa</drawable>
</resources>
""".trimIndent())
""".trimIndent()
)
gradleFailure("prepareKotlinIdeaImport").checks {
check.logContains("${testXml.name} is not valid. Unknown string resource type: 'drawable'.")
}

testXml.writeText("""
testXml.writeText(
"""
<resources>
<string name="v1">aaa</string>
<string name="v2">aaa</string>
<string name="v3">aaa</string>
<string name="v1">aaa</string>
</resources>
""".trimIndent())
""".trimIndent()
)
gradleFailure("prepareKotlinIdeaImport").checks {
check.logContains("${testXml.name} is not valid. Duplicated key 'v1'.")
}

testXml.writeText("""
testXml.writeText(
"""
<resources>
<string name="v1">aaa</string>
<string foo="v2">aaa</string>
</resources>
""".trimIndent())
""".trimIndent()
)
gradleFailure("prepareKotlinIdeaImport").checks {
check.logContains("${testXml.name} is not valid. Attribute 'name' not found.")
}
Expand All @@ -219,7 +227,7 @@ class ResourcesTest : GradlePluginTestBase() {
@Test
fun testMultiModuleResources() {
val environment = defaultTestEnvironment.copy(
kotlinVersion = "2.0.0-Beta5"
kotlinVersion = "2.0.0-RC2"
)
with(
testProject("misc/kmpResourcePublication", environment)
Expand Down Expand Up @@ -317,6 +325,25 @@ class ResourcesTest : GradlePluginTestBase() {
}
}

@Test
fun testOldResourcesWithNewKotlin() {
val environment = defaultTestEnvironment.copy(
kotlinVersion = "2.0.0-RC2"
)
with(
testProject("misc/kmpResourcePublication", environment)
) {
file("gradle.properties").modify { content ->
content.replace("compose.resources.multimodule.enable=true", "")
}

gradle(":cmplib:build").checks {
check.logContains("Multimodule Compose Resources are disabled by default.")
check.logContains("To enable it add 'compose.resources.multimodule.enable=true' int the root gradle.properties file.")
}
}
}

private fun checkResourcesZip(zipFile: File, resourcesFiles: Sequence<String>, subdir: String) {
assertTrue(zipFile.exists(), "File not found: " + zipFile.path)
ZipFile(zipFile).use { zip ->
Expand Down
1 change: 1 addition & 0 deletions ...ins/compose/src/test/test-projects/misc/kmpResourcePublication/appModule/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import org.jetbrains.compose.ExperimentalComposeLibrary
plugins {
id("org.jetbrains.compose")
kotlin("multiplatform")
kotlin("plugin.compose")
id("com.android.application")
}

Expand Down
1 change: 1 addition & 0 deletions gradle-plugins/compose/src/test/test-projects/misc/kmpResourcePublication/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
plugins {
id("org.jetbrains.compose").apply(false)
kotlin("multiplatform").apply(false)
kotlin("plugin.compose").apply(false)
id("com.android.library").apply(false)
id("com.android.application").apply(false)
}
1 change: 1 addition & 0 deletions ...lugins/compose/src/test/test-projects/misc/kmpResourcePublication/cmplib/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
plugins {
id("org.jetbrains.compose")
kotlin("multiplatform")
kotlin("plugin.compose")
id("maven-publish")
id("com.android.library")
}
Expand Down
1 change: 1 addition & 0 deletions ...compose/src/test/test-projects/misc/kmpResourcePublication/featureModule/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
plugins {
id("org.jetbrains.compose")
kotlin("multiplatform")
kotlin("plugin.compose")
id("com.android.library")
}

Expand Down
1 change: 1 addition & 0 deletions gradle-plugins/compose/src/test/test-projects/misc/kmpResourcePublication/gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ android.useAndroidX=true
org.jetbrains.compose.experimental.uikit.enabled=true
org.jetbrains.compose.experimental.jscanvas.enabled=true
org.jetbrains.compose.experimental.wasm.enabled=true
compose.resources.multimodule.enable=true
terrakok marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions ...le-plugins/compose/src/test/test-projects/misc/kmpResourcePublication/settings.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pluginManagement {
}
plugins {
id("org.jetbrains.kotlin.multiplatform").version("KOTLIN_VERSION_PLACEHOLDER")
id("org.jetbrains.kotlin.plugin.compose").version("KOTLIN_VERSION_PLACEHOLDER")
id("org.jetbrains.compose").version("COMPOSE_GRADLE_PLUGIN_VERSION_PLACEHOLDER")
id("com.android.library").version("AGP_VERSION_PLACEHOLDER")
id("com.android.application").version("AGP_VERSION_PLACEHOLDER")
Expand Down