From 0f670f806f8e6d87421775af18393ec6566eeba6 Mon Sep 17 00:00:00 2001 From: Sergey Igushkin Date: Wed, 28 Nov 2018 21:25:07 +0300 Subject: [PATCH] Fix Android Extensions early dependency resolution Make compiler plugin options evaluate lazily, with Lazy and FileCollection as replacements for the eagerly-evaluated data. Adjust compiler plugin option usages so that their evaluation is not triggered when not necessary. Issue #KT-26065 Fixed --- .../gradle/plugin/KotlinGradleSubplugin.kt | 32 +++++++--- .../AbstractKotlinAndroidGradleTests.kt | 23 ++++++++ .../plugin/android/Android25ProjectHandler.kt | 8 ++- .../gradle/internal/AndroidSubplugin.kt | 58 ++++++++++--------- .../kapt/Kapt3KotlinGradleSubplugin.kt | 2 +- .../kotlin/gradle/internal/subpluginUtils.kt | 8 ++- .../kotlin/gradle/plugin/KotlinPlugin.kt | 7 +-- .../LegacyAndroidAndroidProjectHandler.kt | 9 ++- .../gradle/tasks/CompilerPluginOptions.kt | 9 +-- 9 files changed, 104 insertions(+), 52 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-api/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinGradleSubplugin.kt b/libraries/tools/kotlin-gradle-plugin-api/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinGradleSubplugin.kt index 796f07f5078..b56d24b9b31 100644 --- a/libraries/tools/kotlin-gradle-plugin-api/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinGradleSubplugin.kt +++ b/libraries/tools/kotlin-gradle-plugin-api/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinGradleSubplugin.kt @@ -21,20 +21,34 @@ import org.gradle.api.tasks.compile.AbstractCompile import org.jetbrains.kotlin.gradle.dsl.KotlinCommonOptions import java.io.File -open class SubpluginOption(val key: String, val value: String) +open class SubpluginOption(val key: String, private val lazyValue: Lazy) { + constructor(key: String, value: String) : this(key, lazyOf(value)) + + val value: String get() = lazyValue.value +} class FilesSubpluginOption( + key: String, + val files: Iterable, + val kind: FilesOptionKind = FilesOptionKind.INTERNAL, + lazyValue: Lazy = lazy { files.joinToString(File.pathSeparator) { it.canonicalPath } } +) : SubpluginOption(key, lazyValue) { + + constructor( key: String, - val files: List, - val kind: FilesOptionKind = FilesOptionKind.INTERNAL, - value: String = files.joinToString(File.pathSeparator) { it.canonicalPath }) - : SubpluginOption(key, value) + files: List, + kind: FilesOptionKind = FilesOptionKind.INTERNAL, + value: String? = null + ) : this(key, files, kind, lazy { value ?: files.joinToString(File.pathSeparator) { it.canonicalPath } }) +} class CompositeSubpluginOption( - key: String, - value: String, - val originalOptions: List) - : SubpluginOption(key, value) + key: String, + lazyValue: Lazy, + val originalOptions: List +) : SubpluginOption(key, lazyValue) { + constructor(key: String, value: String, originalOptions: List) : this(key, lazyOf(value), originalOptions) +} /** Defines how the files option should be handled with regard to Gradle model */ enum class FilesOptionKind { diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/AbstractKotlinAndroidGradleTests.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/AbstractKotlinAndroidGradleTests.kt index c8d252676ee..153823d0c65 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/AbstractKotlinAndroidGradleTests.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/AbstractKotlinAndroidGradleTests.kt @@ -419,6 +419,29 @@ fun getSomething() = 10 val project = Project("AndroidExtensionsSpecificFeatures") val options = defaultBuildOptions().copy(incremental = false) + if (this is KotlinAndroid30GradleIT) { + project.setupWorkingDir() + project.gradleBuildScript("app").modify { + """ + def projectEvaluated = false + + configurations.all { configuration -> + incoming.beforeResolve { + if (!projectEvaluated) { + throw new RuntimeException("${'$'}configuration resolved during project configuration phase.") + } + } + } + + $it + + afterEvaluate { + projectEvaluated = true + } + """.trimIndent() + } + } + project.build("assemble", options = options) { assertFailed() assertContains("Unresolved reference: textView") diff --git a/libraries/tools/kotlin-gradle-plugin/src/agp25/kotlin/org/jetbrains/kotlin/gradle/plugin/android/Android25ProjectHandler.kt b/libraries/tools/kotlin-gradle-plugin/src/agp25/kotlin/org/jetbrains/kotlin/gradle/plugin/android/Android25ProjectHandler.kt index 928bcc9a780..5ac1bfe660a 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/agp25/kotlin/org/jetbrains/kotlin/gradle/plugin/android/Android25ProjectHandler.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/agp25/kotlin/org/jetbrains/kotlin/gradle/plugin/android/Android25ProjectHandler.kt @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinJvmAndroidCompilation import org.jetbrains.kotlin.gradle.tasks.KotlinCompile import org.jetbrains.kotlin.gradle.utils.addExtendsFromRelation import java.io.File +import java.util.concurrent.Callable @Suppress("unused") class Android25ProjectHandler(kotlinConfigurationTools: KotlinConfigurationTools) @@ -101,15 +102,16 @@ class Android25ProjectHandler(kotlinConfigurationTools: KotlinConfigurationTools override fun addJavaSourceDirectoryToVariantModel(variantData: BaseVariant, javaSourceDirectory: File) = variantData.addJavaSourceFoldersToModel(javaSourceDirectory) - override fun getResDirectories(variantData: BaseVariant): List { + override fun getResDirectories(variantData: BaseVariant): FileCollection { val getAllResourcesMethod = variantData::class.java.methods.firstOrNull { it.name == "getAllRawAndroidResources" } if (getAllResourcesMethod != null) { val allResources = getAllResourcesMethod.invoke(variantData) as FileCollection - return allResources.files.toList() + return allResources } - return variantData.mergeResources?.computeResourceSetList0() ?: emptyList() + val project = variantData.mergeResources.project + return project.files(Callable { variantData.mergeResources?.computeResourceSetList0() ?: emptyList() }) } override fun setUpDependencyResolution(variant: BaseVariant, compilation: KotlinJvmAndroidCompilation) { diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/AndroidSubplugin.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/AndroidSubplugin.kt index 8878526b4b9..7396b048e3c 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/AndroidSubplugin.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/AndroidSubplugin.kt @@ -25,6 +25,8 @@ import com.android.build.gradle.internal.variant.TestVariantData import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.UnknownDomainObjectException +import org.gradle.api.file.FileCollection +import org.gradle.api.tasks.PathSensitivity import org.gradle.api.tasks.compile.AbstractCompile import org.gradle.tooling.provider.model.ToolingModelBuilderRegistry import org.jetbrains.kotlin.gradle.model.builder.KotlinAndroidExtensionModelBuilder @@ -34,6 +36,7 @@ import org.jetbrains.kotlin.gradle.plugin.mpp.multiplatformExtension import org.jetbrains.kotlin.gradle.tasks.KotlinCompile import org.w3c.dom.Document import java.io.File +import java.util.concurrent.Callable import javax.inject.Inject import javax.xml.parsers.DocumentBuilderFactory @@ -133,16 +136,17 @@ class AndroidSubplugin : KotlinGradleSubplugin { pluginOptions += SubpluginOption("package", applicationPackage) fun addVariant(sourceSet: AndroidSourceSet) { - val optionValue = sourceSet.name + ';' + - sourceSet.res.srcDirs.joinToString(";") { it.absolutePath } + val optionValue = lazy { + sourceSet.name + ';' + sourceSet.res.srcDirs.joinToString(";") { it.absolutePath } + } pluginOptions += CompositeSubpluginOption( "variant", optionValue, listOf( SubpluginOption("sourceSetName", sourceSet.name), //use the INTERNAL option kind since the resources are tracked as sources (see below) - FilesSubpluginOption("resDirs", sourceSet.res.srcDirs.toList()) + FilesSubpluginOption("resDirs", project.files(Callable { sourceSet.res.srcDirs })) ) ) - kotlinCompile.source(project.files(getLayoutDirectories(sourceSet.res.srcDirs))) + kotlinCompile.inputs.files(getLayoutDirectories(project, sourceSet.res.srcDirs)).withPathSensitivity(PathSensitivity.RELATIVE) } addVariant(mainSourceSet) @@ -157,12 +161,14 @@ class AndroidSubplugin : KotlinGradleSubplugin { return wrapPluginOptions(pluginOptions, "configuration") } - private fun getLayoutDirectories(resDirectories: Collection): List { + private fun getLayoutDirectories(project: Project, resDirectories: Iterable): FileCollection { fun isLayoutDirectory(file: File) = file.name == "layout" || file.name.startsWith("layout-") - return resDirectories.flatMap { resDir -> - (resDir.listFiles(::isLayoutDirectory)).orEmpty().asList() - } + return project.files(Callable { + resDirectories.flatMap { resDir -> + (resDir.listFiles(::isLayoutDirectory)).orEmpty().asList() + } + }) } private fun applyExperimental( @@ -189,11 +195,13 @@ class AndroidSubplugin : KotlinGradleSubplugin { val mainSourceSet = androidExtension.sourceSets.getByName("main") pluginOptions += SubpluginOption("package", getApplicationPackage(project, mainSourceSet)) - fun addVariant(name: String, resDirectories: List) { - val optionValue = buildString { - append(name) - append(';') - resDirectories.joinTo(this, separator = ";") { it.canonicalPath } + fun addVariant(name: String, resDirectories: FileCollection) { + val optionValue = lazy { + buildString { + append(name) + append(';') + resDirectories.joinTo(this, separator = ";") { it.canonicalPath } + } } pluginOptions += CompositeSubpluginOption( "variant", optionValue, listOf( @@ -203,27 +211,27 @@ class AndroidSubplugin : KotlinGradleSubplugin { ) ) - kotlinCompile.source(project.files(getLayoutDirectories(resDirectories))) + kotlinCompile.inputs.files(getLayoutDirectories(project, resDirectories)).withPathSensitivity(PathSensitivity.RELATIVE) } fun addSourceSetAsVariant(name: String) { val sourceSet = androidExtension.sourceSets.findByName(name) ?: return val srcDirs = sourceSet.res.srcDirs.toList() if (srcDirs.isNotEmpty()) { - addVariant(sourceSet.name, srcDirs) + addVariant(sourceSet.name, project.files(srcDirs)) } } - val resDirectoriesForAllVariants = mutableListOf>() + val resDirectoriesForAllVariants = mutableListOf() androidProjectHandler.forEachVariant(project) { variant -> if (androidProjectHandler.getTestedVariantData(variant) != null) return@forEachVariant resDirectoriesForAllVariants += androidProjectHandler.getResDirectories(variant) } - val commonResDirectories = getCommonResDirectories(resDirectoriesForAllVariants) + val commonResDirectories = getCommonResDirectories(project, resDirectoriesForAllVariants) - addVariant("main", commonResDirectories.toList()) + addVariant("main", commonResDirectories) getVariantComponentNames(variantData)?.let { (variantName, flavorName, buildTypeName) -> addSourceSetAsVariant(buildTypeName) @@ -255,15 +263,11 @@ class AndroidSubplugin : KotlinGradleSubplugin { private data class VariantComponentNames(val variantName: String, val flavorName: String, val buildTypeName: String) - private fun getCommonResDirectories(resDirectories: List>): Set { - var common = resDirectories.firstOrNull()?.toSet() ?: return emptySet() - - for (resDirs in resDirectories.drop(1)) { - common = common.intersect(resDirs) - } - - return common - } + private fun getCommonResDirectories(project: Project, resDirectories: List): FileCollection = + if (resDirectories.isEmpty()) + project.files() + else + resDirectories.reduce { acc, other -> acc.filter { other.contains(it) } } private fun getApplicationPackage(project: Project, mainSourceSet: AndroidSourceSet): String { val manifestFile = mainSourceSet.manifest.srcFile diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/Kapt3KotlinGradleSubplugin.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/Kapt3KotlinGradleSubplugin.kt index 666e90b9a3c..56baa853c67 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/Kapt3KotlinGradleSubplugin.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/Kapt3KotlinGradleSubplugin.kt @@ -262,7 +262,7 @@ class Kapt3KotlinGradleSubplugin : KotlinGradleSubplugin { val apOptions = getAPOptions() - pluginOptions += CompositeSubpluginOption("apoptions", encodeList(apOptions.associate { it.key to it.value }), apOptions) + pluginOptions += CompositeSubpluginOption("apoptions", lazy { encodeList(apOptions.associate { it.key to it.value }) }, apOptions) pluginOptions += SubpluginOption("javacArguments", encodeList(kaptExtension.getJavacOptions())) diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/subpluginUtils.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/subpluginUtils.kt index cc7a21edb3d..fbddd3c33d1 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/subpluginUtils.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/subpluginUtils.kt @@ -62,8 +62,12 @@ internal fun CompilerPluginOptions.withWrappedKaptOptions(withApClasspath: Itera } fun wrapPluginOptions(options: List, newOptionName: String): List { - val groupedOptions = options.groupBy { it.key }.mapValues { opt -> opt.value.map { it.value } } - val encodedOptions = encodePluginOptions(groupedOptions) + val encodedOptions = lazy { + val groupedOptions = options + .groupBy { it.key } + .mapValues { (_, options) -> options.map { it.value } } + encodePluginOptions(groupedOptions) + } val singleOption = CompositeSubpluginOption(newOptionName, encodedOptions, options) return listOf(singleOption) } \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPlugin.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPlugin.kt index 62176aae255..5816133a5c3 100755 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPlugin.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPlugin.kt @@ -25,7 +25,6 @@ import org.jetbrains.kotlin.gradle.dsl.KotlinCommonOptions import org.jetbrains.kotlin.gradle.dsl.KotlinJvmOptionsImpl import org.jetbrains.kotlin.gradle.dsl.KotlinSingleJavaTargetExtension import org.jetbrains.kotlin.gradle.dsl.kotlinExtension -import org.jetbrains.kotlin.gradle.internal.Kapt3GradleSubplugin import org.jetbrains.kotlin.gradle.internal.Kapt3GradleSubplugin.Companion.getKaptGeneratedClassesDir import org.jetbrains.kotlin.gradle.internal.Kapt3KotlinGradleSubplugin import org.jetbrains.kotlin.gradle.internal.KaptVariantData @@ -630,7 +629,7 @@ abstract class AbstractAndroidProjectHandler(private val kotlinConfigurationT abstract fun forEachVariant(project: Project, action: (V) -> Unit): Unit abstract fun getTestedVariantData(variantData: V): V? - abstract fun getResDirectories(variantData: V): List + abstract fun getResDirectories(variantData: V): FileCollection protected abstract fun getSourceProviders(variantData: V): Iterable protected abstract fun getAllJavaSources(variantData: V): Iterable @@ -875,7 +874,7 @@ internal fun Task.registerSubpluginOptionsAsInputs(subpluginId: String, subplugi is InternalSubpluginOption -> Unit is CompositeSubpluginOption -> { - val subpluginIdWithWrapperKey = "$subpluginId.${optionKey}$indexSuffix" + val subpluginIdWithWrapperKey = "$subpluginId.$optionKey$indexSuffix" registerSubpluginOptionsAsInputs(subpluginIdWithWrapperKey, option.originalOptions) } @@ -884,7 +883,7 @@ internal fun Task.registerSubpluginOptionsAsInputs(subpluginId: String, subplugi }.run { /* exhaustive when */ } else -> { - inputsCompatible.propertyCompatible("$subpluginId." + option.key + indexSuffix, option.value) + inputsCompatible.propertyCompatible("$subpluginId." + option.key + indexSuffix, Callable { option.value }) } } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/LegacyAndroidAndroidProjectHandler.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/LegacyAndroidAndroidProjectHandler.kt index e15255b2fd6..bd95d0cf049 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/LegacyAndroidAndroidProjectHandler.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/LegacyAndroidAndroidProjectHandler.kt @@ -14,6 +14,7 @@ import com.android.build.gradle.internal.variant.TestVariantData import com.android.builder.model.SourceProvider import org.gradle.api.Project import org.gradle.api.ProjectConfigurationException +import org.gradle.api.file.FileCollection import org.gradle.api.tasks.compile.AbstractCompile import org.jetbrains.kotlin.gradle.internal.KaptTask import org.jetbrains.kotlin.gradle.internal.KaptVariantData @@ -23,6 +24,7 @@ import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinJvmAndroidCompilation import org.jetbrains.kotlin.gradle.tasks.KotlinCompile import org.jetbrains.kotlin.gradle.utils.checkedReflection import java.io.File +import java.util.concurrent.Callable internal class LegacyAndroidAndroidProjectHandler(kotlinConfigurationTools: KotlinConfigurationTools) : AbstractAndroidProjectHandler>(kotlinConfigurationTools) { @@ -110,8 +112,11 @@ internal class LegacyAndroidAndroidProjectHandler(kotlinConfigurationTools: Kotl override fun getTestedVariantData(variantData: BaseVariantData<*>): BaseVariantData<*>? = ((variantData as? TestVariantData)?.testedVariantData as? BaseVariantData<*>) - override fun getResDirectories(variantData: BaseVariantData): List { - return variantData.mergeResourcesTask?.rawInputFolders?.toList() ?: emptyList() + override fun getResDirectories(variantData: BaseVariantData): FileCollection { + val project = variantData.mergeResourcesTask.project + return project.files( + Callable { variantData.mergeResourcesTask?.rawInputFolders?.toList().orEmpty() } + ) } private val BaseVariantData<*>.sourceProviders: List diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/CompilerPluginOptions.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/CompilerPluginOptions.kt index edc67d67e39..f5d32fca392 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/CompilerPluginOptions.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/CompilerPluginOptions.kt @@ -19,16 +19,17 @@ package org.jetbrains.kotlin.gradle.tasks import org.jetbrains.kotlin.gradle.plugin.SubpluginOption class CompilerPluginOptions { - private val mutableArguments = arrayListOf() - internal val subpluginOptionsByPluginId = mutableMapOf>() val arguments: List - get() = mutableArguments + get() = subpluginOptionsByPluginId.flatMap { (pluginId, subplubinOptions) -> + subplubinOptions.map { option -> + "plugin:$pluginId:${option.key}=${option.value}" + } + } fun addPluginArgument(pluginId: String, option: SubpluginOption) { - mutableArguments.add("plugin:$pluginId:${option.key}=${option.value}") subpluginOptionsByPluginId.getOrPut(pluginId) { mutableListOf() }.add(option) } }