Fix JVM maven-publish caching by not capturing state in POM rewriting

In POM rewriting logic, ensure that non-serializable entities are accessed from within project.provider { ... } and their evaluation results are therefore properly serialized. Issue #KT-43054 Fixed
2020-11-06 21:18:06 +03:00
parent d1ba2f3d46
commit 3666327c81
4 changed files with 184 additions and 117 deletions
@@ -18,6 +18,27 @@ class ConfigurationCacheIT : AbstractConfigurationCacheIT() {
        testConfigurationCacheOf(":compileKotlin")
    }

+    @Test
+    fun testJvmWithMavenPublish() = with(Project("kotlinProject")) {
+        setupWorkingDir()
+        gradleBuildScript().appendText("""
+            apply plugin: "maven-publish"
+            group = "com.example"
+            version = "1.0"
+            publishing.repositories {
+                maven { 
+                    url = "${'$'}buildDir/repo"
+                }
+            }
+            publishing.publications {
+                maven(MavenPublication) {
+                    from(components["java"])
+                }
+            }
+        """.trimIndent())
+        testConfigurationCacheOf(":publishMavenPublicationToMavenRepository", checkUpToDateOnRebuild = false)
+    }
+
    @Test
    fun testIncrementalKaptProject() = with(Project("kaptIncrementalCompilationProject")) {
        setupIncrementalAptProject("AGGREGATING")
@@ -57,6 +78,7 @@ abstract class AbstractConfigurationCacheIT : BaseGradleIT() {
    protected fun Project.testConfigurationCacheOf(
        vararg taskNames: String,
        executedTaskNames: List<String>? = null,
+        checkUpToDateOnRebuild: Boolean = true,
        buildOptions: BuildOptions = defaultBuildOptions()
    ) {
        // First, run a build that serializes the tasks state for instant execution in further builds
@@ -81,9 +103,11 @@ abstract class AbstractConfigurationCacheIT : BaseGradleIT() {
            assertContains("Reusing configuration cache.")
        }

-        build(*taskNames, options = buildOptions) {
-            assertSuccessful()
-            assertTasksUpToDate(executedTask)
+        if (checkUpToDateOnRebuild) {
+            build(*taskNames, options = buildOptions) {
+                assertSuccessful()
+                assertTasksUpToDate(executedTask)
+            }
        }
    }

@@ -22,6 +22,8 @@ import org.gradle.api.plugins.JavaPluginConvention
 import org.gradle.api.plugins.MavenPluginConvention
 import org.gradle.api.provider.Provider
 import org.gradle.api.publish.PublishingExtension
+import org.gradle.api.publish.maven.MavenPom
+import org.gradle.api.artifacts.maven.MavenPom as OldMavenPom
 import org.gradle.api.publish.maven.MavenPublication
 import org.gradle.api.tasks.*
 import org.gradle.api.tasks.compile.AbstractCompile
@@ -415,19 +417,32 @@ internal abstract class AbstractKotlinPlugin(
        project.components.addAll(target.components)
    }

+    private fun rewritePom(pom: MavenPom, rewriter: PomDependenciesRewriter, shouldRewritePom: Provider<Boolean>) {
+        pom.withXml { xml ->
+            if (shouldRewritePom.get())
+                rewriter.rewritePomMppDependenciesToActualTargetModules(xml)
+        }
+    }
+
+    private fun rewritePom(pom: OldMavenPom, rewriter: PomDependenciesRewriter, shouldRewritePom: Provider<Boolean>) {
+        pom.withXml { xml ->
+            if (shouldRewritePom.get())
+                rewriter.rewritePomMppDependenciesToActualTargetModules(xml)
+        }
+    }
+
    private fun rewriteMppDependenciesInPom(target: AbstractKotlinTarget) {
        val project = target.project

-        fun shouldRewritePoms(): Boolean =
+        val shouldRewritePoms = project.provider {
            PropertiesProvider(project).keepMppDependenciesIntactInPoms != true
+        }

        project.pluginManager.withPlugin("maven-publish") {
            project.extensions.configure(PublishingExtension::class.java) { publishing ->
+                val pomRewriter = PomDependenciesRewriter(project, target.kotlinComponents.single())
                publishing.publications.withType(MavenPublication::class.java).all { publication ->
-                    publication.pom.withXml { xml ->
-                        if (shouldRewritePoms())
-                            project.rewritePomMppDependenciesToActualTargetModules(xml, target.kotlinComponents.single())
-                    }
+                    rewritePom(publication.pom, pomRewriter, shouldRewritePoms)
                }
            }
        }
@@ -435,10 +450,8 @@ internal abstract class AbstractKotlinPlugin(
        project.pluginManager.withPlugin("maven") {
            project.tasks.withType(Upload::class.java).all { uploadTask ->
                uploadTask.repositories.withType(MavenResolver::class.java).all { mavenResolver ->
-                    mavenResolver.pom.withXml { xml ->
-                        if (shouldRewritePoms())
-                            project.rewritePomMppDependenciesToActualTargetModules(xml, target.kotlinComponents.single())
-                    }
+                    val pomRewriter = PomDependenciesRewriter(project, target.kotlinComponents.single())
+                    rewritePom(mavenResolver.pom, pomRewriter, shouldRewritePoms)
                }
            }

@@ -13,8 +13,10 @@ import org.gradle.api.attributes.AttributeContainer
 import org.gradle.api.internal.FeaturePreviews
 import org.gradle.api.internal.plugins.DslObject
 import org.gradle.api.plugins.JavaBasePlugin
+import org.gradle.api.provider.Provider
 import org.gradle.api.publish.PublicationContainer
 import org.gradle.api.publish.PublishingExtension
+import org.gradle.api.publish.maven.MavenPom
 import org.gradle.api.publish.maven.MavenPublication
 import org.gradle.api.publish.maven.internal.publication.MavenPublicationInternal
 import org.gradle.api.tasks.SourceTask
@@ -234,6 +236,18 @@ class KotlinMultiplatformPlugin(
        project.components.add(kotlinSoftwareComponent)
    }

+    private fun rewritePom(
+        pom: MavenPom,
+        pomRewriter: PomDependenciesRewriter,
+        shouldRewritePomDependencies: Provider<Boolean>,
+        includeOnlySpecifiedDependencies: Provider<Set<ModuleCoordinates>>?
+    ) {
+        pom.withXml { xml ->
+            if (shouldRewritePomDependencies.get())
+                pomRewriter.rewritePomMppDependenciesToActualTargetModules(xml, includeOnlySpecifiedDependencies)
+        }
+    }
+
    private fun AbstractKotlinTarget.createMavenPublications(publications: PublicationContainer) {
        components
            .map { gradleComponent -> gradleComponent to kotlinComponents.single { it.name == gradleComponent.name } }
@@ -250,12 +264,16 @@ class KotlinMultiplatformPlugin(
                    (this as MavenPublicationInternal).publishWithOriginalFileName()
                    artifactId = kotlinComponent.defaultArtifactId

-                    pom.withXml { xml ->
-                        if (PropertiesProvider(project).keepMppDependenciesIntactInPoms != true)
-                            project.rewritePomMppDependenciesToActualTargetModules(xml, kotlinComponent) { id ->
-                                filterMetadataDependencies(this@createMavenPublications, id)
-                            }
-                    }
+                    val pomRewriter = PomDependenciesRewriter(project, kotlinComponent)
+                    val shouldRewritePomDependencies =
+                        project.provider { PropertiesProvider(project).keepMppDependenciesIntactInPoms != true }
+
+                    rewritePom(
+                        pom,
+                        pomRewriter,
+                        shouldRewritePomDependencies,
+                        dependenciesForPomRewriting(this@createMavenPublications)
+                    )
                }

                (kotlinComponent as? KotlinTargetComponentWithPublication)?.publicationDelegate = componentPublication
@@ -269,23 +287,24 @@ class KotlinMultiplatformPlugin(
     * can't read Gradle module metadata won't resolve a dependency on an MPP to the granular metadata variant and won't then choose the
     * right dependencies for each source set, we put only the dependencies of the legacy common variant into the POM, i.e. commonMain API.
     */
-    private fun filterMetadataDependencies(target: AbstractKotlinTarget, groupNameVersion: Triple<String?, String, String?>): Boolean {
-        if (target !is KotlinMetadataTarget || !target.project.isKotlinGranularMetadataEnabled) {
-            return true
+    private fun dependenciesForPomRewriting(target: AbstractKotlinTarget): Provider<Set<ModuleCoordinates>>? =
+        if (target !is KotlinMetadataTarget || !target.project.isKotlinGranularMetadataEnabled)
+            null
+        else {
+            val commonMain = target.project.kotlinExtension.sourceSets.findByName(KotlinSourceSet.COMMON_MAIN_SOURCE_SET_NAME)
+            if (commonMain == null)
+                null
+            else
+                target.project.provider {
+                    val project = target.project
+
+                    // Only the commonMain API dependencies can be published for consumers who can't read Gradle project metadata
+                    val commonMainApi = project.sourceSetDependencyConfigurationByScope(commonMain, KotlinDependencyScope.API_SCOPE)
+                    val commonMainDependencies = commonMainApi.allDependencies
+                    commonMainDependencies.map { ModuleCoordinates(it.group, it.name, it.version) }.toSet()
+                }
        }

-        val (group, name, _) = groupNameVersion
-
-        val project = target.project
-        val commonMain = project.kotlinExtension.sourceSets?.findByName(KotlinSourceSet.COMMON_MAIN_SOURCE_SET_NAME)
-            ?: return true
-
-        // Only the commonMain API dependencies can be published for consumers who can't read Gradle project metadata
-        val commonMainApi = project.sourceSetDependencyConfigurationByScope(commonMain, KotlinDependencyScope.API_SCOPE)
-
-        return commonMainApi.allDependencies.any { it.group == group && it.name == name }
-    }
-
    private fun configureSourceSets(project: Project) = with(project.multiplatformExtension) {
        val production = sourceSets.create(KotlinSourceSet.COMMON_MAIN_SOURCE_SET_NAME)
        val test = sourceSets.create(KotlinSourceSet.COMMON_TEST_SOURCE_SET_NAME)
@@ -14,82 +14,103 @@ import org.gradle.api.artifacts.ProjectDependency
 import org.gradle.api.attributes.Usage
 import org.gradle.api.internal.component.SoftwareComponentInternal
 import org.gradle.api.internal.component.UsageContext
+import org.gradle.api.provider.Provider
 import org.jetbrains.kotlin.gradle.dsl.multiplatformExtensionOrNull
 import org.jetbrains.kotlin.gradle.plugin.KotlinCompilationToRunnableFiles
 import org.jetbrains.kotlin.gradle.plugin.KotlinTargetComponent
+import org.jetbrains.kotlin.gradle.utils.getValue

-internal fun Project.rewritePomMppDependenciesToActualTargetModules(
-    pomXml: XmlProvider,
-    component: KotlinTargetComponent,
-    filterDependencies: (groupNameVersion: Triple<String?, String, String?>) -> Boolean = { true }
+internal data class ModuleCoordinates(
+    val group: String?,
+    val name: String,
+    val version: String?
+)
+
+internal class PomDependenciesRewriter(
+    project: Project,
+
+    @field:Transient
+    private val component: KotlinTargetComponent
 ) {
-    if (component !is SoftwareComponentInternal)
-        return
-
-    val dependenciesNode = (pomXml.asNode().get("dependencies") as NodeList).filterIsInstance<Node>().singleOrNull() ?: return
-
-    val dependencyNodes = (dependenciesNode.get("dependency") as? NodeList).orEmpty().filterIsInstance<Node>()
-
-    val dependencyByNode = mutableMapOf<Node, ModuleDependency>()
-
-    // Collect all the dependencies from the nodes:
-    val dependencies = dependencyNodes.map { dependencyNode ->
-        fun Node.getSingleChildValueOrNull(childName: String): String? =
-            ((get(childName) as NodeList?)?.singleOrNull() as Node?)?.text()
-
-        val groupId = dependencyNode.getSingleChildValueOrNull("groupId")
-        val artifactId = dependencyNode.getSingleChildValueOrNull("artifactId")
-        val version = dependencyNode.getSingleChildValueOrNull("version")
-        (project.dependencies.module("$groupId:$artifactId:$version") as ModuleDependency)
-            .also { dependencyByNode[dependencyNode] = it }
-    }.toSet()

    // Get the dependencies mapping according to the component's UsageContexts:
-    val resultDependenciesForEachUsageContext =
-        component.usages.mapNotNull { usage ->
+    private val dependenciesMappingForEachUsageContext by project.provider {
+        (component as SoftwareComponentInternal).usages.mapNotNull { usage ->
            if (usage is KotlinUsageContext)
-                associateDependenciesWithActualModuleDependencies(usage, dependencies)
+                associateDependenciesWithActualModuleDependencies(usage)
                    // We are only interested in dependencies that are mapped to some other dependencies:
                    .filter { (from, to) -> Triple(from.group, from.name, from.version) != Triple(to.group, to.name, to.version) }
            else null
        }
+    }

-    // Rewrite the dependency nodes according to the mapping:
-    dependencyNodes.forEach { dependencyNode ->
-        val moduleDependency = dependencyByNode[dependencyNode]
+    fun rewritePomMppDependenciesToActualTargetModules(
+        pomXml: XmlProvider,
+        includeOnlySpecifiedDependencies: Provider<Set<ModuleCoordinates>>? = null
+    ) {
+        if (component !is SoftwareComponentInternal)
+            return

-        if (moduleDependency != null) {
-            val groupNameVersion = Triple(moduleDependency.group, moduleDependency.name, moduleDependency.version)
-            if (!filterDependencies(groupNameVersion)) {
-                dependenciesNode.remove(dependencyNode)
-                return@forEach
-            }
+        val dependenciesNode = (pomXml.asNode().get("dependencies") as NodeList).filterIsInstance<Node>().singleOrNull() ?: return
+
+        val dependencyNodes = (dependenciesNode.get("dependency") as? NodeList).orEmpty().filterIsInstance<Node>()
+
+        val dependencyByNode = mutableMapOf<Node, ModuleCoordinates>()
+
+        // Collect all the dependencies from the nodes:
+        val dependencies = dependencyNodes.map { dependencyNode ->
+            fun Node.getSingleChildValueOrNull(childName: String): String? =
+                ((get(childName) as NodeList?)?.singleOrNull() as Node?)?.text()
+
+            val groupId = dependencyNode.getSingleChildValueOrNull("groupId")
+            val artifactId = dependencyNode.getSingleChildValueOrNull("artifactId")
+                ?: error("unexpected dependency in POM with no artifact ID: $dependenciesNode")
+            val version = dependencyNode.getSingleChildValueOrNull("version")
+            (ModuleCoordinates(groupId, artifactId, version)).also { dependencyByNode[dependencyNode] = it }
+        }.toSet()
+
+        val resultDependenciesForEachUsageContext = dependencies.associate { key ->
+            val map = dependenciesMappingForEachUsageContext.find { key in it }
+            val value = map?.get(key) ?: key
+            key to value
        }

-        val mapDependencyTo = resultDependenciesForEachUsageContext.find { moduleDependency in it }?.get(moduleDependency)
+        val includeOnlySpecifiedDependenciesSet = includeOnlySpecifiedDependencies?.get()

-        if (mapDependencyTo != null) {
+        // Rewrite the dependency nodes according to the mapping:
+        dependencyNodes.forEach { dependencyNode ->
+            val moduleDependency = dependencyByNode[dependencyNode]

-            fun Node.setChildNodeByName(name: String, value: String?) {
-                val childNode: Node? = (get(name) as NodeList?)?.firstOrNull() as Node?
-                if (value != null) {
-                    (childNode ?: appendNode(name)).setValue(value)
-                } else {
-                    childNode?.let { remove(it) }
+            if (moduleDependency != null) {
+                if (includeOnlySpecifiedDependenciesSet != null && moduleDependency !in includeOnlySpecifiedDependenciesSet) {
+                    dependenciesNode.remove(dependencyNode)
+                    return@forEach
                }
            }

-            dependencyNode.setChildNodeByName("groupId", mapDependencyTo.group)
-            dependencyNode.setChildNodeByName("artifactId", mapDependencyTo.name)
-            dependencyNode.setChildNodeByName("version", mapDependencyTo.version)
+            val mapDependencyTo = resultDependenciesForEachUsageContext.get(moduleDependency)
+
+            if (mapDependencyTo != null) {
+                fun Node.setChildNodeByName(name: String, value: String?) {
+                    val childNode: Node? = (get(name) as NodeList?)?.firstOrNull() as Node?
+                    if (value != null) {
+                        (childNode ?: appendNode(name)).setValue(value)
+                    } else {
+                        childNode?.let { remove(it) }
+                    }
+                }
+
+                dependencyNode.setChildNodeByName("groupId", mapDependencyTo.group)
+                dependencyNode.setChildNodeByName("artifactId", mapDependencyTo.name)
+                dependencyNode.setChildNodeByName("version", mapDependencyTo.version)
+            }
        }
    }
 }

 private fun associateDependenciesWithActualModuleDependencies(
-    usageContext: KotlinUsageContext,
-    moduleDependencies: Set<ModuleDependency>
-): Map<ModuleDependency, ModuleDependency> {
+    usageContext: KotlinUsageContext
+): Map<ModuleCoordinates, ModuleCoordinates> {
    val compilation = usageContext.compilation
    val project = compilation.target.project

@@ -119,17 +140,19 @@ private fun associateDependenciesWithActualModuleDependencies(
        }
    }

-    val resolvedModulesByRootModuleCoordinates = targetDependenciesConfiguration
+    return targetDependenciesConfiguration
        .allDependencies.withType(ModuleDependency::class.java)
        .associate { dependency ->
+            val coordinates = ModuleCoordinates(dependency.group, dependency.name, dependency.version)
+            val noMapping = coordinates to coordinates
            when (dependency) {
                is ProjectDependency -> {
                    val dependencyProject = dependency.dependencyProject
                    val dependencyProjectKotlinExtension = dependencyProject.multiplatformExtensionOrNull
-                        ?: return@associate dependency to dependency
+                        ?: return@associate noMapping

                    val resolved = resolvedDependencies[Triple(dependency.group, dependency.name, dependency.version)]
-                        ?: return@associate dependency to dependency
+                        ?: return@associate noMapping

                    val resolvedToConfiguration = resolved.configuration
                    val dependencyTargetComponent: KotlinTargetComponent = run {
@@ -140,7 +163,7 @@ private fun associateDependenciesWithActualModuleDependencies(
                            }
                        }
                        // Failed to find a matching component:
-                        return@associate dependency to dependency
+                        return@associate noMapping
                    }

                    val targetModulePublication = (dependencyTargetComponent as? KotlinTargetComponentWithPublication)?.publicationDelegate
@@ -149,49 +172,37 @@ private fun associateDependenciesWithActualModuleDependencies(
                    // During Gradle POM generation, a project dependency is already written as the root module's coordinates. In the
                    // dependencies mapping, map the root module to the target's module:

-                    val rootModule = project.dependencies.module(
-                        listOf(
-                            rootModulePublication?.groupId ?: dependency.group,
-                            rootModulePublication?.artifactId ?: dependencyProject.name,
-                            rootModulePublication?.version ?: dependency.version
-                        ).joinToString(":")
-                    ) as ModuleDependency
+                    val rootModule = ModuleCoordinates(
+                        rootModulePublication?.groupId ?: dependency.group,
+                        rootModulePublication?.artifactId ?: dependencyProject.name,
+                        rootModulePublication?.version ?: dependency.version
+                    )

-                    rootModule to project.dependencies.module(
-                        listOf(
-                            targetModulePublication?.groupId ?: dependency.group,
-                            targetModulePublication?.artifactId ?: dependencyTargetComponent.defaultArtifactId,
-                            targetModulePublication?.version ?: dependency.version
-                        ).joinToString(":")
-                    ) as ModuleDependency
+                    rootModule to ModuleCoordinates(
+                        targetModulePublication?.groupId ?: dependency.group,
+                        targetModulePublication?.artifactId ?: dependencyTargetComponent.defaultArtifactId,
+                        targetModulePublication?.version ?: dependency.version
+                    )
                }
                else -> {
                    val resolvedDependency = resolvedDependencies[Triple(dependency.group, dependency.name, dependency.version)]
-                        ?: return@associate dependency to dependency
+                        ?: return@associate noMapping

                    if (resolvedDependency.moduleArtifacts.isEmpty() && resolvedDependency.children.size == 1) {
                        // This is a dependency on a module that resolved to another module; map the original dependency to the target module
                        val targetModule = resolvedDependency.children.single()
-                        dependency to project.dependencies.module(
-                            listOf(
-                                targetModule.moduleGroup,
-                                targetModule.moduleName,
-                                targetModule.moduleVersion
-                            ).joinToString(":")
-                        ) as ModuleDependency
+                        coordinates to ModuleCoordinates(
+                            targetModule.moduleGroup,
+                            targetModule.moduleName,
+                            targetModule.moduleVersion
+                        )

                    } else {
-                        dependency to dependency
+                        noMapping
                    }
                }
            }
-        }.mapKeys { (key, _) -> Triple(key.group, key.name, key.version) }
-
-    return moduleDependencies.associate { dependency ->
-        val key = Triple(dependency.group, dependency.name, dependency.version)
-        val value = resolvedModulesByRootModuleCoordinates[key] ?: dependency
-        dependency to value
-    }
+        }
 }

 private fun KotlinTargetComponent.findUsageContext(configurationName: String): UsageContext? {