From 6a6a2056873c1643ddd8170a5742357a8dfa11fc Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 5 Jun 2024 11:06:31 +0200 Subject: [PATCH 1/4] Improve exception message --- src/main/kotlin/KotlinKlibExtractAbiTask.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/KotlinKlibExtractAbiTask.kt b/src/main/kotlin/KotlinKlibExtractAbiTask.kt index 2722f7a6..22602f4f 100644 --- a/src/main/kotlin/KotlinKlibExtractAbiTask.kt +++ b/src/main/kotlin/KotlinKlibExtractAbiTask.kt @@ -67,7 +67,7 @@ public abstract class KotlinKlibExtractAbiTask : DefaultTask() { val targetsToRemove = dump.targets.filter { it.targetName !in enabledTargets } if (targetsToRemove.isNotEmpty() && strictValidation.get()) { throw IllegalStateException( - "Validation could not be performed as some targets are not available " + + "Validation could not be performed as some targets (namely, $targetsToRemove) are not available " + "and the strictValidation mode was enabled." ) } From e44424ff67b66ca1ff6254efc2e2ce2f731b2c72 Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 5 Jun 2024 11:17:07 +0200 Subject: [PATCH 2/4] Added a test reproducing the issue from #234 (p. 1 & 2) --- .../validation/test/KlibVerificationTests.kt | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt index 372f9a60..9b18e486 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt @@ -791,4 +791,26 @@ internal class KlibVerificationTests : BaseKotlinGradleTest() { assertTaskFailure(":klibApiCheck") } } + + @Test + fun `apiCheck should fail after target removal`() { + val runner = test { + settingsGradleKts { + resolve("/examples/gradle/settings/settings-name-testproject.gradle.kts") + } + // only a single native target is defined there + buildGradleKts { + resolve("/examples/gradle/base/withNativePluginAndSingleTarget.gradle.kts") + } + addToSrcSet("/examples/classes/AnotherBuildConfig.kt") + // dump was created for multiple native targets + abiFile(projectName = "testproject") { + resolve("/examples/classes/AnotherBuildConfig.klib.dump") + } + runApiCheck() + } + runner.buildAndFail().apply { + assertTaskFailure(":klibApiCheck") + } + } } From 4176bbb21cdc2193e4611580cc83b585e41ccfee Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 5 Jun 2024 11:23:57 +0200 Subject: [PATCH 3/4] Removed native targets should trigger validation failure Fixes #234 --- api/binary-compatibility-validator.api | 2 +- src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt | 10 +++++----- src/main/kotlin/KotlinKlibExtractAbiTask.kt | 11 +++++------ 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/api/binary-compatibility-validator.api b/api/binary-compatibility-validator.api index ad8325d0..892ac82e 100644 --- a/api/binary-compatibility-validator.api +++ b/api/binary-compatibility-validator.api @@ -88,8 +88,8 @@ public abstract class kotlinx/validation/KotlinKlibExtractAbiTask : org/gradle/a public fun ()V public abstract fun getInputAbiFile ()Lorg/gradle/api/file/RegularFileProperty; public abstract fun getOutputAbiFile ()Lorg/gradle/api/file/RegularFileProperty; - public abstract fun getRequiredTargets ()Lorg/gradle/api/provider/SetProperty; public final fun getStrictValidation ()Lorg/gradle/api/provider/Property; + public abstract fun getTargetsToRemove ()Lorg/gradle/api/provider/SetProperty; } public abstract class kotlinx/validation/KotlinKlibInferAbiTask : org/gradle/api/DefaultTask { diff --git a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt index c498aa3c..ccaf4d2c 100644 --- a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt +++ b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt @@ -423,7 +423,7 @@ private class KlibValidationPipelineBuilder( "the golden file stored in the project" group = "other" strictValidation.set(extension.klib.strictValidation) - requiredTargets.addAll(supportedTargets()) + targetsToRemove.addAll(unsupportedTargets()) inputAbiFile.fileProvider(klibApiDir.map { it.resolve(klibDumpFileName) }) outputAbiFile.fileProvider(klibOutputDir.map { it.resolve(klibDumpFileName) }) } @@ -534,8 +534,8 @@ private class KlibValidationPipelineBuilder( } } - // Compilable targets supported by the host compiler - private fun Project.supportedTargets(): Provider> { + // Compilable targets not supported by the host compiler + private fun Project.unsupportedTargets(): Provider> { val banned = bannedTargets() // for testing only return project.provider { val hm = HostManager() @@ -543,9 +543,9 @@ private class KlibValidationPipelineBuilder( .asSequence() .filter { if (it is KotlinNativeTarget) { - hm.isEnabled(it.konanTarget) && it.targetName !in banned + !hm.isEnabled(it.konanTarget) || it.targetName in banned } else { - true + false } } .map { it.toKlibTarget() } diff --git a/src/main/kotlin/KotlinKlibExtractAbiTask.kt b/src/main/kotlin/KotlinKlibExtractAbiTask.kt index 22602f4f..33b3b3e5 100644 --- a/src/main/kotlin/KotlinKlibExtractAbiTask.kt +++ b/src/main/kotlin/KotlinKlibExtractAbiTask.kt @@ -30,10 +30,10 @@ public abstract class KotlinKlibExtractAbiTask : DefaultTask() { public abstract val inputAbiFile: RegularFileProperty /** - * List of the targets that the resulting dump should contain. + * List of the targets that need to be filtered out from [inputAbiFile]. */ @get:Input - public abstract val requiredTargets: SetProperty + public abstract val targetsToRemove: SetProperty /** * Refer to [KlibValidationSettings.strictValidation] for details. @@ -61,17 +61,16 @@ public abstract class KotlinKlibExtractAbiTask : DefaultTask() { error("Project ABI file ${inputFile.relativeTo(rootDir)} is empty.") } val dump = KlibDump.from(inputFile) - val enabledTargets = requiredTargets.get().map(KlibTarget::targetName).toSet() + val unsupportedTargets = targetsToRemove.get().map(KlibTarget::targetName).toSet() // Filter out only unsupported files. // That ensures that target renaming will be caught and reported as a change. - val targetsToRemove = dump.targets.filter { it.targetName !in enabledTargets } - if (targetsToRemove.isNotEmpty() && strictValidation.get()) { + if (unsupportedTargets.isNotEmpty() && strictValidation.get()) { throw IllegalStateException( "Validation could not be performed as some targets (namely, $targetsToRemove) are not available " + "and the strictValidation mode was enabled." ) } - dump.remove(targetsToRemove) + dump.remove(unsupportedTargets.map(KlibTarget::parse)) dump.saveTo(outputAbiFile.asFile.get()) } } From 3ed0bf94fce6b944a3730a159dab85240fae61f9 Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 5 Jun 2024 18:28:59 +0200 Subject: [PATCH 4/4] Validate that check failed due to changed targets --- .../kotlin/kotlinx/validation/test/KlibVerificationTests.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt index 9b18e486..1a7f9bc4 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt @@ -811,6 +811,10 @@ internal class KlibVerificationTests : BaseKotlinGradleTest() { } runner.buildAndFail().apply { assertTaskFailure(":klibApiCheck") + Assertions.assertThat(output) + .contains("-// Targets: [androidNativeArm32, androidNativeArm64, androidNativeX64, " + + "androidNativeX86, linuxArm64, linuxX64, mingwX64]") + .contains("+// Targets: [linuxArm64]") } } }