From 557fb07f4478939b3f60e70ab08eb5ef69ee679d Mon Sep 17 00:00:00 2001 From: Sergey Igushkin Date: Wed, 12 Dec 2018 15:58:13 +0300 Subject: [PATCH] Fix the localToProject attribute schema for Gradle 4.10.2+ In Gradle 4.10.2, a change was made in the attributes disambiguation process: if some of the candidates have a single attribute value and others don't have that attribute (i.e. have a null value), the disambiguation rule for that attribute doesn't get called. Effectively, null values are excluded from disambiguation, but still may cause ambiguity (sic!) See: https://github.com/gradle/gradle/issues/6747#issuecomment-445921139 This change affected our attribute `localToProject` that we use to disambiguate the deprecated but still consumable configurations like `compile`, `runtime`, `testCompile`, `testRuntime` from those configurations which should have priority during project dependency resolution: the `*Element` ones. Our scheme marked the former configurations with the attribute and the latter were not marked, with a disambiguation rule that preferred null values. To fix this logic with Gradle 4.10.2, we instead mark both kinds of configurations with the attribute, which now has a value 'public' that is preferred by the rule over the other values. We also make sure that the attribute doesn't leak into the published Gradle metadata, as it is only needed for project-to-project dependencies resolution. Issue #KT-28795 Fixed --- .../kotlin/gradle/NewMultiplatformIT.kt | 4 ++++ .../kotlin/gradle/plugin/KotlinPlugin.kt | 2 ++ .../gradle/plugin/KotlinTargetConfigurator.kt | 2 ++ .../plugin/ProjectLocalConfigurations.kt | 22 ++++++++++++++----- .../plugin/mpp/HierarchyAttributeContainer.kt | 16 +++++++++----- .../plugin/mpp/KotlinSoftwareComponent.kt | 7 ++---- 6 files changed, 37 insertions(+), 16 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt index ed5e296e65e..1d9b0801e21 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt @@ -4,6 +4,7 @@ */ package org.jetbrains.kotlin.gradle +import org.jetbrains.kotlin.gradle.plugin.ProjectLocalConfigurations import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinMultiplatformPlugin import org.jetbrains.kotlin.gradle.plugin.mpp.UnusedSourceSetsChecker import org.jetbrains.kotlin.gradle.plugin.sources.METADATA_CONFIGURATION_NAME_SUFFIX @@ -75,6 +76,9 @@ class NewMultiplatformIT : BaseGradleIT() { Assert.assertTrue("$it should exist", groupDir.resolve(it).exists()) } + val gradleMetadata = groupDir.resolve("sample-lib/1.0/sample-lib-1.0.module").readText() + assertFalse(gradleMetadata.contains(ProjectLocalConfigurations.ATTRIBUTE.name)) + listOf(jvmJarName, jsJarName, metadataJarName, wasmKlibName, nativeKlibName).forEach { val pom = groupDir.resolve(it.replaceAfterLast('.', "pom")) Assert.assertTrue( 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 5816133a5c3..b13017853f1 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 @@ -448,11 +448,13 @@ internal abstract class AbstractKotlinPlugin( if (kotlinTarget.platformType != KotlinPlatformType.common) { project.configurations.getByName(kotlinTarget.apiElementsConfigurationName).run { attributes.attribute(Usage.USAGE_ATTRIBUTE, KotlinUsages.producerApiUsage(kotlinTarget)) + setupAsPublicConfigurationIfSupported(kotlinTarget) usesPlatformOf(kotlinTarget) } project.configurations.getByName(kotlinTarget.runtimeElementsConfigurationName).run { attributes.attribute(Usage.USAGE_ATTRIBUTE, KotlinUsages.producerRuntimeUsage(kotlinTarget)) + setupAsPublicConfigurationIfSupported(kotlinTarget) usesPlatformOf(kotlinTarget) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinTargetConfigurator.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinTargetConfigurator.kt index 0b38032ab39..3ba20c472dc 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinTargetConfigurator.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinTargetConfigurator.kt @@ -187,6 +187,7 @@ abstract class AbstractKotlinTargetConfigurator extendsFrom(runtimeConfiguration) } usesPlatformOf(target) + setupAsPublicConfigurationIfSupported(target) } if (mainCompilation is KotlinCompilationToRunnableFiles<*>) { @@ -199,6 +200,7 @@ abstract class AbstractKotlinTargetConfigurator val runtimeConfiguration = configurations.maybeCreate(mainCompilation.deprecatedRuntimeConfigurationName) extendsFrom(implementationConfiguration, runtimeOnlyConfiguration, runtimeConfiguration) usesPlatformOf(target) + setupAsPublicConfigurationIfSupported(target) } defaultConfiguration.extendsFrom(runtimeElementsConfiguration) } else { diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/ProjectLocalConfigurations.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/ProjectLocalConfigurations.kt index ccb08b90f08..2b7f4f347e9 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/ProjectLocalConfigurations.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/ProjectLocalConfigurations.kt @@ -10,8 +10,11 @@ import org.gradle.api.attributes.* import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinWithJavaTarget import org.jetbrains.kotlin.gradle.utils.isGradleVersionAtLeast -internal object ProjectLocalConfigurations { - val ATTRIBUTE = Attribute.of("org.jetbrains.kotlin.localToProject", String::class.java) +object ProjectLocalConfigurations { + val ATTRIBUTE: Attribute = Attribute.of("org.jetbrains.kotlin.localToProject", String::class.java) + + const val PUBLIC_VALUE = "public" + const val LOCAL_TO_PROJECT_PREFIX = "local to " fun setupAttributesMatchingStrategy(attributesSchema: AttributesSchema) = with(attributesSchema) { attribute(ATTRIBUTE) { @@ -30,9 +33,8 @@ internal object ProjectLocalConfigurations { class ProjectLocalDisambiguation : AttributeDisambiguationRule { override fun execute(details: MultipleCandidatesDetails) = with(details) { - if (candidateValues.contains(null)) { - @Suppress("NULLABILITY_MISMATCH_BASED_ON_JAVA_ANNOTATIONS") - closestMatch(null as String?) + if (candidateValues.contains(PUBLIC_VALUE)) { + closestMatch(PUBLIC_VALUE) } } } @@ -46,7 +48,15 @@ internal fun Configuration.setupAsLocalTargetSpecificConfigurationIfSupported(ta (target !is KotlinWithJavaTarget<*> || target.platformType != KotlinPlatformType.common) ) { usesPlatformOf(target) - attributes.attribute(ProjectLocalConfigurations.ATTRIBUTE, target.project.path) + attributes.attribute(ProjectLocalConfigurations.ATTRIBUTE, ProjectLocalConfigurations.LOCAL_TO_PROJECT_PREFIX + target.project.path) + } +} + +internal fun Configuration.setupAsPublicConfigurationIfSupported(target: KotlinTarget) { + if (gradleVersionSupportsAttributeRules && + (target !is KotlinWithJavaTarget<*> || target.platformType != KotlinPlatformType.common) + ) { + attributes.attribute(ProjectLocalConfigurations.ATTRIBUTE, ProjectLocalConfigurations.PUBLIC_VALUE) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/HierarchyAttributeContainer.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/HierarchyAttributeContainer.kt index 1c6c4f96fda..f1e5a9d50a1 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/HierarchyAttributeContainer.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/HierarchyAttributeContainer.kt @@ -20,19 +20,25 @@ import org.gradle.api.attributes.AttributeContainer import java.util.* // TODO better implementation: attribute invariants (no attrs with same name and different types allowed), thread safety? -class HierarchyAttributeContainer(val parent: AttributeContainer?) : AttributeContainer { +class HierarchyAttributeContainer( + val parent: AttributeContainer?, + val filterParentAttributes: (Attribute<*>) -> Boolean = { true } +) : AttributeContainer { private val attributesMap = Collections.synchronizedMap(mutableMapOf, Any>()) + private fun getFilteredParentAttribute(key: Attribute<*>) = + if (parent != null && filterParentAttributes(key)) parent.getAttribute(key) else null + override fun contains(key: Attribute<*>): Boolean = - attributesMap.contains(key) || parent?.contains(key) ?: false + attributesMap.contains(key) || getFilteredParentAttribute(key) != null @Suppress("UNCHECKED_CAST") override fun getAttribute(key: Attribute?): T? = - attributesMap.get(key as Attribute<*>) as T? ?: parent?.getAttribute(key) + attributesMap.get(key as Attribute<*>) as T? ?: getFilteredParentAttribute(key) as T? - override fun isEmpty(): Boolean = attributesMap.isEmpty() && (parent?.isEmpty ?: true) + override fun isEmpty(): Boolean = attributesMap.isEmpty() && (parent?.keySet().orEmpty().filter(filterParentAttributes).isEmpty()) - override fun keySet(): Set> = attributesMap.keys + parent?.keySet().orEmpty() + override fun keySet(): Set> = attributesMap.keys + parent?.keySet().orEmpty().filter(filterParentAttributes) override fun attribute(key: Attribute?, value: T): AttributeContainer { val checkedValue = requireNotNull(value as Any?) { "null values for attributes are not supported" } 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 620148517e2..fbb9acc1420 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 @@ -13,10 +13,7 @@ import org.gradle.api.capabilities.Capability import org.gradle.api.component.ComponentWithVariants import org.gradle.api.internal.component.SoftwareComponentInternal import org.gradle.api.internal.component.UsageContext -import org.jetbrains.kotlin.gradle.plugin.KotlinCompilation -import org.jetbrains.kotlin.gradle.plugin.KotlinCompilationToRunnableFiles -import org.jetbrains.kotlin.gradle.plugin.KotlinTarget -import org.jetbrains.kotlin.gradle.plugin.KotlinTargetComponent +import org.jetbrains.kotlin.gradle.plugin.* import org.jetbrains.kotlin.utils.ifEmpty class KotlinSoftwareComponent( @@ -72,7 +69,7 @@ internal class KotlinPlatformUsageContext( configuration.artifacts override fun getAttributes(): AttributeContainer = - configuration.outgoing.attributes + HierarchyAttributeContainer(configuration.outgoing.attributes) { it != ProjectLocalConfigurations.ATTRIBUTE } override fun getCapabilities(): Set = emptySet()