Correctly handle disappeared native/web targets (#236)

* Improve exception message * Added a test reproducing the issue from #234 (p. 1 & 2) * Removed native targets should trigger validation failure Fixes #234
Kotlin · Jun 24, 2024 · db8c335 · db8c335
1 parent d72d9b6
commit db8c335
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 13 deletions.
diff --git 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 <init> ()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/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt
@@ -791,4 +791,30 @@ 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")
+            Assertions.assertThat(output)
+                .contains("-// Targets: [androidNativeArm32, androidNativeArm64, androidNativeX64, " +
+                        "androidNativeX86, linuxArm64, linuxX64, mingwX64]")
+                .contains("+// Targets: [linuxArm64]")
+        }
+    }
 }
diff --git 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,18 +534,18 @@ private class KlibValidationPipelineBuilder(
         }
     }
 
-    // Compilable targets supported by the host compiler
-    private fun Project.supportedTargets(): Provider<Set<KlibTarget>> {
+    // Compilable targets not supported by the host compiler
+    private fun Project.unsupportedTargets(): Provider<Set<KlibTarget>> {
         val banned = bannedTargets() // for testing only
         return project.provider {
             val hm = HostManager()
             project.kotlinMultiplatform.targets.matching { it.emitsKlib }
                 .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
@@ -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<KlibTarget>
+    public abstract val targetsToRemove: SetProperty<KlibTarget>
 
     /**
      * 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 are not available " +
+                "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())
     }
 }