From 16d493558efec3140b842ad123b24ca05842c71f Mon Sep 17 00:00:00 2001 From: Sergey Igushkin Date: Sat, 26 Sep 2020 21:05:04 +0300 Subject: [PATCH] HMPP resolution fixes While we already had the associateWith relationship that established additional visibility between such source sets as fooMain and fooTest, this relationship was not used properly when requested dependencies were built for source set visibility inference in HMPP. Instead, an old workaround was used that just added dependencies of commonMain to commonTest. Fix this by using the associateWith relationship in the preparation logic of source sets visibiltiy. ALso, make the artifact view used in source sets visibility inference lenient, so that a host-specific module that has not yet been published won't lead to fail in source sets visibility and will instead be ingored. --- .../kotlin/gradle/HierarchicalMppIT.kt | 52 +++++++++++++++++-- .../kotlin/gradle/plugin/KotlinPlugin.kt | 2 +- .../plugin/mpp/ResolvedMppVariantsProvider.kt | 1 + .../plugin/mpp/SourceSetVisibilityProvider.kt | 12 ++--- .../KotlinMetadataTargetConfigurator.kt | 5 +- .../kotlin/gradle/tasks/TasksProvider.kt | 13 ++--- 6 files changed, 63 insertions(+), 22 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/HierarchicalMppIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/HierarchicalMppIT.kt index 7bb34789792..4d52b3f8a65 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/HierarchicalMppIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/HierarchicalMppIT.kt @@ -58,9 +58,42 @@ class HierarchicalMppIT : BaseGradleIT() { } @Test - fun testDependenciesInTests() { + fun testNoSourceSetsVisibleIfNoVariantMatched() { publishThirdPartyLib(withGranularMetadata = true) + Project("my-lib-foo", gradleVersion, "hierarchical-mpp-published-modules").run { + setupWorkingDir() + gradleBuildScript().modify(::transformBuildScriptWithPluginsDsl) + + // --- Move the dependency from jvmAndJsMain to commonMain, where there's a linuxX64 target missing in the lib + gradleBuildScript().modify { + it.checkedReplace("api(\"com.example.thirdparty:third-party-lib:1.0\")", "//") + "\n" + """ + dependencies { + "commonMainApi"("com.example.thirdparty:third-party-lib:1.0") + } + """.trimIndent() + } + + testDependencyTransformations { reports -> + val thirdPartyLibApiVisibility = reports.filter { report -> + report.groupAndModule.startsWith("com.example.thirdparty:third-party-lib") && report.scope == "api" + } + val jvmJsSourceSets = setOf("jvmMain", "jsMain", "jvmTest", "jsTest", "jvmAndJsMain", "jvmAndJsTest") + thirdPartyLibApiVisibility.forEach { + if (it.sourceSetName in jvmJsSourceSets) + assertTrue("$it") { it.allVisibleSourceSets == setOf("commonMain") } + } + } + } + } + + @Test + fun testDependenciesInTests() { + publishThirdPartyLib(withGranularMetadata = true) { + projectDir.resolve("src/jvmMain").copyRecursively(projectDir.resolve("src/linuxX64Main")) + gradleBuildScript().appendText("\nkotlin.linuxX64()") + } + Project("my-lib-foo", gradleVersion, "hierarchical-mpp-published-modules").run { setupWorkingDir() gradleBuildScript().modify(::transformBuildScriptWithPluginsDsl) @@ -68,12 +101,18 @@ class HierarchicalMppIT : BaseGradleIT() { testDependencyTransformations { reports -> val testApiTransformationReports = reports.filter { report -> - report.groupAndModule.startsWith("com.example.thirdparty") && - report.sourceSetName.let { it == "commonTest" || it == "jvmAndJsTest" } + report.groupAndModule.startsWith("com.example.thirdparty:third-party-lib") && + report.sourceSetName.let { it == "commonTest" || it == "jvmAndJsTest" } && + report.scope == "api" } testApiTransformationReports.forEach { - assertTrue("$it") { it.isExcluded } // should not be visible in test source sets + if (it.sourceSetName == "commonTest") + assertTrue("$it") { it.isExcluded } // should not be visible in commonTest + else { + assertTrue("$it") { it.allVisibleSourceSets == setOf("commonMain") } + assertTrue("$it") { it.newVisibleSourceSets == emptySet() } + } } } @@ -187,12 +226,15 @@ class HierarchicalMppIT : BaseGradleIT() { projectName: String = "third-party-lib", directoryPrefix: String = "hierarchical-mpp-published-modules", withGranularMetadata: Boolean, - jsCompilerType: KotlinJsCompilerType = KotlinJsCompilerType.LEGACY + jsCompilerType: KotlinJsCompilerType = KotlinJsCompilerType.LEGACY, + beforePublishing: Project.() -> Unit = { } ): Project = Project(projectName, gradleVersion, directoryPrefix).apply { setupWorkingDir() gradleBuildScript().modify(::transformBuildScriptWithPluginsDsl) + beforePublishing() + if (withGranularMetadata) { projectDir.resolve("gradle.properties").appendText("kotlin.mpp.enableGranularSourceSetsMetadata=true") } 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 22b1474eba5..ac62f5706ad 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 @@ -62,7 +62,7 @@ val KOTLIN_JS_DSL_NAME = "kotlin2js" val KOTLIN_OPTIONS_DSL_NAME = "kotlinOptions" abstract class KotlinCompilationProcessor( - open val kotlinCompilation: AbstractKotlinCompilation<*> + open val kotlinCompilation: KotlinCompilation<*> ) { abstract val kotlinTask: TaskProvider abstract fun run() diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ResolvedMppVariantsProvider.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ResolvedMppVariantsProvider.kt index 2a3b9c09bbc..22507d165d4 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ResolvedMppVariantsProvider.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ResolvedMppVariantsProvider.kt @@ -146,6 +146,7 @@ internal class ResolvedMppVariantsProvider private constructor(private val proje return artifactsConfiguration.incoming.artifactView { view -> view.componentFilter { it in mppComponentIds } view.attributes { attrs -> attrs.attribute(Usage.USAGE_ATTRIBUTE, project.usageByName(KotlinUsages.KOTLIN_METADATA)) } + view.lenient(true) }.artifacts.associateBy { it.id.componentIdentifier } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/SourceSetVisibilityProvider.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/SourceSetVisibilityProvider.kt index 18dd74ebcc0..05f11d3b71c 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/SourceSetVisibilityProvider.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/SourceSetVisibilityProvider.kt @@ -71,7 +71,7 @@ internal class SourceSetVisibilityProvider( val firstConfigurationByVariant = mutableMapOf() - val visiblePlatformVariantNames: Set = + val visiblePlatformVariantNames: Set = compilations .filter { it.target.platformType != KotlinPlatformType.common } .flatMapTo(mutableSetOf()) { compilation -> @@ -79,10 +79,10 @@ internal class SourceSetVisibilityProvider( // that we have in the compilations: dependencyScopes.mapNotNull { scope -> project.resolvableConfigurationFromCompilationByScope(compilation, scope) } } - .mapNotNullTo(mutableSetOf()) { configuration -> + .mapTo(mutableSetOf()) { configuration -> val resolvedVariant = resolvedVariantsProvider.getResolvedVariantName(mppModuleIdentifier, configuration) ?.let { platformVariantName(it) } - ?: return@mapNotNullTo null + ?: return@mapTo null firstConfigurationByVariant.putIfAbsent(resolvedVariant, configuration) resolvedVariant @@ -121,11 +121,11 @@ internal class SourceSetVisibilityProvider( .keys.first() } - someVariantByHostSpecificSourceSet.mapValues { (_, variantName) -> + someVariantByHostSpecificSourceSet.entries.mapNotNull { (sourceSetName, variantName) -> val configuration = firstConfigurationByVariant.getValue(variantName) resolvedVariantsProvider.getMetadataArtifactByRootModule(mppModuleIdentifier, configuration) - ?: error("Couldn't resolve metadata artifact for $mppModuleIdentifier in $configuration") - } + ?.let { sourceSetName to it } + }.toMap() } return SourceSetVisibilityResult( diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt index 0c14318879e..7dd98daadc3 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt @@ -51,12 +51,9 @@ class KotlinMetadataTargetConfigurator(kotlinPluginVersion: String) : internal fun transformGranularMetadataTaskName(sourceSetName: String) = lowerCamelCaseName("transform", sourceSetName, "DependenciesMetadata") - // TODO generalize once a general production-test and other kinds of inter-compilation visibility are supported - // Currently, this is a temporary ad-hoc mechanism for exposing the commonMain dependencies to the test source sets internal fun dependsOnWithInterCompilationDependencies(project: Project, sourceSet: KotlinSourceSet): Set = sourceSet.dependsOn.toMutableSet().apply { - if (sourceSet.name == KotlinSourceSet.COMMON_TEST_SOURCE_SET_NAME) - add(project.kotlinExtension.sourceSets.getByName(KotlinSourceSet.COMMON_MAIN_SOURCE_SET_NAME)) + addAll(getVisibleSourceSetsFromAssociateCompilations(project, sourceSet)) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/TasksProvider.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/TasksProvider.kt index 0b42781daab..5822afe6e93 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/TasksProvider.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/TasksProvider.kt @@ -21,6 +21,7 @@ import org.gradle.api.Task import org.gradle.api.UnknownTaskException import org.gradle.api.tasks.TaskCollection import org.gradle.api.tasks.TaskProvider +import org.jetbrains.kotlin.gradle.plugin.KotlinCompilation import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider import org.jetbrains.kotlin.gradle.plugin.mapKotlinTaskProperties import org.jetbrains.kotlin.gradle.plugin.mpp.AbstractKotlinCompilation @@ -83,7 +84,7 @@ internal open class KotlinTasksProvider(val targetName: String) { open fun registerKotlinJVMTask( project: Project, name: String, - compilation: AbstractKotlinCompilation<*>, + compilation: KotlinCompilation<*>, configureAction: (KotlinCompile) -> (Unit) ): TaskProvider { val properties = PropertiesProvider(project) @@ -98,7 +99,7 @@ internal open class KotlinTasksProvider(val targetName: String) { fun registerKotlinJSTask( project: Project, name: String, - compilation: AbstractKotlinCompilation<*>, + compilation: KotlinCompilation<*>, configureAction: (Kotlin2JsCompile) -> Unit ): TaskProvider { val properties = PropertiesProvider(project) @@ -113,7 +114,7 @@ internal open class KotlinTasksProvider(val targetName: String) { fun registerKotlinJsIrTask( project: Project, name: String, - compilation: AbstractKotlinCompilation<*>, + compilation: KotlinCompilation<*>, configureAction: (KotlinJsIrLink) -> Unit ): TaskProvider { val properties = PropertiesProvider(project) @@ -128,7 +129,7 @@ internal open class KotlinTasksProvider(val targetName: String) { fun registerKotlinCommonTask( project: Project, name: String, - compilation: AbstractKotlinCompilation<*>, + compilation: KotlinCompilation<*>, configureAction: (KotlinCompileCommon) -> (Unit) ): TaskProvider { val properties = PropertiesProvider(project) @@ -144,7 +145,7 @@ internal open class KotlinTasksProvider(val targetName: String) { kotlinTaskHolder: TaskProvider>, project: Project, propertiesProvider: PropertiesProvider, - compilation: AbstractKotlinCompilation<*> + compilation: KotlinCompilation<*> ) { project.runOnceAfterEvaluated("apply properties and language settings to ${kotlinTaskHolder.name}", kotlinTaskHolder) { propertiesProvider.mapKotlinTaskProperties(kotlinTaskHolder.get()) @@ -165,7 +166,7 @@ internal class AndroidTasksProvider(targetName: String) : KotlinTasksProvider(ta kotlinTaskHolder: TaskProvider>, project: Project, propertiesProvider: PropertiesProvider, - compilation: AbstractKotlinCompilation<*> + compilation: KotlinCompilation<*> ) { super.configure(kotlinTaskHolder, project, propertiesProvider, compilation) kotlinTaskHolder.configure {