From d0604ffde8a6522dfe30e3d91e74b73316efc123 Mon Sep 17 00:00:00 2001 From: Sergey Igushkin Date: Fri, 11 Oct 2019 18:37:08 +0300 Subject: [PATCH] Fix `requiresVisibilityOf` in the Gradle plugin (KT-34312) * Fix adding new items (which was untested, as the logic was only tested by unit tests) * Exclude the source set's dependsOn transitive closure from the requiresVisibilityOf checks (i.e. these source sets are considered visible) * Fix minor issues in the `UnsatisfiedSourceSetVisibilityException` message * Add `requiresVisibilityOf` to integration tests Issue #KT-34312 Fixed --- .../org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt | 9 +++++++++ .../new-mpp-associate-compilations/build.gradle | 8 +++++++- .../gradle/plugin/sources/DefaultKotlinSourceSet.kt | 2 +- .../plugin/sources/SourceSetsVisibilityInference.kt | 10 +++++----- 4 files changed, 22 insertions(+), 7 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 20c9762b867..52326ed486b 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 @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.gradle.plugin.mpp.NativeOutputKind import org.jetbrains.kotlin.gradle.plugin.mpp.UnusedSourceSetsChecker import org.jetbrains.kotlin.gradle.plugin.sources.METADATA_CONFIGURATION_NAME_SUFFIX import org.jetbrains.kotlin.gradle.plugin.sources.SourceSetConsistencyChecks +import org.jetbrains.kotlin.gradle.plugin.sources.UnsatisfiedSourceSetVisibilityException import org.jetbrains.kotlin.gradle.targets.jvm.KotlinJvmTarget import org.jetbrains.kotlin.gradle.util.* import org.jetbrains.kotlin.konan.target.CompilerOutputKind @@ -2104,6 +2105,14 @@ class NewMultiplatformIT : BaseGradleIT() { // Native: assertFileExists("build/classes/kotlin/$nativeHostTargetName/integrationTest/integrationTest.klib") } + + gradleBuildScript().appendText( + "\nkotlin.sourceSets { getByName(\"commonTest\").requiresVisibilityOf(getByName(\"commonIntegrationTest\")) }" + ) + build { + assertFailed() + assertContains(UnsatisfiedSourceSetVisibilityException::class.java.simpleName) + } } @Test diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/new-mpp-associate-compilations/build.gradle b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/new-mpp-associate-compilations/build.gradle index af24737161d..e3891e64792 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/new-mpp-associate-compilations/build.gradle +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/new-mpp-associate-compilations/build.gradle @@ -22,7 +22,10 @@ kotlin { } } - create("commonIntegrationTest") + create("commonIntegrationTest") { + requiresVisibilityOf(getByName("commonMain")) + requiresVisibilityOf(getByName("commonTest")) + } } jvm { @@ -51,6 +54,9 @@ kotlin { compilations.create("integrationTest") { associateWith(compilations["test"]) defaultSourceSet.dependsOn(sourceSets["commonIntegrationTest"]) + defaultSourceSet.requiresVisibilityOf(sourceSets["commonMain"]) + defaultSourceSet.requiresVisibilityOf(sourceSets["commonTest"]) + defaultSourceSet.requiresVisibilityOf(sourceSets["commonIntegrationTest"]) } } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/sources/DefaultKotlinSourceSet.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/sources/DefaultKotlinSourceSet.kt index 9d5bb9532de..2d5a82e7623 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/sources/DefaultKotlinSourceSet.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/sources/DefaultKotlinSourceSet.kt @@ -124,7 +124,7 @@ class DefaultKotlinSourceSet( get() = Collections.unmodifiableSet(_requiresVisibilityOf) override fun requiresVisibilityOf(other: KotlinSourceSet) { - requiresVisibilityOf += other + _requiresVisibilityOf += other } //region IDE import for Granular source sets metadata diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/sources/SourceSetsVisibilityInference.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/sources/SourceSetsVisibilityInference.kt index eaea8e538c1..ab5793cb255 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/sources/SourceSetsVisibilityInference.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/sources/SourceSetsVisibilityInference.kt @@ -66,7 +66,7 @@ class UnsatisfiedSourceSetVisibilityException( append( "The source set ${sourceSet.name} requires visibility of the " + singularOrPlural(requiredButNotVisible, "source set", "source sets:") + " " + - "${requiredButNotVisible.joinToString()}. " + + "${requiredButNotVisible.joinToString { it.name }}. " + "This requirement was not satisfied.\n\n" ) @@ -90,7 +90,7 @@ class UnsatisfiedSourceSetVisibilityException( append( if (isAssociatedCompilation) - ", which compiles" + + ", which compiles " + singularOrPlural(allKotlinSourceSets, "source set ", "source sets: ") + allKotlinSourceSets.joinToString { it.name } + "\n" @@ -109,9 +109,9 @@ class UnsatisfiedSourceSetVisibilityException( } else { append( "${indent}To ensure the required visibility, the compilation " + compilationWithTarget(compilation) + - "must have a direct or indirect associate that compiles the source " + + " must have a direct or indirect associate that compiles the source " + singularOrPlural(missingRequiredSourceSets, "set ", "sets: ") + - missingRequiredSourceSets.joinToString() + "\n" + missingRequiredSourceSets.joinToString { it.name } + "\n" ) } } @@ -140,7 +140,7 @@ internal fun checkSourceSetVisibilityRequirements( val inferredVisibility = getVisibleSourceSetsFromAssociateCompilations(compilationsBySourceSet[sourceSet].orEmpty()) - val requiredButNotVisible = requiredVisibility - inferredVisibility //TODO minus dependsOn? + val requiredButNotVisible = requiredVisibility - inferredVisibility - sourceSet.getSourceSetHierarchy() if (requiredButNotVisible.isNotEmpty()) { val compilations = compilationsBySourceSet.getValue(sourceSet)