From c3f5d57d3bae20c871006cefa190626475389450 Mon Sep 17 00:00:00 2001 From: Dmitry Savvinov Date: Fri, 15 Jan 2021 11:21:10 +0300 Subject: [PATCH] Implement precise platforms importing in MPP - skip metadata target from importing. That lead to metadata compilations being imported is well -> some source-sets were participating in metadata compilations as well -> logic for determining platforms used to consider metadata compilations as well, adding COMMON platform to set of platforms Seems like metadata was never needed in import/IDE, and got there purely by accident - Use only targets, actually present in the project, as the default platform. This is needed mostly for corner-cases/miconfigurations, like orphan source-sets (source-sets which are created but not included into any configuraion). Still, for those source-sets the tooling is required to behave properly; presence of non-existing target can lead to various issues like showing gutters for test runs, which would fail on launch (because tests for that target actually do not exist) ^KT-37127 Fixed --- ...chicalMultiplatformProjectImportingTest.kt | 22 +++++++++---------- .../src/KotlinMPPGradleModel.kt | 1 + .../src/KotlinMPPGradleModelBuilder.kt | 16 ++++++++++++++ .../src/KotlinMPPGradleModelImpl.kt | 2 ++ .../src/MultiplatformModelImportingContext.kt | 9 ++++++++ 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/HierarchicalMultiplatformProjectImportingTest.kt b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/HierarchicalMultiplatformProjectImportingTest.kt index 84f17e81868..0a732580dc6 100644 --- a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/HierarchicalMultiplatformProjectImportingTest.kt +++ b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/HierarchicalMultiplatformProjectImportingTest.kt @@ -360,7 +360,7 @@ class HierarchicalMultiplatformProjectImportingTest : MultiplePluginVersionGradl targetPlatform(jvm) } - module("my-app.commonMain") { targetPlatform(jvm, anyNative, js) } + module("my-app.commonMain") { targetPlatform(jvm, anyNative) } module("my-app.commonTest") { targetPlatform(jvm, anyNative) } module("my-app.jvmAndLinuxMain") { targetPlatform(jvm, anyNative) } @@ -395,7 +395,7 @@ class HierarchicalMultiplatformProjectImportingTest : MultiplePluginVersionGradl targetPlatform(jvm) } - module("my-app.commonMain") { targetPlatform(jvm, anyNative, js) } // :( should be (jvm, anyNative) + module("my-app.commonMain") { targetPlatform(jvm, anyNative) } module("my-app.commonTest") { targetPlatform(jvm, anyNative) } module("my-app.jvmAndLinuxMain") { targetPlatform(jvm, anyNative) } @@ -414,7 +414,7 @@ class HierarchicalMultiplatformProjectImportingTest : MultiplePluginVersionGradl // submodule module("my-app.submodule") { targetPlatform(jvm) } - module("my-app.submodule.commonMain") { targetPlatform(js, jvm, anyNative) } // :( should be (js, jvm) + module("my-app.submodule.commonMain") { targetPlatform(js, jvm) } module("my-app.submodule.commonTest") { targetPlatform(js, jvm) } module("my-app.submodule.jvmMain") { targetPlatform(jvm) } @@ -438,11 +438,11 @@ class HierarchicalMultiplatformProjectImportingTest : MultiplePluginVersionGradl checkProjectStructure(exhaustiveModuleList = false, exhaustiveSourceSourceRootList = false, exhaustiveDependencyList = false) { module("my-app.commonMain") { // must not be (jvm, js, native) - targetPlatform(jvm, js, native) + targetPlatform(jvm, js) } module("my-app.orphan") { - targetPlatform(jvm, js, native) + targetPlatform(jvm, js) } module("my-app") { assertDiagnosticsCount(1) @@ -462,7 +462,7 @@ class HierarchicalMultiplatformProjectImportingTest : MultiplePluginVersionGradl checkProjectStructure(exhaustiveModuleList = false, exhaustiveSourceSourceRootList = false, exhaustiveDependencyList = false) { module("my-app.commonMain") { - targetPlatform(jvm, js, native) // must not be (jvm, js, native) + targetPlatform(jvm, js) // must not be (jvm, js, native) } module("my-app.includedIntoJvm") { @@ -487,7 +487,7 @@ class HierarchicalMultiplatformProjectImportingTest : MultiplePluginVersionGradl checkProjectStructure(exhaustiveModuleList = false, exhaustiveSourceSourceRootList = false, exhaustiveDependencyList = false) { module("my-app.commonMain") { - targetPlatform(jvm, js, native) // must not be (jvm, js, native) + targetPlatform(jvm, js) // must not be (jvm, js, native) } module("my-app.intermediateBetweenJsAndCommon") { @@ -542,19 +542,19 @@ class HierarchicalMultiplatformProjectImportingTest : MultiplePluginVersionGradl // (jvm, js, native) is highly undesirable module("my-app.danglingOnJvm") { - targetPlatform(jvm, js, native) + targetPlatform(jvm, js) } module("my-app.commonMain") { - targetPlatform(jvm, js, native) + targetPlatform(jvm, js) } module("my-app.danglingOnCommon") { - targetPlatform(jvm, js, native) + targetPlatform(jvm, js) } module("my-app.danglingOnJvmAndJs") { - targetPlatform(jvm, js, native) + targetPlatform(jvm, js) } } } diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModel.kt b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModel.kt index e6aa26a7fcc..c58d3e6daf8 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModel.kt +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModel.kt @@ -142,6 +142,7 @@ enum class KotlinPlatform(val id: String) { interface KotlinPlatformContainer : Serializable { val platforms: Collection + val arePlatformsInitialized: Boolean fun supports(simplePlatform: KotlinPlatform): Boolean diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt index f045f701d7a..d8b25af5d9c 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt @@ -305,6 +305,10 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { dependencyMapper: KotlinDependencyMapper ): KotlinTarget? { val targetClass = gradleTarget.javaClass + + val metadataTargetClass = targetClass.classLoader.loadClass("org.jetbrains.kotlin.gradle.plugin.mpp.KotlinMetadataTarget") + if (metadataTargetClass.isInstance(gradleTarget)) return null + val getPlatformType = targetClass.getMethodOrNull("getPlatformType") ?: return null val getDisambiguationClassifier = targetClass.getMethodOrNull("getDisambiguationClassifier") ?: return null val platformId = (getPlatformType.invoke(gradleTarget) as? Named)?.name ?: return null @@ -740,6 +744,18 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { } private fun MultiplatformModelImportingContext.computeSourceSetPlatforms(sourceSet: KotlinSourceSetImpl) { + require(!sourceSet.actualPlatforms.arePlatformsInitialized) { + "Attempt to re-initialize platforms for source set ${sourceSet}. Already present platforms: ${sourceSet.actualPlatforms}" + } + + if (isOrphanSourceSet(sourceSet)) { + // Explicitly set platform of orphan source-sets to only used platforms, not all supported platforms + // Otherwise, the tooling might be upset after trying to provide some support for a target which actually + // doesn't exist in this project (e.g. after trying to draw gutters, while test tasks do not exist) + sourceSet.actualPlatforms.addSimplePlatforms(projectPlatforms) + return + } + if (shouldCoerceToCommon(sourceSet)) { sourceSet.actualPlatforms.addSimplePlatforms(listOf(KotlinPlatform.COMMON)) return diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelImpl.kt b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelImpl.kt index 965fcfb4158..783de245568 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelImpl.kt +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelImpl.kt @@ -285,6 +285,8 @@ class KotlinPlatformContainerImpl() : KotlinPlatformContainer { private val defaultCommonPlatform = setOf(KotlinPlatform.COMMON) private var myPlatforms: MutableSet? = null + override val arePlatformsInitialized: Boolean + get() = myPlatforms != null constructor(platform: KotlinPlatformContainer) : this() { myPlatforms = HashSet(platform.platforms) diff --git a/idea/kotlin-gradle-tooling/src/MultiplatformModelImportingContext.kt b/idea/kotlin-gradle-tooling/src/MultiplatformModelImportingContext.kt index 003236a7a14..4d6c05e344e 100644 --- a/idea/kotlin-gradle-tooling/src/MultiplatformModelImportingContext.kt +++ b/idea/kotlin-gradle-tooling/src/MultiplatformModelImportingContext.kt @@ -23,6 +23,11 @@ internal interface MultiplatformModelImportingContext { val sourceSets: Collection val sourceSetsByNames: Map + /** + * Platforms, which are actually used in this project (i.e. platforms, for which targets has been created) + */ + val projectPlatforms: Collection + fun sourceSetByName(name: String): KotlinSourceSet? fun compilationsBySourceSet(sourceSet: KotlinSourceSet): Collection? @@ -86,6 +91,9 @@ internal class MultiplatformModelImportingContextImpl(override val project: Proj override lateinit var targets: Collection private set + override lateinit var projectPlatforms: Collection + private set + internal fun initializeSourceSets(sourceSetsByNames: Map) { require(!this::sourceSetsByNames.isInitialized) { "Attempt to re-initialize source sets for $this. Previous value: ${this.sourceSetsByNames}" @@ -118,6 +126,7 @@ internal class MultiplatformModelImportingContextImpl(override val project: Proj internal fun initializeTargets(targets: Collection) { require(!this::targets.isInitialized) { "Attempt to re-initialize targets for $this. Previous value: ${this.targets}" } this.targets = targets + this.projectPlatforms = targets.map { it.platform } } // overload for small optimization