Skip to content

Commit

Permalink
Fix reporting configuration problems with configuration cache (#3596)
Browse files Browse the repository at this point in the history
Sometimes we need to report warnings during the configuration phase.
For example, when Androidx Compose Compiler is used with
non-JVM targets (e.g. iOS/js), we want to warn users
that using non-JB compiler with non-JVM targets is not supported.

The default way of reporting warnings in Gradle is using a logger.
For example we could write something like:
```
abstract class ComposePlugin : Plugin<Project> {
    override fun apply(project: Project) {
          if (project.hasNonJvmTargets() && project.usesNonJBComposeCompiler()) {
               project.logger.warn("...")
          }
    }
}
```

This approach has a few issues:
1. When the Configuration Cache is enabled, project's configuration might
get skipped altogether, and the warning won't be printed.
2. If a project contains multiple Gradle modules (subprojects),
the warning might be printed multiple times. That might be OK
for some warnings. But repeating exactly the same warning
10s or 100s is unnecessary.

The only way to share the state between Gradle modules,
while preserving compatibility with the Configuration Cache,
is to define Gradle Build Service.

In 1.5.0 we used Gradle Build Service mechanism for both warnings.
However, I did not know that:
* only the service's parameters are persisted in the Configuration Cache.
The service itself is not persisted.
* if a service instance is materialized during the configuration
phase, then all changes made to its parameters will not be
visible to that particular instance (but they will be visible to the
next instance).

So the only way to report diagnostics with configuration cache without
repetition is to define a service that is not materialized
during the configuration phase (i.e. serviceProvider.get() is not called),
add to add warnings to a set property of the service.
This change implements that.


Resolves #3595
  • Loading branch information
AlexeyTsvetkov authored Sep 5, 2023
1 parent f81f6fa commit eb124fe
Show file tree
Hide file tree
Showing 16 changed files with 332 additions and 167 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,14 @@ package org.jetbrains.compose

import org.gradle.api.Project
import org.gradle.api.provider.Provider
import org.gradle.build.event.BuildEventsListenerRegistry
import org.jetbrains.compose.internal.ComposeCompilerArtifactProvider
import org.jetbrains.compose.internal.mppExtOrNull
import org.jetbrains.compose.internal.service.ConfigurationProblemReporterService
import org.jetbrains.compose.internal.webExt
import org.jetbrains.kotlin.gradle.plugin.*
import org.jetbrains.kotlin.gradle.targets.js.ir.KotlinJsIrTarget
import javax.inject.Inject

@Suppress("UnstableApiUsage")
class ComposeCompilerKotlinSupportPlugin @Inject constructor(
@Suppress("UnstableApiUsage") private val buildEventsListenerRegistry: BuildEventsListenerRegistry
) : KotlinCompilerPluginSupportPlugin {
class ComposeCompilerKotlinSupportPlugin : KotlinCompilerPluginSupportPlugin {
private lateinit var composeCompilerArtifactProvider: ComposeCompilerArtifactProvider

override fun apply(target: Project) {
Expand All @@ -35,7 +31,7 @@ class ComposeCompilerKotlinSupportPlugin @Inject constructor(
}
}

private fun collectUnsupportedCompilerPluginUsages(target: Project) {
private fun collectUnsupportedCompilerPluginUsages(project: Project) {
fun Project.hasNonJvmTargets(): Boolean {
val nonJvmTargets = setOf(KotlinPlatformType.native, KotlinPlatformType.js, KotlinPlatformType.wasm)
return mppExtOrNull?.targets?.any {
Expand All @@ -47,10 +43,11 @@ class ComposeCompilerKotlinSupportPlugin @Inject constructor(
return !groupId.startsWith("org.jetbrains.compose.compiler")
}

ComposeMultiplatformBuildService.getInstance(target).unsupportedCompilerPlugins.add(
target.provider {
ConfigurationProblemReporterService.registerUnsupportedPluginProvider(
project,
project.provider {
composeCompilerArtifactProvider.compilerArtifact.takeIf {
target.hasNonJvmTargets() && it.isNonJBComposeCompiler()
project.hasNonJvmTargets() && it.isNonJBComposeCompiler()
}
}
)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import org.jetbrains.compose.experimental.uikit.internal.resources.configureSync
import org.jetbrains.compose.internal.KOTLIN_MPP_PLUGIN_ID
import org.jetbrains.compose.internal.mppExt
import org.jetbrains.compose.internal.mppExtOrNull
import org.jetbrains.compose.internal.service.ConfigurationProblemReporterService
import org.jetbrains.compose.internal.service.GradlePropertySnapshotService
import org.jetbrains.compose.internal.utils.currentTarget
import org.jetbrains.compose.web.WebExtension
import org.jetbrains.kotlin.gradle.plugin.KotlinDependencyHandler
Expand All @@ -36,9 +38,14 @@ import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType

internal val composeVersion get() = ComposeBuildConfig.composeVersion

private fun initBuildServices(project: Project) {
ConfigurationProblemReporterService.init(project)
GradlePropertySnapshotService.init(project)
}

abstract class ComposePlugin : Plugin<Project> {
override fun apply(project: Project) {
ComposeMultiplatformBuildService.init(project)
initBuildServices(project)

val composeExtension = project.extensions.create("compose", ComposeExtension::class.java, project)
val desktopExtension = composeExtension.extensions.create("desktop", DesktopExtension::class.java)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
package org.jetbrains.compose.experimental.internal

import org.gradle.api.Project
import org.jetbrains.compose.ComposeMultiplatformBuildService
import org.jetbrains.compose.internal.KOTLIN_MPP_PLUGIN_ID
import org.jetbrains.compose.internal.mppExt
import org.jetbrains.compose.internal.service.ConfigurationProblemReporterService
import org.jetbrains.compose.internal.utils.KGPPropertyProvider
import org.jetbrains.compose.internal.utils.configureEachWithType
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget
Expand Down Expand Up @@ -59,11 +59,10 @@ private fun KotlinNativeTarget.checkExplicitCacheKind() {
for (provider in propertyProviders) {
val value = provider.valueOrNull(cacheKindProperty)
if (value != null) {
ComposeMultiplatformBuildService
.getInstance(project)
.warnOnceAfterBuild(
explicitCacheKindWarningMessage(cacheKindProperty, value, provider)
)
ConfigurationProblemReporterService.reportProblem(
project,
explicitCacheKindWarningMessage(cacheKindProperty, value, provider)
)
return
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Copyright 2020-2023 JetBrains s.r.o. and respective authors and developers.
* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
*/

package org.jetbrains.compose.internal.service

import org.gradle.api.Project
import org.gradle.api.services.BuildService
import org.gradle.api.services.BuildServiceParameters
import org.gradle.api.services.BuildServiceRegistration
import org.gradle.tooling.events.FinishEvent
import org.gradle.tooling.events.OperationCompletionListener

// The service implements OperationCompletionListener just so Gradle would materialize the service even if the service is not used by any task or transformation
abstract class AbstractComposeMultiplatformBuildService<P : BuildServiceParameters> : BuildService<P> , OperationCompletionListener, AutoCloseable {
override fun onFinish(event: FinishEvent) {}
override fun close() {}
}

internal inline fun <reified Service : BuildService<*>> serviceName(instance: Service? = null): String =
fqName(instance)

internal inline fun <reified Service : AbstractComposeMultiplatformBuildService<Params>, reified Params : BuildServiceParameters> registerServiceIfAbsent(
project: Project,
crossinline initParams: Params.() -> Unit = {}
): BuildServiceRegistration<Service, Params> {
if (findRegistration<Service, Params>(project) == null) {
val newService = project.gradle.sharedServices.registerIfAbsent(fqName<Service>(), Service::class.java) {
it.parameters.initParams()
}
// Workaround to materialize a service even if it is not bound to a task
BuildEventsListenerRegistryProvider.getInstance(project).onTaskCompletion(newService)
}

return getExistingServiceRegistration(project)
}

internal inline fun <reified Service : BuildService<Params>, reified Params : BuildServiceParameters> getExistingServiceRegistration(
project: Project
): BuildServiceRegistration<Service, Params> {
val registration = findRegistration<Service, Params>(project)
?: error("Service '${serviceName<Service>()}' was not initialized")
return registration.verified(project)
}

private inline fun <reified Service : BuildService<Params>, reified Params : BuildServiceParameters> BuildServiceRegistration<*, *>.verified(
project: Project
): BuildServiceRegistration<Service, Params> {
val parameters = parameters
// We are checking the type of parameters instead of the type of service
// to avoid materializing the service.
// After a service instance is created all changes made to its parameters won't be visible to
// that particular service instance.
// This is undesirable in some cases. For example, when reporting configuration problems,
// we want to collect all configuration issues from all projects first, then report issues all at once
// in execution phase.
if (parameters !is Params) {
// Compose Gradle plugin was probably loaded more than once
// See https://github.com/JetBrains/compose-multiplatform/issues/3459
if (fqName(parameters) == fqName<Params>()) {
val rootScript = project.rootProject.buildFile
error("""
Compose Multiplatform Gradle plugin has been loaded in multiple classloaders.
To avoid classloading issues, declare Compose Gradle Plugin in root build file $rootScript.
""".trimIndent())
} else {
error("Shared build service '${serviceName<Service>()}' parameters have unexpected type: ${fqName(parameters)}")
}
}

@Suppress("UNCHECKED_CAST")
return this as BuildServiceRegistration<Service, Params>
}

private inline fun <reified S : BuildService<P>, reified P : BuildServiceParameters> findRegistration(
project: Project
): BuildServiceRegistration<*, *>? =
project.gradle.sharedServices.registrations.findByName(fqName<S>())

private inline fun <reified T : Any> fqName(instance: T? = null) = T::class.java.canonicalName
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
*/

package org.jetbrains.compose.internal.utils
package org.jetbrains.compose.internal.service

import org.gradle.api.Project
import org.gradle.build.event.BuildEventsListenerRegistry
import javax.inject.Inject

// a hack to get BuildEventsListenerRegistry conveniently, which can only be injected by Gradle
@Suppress("UnstableApiUsage")
internal abstract class BuildEventsListenerRegistryProvider @Inject constructor(val registry: BuildEventsListenerRegistry) {
companion object {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright 2020-2023 JetBrains s.r.o. and respective authors and developers.
* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
*/

package org.jetbrains.compose.internal.service

import org.gradle.api.Project
import org.gradle.api.logging.Logging
import org.gradle.api.provider.ListProperty
import org.gradle.api.provider.Provider
import org.gradle.api.provider.SetProperty
import org.gradle.api.services.BuildServiceParameters
import org.jetbrains.compose.createWarningAboutNonCompatibleCompiler
import org.jetbrains.kotlin.gradle.plugin.SubpluginArtifact

abstract class ConfigurationProblemReporterService : AbstractComposeMultiplatformBuildService<ConfigurationProblemReporterService.Parameters>() {
interface Parameters : BuildServiceParameters {
val unsupportedPluginWarningProviders: ListProperty<Provider<String?>>
val warnings: SetProperty<String>
}

private val log = Logging.getLogger(this.javaClass)

override fun close() {
warnAboutUnsupportedCompilerPlugin()
logWarnings()
}

private fun warnAboutUnsupportedCompilerPlugin() {
for (warningProvider in parameters.unsupportedPluginWarningProviders.get()) {
val warning = warningProvider.orNull
if (warning != null) {
log.warn(warning)
}
}
}

private fun logWarnings() {
for (warning in parameters.warnings.get()) {
log.warn(warning)
}
}
companion object {
fun init(project: Project) {
registerServiceIfAbsent<ConfigurationProblemReporterService, Parameters>(project)
}

private inline fun configureParameters(project: Project, fn: Parameters.() -> Unit) {
getExistingServiceRegistration<ConfigurationProblemReporterService, Parameters>(project)
.parameters.fn()
}

fun reportProblem(project: Project, message: String) {
configureParameters(project) { warnings.add(message) }
}

fun registerUnsupportedPluginProvider(project: Project, unsupportedPlugin: Provider<SubpluginArtifact?>) {
configureParameters(project) {
unsupportedPluginWarningProviders.add(unsupportedPlugin.map { unsupportedCompiler ->
unsupportedCompiler?.groupId?.let { createWarningAboutNonCompatibleCompiler(it) }
})
}
}
}
}
Loading

0 comments on commit eb124fe

Please sign in to comment.