Fixes for review KOTLIN-CR-2599

* Move source artifacts to KotlinVariant classes (since joint Android variants may have multiple source artifacts, make it a set) * Change lowerSpinalCaseName to dashSeparatedName (doesn't transform the strings to lower case inside) * Don't transform the project name to lower case (in the future, it may be reasonable to stop transforming the target and component names, too) Still transform the target names and the component names. Currently, this is the way we don't break existing publications and may still reconsider this in the future. * Add a workaround for a list of GString's assigned to publishLibraryVariants * Check for Android library variants existence in a more strict way, report existing variants that are not library variants.
2019-01-11 17:17:56 +03:00
parent 7e6527ca65
commit 8bc7ac32f3
8 changed files with 82 additions and 63 deletions
@@ -10,6 +10,7 @@ import org.gradle.api.Action
 import org.gradle.api.Named
 import org.gradle.api.NamedDomainObjectContainer
 import org.gradle.api.Project
+import org.gradle.api.artifacts.PublishArtifact
 import org.gradle.api.attributes.AttributeContainer
 import org.gradle.api.attributes.HasAttributes
 import org.gradle.api.component.SoftwareComponent
@@ -21,6 +22,7 @@ interface KotlinTargetComponent : SoftwareComponent {
    val target: KotlinTarget
    val publishable: Boolean
    val defaultArtifactId: String
+    val sourcesArtifacts: Set<PublishArtifact>
 }

 interface KotlinTarget : Named, HasAttributes {
@@ -116,11 +116,11 @@ class KotlinAndroid32GradleIT : KotlinAndroid3GradleIT(androidGradlePluginVersio

        // Add one flavor dimension with two flavors, check that the flavors produce grouped publications:
        gradleBuildScript("lib").appendText(
-            "\nandroid { flavorDimensions('foo'); productFlavors { foo1 { dimension 'foo' }; foo2 { dimension 'foo' } } }"
+            "\nandroid { flavorDimensions('foo'); productFlavors { fooBar { dimension 'foo' }; fooBaz { dimension 'foo' } } }"
        )
        build("publish") {
            assertSuccessful()
-            listOf("foo1", "foo2").forEach { flavor ->
+            listOf("foobar", "foobaz").forEach { flavor ->
                assertFileExists(groupDir + "lib-androidlib-$flavor/1.0/lib-androidlib-$flavor-1.0.aar")
                assertFileExists(groupDir + "lib-androidlib-$flavor/1.0/lib-androidlib-$flavor-1.0-sources.jar")
                assertFileExists(groupDir + "lib-androidlib-$flavor/1.0/lib-androidlib-$flavor-1.0-debug.aar")
@@ -135,7 +135,7 @@ class KotlinAndroid32GradleIT : KotlinAndroid3GradleIT(androidGradlePluginVersio
        )
        build("publish") {
            assertSuccessful()
-            listOf("foo1", "foo2").forEach { flavor ->
+            listOf("foobar", "foobaz").forEach { flavor ->
                listOf("-debug", "").forEach { buildType ->
                    assertFileExists(groupDir + "lib-androidlib-$flavor$buildType/1.0/lib-androidlib-$flavor$buildType-1.0.aar")
                    assertFileExists(groupDir + "lib-androidlib-$flavor$buildType/1.0/lib-androidlib-$flavor$buildType-1.0-sources.jar")
@@ -153,13 +153,13 @@ class KotlinAndroid32GradleIT : KotlinAndroid3GradleIT(androidGradlePluginVersio
                    apply plugin: 'maven-publish'
                    publishing { repositories { maven { url = uri("${'$'}buildDir/repo") } } }
                    kotlin.android('androidApp') { publishAllLibraryVariants() }
-                    android { flavorDimensions('foo'); productFlavors { foo1 { dimension 'foo' }; foo2 { dimension 'foo' } } }
+                    android { flavorDimensions('foo'); productFlavors { fooBar { dimension 'foo' }; fooBaz { dimension 'foo' } } }
                """.trimIndent()
        }
        build("publish") {
            assertSuccessful()
            val appGroupDir = "app/build/repo/com/example/"
-            listOf("foo1", "foo2").forEach { flavor ->
+            listOf("foobar", "foobaz").forEach { flavor ->
                listOf("-debug", "").forEach { buildType ->
                    assertFileExists(appGroupDir + "app-androidapp-$flavor$buildType/1.0/app-androidapp-$flavor$buildType-1.0.aar")
                    assertFileExists(appGroupDir + "app-androidapp-$flavor$buildType/1.0/app-androidapp-$flavor$buildType-1.0-sources.jar")
@@ -1117,7 +1117,7 @@ class NewMultiplatformIT : BaseGradleIT() {
                assertTasksExecuted(":publishedLibrary:cinteropStdio${host.capitalize()}")
                assertTrue(output.contains("Published test"), "No test output found")
                assertFileExists("publishedLibrary/build/classes/kotlin/$host/main/publishedLibrary-cinterop-stdio.klib")
-                assertFileExists("repo/org/example/publishedlibrary-$host/1.0/publishedlibrary-$host-1.0-cinterop-stdio.klib")
+                assertFileExists("repo/org/example/publishedLibrary-$host/1.0/publishedLibrary-$host-1.0-cinterop-stdio.klib")
            }

            build(":build") {
@@ -166,7 +166,6 @@ class KotlinMultiplatformPlugin(

            // The root publication that references the platform specific publications as its variants:
            val rootPublication = publishing.publications.create("kotlinMultiplatform", MavenPublication::class.java).apply {
-                artifactId = project.name.toLowerCase()
                from(kotlinSoftwareComponent)
                (this as MavenPublicationInternal).publishWithOriginalFileName()
            }
@@ -200,9 +199,8 @@ class KotlinMultiplatformPlugin(
                    // do this in whenEvaluated since older Gradle versions seem to check the files in the variant eagerly:
                    project.whenEvaluated {
                        from(variant)
-                        if (variant is SoftwareComponentInternal) {
-                            variant.usages.filterIsInstance<KotlinUsageContext>().map { it.sourcesArtifact }.distinct()
-                                .forEach { artifact(it) }
+                        variant.sourcesArtifacts.forEach { sourceArtifact ->
+                            artifact(sourceArtifact)
                        }
                    }
                    (this as MavenPublicationInternal).publishWithOriginalFileName()
@@ -16,7 +16,6 @@ import org.gradle.api.internal.component.SoftwareComponentInternal
 import org.gradle.api.internal.component.UsageContext
 import org.gradle.api.publish.maven.MavenPublication
 import org.jetbrains.kotlin.gradle.plugin.*
-import org.jetbrains.kotlin.utils.ifEmpty

 open class KotlinSoftwareComponent(
    private val name: String,
@@ -52,14 +51,12 @@ object NativeUsage {
 interface KotlinUsageContext : UsageContext {
    val compilation: KotlinCompilation<*>
    val dependencyConfigurationName: String
-    val sourcesArtifact: PublishArtifact?
 }

 class DefaultKotlinUsageContext(
    override val compilation: KotlinCompilation<*>,
    private val usage: Usage,
    override val dependencyConfigurationName: String,
-    override val sourcesArtifact: PublishArtifact? = null,
    private val overrideConfigurationArtifacts: Set<PublishArtifact>? = null
 ) : KotlinUsageContext {

@@ -22,9 +22,9 @@ import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension
 import org.jetbrains.kotlin.gradle.dsl.KotlinNativeBinaryContainer
 import org.jetbrains.kotlin.gradle.dsl.kotlinExtension
 import org.jetbrains.kotlin.gradle.plugin.*
+import org.jetbrains.kotlin.gradle.utils.dashSeparatedName
 import org.jetbrains.kotlin.gradle.utils.isGradleVersionAtLeast
 import org.jetbrains.kotlin.gradle.utils.lowerCamelCaseName
-import org.jetbrains.kotlin.gradle.utils.lowerSpinalCaseName
 import org.jetbrains.kotlin.konan.target.KonanTarget

 abstract class AbstractKotlinTarget(
@@ -53,10 +53,18 @@ abstract class AbstractKotlinTarget(

    override val components: Set<KotlinTargetComponent> by lazy {
        val mainCompilation = compilations.getByName(KotlinCompilation.MAIN_COMPILATION_NAME)
-        val usageContexts = createUsageContexts(mainCompilation, targetName, targetName.toLowerCase())
+        val usageContexts = createUsageContexts(mainCompilation)
        setOf(
            if (isGradleVersionAtLeast(4, 7)) {
-                createKotlinVariant(mainCompilation.target.name, mainCompilation, usageContexts)
+                val componentName = mainCompilation.target.name
+                createKotlinVariant(componentName, mainCompilation, usageContexts).apply {
+                    sourcesArtifacts = setOf(
+                        sourcesJarArtifact(
+                            mainCompilation, componentName,
+                            dashSeparatedName(target.name.toLowerCase(), componentName.toLowerCase())
+                        )
+                    )
+                }
            } else {
                KotlinVariant(mainCompilation, usageContexts)
            }
@@ -91,12 +99,8 @@ abstract class AbstractKotlinTarget(
    }

    private fun createUsageContexts(
-        producingCompilation: KotlinCompilation<*>,
-        componentName: String,
-        artifactNameAppendix: String
+        producingCompilation: KotlinCompilation<*>
    ): Set<DefaultKotlinUsageContext> {
-        val sourcesJarArtifact = sourcesJarArtifact(producingCompilation, componentName, artifactNameAppendix)
-
        // Here, `JAVA_API` and `JAVA_RUNTIME_JARS` are used intentionally as Gradle needs this for
        // ordering of the usage contexts (prioritizing the dependencies) when merging them into the POM;
        // These Java usages should not be replaced with the custom Kotlin usages.
@@ -107,8 +111,7 @@ abstract class AbstractKotlinTarget(
            DefaultKotlinUsageContext(
                producingCompilation,
                project.usageByName(usageName),
-                dependenciesConfigurationName,
-                sourcesJarArtifact
+                dependenciesConfigurationName
            )
        }
    }
@@ -121,7 +124,7 @@ abstract class AbstractKotlinTarget(
    ): PublishArtifact {
        val sourcesJarTask = sourcesJarTask(producingCompilation, componentName, artifactNameAppendix)
        val sourceArtifactConfigurationName = producingCompilation.disambiguateName("sourceArtifacts")
-        val sourcesJarArtifact = producingCompilation.target.project.run {
+        return producingCompilation.target.project.run {
            (configurations.findByName(sourceArtifactConfigurationName) ?: run {
                val configuration = configurations.create(sourceArtifactConfigurationName) {
                    it.isCanBeResolved = false
@@ -131,10 +134,9 @@ abstract class AbstractKotlinTarget(
                configuration
            }).artifacts.single().apply {
                this as ConfigurablePublishArtifact
-                classifier = lowerSpinalCaseName(classifierPrefix, "sources")
+                classifier = dashSeparatedName(classifierPrefix, "sources")
            }
        }
-        return sourcesJarArtifact
    }

    @Suppress("UNCHECKED_CAST")
@@ -182,6 +184,8 @@ open class KotlinAndroidTarget(
     * If set to null, which can also be done with [publishAllLibraryVariants],
     * all library variants will be published, but not test or application variants. */
    var publishLibraryVariants: List<String>? = listOf()
+        // Workaround for Groovy GString items in a list:
+        set(value) { field = value?.map(Any::toString) }

    /** Add Android library variant names to [publishLibraryVariants]. */
    fun publishLibraryVariants(vararg names: String) {
@@ -200,21 +204,27 @@ open class KotlinAndroidTarget(

    private fun checkPublishLibraryVariantsExist() {
        // Capture type parameter T
-        fun <T> AbstractAndroidProjectHandler<T>.getVariantNames() =
-            mutableSetOf<String>().apply { forEachVariant(project) { add(getVariantName(it)) } }
+        fun <T> AbstractAndroidProjectHandler<T>.getLibraryVariantNames() =
+            mutableSetOf<String>().apply {
+                forEachVariant(project) {
+                    if (getLibraryOutputTask(it) != null)
+                        add(getVariantName(it))
+                }
+            }

        val variantNames =
            KotlinAndroidPlugin.androidTargetHandler(project.getKotlinPluginVersion()!!, this)
-                .getVariantNames()
+                .getLibraryVariantNames()

        val missingVariants =
            publishLibraryVariants?.minus(variantNames).orEmpty()

        if (missingVariants.isNotEmpty())
            throw InvalidUserDataException(
-                "Kotlin target '$targetName' tried to set up publishing for Android library variants that are not present " +
-                        "in the project:\n" + missingVariants.joinToString("\n") { "* $it" } +
-                        "\nCheck the 'publishLibraryVariants' property, it should point to existing Android build variants."
+                "Kotlin target '$targetName' tried to set up publishing for Android build variants that are not library variants " +
+                        "or do not exist:\n" + missingVariants.joinToString("\n") { "* $it" } +
+                        "\nCheck the 'publishLibraryVariants' property, it should point to existing Android library variants. Publishing " +
+                        "of application and test variants is not supported."
            )
    }

@@ -250,17 +260,36 @@ open class KotlinAndroidTarget(
            val nestedVariants = androidVariants.mapTo(mutableSetOf()) { androidVariant ->
                val androidVariantName = getVariantName(androidVariant)
                val compilation = compilations.getByName(androidVariantName)
+
+                val flavorNames = getFlavorNames(androidVariant)
                val buildTypeName = getBuildTypeName(androidVariant)
-                val usageContexts = createAndroidUsageContexts(androidVariant, compilation, getFlavorNames(androidVariant), buildTypeName)
+
+                val artifactClassifier = buildTypeName.takeIf { it != "release" && publishLibraryVariantsGroupedByFlavor }
+
+                val usageContexts = createAndroidUsageContexts(androidVariant, compilation, artifactClassifier)
                createKotlinVariant(
                    lowerCamelCaseName(compilation.target.name, *flavorGroupNameParts.toTypedArray()),
                    compilation,
                    usageContexts
                ).apply {
+                    sourcesArtifacts = setOf(
+                        sourcesJarArtifact(
+                            compilation, compilation.disambiguateName(""),
+                            dashSeparatedName(
+                                compilation.target.name.toLowerCase(),
+                                *flavorNames.map { it.toLowerCase() }.toTypedArray(),
+                                buildTypeName.takeIf { it != "release" }?.toLowerCase()
+                            ),
+                            classifierPrefix = artifactClassifier
+                        )
+                    )
+
                    if (!publishLibraryVariantsGroupedByFlavor) {
                        defaultArtifactIdSuffix =
-                            lowerSpinalCaseName(getFlavorNames(androidVariant) + getBuildTypeName(androidVariant).takeIf { it != "release" })
-                                .takeIf { it.isNotEmpty() }
+                            dashSeparatedName(
+                                (getFlavorNames(androidVariant) + getBuildTypeName(androidVariant).takeIf { it != "release" })
+                                    .map { it?.toLowerCase() }
+                            ).takeIf { it.isNotEmpty() }
                    }
                }
            }
@@ -269,7 +298,8 @@ open class KotlinAndroidTarget(
                JointAndroidKotlinTargetComponent(
                    this@KotlinAndroidTarget,
                    nestedVariants,
-                    flavorGroupNameParts
+                    flavorGroupNameParts,
+                    nestedVariants.flatMap { it.sourcesArtifacts }.toSet()
                )
            } else {
                nestedVariants.single()
@@ -280,17 +310,8 @@ open class KotlinAndroidTarget(
    private fun <T> AbstractAndroidProjectHandler<T>.createAndroidUsageContexts(
        variant: T,
        compilation: KotlinCompilation<*>,
-        flavorNames: List<String>,
-        buildTypeName: String
+        artifactClassifier: String?
    ): Set<DefaultKotlinUsageContext> {
-        val artifactClassifier = buildTypeName.takeIf { it != "release" && publishLibraryVariantsGroupedByFlavor }
-
-        val sourcesJarArtifact = sourcesJarArtifact(
-            compilation, compilation.disambiguateName(""),
-            lowerSpinalCaseName(compilation.target.name, *flavorNames.toTypedArray(), buildTypeName.takeIf { it != "release" }),
-            classifierPrefix = artifactClassifier
-        )
-
        val variantName = getVariantName(variant)
        val outputTask = getLibraryOutputTask(variant) ?: return emptySet()
        val artifact = run {
@@ -318,7 +339,6 @@ open class KotlinAndroidTarget(
                compilation,
                project.usageByName(usageName),
                dependencyConfigurationName,
-                sourcesJarArtifact,
                overrideConfigurationArtifacts = setOf(artifact)
            )
        }
@@ -6,10 +6,7 @@
 package org.jetbrains.kotlin.gradle.plugin.mpp

 import org.gradle.api.Project
-import org.gradle.api.artifacts.ExcludeRule
-import org.gradle.api.artifacts.ModuleDependency
-import org.gradle.api.artifacts.ModuleIdentifier
-import org.gradle.api.artifacts.ModuleVersionIdentifier
+import org.gradle.api.artifacts.*
 import org.gradle.api.component.ComponentWithCoordinates
 import org.gradle.api.component.ComponentWithVariants
 import org.gradle.api.internal.component.SoftwareComponentInternal
@@ -18,8 +15,8 @@ import org.gradle.api.publish.maven.MavenPublication
 import org.jetbrains.kotlin.gradle.plugin.KotlinCompilation
 import org.jetbrains.kotlin.gradle.plugin.KotlinTarget
 import org.jetbrains.kotlin.gradle.plugin.KotlinTargetComponent
+import org.jetbrains.kotlin.gradle.utils.dashSeparatedName
 import org.jetbrains.kotlin.gradle.utils.lowerCamelCaseName
-import org.jetbrains.kotlin.gradle.utils.lowerSpinalCaseName

 internal interface KotlinTargetComponentWithPublication : KotlinTargetComponent {
    // This property is declared in the separate parent type to allow the usages to reference it without forcing the subtypes to load,
@@ -35,7 +32,7 @@ internal fun getCoordinatesFromPublicationDelegateAndProject(
    object : ModuleVersionIdentifier {
        private val moduleName: String
            get() =
-                publication?.artifactId ?: lowerSpinalCaseName(project.name, target?.name)
+                publication?.artifactId ?: dashSeparatedName(project.name, target?.name?.toLowerCase())

        private val moduleGroup: String
            get() =
@@ -74,10 +71,13 @@ open class KotlinVariant(
    override val publishable: Boolean
        get() = target.publishable

+    override var sourcesArtifacts: Set<PublishArtifact> = emptySet()
+        internal set
+
    internal var defaultArtifactIdSuffix: String? = null

    override val defaultArtifactId: String
-        get() = lowerSpinalCaseName(target.project.name, target.targetName, defaultArtifactIdSuffix)
+        get() = dashSeparatedName(target.project.name, target.targetName.toLowerCase(), defaultArtifactIdSuffix)

    override var publicationDelegate: MavenPublication? = null
 }
@@ -126,8 +126,9 @@ class KotlinVariantWithMetadataDependency(
 class JointAndroidKotlinTargetComponent(
    override val target: KotlinAndroidTarget,
    private val nestedVariants: Set<KotlinVariant>,
-    val flavorNames: List<String>
-) : KotlinTargetComponentWithCoordinatesAndPublication, SoftwareComponentInternal {
+    val flavorNames: List<String>,
+    override val sourcesArtifacts: Set<PublishArtifact>
+    ) : KotlinTargetComponentWithCoordinatesAndPublication, SoftwareComponentInternal {

    override fun getUsages(): Set<UsageContext> = nestedVariants.flatMap { it.usages }.toSet()

@@ -137,7 +138,11 @@ class JointAndroidKotlinTargetComponent(
        get() = target.publishable

    override val defaultArtifactId: String =
-        lowerSpinalCaseName(target.project.name, target.targetName, *flavorNames.toTypedArray())
+        dashSeparatedName(
+            target.project.name,
+            target.targetName.toLowerCase(),
+            *flavorNames.map { it.toLowerCase() }.toTypedArray()
+        )

    override var publicationDelegate: MavenPublication? = null
 }
@@ -14,12 +14,9 @@ internal fun lowerCamelCaseName(vararg nameParts: String?): String {
    )
 }

-internal fun lowerSpinalCaseName(nameParts: Iterable<String?>) = lowerSpinalCaseName(*nameParts.toList().toTypedArray())
+internal fun dashSeparatedName(nameParts: Iterable<String?>) = dashSeparatedName(*nameParts.toList().toTypedArray())

-internal fun lowerSpinalCaseName(vararg nameParts: String?): String {
+internal fun dashSeparatedName(vararg nameParts: String?): String {
    val nonEmptyParts = nameParts.mapNotNull { it?.takeIf(String::isNotEmpty) }
-    return nonEmptyParts.joinToString(
-        separator = "-",
-        transform = String::toLowerCase
-    )
+    return nonEmptyParts.joinToString(separator = "-")
 }