From 5094ac945c584c5c1ba18043784ca0360cc01bd1 Mon Sep 17 00:00:00 2001 From: Anton Lakotka Date: Tue, 17 Jan 2023 09:19:57 +0100 Subject: [PATCH] [Gradle] Fix code review NITs * rename parentVisibleSourceSetsProvider -> visibleSourceSetsFromParentsProvider * use flatMap.toSet() instead flatMapTo(mutableSetOf()) * Use `extraProperties` instead of extensions * KotlinMultiplatformExtension.metadat() instead of finding it by type * rename PlatformCompilationData.sourceSets to allSourceSets for clarity * Use FileCollection instead of Provider> * Use System.lineSeparator() instead of `\n` for better platform support * Report meaningful error when `participatingSourceSets` is accessed in execution state when Task state is loaded from Configuration Cache * Mark newly added API as internal since there is no use of them in user's build scripts. ^KT-49933 --- .../mpp/GranularMetadataTransformation.kt | 10 ++-- .../MetadataDependencyTransformationTask.kt | 46 ++++++++++--------- ...rojectStructureMetadataExtractorFactory.kt | 23 ++++------ .../plugin/mpp/ProjectMetadataProviderImpl.kt | 2 +- .../plugin/mpp/SourceSetVisibilityProvider.kt | 6 +-- .../KotlinMetadataTargetConfigurator.kt | 5 +- ...ropMetadataDependencyTransformationTask.kt | 14 +++--- 7 files changed, 50 insertions(+), 56 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/GranularMetadataTransformation.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/GranularMetadataTransformation.kt index 6a16a553586..cf513c30ac7 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/GranularMetadataTransformation.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/GranularMetadataTransformation.kt @@ -103,7 +103,7 @@ private val ComponentIdentifier.uniqueKey get(): ComponentIdentifierKey = internal class GranularMetadataTransformation( val params: Params, - parentVisibleSourceSetsProvider: () -> Iterable>> + visibleSourceSetsFromParentsProvider: () -> Iterable>> ) { class Params( val sourceSetName: String, @@ -130,8 +130,8 @@ internal class GranularMetadataTransformation( val moduleId: Provider ) - private val parentVisibleSourceSets: Map> by lazy { - parentVisibleSourceSetsProvider().reduceOrNull { acc, map -> acc mergeWith map }.orEmpty() + private val visibleSourceSetsFromParents: Map> by lazy { + visibleSourceSetsFromParentsProvider().reduceOrNull { acc, map -> acc mergeWith map }.orEmpty() } val metadataDependencyResolutions: Iterable by lazy { doTransform() } @@ -140,7 +140,7 @@ internal class GranularMetadataTransformation( metadataDependencyResolutions .filterIsInstance() .groupBy { it.dependency.id.uniqueKey } - .mapValues { (_, visibleSourceSets) -> visibleSourceSets.flatMapTo(mutableSetOf()) { it.allVisibleSourceSetNames } } + .mapValues { (_, visibleSourceSets) -> visibleSourceSets.flatMap { it.allVisibleSourceSetNames }.toSet() } } private fun doTransform(): Iterable { @@ -169,7 +169,7 @@ internal class GranularMetadataTransformation( val dependencyResult = processDependency( resolvedDependency, - parentVisibleSourceSets[componentId.uniqueKey].orEmpty() + visibleSourceSetsFromParents[componentId.uniqueKey].orEmpty() ) result.add(dependencyResult) diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/MetadataDependencyTransformationTask.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/MetadataDependencyTransformationTask.kt index c7f53c18f66..8acb5cf4866 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/MetadataDependencyTransformationTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/MetadataDependencyTransformationTask.kt @@ -41,7 +41,7 @@ open class MetadataDependencyTransformationTask private val transformationParameters = GranularMetadataTransformation.Params(project, kotlinSourceSet) @get:OutputDirectory - val outputsDir: File get() = projectLayout.kotlinTransformedMetadataLibraryDirectoryForBuild(transformationParameters.sourceSetName) + internal val outputsDir: File get() = projectLayout.kotlinTransformedMetadataLibraryDirectoryForBuild(transformationParameters.sourceSetName) @Suppress("unused") // Gradle input @get:InputFiles @@ -50,14 +50,18 @@ open class MetadataDependencyTransformationTask @get:NormalizeLineEndings internal val configurationToResolve: FileCollection = kotlinSourceSet.internal.resolvableMetadataConfiguration - @delegate:Transient // Only needed for configuring task inputs - private val participatingSourceSets: Set by lazy { + @Transient // Only needed for configuring task inputs + private val participatingSourceSetsLazy: Lazy>? = lazy { kotlinSourceSet.internal.withDependsOnClosure.toMutableSet().apply { if (any { it.name == KotlinSourceSet.COMMON_MAIN_SOURCE_SET_NAME }) add(project.kotlinExtension.sourceSets.getByName(KotlinSourceSet.COMMON_MAIN_SOURCE_SET_NAME)) } } + private val participatingSourceSets: Set get() = participatingSourceSetsLazy?.value + ?: error("`participatingSourceSets` is null. " + + "Probably it is accessed it during Task Execution with state loaded from Configuration Cache") + @Suppress("unused") // Gradle input @get:Input internal val inputSourceSetsAndCompilations: Map> by lazy { @@ -76,30 +80,26 @@ open class MetadataDependencyTransformationTask } @get:OutputFile - val transformedLibrariesFileIndex: RegularFileProperty = objectFactory + internal val transformedLibrariesFileIndex: RegularFileProperty = objectFactory .fileProperty() .apply { set(outputsDir.resolve("${kotlinSourceSet.name}.transformedLibraries")) } @get:OutputFile - val visibleSourceSetsFile: RegularFileProperty = objectFactory + internal val visibleSourceSetsFile: RegularFileProperty = objectFactory .fileProperty() .apply { set(outputsDir.resolve("${kotlinSourceSet.name}.visibleSourceSets")) } @get:InputFiles - val parentVisibleSourceSetFiles: ConfigurableFileCollection = objectFactory - .fileCollection() - .from( - { - val parentSourceSets: List> = dependsOnClosureWithInterCompilationDependencies(kotlinSourceSet).mapNotNull { - project - .tasks - .locateTask(KotlinMetadataTargetConfigurator.transformGranularMetadataTaskName(it.name)) - ?.flatMap { it.visibleSourceSetsFile.map { it.asFile } } - } + val parentVisibleSourceSetFiles: ConfigurableFileCollection = project.filesProvider { + val parentSourceSets: List> = dependsOnClosureWithInterCompilationDependencies(kotlinSourceSet).mapNotNull { + project + .tasks + .locateTask(KotlinMetadataTargetConfigurator.transformGranularMetadataTaskName(it.name)) + ?.flatMap { it.visibleSourceSetsFile.map { it.asFile } } + } - parentSourceSets - } - ) + parentSourceSets + } //endregion Task Configuration State & Inputs @@ -107,7 +107,7 @@ open class MetadataDependencyTransformationTask fun transformMetadata() { val transformation = GranularMetadataTransformation( params = transformationParameters, - parentVisibleSourceSetsProvider = { parentVisibleSourceSetFiles.map(::readVisibleSourceSetsFile) } + visibleSourceSetsFromParentsProvider = { parentVisibleSourceSetFiles.map(::readVisibleSourceSetsFile) } ) if (outputsDir.isDirectory) { @@ -155,7 +155,7 @@ open class MetadataDependencyTransformationTask files } - val content = fileList.joinToString("\n") + val content = fileList.joinToString(System.lineSeparator()) transformedLibrariesFileIndex.get().asFile.writeText(content) } @@ -175,7 +175,9 @@ open class MetadataDependencyTransformationTask } @get:Internal // Warning! transformedLibraries is available only after Task Execution - val transformedLibraries: Provider> get() = transformedLibrariesFileIndex.map { regularFile -> - regularFile.asFile.readLines().map { File(it) } + val transformedLibraries: FileCollection = project.filesProvider { + transformedLibrariesFileIndex.map { regularFile -> + regularFile.asFile.readLines().map { File(it) } + } } } \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/MppDependencyProjectStructureMetadataExtractorFactory.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/MppDependencyProjectStructureMetadataExtractorFactory.kt index 9d1ef95f996..772e1ee92e6 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/MppDependencyProjectStructureMetadataExtractorFactory.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/MppDependencyProjectStructureMetadataExtractorFactory.kt @@ -13,12 +13,12 @@ import org.gradle.api.artifacts.result.ResolvedComponentResult import org.gradle.api.artifacts.result.ResolvedDependencyResult import org.gradle.api.provider.Provider import org.jetbrains.kotlin.gradle.dsl.multiplatformExtensionOrNull -import org.jetbrains.kotlin.gradle.plugin.addExtension -import org.jetbrains.kotlin.gradle.plugin.findExtension +import org.jetbrains.kotlin.gradle.plugin.extraProperties import org.jetbrains.kotlin.gradle.plugin.mpp.pm20.toSingleKpmModuleIdentifier +import org.jetbrains.kotlin.gradle.utils.getOrPut import org.jetbrains.kotlin.project.model.KpmModuleIdentifier -class MppDependencyProjectStructureMetadataExtractorFactory( +internal class MppDependencyProjectStructureMetadataExtractorFactory( private val projectStructureMetadataByProjectPath: Map> ) { fun create( @@ -41,19 +41,14 @@ class MppDependencyProjectStructureMetadataExtractorFactory( companion object { private val extensionName = MppDependencyProjectStructureMetadataExtractorFactory::class.java.simpleName - fun getOrCreate(project: Project): MppDependencyProjectStructureMetadataExtractorFactory { - val existing = project.findExtension(extensionName) - if (existing != null) return existing - - val projectStructureMetadataByProjectPath = collectProjectStructureMetadataFromAllProjects(project) - val newFactory = MppDependencyProjectStructureMetadataExtractorFactory(projectStructureMetadataByProjectPath) - project.addExtension(extensionName, newFactory) - return newFactory - } + fun getOrCreate(project: Project): MppDependencyProjectStructureMetadataExtractorFactory = + project.extraProperties.getOrPut(extensionName) { + val projectStructureMetadataByProjectPath = collectProjectStructureMetadataFromAllProjects(project) + MppDependencyProjectStructureMetadataExtractorFactory(projectStructureMetadataByProjectPath) + } /** - * Collect Kotlin Project StructureMetadata only for TCS model. - * TODO: Add support for KPM and auxiliary modules + * Collect Kotlin Project StructureMetadata. */ private fun collectProjectStructureMetadataFromAllProjects(project: Project): Map> { return project.rootProject.allprojects.associateBy { it.path }.mapValues { (_, subProject) -> diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ProjectMetadataProviderImpl.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ProjectMetadataProviderImpl.kt index aae7fa19624..a8447fa453f 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ProjectMetadataProviderImpl.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ProjectMetadataProviderImpl.kt @@ -106,7 +106,7 @@ internal fun Project.collectSourceSetMetadataOutputs(): Map { - val commonTarget = targets.withType().singleOrNull() ?: return emptyMap() + val commonTarget = metadata() val compilations = commonTarget.compilations diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/SourceSetVisibilityProvider.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/SourceSetVisibilityProvider.kt index 849d1327ee4..704e96c2336 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/SourceSetVisibilityProvider.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/SourceSetVisibilityProvider.kt @@ -39,7 +39,7 @@ private fun Project.collectAllPlatformCompilationData(): List.toPlatformCompilationData() = SourceSetVisibilityProvider.PlatformCompilationData( - sourceSets = allKotlinSourceSets.map { it.name }.toSet(), + allSourceSets = allKotlinSourceSets.map { it.name }.toSet(), resolvedDependenciesConfiguration = LazyResolvedConfiguration(project.configurations.getByName(compileDependencyConfigurationName)), hostSpecificMetadataConfiguration = project .configurations @@ -56,7 +56,7 @@ internal class SourceSetVisibilityProvider( ) class PlatformCompilationData( - val sourceSets: Set, + val allSourceSets: Set, val resolvedDependenciesConfiguration: LazyResolvedConfiguration, val hostSpecificMetadataConfiguration: LazyResolvedConfiguration? ) @@ -86,7 +86,7 @@ internal class SourceSetVisibilityProvider( val visiblePlatformVariantNames: Set = platformCompilations - .filter { visibleFromSourceSet in it.sourceSets } + .filter { visibleFromSourceSet in it.allSourceSets } .mapTo(mutableSetOf()) { resolvedConfiguration -> val resolvedVariant = resolvedConfiguration .resolvedDependenciesConfiguration diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt index ffbc8116841..7f63f1f4046 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt @@ -31,7 +31,6 @@ import org.jetbrains.kotlin.gradle.tasks.* import org.jetbrains.kotlin.gradle.utils.filesProvider import org.jetbrains.kotlin.gradle.utils.lowerCamelCaseName import org.jetbrains.kotlin.statistics.metrics.BooleanMetrics -import java.util.concurrent.Callable internal const val COMMON_MAIN_ELEMENTS_CONFIGURATION_NAME = "commonMainMetadataElements" internal const val ALL_COMPILE_METADATA_CONFIGURATION_NAME = "allSourceSetsCompileDependenciesMetadata" @@ -360,7 +359,7 @@ class KotlinMetadataTargetConfigurator : compilation.compileDependencyFiles += project.filesProvider { artifacts.filterNot { it.isMpp }.map { it.file } } // Transformed Multiplatform Libraries based on source set visibility - compilation.compileDependencyFiles += project.files(transformationTask.flatMap { it.transformedLibraries }) + compilation.compileDependencyFiles += project.files(transformationTask.map { it.transformedLibraries }) if (compilation is KotlinSharedNativeCompilation && sourceSet is DefaultKotlinSourceSet) { compilation.compileDependencyFiles += project.createCInteropMetadataDependencyClasspath(sourceSet) @@ -388,7 +387,7 @@ class KotlinMetadataTargetConfigurator : ) { val granularMetadataTransformation = GranularMetadataTransformation( params = GranularMetadataTransformation.Params(project, sourceSet), - parentVisibleSourceSetsProvider = { + visibleSourceSetsFromParentsProvider = { dependsOnClosureWithInterCompilationDependencies(sourceSet).filterIsInstance() .map { it.compileDependenciesTransformationOrFail.visibleSourceSetsByComponentId } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CInteropMetadataDependencyTransformationTask.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CInteropMetadataDependencyTransformationTask.kt index d460ba386ae..4306ae6261d 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CInteropMetadataDependencyTransformationTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/native/internal/CInteropMetadataDependencyTransformationTask.kt @@ -29,6 +29,7 @@ import org.jetbrains.kotlin.gradle.plugin.sources.internal import org.jetbrains.kotlin.gradle.tasks.dependsOn import org.jetbrains.kotlin.gradle.tasks.locateOrRegisterTask import org.jetbrains.kotlin.gradle.tasks.withType +import org.jetbrains.kotlin.gradle.utils.filesProvider import org.jetbrains.kotlin.gradle.utils.lowerCamelCaseName import org.jetbrains.kotlin.library.KLIB_FILE_EXTENSION import org.jetbrains.kotlin.project.model.KpmModuleIdentifier @@ -199,8 +200,10 @@ internal open class CInteropMetadataDependencyTransformationTask @Inject constru .apply { set(outputDirectory.resolve("${sourceSet.name}.transformedCinteropLibraries")) } @get:Internal - val outputLibraryFiles: Provider> get() = outputLibrariesFileIndex.map { file -> - TransformedCinteropLibrariesFile(file.asFile).read() + val outputLibraryFiles: FileCollection = project.filesProvider { + outputLibrariesFileIndex.map { file -> + TransformedCinteropLibrariesFile(file.asFile).read() + } } @TaskAction @@ -215,11 +218,6 @@ internal open class CInteropMetadataDependencyTransformationTask @Inject constru TransformedCinteropLibrariesFile(outputLibrariesFileIndex.get().asFile).write(transformedLibraries) } - private fun writeTransformedLibraries(files: Set) { - val content = files.joinToString("\n") - outputLibrariesFileIndex.get().asFile.writeText(content) - } - private fun materializeMetadata( chooseVisibleSourceSets: ChooseVisibleSourceSets ): Unit = when (chooseVisibleSourceSets.metadataProvider) { @@ -250,7 +248,7 @@ private class TransformedCinteropLibrariesFile( fun read(): Set = indexFile.readLines().mapTo(mutableSetOf()) { File(it) } fun write(files: Iterable) { - val content = files.joinToString("\n") + val content = files.joinToString(System.lineSeparator()) indexFile.writeText(content) } }