Avoid using hasAnySourcesPredicate-predicate to filter klib-related tasks

Author: fzhinkin

src/functionalTest/kotlin/kotlinx/validation/test/KlibVerificationTests.kt

@@ -666,12 +666,29 @@ internal class KlibVerificationTests : BaseKotlinGradleTest() {
             runApiDump()
         }
 
-        runner.build().apply {
+        runner.withDebug(true).build().apply {
             assertTaskSkipped(":klibApiDump")
         }
         assertFalse(runner.projectDir.resolve("api").exists())
     }
 
+    @Test
+    fun `apiDump should remove dump file if the project does not contain sources anymore`() {
+        val runner = test {
+            baseProjectSetting()
+            addToSrcSet("/examples/classes/AnotherBuildConfig.kt", sourceSet = "commonTest")
+            abiFile(projectName = "testproject") {
+                resolve("/examples/classes/AnotherBuildConfig.klib.dump")
+            }
+            runApiDump()
+        }
+
+        runner.build().apply {
+            assertTaskSuccess(":klibApiDump")
+        }
+        assertFalse(runner.projectDir.resolve("api").resolve("testproject.klib.api").exists())
+    }
+
     @Test
     fun `apiDump should not fail if there is only one target`() {
         val runner = test {
@@ -698,8 +715,7 @@ internal class KlibVerificationTests : BaseKotlinGradleTest() {
         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) {
+            runner {
                 arguments.add(":apiDump")
             }
         }
@@ -714,8 +730,7 @@ internal class KlibVerificationTests : BaseKotlinGradleTest() {
             abiFile(projectName = "testproject") {
                 resolve("/examples/classes/GeneratedSources.klib.dump")
             }
-            // TODO: enable configuration cache back when we start skipping tasks correctly
-            runner(withConfigurationCache = false) {
+            runner {
                 arguments.add(":apiCheck")
             }
         }

src/main/kotlin/-Utils.kt

@@ -10,6 +10,7 @@ import kotlinx.validation.api.klib.konanTargetNameMapping
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.InputFiles
+import org.gradle.api.tasks.SkipWhenEmpty
 import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType
 import org.jetbrains.kotlin.gradle.plugin.KotlinTarget
 import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget
@@ -51,5 +52,6 @@ public class KlibDumpMetadata(
      * Path to a resulting dump file.
      */
     @get:InputFiles
+    @get:SkipWhenEmpty
     public val dumpFile: RegularFileProperty
 ) : Serializable

src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt

@@ -304,7 +304,7 @@ private fun Project.configureCheckTasks(
     }
 
     val dumpFileName = project.jvmDumpFileName
-    val apiDump = task<CopyFile>(targetConfig.apiTaskName("Dump")) {
+    val apiDump = task<SyncFile>(targetConfig.apiTaskName("Dump")) {
         isEnabled = apiCheckEnabled(projectName, extension) && apiBuild.map { it.enabled }.getOrElse(true)
         group = "other"
         description = "Syncs API from build dir to ${targetConfig.apiDir} dir for $projectName"
@@ -343,7 +343,7 @@ private const val KLIB_INFERRED_DUMPS_DIRECTORY = "klib-all"
  * Here's how different tasks depend on each other:
  * - `klibApiCheck` ([KotlinApiCompareTask]) depends on `klibApiMerge` and `klibApiExtractForValidation` tasks;
  * this task itself does not perform anything except comparing the result of a merge, with a preprocessed golden value;
- * - `klibApiDump` ([CopyFile]) depends on `klibApiMergeInferred` and simply moves the merged ABI dump into a configured
+ * - `klibApiDump` ([SyncFile]) depends on `klibApiMergeInferred` and simply moves the merged ABI dump into a configured
  * api directory within a project;
  * - `klibApiMerge` and `klibApiMergeInferred` are both [KotlinKlibMergeAbiTask] instances merging multiple individual
  * KLib ABI dumps into a single merged dump file; these tasks differs only by their dependencies and input dump files
@@ -387,51 +387,36 @@ private class KlibValidationPipelineBuilder(
         val klibMergeInferred = project.mergeInferredKlibsUmbrellaTask(klibDumpConfig, klibMergeInferredDir)
         val klibDump = project.dumpKlibsTask(klibDumpConfig)
         val klibExtractAbiForSupportedTargets = project.extractAbi(klibDumpConfig, klibApiDir, klibExtractedFileDir)
-        val klibCheck = project.checkKlibsTask(klibDumpConfig, project.provider { klibExtractedFileDir }, klibMergeDir)
+        val klibCheck = project.checkKlibsTask(klibDumpConfig)
         klibDump.configure {
             it.from.set(klibMergeInferred.flatMap { it.mergedApiFile })
             val filename = project.klibDumpFileName
             it.to.fileProvider(klibApiDir.map { it.resolve(filename) })
         }
-        commonApiDump.configure { it.dependsOn(klibDump) }
-
-        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.projectApiFile.set(klibExtractAbiForSupportedTargets.flatMap { it.outputAbiFile })
+            it.generatedApiFile.set(klibMerge.flatMap { it.mergedApiFile })
         }
+        commonApiDump.configure { it.dependsOn(klibDump) }
         commonApiCheck.configure { it.dependsOn(klibCheck) }
         project.configureTargets(klibApiDir, klibMerge, klibMergeInferred)
     }
 
-    private fun Project.checkKlibsTask(
-        klibDumpConfig: TargetConfig,
-        klibApiDir: Provider<File>,
-        klibMergeDir: File
-    ) = project.task<KotlinApiCompareTask>(klibDumpConfig.apiTaskName("Check")) {
+    private fun Project.checkKlibsTask(klibDumpConfig: TargetConfig)
+    = project.task<KotlinApiCompareLazyTask>(klibDumpConfig.apiTaskName("Check")) {
         isEnabled = klibAbiCheckEnabled(project.name, extension)
         group = "verification"
-        description = "Checks signatures of a public KLib ABI against the golden value in ABI folder for " +
-                project.name
-        projectApiFile = klibApiDir.get().resolve(klibDumpFileName)
-        generatedApiFile = klibMergeDir.resolve(klibDumpFileName)
-        val hasCompilableTargets = project.hasCompilableTargetsPredicate()
-        onlyIf("There are no klibs compiled for the project") { hasCompilableTargets.get() }
+        description = "Checks signatures of a public KLib ABI against the golden value in ABI folder for ${project.name}"
     }
 
-    private fun Project.dumpKlibsTask(klibDumpConfig: TargetConfig) = project.task<CopyFile>(klibDumpConfig.apiTaskName("Dump")) {
+    private fun Project.dumpKlibsTask(klibDumpConfig: TargetConfig) = project.task<SyncFile>(klibDumpConfig.apiTaskName("Dump")) {
         isEnabled = klibAbiCheckEnabled(project.name, extension)
         description = "Syncs a KLib ABI dump from a build dir to the ${klibDumpConfig.apiDir} dir for ${project.name}"
         group = "other"
-        val hasCompilableTargets = project.hasCompilableTargetsPredicate()
-        onlyIf("There are no klibs compiled for the project") { hasCompilableTargets.get() }
+        onlyIf {
+            it as SyncFile
+            it.to.get().asFile.exists() || it.from.get().asFile.exists()
+        }
     }
 
     private fun Project.extractAbi(
@@ -450,8 +435,6 @@ private class KlibValidationPipelineBuilder(
         requiredTargets.addAll(supportedTargets())
         inputAbiFile.set(klibApiDir.get().resolve(klibDumpFileName))
         outputAbiFile.set(klibOutputDir.resolve(klibDumpFileName))
-        val hasCompilableTargets = project.hasCompilableTargetsPredicate()
-        onlyIf("There are no klibs compiled for the project") { hasCompilableTargets.get() }
     }
 
     private fun Project.mergeInferredKlibsUmbrellaTask(
@@ -466,8 +449,6 @@ private class KlibValidationPipelineBuilder(
                 "different targets (including inferred dumps for unsupported targets) " +
                 "into a single merged KLib ABI dump"
         mergedApiFile.set(klibMergeDir.resolve(klibDumpFileName))
-        val hasCompilableTargets = project.hasCompilableTargetsPredicate()
-        onlyIf("There are no dumps to merge") { hasCompilableTargets.get() }
     }
 
     private fun Project.mergeKlibsUmbrellaTask(
@@ -478,8 +459,6 @@ private class KlibValidationPipelineBuilder(
         description = "Merges multiple KLib ABI dump files generated for " +
                 "different targets into a single merged KLib ABI dump"
         mergedApiFile.set(klibMergeDir.resolve(klibDumpFileName))
-        val hasCompilableTargets = project.hasCompilableTargetsPredicate()
-        onlyIf("There are no dumps to merge") { hasCompilableTargets.get() }
     }
 
     fun Project.bannedTargets(): Set<String> {
@@ -597,8 +576,6 @@ private class KlibValidationPipelineBuilder(
         val buildTask = project.task<KotlinKlibAbiBuildTask>(targetConfig.apiTaskName("Build")) {
             // Do not enable task for empty umbrella modules
             isEnabled = klibAbiCheckEnabled(projectName, extension)
-            val hasSourcesPredicate = compilation.hasAnySourcesPredicate()
-            onlyIf { hasSourcesPredicate.get() }
             // 'group' is not specified deliberately, so it will be hidden from ./gradlew tasks
             description = "Builds Kotlin KLib ABI dump for 'main' compilations of $projectName. " +
                     "Complementary task and shouldn't be called manually"

src/main/kotlin/KotlinApiCompareTask.kt

@@ -104,3 +104,96 @@ public open class KotlinApiCompareTask @Inject constructor(private val objects:
         return diff.joinToString("\n")
     }
 }
+
+// TODO: decide what to do with to old compare task
+internal abstract class KotlinApiCompareLazyTask @Inject constructor(private val objects: ObjectFactory): DefaultTask() {
+
+    @get:SkipWhenEmpty
+    @get:InputFiles
+    @get:PathSensitive(PathSensitivity.RELATIVE)
+    abstract val projectApiFile: RegularFileProperty
+
+    @get:SkipWhenEmpty
+    @get:InputFiles
+    abstract val generatedApiFile: RegularFileProperty
+
+    private val projectName = project.name
+
+    private val rootDir = project.rootProject.rootDir
+
+    @TaskAction
+    internal fun verify() {
+        val projectApiDir = projectApiFile.get().asFile.parentFile
+        if (!projectApiDir.exists()) {
+            error("Expected folder with API declarations '$projectApiDir' does not exist.\n" +
+                    "Please ensure that ':apiDump' was executed in order to get API dump to compare the build against")
+        }
+        val buildApiDir = generatedApiFile.get().asFile.parentFile
+        if (!buildApiDir.exists()) {
+            error("Expected folder with generate API declarations '$buildApiDir' does not exist.")
+        }
+        val subject = projectName
+
+        /*
+         * We use case-insensitive comparison to workaround issues with case-insensitive OSes
+         * and Gradle behaving slightly different on different platforms.
+         * We neither know original sensitivity of existing .api files, not
+         * build ones, because projectName that is part of the path can have any sensitvity.
+         * To workaround that, we replace paths we are looking for the same paths that
+         * actually exist on FS.
+         */
+        fun caseInsensitiveMap() = TreeMap<String, RelativePath> { rp, rp2 ->
+            rp.compareTo(rp2, true)
+        }
+
+        val apiBuildDirFiles = caseInsensitiveMap()
+        val expectedApiFiles = caseInsensitiveMap()
+
+        objects.fileTree().from(buildApiDir).visit { file ->
+            apiBuildDirFiles[file.name] = file.relativePath
+        }
+        objects.fileTree().from(projectApiDir).visit { file ->
+            expectedApiFiles[file.name] = file.relativePath
+        }
+
+        if (!expectedApiFiles.containsKey(projectApiFile.get().asFile.name)) {
+            error("File ${projectApiFile.get().asFile.name} is missing from ${projectApiDir.relativeDirPath()}, please run " +
+                    ":$subject:apiDump task to generate one")
+        }
+        if (!apiBuildDirFiles.containsKey(generatedApiFile.get().asFile.name)) {
+            error("File ${generatedApiFile.get().asFile.name} is missing from dump results.")
+        }
+
+        // Normalize case-sensitivity
+        val expectedApiDeclaration = expectedApiFiles.getValue(projectApiFile.get().asFile.name)
+        val actualApiDeclaration = apiBuildDirFiles.getValue(generatedApiFile.get().asFile.name)
+        val diffSet = mutableSetOf<String>()
+        val expectedFile = expectedApiDeclaration.getFile(projectApiDir)
+        val actualFile = actualApiDeclaration.getFile(buildApiDir)
+        val diff = compareFiles(expectedFile, actualFile)
+        if (diff != null) diffSet.add(diff)
+        if (diffSet.isNotEmpty()) {
+            val diffText = diffSet.joinToString("\n\n")
+            error("API check failed for project $subject.\n$diffText\n\n You can run :$subject:apiDump task to overwrite API declarations")
+        }
+    }
+
+    private fun File.relativeDirPath(): String {
+        return toRelativeString(rootDir) + File.separator
+    }
+
+    private fun compareFiles(checkFile: File, builtFile: File): String? {
+        val checkText = checkFile.readText()
+        val builtText = builtFile.readText()
+
+        // We don't compare full text because newlines on Windows & Linux/macOS are different
+        val checkLines = checkText.lines()
+        val builtLines = builtText.lines()
+        if (checkLines == builtLines)
+            return null
+
+        val patch = DiffUtils.diff(checkLines, builtLines)
+        val diff = UnifiedDiffUtils.generateUnifiedDiff(checkFile.toString(), builtFile.toString(), checkLines, patch, 3)
+        return diff.joinToString("\n")
+    }
+}

src/main/kotlin/KotlinKlibAbiBuildTask.kt

@@ -12,6 +12,7 @@ import org.gradle.api.provider.Property
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.InputFiles
 import org.gradle.api.tasks.OutputFile
+import org.gradle.api.tasks.SkipWhenEmpty
 import org.gradle.api.tasks.TaskAction
 
 /**
@@ -23,6 +24,7 @@ public abstract class KotlinKlibAbiBuildTask : BuildTaskBase() {
      * Collection consisting of a single path to a compiled klib (either file, or directory).
      */
     @get:InputFiles
+    @get:SkipWhenEmpty
     public abstract val klibFile: ConfigurableFileCollection
 
     /**

src/main/kotlin/KotlinKlibExtractAbiTask.kt

@@ -24,7 +24,7 @@ public abstract class KotlinKlibExtractAbiTask : DefaultTask() {
     /**
      * Merged KLib dump that should be filtered by this task.
      */
-    @get:InputFile
+    @get:InputFiles
     public abstract val inputAbiFile: RegularFileProperty
 
     /**
@@ -49,6 +49,7 @@ public abstract class KotlinKlibExtractAbiTask : DefaultTask() {
     @TaskAction
     internal fun generate() {
         val inputFile = inputAbiFile.asFile.get()
+        if (!inputFile.exists()) return
         if (inputFile.length() == 0L) {
             error("Project ABI file $inputAbiFile is empty.")
         }

src/main/kotlin/CopyFile.kt renamed to src/main/kotlin/SyncFile.kt

@@ -7,27 +7,33 @@ package kotlinx.validation
 
 import org.gradle.api.DefaultTask
 import org.gradle.api.file.RegularFileProperty
-import org.gradle.api.tasks.InputFile
 import org.gradle.api.tasks.InputFiles
 import org.gradle.api.tasks.OutputFile
+import org.gradle.api.tasks.OutputFiles
 import org.gradle.api.tasks.TaskAction
-import java.io.File
 import java.nio.file.Files
 import java.nio.file.StandardCopyOption
 
 // Built-in Gradle's Copy/Sync tasks accepts only a destination directory (not a single file)
 // and registers it as an output dependency. If there's another task reading from that particular
 // directory or writing into it, their input/output dependencies would clash and as long as
 // there will be no explicit ordering or dependencies between these tasks, Gradle would be unhappy.
-internal abstract class CopyFile : DefaultTask() {
-    @get:InputFile
+internal abstract class SyncFile : DefaultTask() {
+    @get:InputFiles
     abstract val from: RegularFileProperty
 
     @get:OutputFile
     abstract val to: RegularFileProperty
 
     @TaskAction
     fun copy() {
-        Files.copy(from.asFile.get().toPath(), to.asFile.get().toPath(), StandardCopyOption.REPLACE_EXISTING)
+        val fromFile = from.asFile.get()
+        val toFile = to.asFile.get()
+        if (fromFile.exists()) {
+            toFile.parentFile.mkdirs()
+            Files.copy(fromFile.toPath(), toFile.toPath(), StandardCopyOption.REPLACE_EXISTING)
+        } else {
+            Files.deleteIfExists(toFile.toPath())
+        }
     }
 }