From f3b53bbe4eb7d3819eb3f14365d28f718a66d0dc Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 3 Apr 2024 20:02:20 +0200 Subject: [PATCH 1/3] Check source sets existence for Klib dump tasks only after compilation (#210) * Added tests * Change dependencies between tasks so that tasks using information about source sets depended on klib compilation --- .../kotlin/kotlinx/validation/api/TestDsl.kt | 8 ++--- .../validation/test/KlibVerificationTests.kt | 29 +++++++++++++++++++ .../classes/GeneratedSources.klib.dump | 12 ++++++++ .../generatedSources.gradle.kts | 22 ++++++++++++++ .../BinaryCompatibilityValidatorPlugin.kt | 9 +++++- 5 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 src/functionalTest/resources/examples/classes/GeneratedSources.klib.dump create mode 100644 src/functionalTest/resources/examples/gradle/configuration/generatedSources/generatedSources.gradle.kts diff --git a/src/functionalTest/kotlin/kotlinx/validation/api/TestDsl.kt b/src/functionalTest/kotlin/kotlinx/validation/api/TestDsl.kt index 42f3c914..43d96770 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/api/TestDsl.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/api/TestDsl.kt @@ -148,8 +148,8 @@ internal fun FileContainer.emptyApiFile(projectName: String) { apiFile(projectName) {} } -internal fun BaseKotlinScope.runner(fn: Runner.() -> Unit) { - val runner = Runner() +internal fun BaseKotlinScope.runner(withConfigurationCache: Boolean = true, fn: Runner.() -> Unit) { + val runner = Runner(withConfigurationCache) fn(runner) this.runner = runner @@ -188,9 +188,9 @@ internal class AppendableScope(val filePath: String) { val files: MutableList = mutableListOf() } -internal class Runner { +internal class Runner(withConfigurationCache: Boolean = true) { val arguments: MutableList = mutableListOf().apply { - if (!koverEnabled) { + if (!koverEnabled && withConfigurationCache) { // Configuration cache is incompatible with javaagents being enabled for Gradle // See https://github.com/gradle/gradle/issues/25979 add("--configuration-cache") diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt index 5307e86f..a544f032 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt @@ -669,4 +669,33 @@ internal class KlibVerificationTests : BaseKotlinGradleTest() { } runner.build() } + + @Test + fun `apiDump for a project with generated sources only`() { + val runner = test { + baseProjectSetting() + additionalBuildConfig("/examples/gradle/configuration/generatedSources/generatedSources.gradle.kts") + // TODO: enable configuration cache back when we start skipping tasks correctly + runner(withConfigurationCache = false) { + arguments.add(":apiDump") + } + } + checkKlibDump(runner.build(), "/examples/classes/GeneratedSources.klib.dump") + } + + @Test + fun `apiCheck for a project with generated sources only`() { + val runner = test { + baseProjectSetting() + additionalBuildConfig("/examples/gradle/configuration/generatedSources/generatedSources.gradle.kts") + abiFile(projectName = "testproject") { + resolve("/examples/classes/GeneratedSources.klib.dump") + } + // TODO: enable configuration cache back when we start skipping tasks correctly + runner(withConfigurationCache = false) { + arguments.add(":apiCheck") + } + } + assertApiCheckPassed(runner.build()) + } } diff --git a/src/functionalTest/resources/examples/classes/GeneratedSources.klib.dump b/src/functionalTest/resources/examples/classes/GeneratedSources.klib.dump new file mode 100644 index 00000000..97ba68f3 --- /dev/null +++ b/src/functionalTest/resources/examples/classes/GeneratedSources.klib.dump @@ -0,0 +1,12 @@ +// Klib ABI Dump +// Targets: [androidNativeArm32, androidNativeArm64, androidNativeX64, androidNativeX86, linuxArm64, linuxX64, mingwX64] +// Rendering settings: +// - Signature version: 2 +// - Show manifest properties: true +// - Show declarations: true + +// Library unique name: +final class /Generated { // /Generated|null[0] + constructor () // /Generated.|(){}[0] + final fun helloCreator(): kotlin/Int // /Generated.helloCreator|helloCreator(){}[0] +} diff --git a/src/functionalTest/resources/examples/gradle/configuration/generatedSources/generatedSources.gradle.kts b/src/functionalTest/resources/examples/gradle/configuration/generatedSources/generatedSources.gradle.kts new file mode 100644 index 00000000..ca514f05 --- /dev/null +++ b/src/functionalTest/resources/examples/gradle/configuration/generatedSources/generatedSources.gradle.kts @@ -0,0 +1,22 @@ +abstract class GenerateSourcesTask : org.gradle.api.DefaultTask() { + @get:org.gradle.api.tasks.OutputDirectory + abstract val outputDirectory: org.gradle.api.file.DirectoryProperty + + @org.gradle.api.tasks.TaskAction + fun generate() { + outputDirectory.asFile.get().mkdirs() + outputDirectory.file("Generated.kt").get().asFile.writeText(""" + public class Generated { public fun helloCreator(): Int = 42 } + """.trimIndent()) + } +} + +val srcgen = project.tasks.register("generateSources", GenerateSourcesTask::class.java) +srcgen.configure { + outputDirectory.set(project.layout.buildDirectory.get().dir("generated").dir("kotlin")) +} + +val kotlin = project.extensions.getByType(org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension::class.java) +kotlin.sourceSets.getByName("commonMain") { + kotlin.srcDir(srcgen) +} diff --git a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt index 8fda84a6..38efe307 100644 --- a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt +++ b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt @@ -398,9 +398,16 @@ private class KlibValidationPipelineBuilder( commonApiCheck.configure { it.dependsOn(klibCheck) } klibDump.configure { it.dependsOn(klibMergeInferred) } + // Extraction task depends on supportedTargets() provider which returns a set of targets supported + // by the host compiler and having some sources. A set of sources for a target may change until the actual + // klib compilation will take place, so we may observe incorrect value if check source sets earlier. + // Merge task already depends on compilations, so instead of adding each compilation task to the extraction's + // dependency set, we can depend on the merge task itself. + klibExtractAbiForSupportedTargets.configure { + it.dependsOn(klibMerge) + } klibCheck.configure { it.dependsOn(klibExtractAbiForSupportedTargets) - it.dependsOn(klibMerge) } project.configureTargets(klibApiDir, klibMerge, klibMergeInferred) From 4de530aba808fa0e335ceade760c05e57a9ab4d9 Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 3 Apr 2024 20:02:50 +0200 Subject: [PATCH 2/3] Revert JVM tasks configuration changes introduced in #200 (#212) * Rollback task skipping logic based on srcset emptyness check for JVM tasks --- .../kotlin/kotlinx/validation/api/Assert.kt | 7 +++ .../validation/test/JvmProjectTests.kt | 54 +++++++++++++++++++ ...kt => MultiPlatformSingleJvmTargetTest.kt} | 44 ++++++++++++++- .../examples/classes/GeneratedSources.dump | 5 ++ .../generatedJvmSources.gradle.kts | 21 ++++++++ .../BinaryCompatibilityValidatorPlugin.kt | 4 +- 6 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 src/functionalTest/kotlin/kotlinx/validation/test/JvmProjectTests.kt rename src/functionalTest/kotlin/kotlinx/validation/test/{MultiPlatformSingleJvmKlibTargetTest.kt => MultiPlatformSingleJvmTargetTest.kt} (77%) create mode 100644 src/functionalTest/resources/examples/classes/GeneratedSources.dump create mode 100644 src/functionalTest/resources/examples/gradle/configuration/generatedSources/generatedJvmSources.gradle.kts diff --git a/src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt b/src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt index 8a3950b4..64dd2e4f 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt @@ -31,6 +31,13 @@ internal fun BuildResult.assertTaskSkipped(task: String) { assertTaskOutcome(TaskOutcome.SKIPPED, task) } +/** + * Helper `fun` for asserting a [TaskOutcome] to be equal to [TaskOutcome.UP_TO_DATE] + */ +internal fun BuildResult.assertTaskUpToDate(task: String) { + assertTaskOutcome(TaskOutcome.UP_TO_DATE, task) +} + private fun BuildResult.assertTaskOutcome(taskOutcome: TaskOutcome, taskName: String) { assertEquals(taskOutcome, task(taskName)?.outcome) } diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/JvmProjectTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/JvmProjectTests.kt new file mode 100644 index 00000000..026f3e59 --- /dev/null +++ b/src/functionalTest/kotlin/kotlinx/validation/test/JvmProjectTests.kt @@ -0,0 +1,54 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package kotlinx.validation.test + +import kotlinx.validation.api.* +import kotlinx.validation.api.resolve +import kotlinx.validation.api.test +import org.assertj.core.api.Assertions +import org.junit.Test + +class JvmProjectTests : BaseKotlinGradleTest() { + @Test + fun `apiDump for a project with generated sources only`() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + resolve("/examples/gradle/configuration/generatedSources/generatedJvmSources.gradle.kts") + } + // TODO: enable configuration cache back when we start skipping tasks correctly + runner(withConfigurationCache = false) { + arguments.add(":apiDump") + } + } + runner.build().apply { + assertTaskSuccess(":apiDump") + + val expectedApi = readFileList("/examples/classes/GeneratedSources.dump") + Assertions.assertThat(rootProjectApiDump.readText()).isEqualToIgnoringNewLines(expectedApi) + } + } + + @Test + fun `apiCheck for a project with generated sources only`() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + resolve("/examples/gradle/configuration/generatedSources/generatedJvmSources.gradle.kts") + } + apiFile(projectName = rootProjectDir.name) { + resolve("/examples/classes/GeneratedSources.dump") + } + // TODO: enable configuration cache back when we start skipping tasks correctly + runner(withConfigurationCache = false) { + arguments.add(":apiCheck") + } + } + runner.build().apply { + assertTaskSuccess(":apiCheck") + } + } +} diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/MultiPlatformSingleJvmKlibTargetTest.kt b/src/functionalTest/kotlin/kotlinx/validation/test/MultiPlatformSingleJvmTargetTest.kt similarity index 77% rename from src/functionalTest/kotlin/kotlinx/validation/test/MultiPlatformSingleJvmKlibTargetTest.kt rename to src/functionalTest/kotlin/kotlinx/validation/test/MultiPlatformSingleJvmTargetTest.kt index dde16426..0e32ce33 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/MultiPlatformSingleJvmKlibTargetTest.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/MultiPlatformSingleJvmTargetTest.kt @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 JetBrains s.r.o. + * Copyright 2016-2024 JetBrains s.r.o. * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. */ @@ -7,10 +7,11 @@ package kotlinx.validation.test import kotlinx.validation.api.* import org.assertj.core.api.Assertions.assertThat +import org.gradle.testkit.runner.TaskOutcome import org.junit.Test import java.io.File -internal class MultiPlatformSingleJvmKlibTargetTest : BaseKotlinGradleTest() { +internal class MultiPlatformSingleJvmTargetTest : BaseKotlinGradleTest() { private fun BaseKotlinScope.createProjectHierarchyWithPluginOnRoot() { settingsGradleKts { resolve("/examples/gradle/settings/settings-name-testproject.gradle.kts") @@ -116,6 +117,45 @@ internal class MultiPlatformSingleJvmKlibTargetTest : BaseKotlinGradleTest() { } } + @Test + fun testApiDumpPassesForEmptyProject() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/multiplatformWithSingleJvmTarget.gradle.kts") + } + + runner { + arguments.add(":apiDump") + } + } + + runner.build().apply { + assertTaskSkipped(":jvmApiDump") + assertTaskUpToDate(":apiDump") + } + } + + @Test + fun testApiCheckPassesForEmptyProject() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/multiplatformWithSingleJvmTarget.gradle.kts") + } + + emptyApiFile(projectName = rootProjectDir.name) + + runner { + arguments.add(":apiCheck") + } + } + + runner.build().apply { + assertTaskSkipped(":jvmApiCheck") + assertTaskUpToDate(":apiCheck") + + } + } + private val jvmApiDump: File get() = rootProjectDir.resolve("$API_DIR/testproject.api") } diff --git a/src/functionalTest/resources/examples/classes/GeneratedSources.dump b/src/functionalTest/resources/examples/classes/GeneratedSources.dump new file mode 100644 index 00000000..f69e8d22 --- /dev/null +++ b/src/functionalTest/resources/examples/classes/GeneratedSources.dump @@ -0,0 +1,5 @@ +public final class Generated { + public fun ()V + public final fun helloCreator ()I +} + diff --git a/src/functionalTest/resources/examples/gradle/configuration/generatedSources/generatedJvmSources.gradle.kts b/src/functionalTest/resources/examples/gradle/configuration/generatedSources/generatedJvmSources.gradle.kts new file mode 100644 index 00000000..57f7ef19 --- /dev/null +++ b/src/functionalTest/resources/examples/gradle/configuration/generatedSources/generatedJvmSources.gradle.kts @@ -0,0 +1,21 @@ +abstract class GenerateSourcesTask : org.gradle.api.DefaultTask() { + @get:org.gradle.api.tasks.OutputDirectory + abstract val outputDirectory: org.gradle.api.file.DirectoryProperty + + @org.gradle.api.tasks.TaskAction + fun generate() { + outputDirectory.asFile.get().mkdirs() + outputDirectory.file("Generated.kt").get().asFile.writeText(""" + public class Generated { public fun helloCreator(): Int = 42 } + """.trimIndent()) + } +} + +val srcgen = project.tasks.register("generateSources", GenerateSourcesTask::class.java) +srcgen.configure { + outputDirectory.set(project.layout.buildDirectory.get().dir("generated").dir("kotlin")) +} + +project.sourceSets.getByName("main") { + kotlin.srcDir(srcgen) +} diff --git a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt index 38efe307..8f9f6b06 100644 --- a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt +++ b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt @@ -219,9 +219,7 @@ private fun Project.configureKotlinCompilation( val apiBuild = task(targetConfig.apiTaskName("Build")) { // Do not enable task for empty umbrella modules - isEnabled = apiCheckEnabled(projectName, extension) - val hasSourcesPredicate = compilation.hasAnySourcesPredicate() - onlyIf { hasSourcesPredicate.get() } + isEnabled = apiCheckEnabled(projectName, extension) && compilation.hasAnySources() // 'group' is not specified deliberately, so it will be hidden from ./gradlew tasks description = "Builds Kotlin API for 'main' compilations of $projectName. Complementary task and shouldn't be called manually" From f8e3ded412a7fda76d561456161fa0c1eaccfb51 Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Thu, 4 Apr 2024 10:10:09 +0200 Subject: [PATCH 3/3] Version 0.15.0-Beta.2 --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 594ee5fd..55e2c031 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ Binary compatibility validator is a Gradle plugin that can be added to your buil - in `build.gradle.kts` ```kotlin plugins { - id("org.jetbrains.kotlinx.binary-compatibility-validator") version "0.15.0-Beta.1" + id("org.jetbrains.kotlinx.binary-compatibility-validator") version "0.15.0-Beta.2" } ``` @@ -38,7 +38,7 @@ plugins { ```groovy plugins { - id 'org.jetbrains.kotlinx.binary-compatibility-validator' version '0.15.0-Beta.1' + id 'org.jetbrains.kotlinx.binary-compatibility-validator' version '0.15.0-Beta.2' } ```