From 14a7e90c696e28eb005e047520cf10515ebf5bca Mon Sep 17 00:00:00 2001 From: Sergey Igushkin Date: Wed, 9 Jun 2021 10:18:15 +0000 Subject: [PATCH] KT-38954: Filter Android attributes during publishing * Always filter out the variant name attribute: it is never requested by consumers, while its presence makes Gradle count it as an unmatched attribute, sometimes leading to ambiguity; * Filter out the build type attribute: if all variants have the same build type, then remove the build type attribute from all variants; Otherwise, remove the build type attribute from the release variants in order to make them compatible with all other consumer's build types. * Add an opt-out flat for always keeping the attribute: "kotlin.android.buildTypeAttribute.keep" Gradle property Issue #KT-38954 Fixed --- .../kotlin/gradle/plugin/KotlinTarget.kt | 1 + .../AbstractKotlinAndroidGradleTests.kt | 5 +- .../PureAndroidAndJavaConsumeMppLibIT.kt | 163 ++++++++++++++++++ .../kotlin/gradle/util/AGPVersion.kt | 2 +- .../AndroidProject/Lib/build.gradle | 8 +- .../new-mpp-android/lib/build.gradle | 2 +- .../kotlin/gradle/plugin/KotlinProperties.kt | 3 + .../plugin/mpp/KotlinSoftwareComponent.kt | 8 +- .../kotlin/gradle/plugin/mpp/Publishing.kt | 2 +- .../gradle/plugin/mpp/kotlinVariants.kt | 11 +- .../targets/android/KotlinAndroidTarget.kt | 48 +++++- 11 files changed, 232 insertions(+), 21 deletions(-) create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/PureAndroidAndJavaConsumeMppLibIT.kt diff --git a/libraries/tools/kotlin-gradle-plugin-api/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinTarget.kt b/libraries/tools/kotlin-gradle-plugin-api/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinTarget.kt index 24738cab8fb..2ef3c2eb279 100644 --- a/libraries/tools/kotlin-gradle-plugin-api/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinTarget.kt +++ b/libraries/tools/kotlin-gradle-plugin-api/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinTarget.kt @@ -21,6 +21,7 @@ import org.jetbrains.kotlin.gradle.dsl.KotlinCommonOptions interface KotlinTargetComponent : SoftwareComponent { val target: KotlinTarget val publishable: Boolean + val publishableOnCurrentHost: Boolean val defaultArtifactId: String val sourcesArtifacts: Set } 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 944d60c7c9e..b207e3c7631 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 @@ -132,14 +132,13 @@ open class KotlinAndroid36GradleIT : KotlinAndroid34GradleIT() { } } - // By default, no Android variant should be published in 1.3.20: val groupDir = "lib/build/repo/com/example/" build("publish") { assertSuccessful() assertFileExists(groupDir + "lib-jvmlib") assertFileExists(groupDir + "lib-jslib") - assertNoSuchFile(groupDir + "lib-androidlib") - assertNoSuchFile(groupDir + "lib-androidlib-debug") + assertFileExists(groupDir + "lib-androidlib") + assertFileExists(groupDir + "lib-androidlib-debug") projectDir.resolve(groupDir).deleteRecursively() } diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/PureAndroidAndJavaConsumeMppLibIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/PureAndroidAndJavaConsumeMppLibIT.kt new file mode 100644 index 00000000000..f0358354e8c --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/PureAndroidAndJavaConsumeMppLibIT.kt @@ -0,0 +1,163 @@ +/* + * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ +@file:Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") + +package org.jetbrains.kotlin.gradle + +import org.jetbrains.kotlin.gradle.util.AGPVersion +import org.jetbrains.kotlin.gradle.util.checkedReplace +import org.jetbrains.kotlin.gradle.util.modify +import org.jetbrains.kotlin.test.util.KtTestUtil +import org.junit.Assume +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import java.io.File +import java.util.* +import java.lang.Boolean as RefBoolean + +@RunWith(Parameterized::class) +class PureAndroidAndJavaConsumeMppLibIT : BaseGradleIT() { + companion object { + @JvmStatic + @Parameterized.Parameters(name = "useFlavors: {0}, isAndroidDebugOnly: {1}") + fun testCases(): List> = + listOf( /* useFlavors, isAndroidDebugOnly */ + arrayOf(false, false), + arrayOf(false, true), + arrayOf(true, false), + arrayOf(true, true) + ) + } + + @Parameterized.Parameter(0) + lateinit var useFlavorsParameter: RefBoolean + + @Parameterized.Parameter(1) + lateinit var isAndroidDebugOnlyParameter: RefBoolean + + @Test + fun test() { + val jdk11Home = File(System.getProperty("jdk11Home")) + Assume.assumeTrue("This test requires JDK11 for AGP7", jdk11Home.isDirectory) + val buildOptions = defaultBuildOptions().copy( + javaHome = jdk11Home, + androidHome = KtTestUtil.findAndroidSdk(), + androidGradlePluginVersion = AGPVersion.v7_0_0 + ) + val gradleVersionRequirement = GradleVersionRequired.AtLeast("7.0") + + val isAndroidDebugOnly = isAndroidDebugOnlyParameter.booleanValue() + val useFlavors = useFlavorsParameter.booleanValue() + + val publishedProject = Project("new-mpp-android", gradleVersionRequirement).apply { + projectDir.deleteRecursively() + setupWorkingDir() + // Don't need custom attributes here + gradleBuildScript("lib").modify { text -> + text.lines().filterNot { it.trimStart().startsWith("attribute(") }.joinToString("\n") + .let { + if (isAndroidDebugOnly) it.checkedReplace( + "publishAllLibraryVariants()", + "publishLibraryVariants(\"${if (useFlavors) "flavor1Debug" else "debug"}\")" + ) else it + } + .let { + if (useFlavors) { + it + "\n" + """ + android { + flavorDimensions "myFlavor" + productFlavors { + flavor1 { dimension "myFlavor" } + } + } + """.trimIndent() + } else it + } + } + } + + publishedProject.build(":lib:publish", options = buildOptions) { + assertSuccessful() + } + val repoDir = publishedProject.projectDir.resolve("lib/build/repo") + + val consumerProject = Project("AndroidProject", gradleVersionRequirement).apply { + projectDir.deleteRecursively() + setupWorkingDir() + gradleBuildScript("Lib").apply { + writeText( + // Remove the Kotlin plugin from the consumer project to check how pure-AGP Kotlin-less consumers resolve the dependency + readText().checkedReplace("apply plugin: 'kotlin-android'", "//").let { text -> + // If the test case doesn't assume flavors, remove the flavor setup lines: + if (useFlavors) text else text.lines().filter { !it.trim().startsWith("flavor") }.joinToString("\n") + } + "\n" + """ + android { + buildTypes { + // We create a build type that is missing in the library in order to check how it resolves such an MPP lib + create("staging") { initWith(getByName("debug")) } + } + } + repositories { + maven { setUrl("${repoDir.absolutePath}") } + } + dependencies { + implementation("com.example:lib:1.0") + } + """.trimIndent() + ) + } + } + val variantForReleaseAndStaging = + if (isAndroidDebugOnly) "debugApiElements-published" else "releaseApiElements-published" + + fun nameWithFlavorIfNeeded(name: String) = if (useFlavors) "flavor1${name.capitalize()}" else name + + val configurationToExpectedVariant = listOf( + nameWithFlavorIfNeeded("debugCompileClasspath") to nameWithFlavorIfNeeded("debugApiElements-published"), + nameWithFlavorIfNeeded("releaseCompileClasspath") to nameWithFlavorIfNeeded(variantForReleaseAndStaging), + nameWithFlavorIfNeeded("stagingCompileClasspath") to nameWithFlavorIfNeeded(variantForReleaseAndStaging) + ) + configurationToExpectedVariant.forEach { (configuration, expected) -> + consumerProject.build( + ":Lib:dependencyInsight", + "--configuration", configuration, + "--dependency", "com.example:lib", + options = buildOptions + ) { + assertSuccessful() + assertContains("variant \"$expected\" [") + } + } + + // Also test that a pure Java (Kotlin-less) project is able to resolve the MPP library dependency to the JVM variants: + consumerProject.apply { + gradleSettingsScript().appendText("\ninclude(\"pure-java\")") + projectDir.resolve("pure-java/build.gradle.kts").also { it.parentFile.mkdirs() }.writeText( + """ + plugins { + java + } + repositories { + maven { setUrl("${repoDir.absolutePath}") } + } + dependencies { + implementation("com.example:lib:1.0") + } + """.trimIndent() + ) + + build( + ":pure-java:dependencyInsight", + "--configuration", "compileClasspath", + "--dependency", "com.example:lib", + options = buildOptions + ) { + assertSuccessful() + assertContains("variant \"jvmLibApiElements-published\" [") + } + } + } +} \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/AGPVersion.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/AGPVersion.kt index bb72845d2fb..594c668f632 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/AGPVersion.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/util/AGPVersion.kt @@ -22,6 +22,6 @@ class AGPVersion private constructor(private val versionNumber: VersionNumber) { val v3_6_0 = fromString("3.6.4") val v4_1_0 = fromString("4.1.3") val v4_2_0 = fromString("4.2.0") - val v7_0_0 = fromString("7.0.0-alpha15") + val v7_0_0 = fromString("7.0.0-beta02") } } \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/AndroidProject/Lib/build.gradle b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/AndroidProject/Lib/build.gradle index 34e95b00953..4f3ddd1150f 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/AndroidProject/Lib/build.gradle +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/AndroidProject/Lib/build.gradle @@ -20,12 +20,8 @@ android { flavorDimensions "myFlavor" productFlavors { - flavor1 { - dimension "myFlavor" - } - flavor2 { - dimension "myFlavor" - } + flavor1 { dimension "myFlavor" } + flavor2 { dimension "myFlavor" } } publishNonDefault true diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/new-mpp-android/lib/build.gradle b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/new-mpp-android/lib/build.gradle index d9d555d50c1..8c7ec0680a2 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/new-mpp-android/lib/build.gradle +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/new-mpp-android/lib/build.gradle @@ -75,8 +75,8 @@ kotlin { attribute(Attribute.of("com.example.compilation", String), compilationName) } } + publishAllLibraryVariants() } - fromPreset(presets.jvm, 'jvmLib') fromPreset(presets.js, 'jsLib') } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinProperties.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinProperties.kt index 55df0e4782a..99b0df8bd63 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinProperties.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinProperties.kt @@ -134,6 +134,9 @@ internal class PropertiesProvider private constructor(private val project: Proje val setJvmTargetFromAndroidCompileOptions: Boolean? get() = booleanProperty("kotlin.setJvmTargetFromAndroidCompileOptions") + val keepAndroidBuildTypeAttribute: Boolean + get() = booleanProperty("kotlin.android.buildTypeAttribute.keep") ?: false + val enableGranularSourceSetsMetadata: Boolean? get() = booleanProperty("kotlin.mpp.enableGranularSourceSetsMetadata") diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/KotlinSoftwareComponent.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/KotlinSoftwareComponent.kt index 5b8d94e78b4..d0113c7287e 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/KotlinSoftwareComponent.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/KotlinSoftwareComponent.kt @@ -38,7 +38,13 @@ abstract class KotlinSoftwareComponent( override fun getVariants(): Set = kotlinTargets .filter { target -> target !is KotlinMetadataTarget } - .flatMap { it.components }.toSet() + .flatMap { target -> + val targetPublishableComponentNames = + (target as? AbstractKotlinTarget)?.kotlinComponents?.mapNotNullTo(mutableSetOf()) { component -> + component.name.takeIf { component.publishable } + } + target.components.filter { targetPublishableComponentNames?.contains(it.name) ?: true } + }.toSet() private val _usages: Set by lazy { val metadataTarget = project.multiplatformExtension.metadata() diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/Publishing.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/Publishing.kt index cdd1bb39503..90325804347 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/Publishing.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/Publishing.kt @@ -76,7 +76,7 @@ private fun createTargetPublications(project: Project, publishing: PublishingExt private fun AbstractKotlinTarget.createMavenPublications(publications: PublicationContainer) { components .map { gradleComponent -> gradleComponent to kotlinComponents.single { it.name == gradleComponent.name } } - .filter { (_, kotlinComponent) -> kotlinComponent.publishable } + .filter { (_, kotlinComponent) -> kotlinComponent.publishableOnCurrentHost } .forEach { (gradleComponent, kotlinComponent) -> val componentPublication = publications.create(kotlinComponent.name, MavenPublication::class.java).apply { // do this in whenEvaluated since older Gradle versions seem to check the files in the variant eagerly: diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/kotlinVariants.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/kotlinVariants.kt index 6722cb441e0..47f6271d4dd 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/kotlinVariants.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/kotlinVariants.kt @@ -69,7 +69,9 @@ open class KotlinVariant( override fun getName(): String = componentName ?: producingCompilation.target.targetName - override var publishable: Boolean = target.publishable + override var publishable: Boolean = true + override val publishableOnCurrentHost: Boolean + get() = publishable && target.publishable override var sourcesArtifacts: Set = emptySet() internal set @@ -103,12 +105,15 @@ class JointAndroidKotlinTargetComponent( override val sourcesArtifacts: Set ) : KotlinTargetComponentWithCoordinatesAndPublication, SoftwareComponentInternal { - override fun getUsages(): Set = nestedVariants.flatMap { it.usages }.toSet() + override fun getUsages(): Set = nestedVariants.filter { it.publishable }.flatMap { it.usages }.toSet() override fun getName(): String = lowerCamelCaseName(target.targetName, *flavorNames.toTypedArray()) override val publishable: Boolean - get() = target.publishable + get() = nestedVariants.any { it.publishable } + + override val publishableOnCurrentHost: Boolean + get() = publishable override val defaultArtifactId: String = dashSeparatedName( diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/android/KotlinAndroidTarget.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/android/KotlinAndroidTarget.kt index 1c63932feed..412660ad031 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/android/KotlinAndroidTarget.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/android/KotlinAndroidTarget.kt @@ -8,8 +8,10 @@ package org.jetbrains.kotlin.gradle.plugin.mpp import com.android.build.gradle.api.BaseVariant import org.gradle.api.InvalidUserDataException +import org.gradle.api.Named import org.gradle.api.NamedDomainObjectContainer import org.gradle.api.Project +import org.gradle.api.attributes.Attribute import org.gradle.api.attributes.Usage.JAVA_RUNTIME_JARS import org.jetbrains.kotlin.gradle.plugin.* import org.jetbrains.kotlin.gradle.utils.dashSeparatedName @@ -126,7 +128,12 @@ open class KotlinAndroidTarget( createKotlinVariant( lowerCamelCaseName(compilation.target.name, *flavorGroupNameParts.toTypedArray()), compilation, - createAndroidUsageContexts(androidVariant, compilation, artifactClassifier) + createAndroidUsageContexts( + androidVariant, + compilation, + artifactClassifier, + publishableVariants.filter(::isVariantPublished).map(::getBuildTypeName).distinct().size == 1 + ) ).apply { publishable = isVariantPublished(androidVariant) sourcesArtifacts = setOf( @@ -154,20 +161,21 @@ open class KotlinAndroidTarget( if (publishLibraryVariantsGroupedByFlavor) { JointAndroidKotlinTargetComponent( target = this@KotlinAndroidTarget, - nestedVariants = nestedVariants.filter { it.publishable }.toSet(), + nestedVariants = nestedVariants, flavorNames = flavorGroupNameParts, sourcesArtifacts = nestedVariants.filter { it.publishable }.flatMap { it.sourcesArtifacts }.toSet() ) } else { nestedVariants.single() - } as KotlinTargetComponent + } as KotlinTargetComponent // Type inference corner case or bug? this cast in each branch is redundant but required here }.toSet() } private fun AbstractAndroidProjectHandler.createAndroidUsageContexts( variant: BaseVariant, compilation: KotlinCompilation<*>, - artifactClassifier: String? + artifactClassifier: String?, + isSingleBuildType: Boolean ): Set { val variantName = getVariantName(variant) val outputTaskOrProvider = getLibraryOutputTask(variant) ?: return emptySet() @@ -192,12 +200,42 @@ open class KotlinAndroidTarget( apiElementsConfigurationName to javaApiUsageForMavenScoping(), runtimeElementsConfigurationName to JAVA_RUNTIME_JARS ).mapTo(mutableSetOf()) { (dependencyConfigurationName, usageName) -> + val configuration = project.configurations.getByName(dependencyConfigurationName) DefaultKotlinUsageContext( compilation, project.usageByName(usageName), dependencyConfigurationName, - overrideConfigurationArtifacts = setOf(artifact) + overrideConfigurationArtifacts = setOf(artifact), + overrideConfigurationAttributes = HierarchyAttributeContainer(configuration.attributes) { + val valueString = run { + val value = configuration.attributes.getAttribute(it) + (value as? Named)?.name ?: value.toString() + } + filterOutAndroidVariantAttribute(it) && filterOutAndroidBuildTypeAttribute(it, valueString, isSingleBuildType) + } ) } } + + /** We filter this variant out as it is never requested on the consumer side, while keeping it leads to ambiguity between Android and + * JVM variants due to non-nesting sets of unmatched attributes. */ + private fun filterOutAndroidVariantAttribute( + attribute: Attribute<*> + ): Boolean = + attribute.name != "com.android.build.gradle.internal.attributes.VariantAttr" && + attribute.name != "com.android.build.api.attributes.VariantAttr" + + private fun filterOutAndroidBuildTypeAttribute( + it: Attribute<*>, + valueString: String, + isSinglePublishedVariant: Boolean + ) = when { + PropertiesProvider(project).keepAndroidBuildTypeAttribute -> true + it.name != "com.android.build.api.attributes.BuildTypeAttr" -> true + + // then the name is "com.android.build.api.attributes.BuildTypeAttr", so we omit it if there's just the single variant and always for the release one: + valueString == "release" -> false + isSinglePublishedVariant -> false + else -> true + } }