Skip to content

Commit

Permalink
Avoid using hasAnySourcesPredicate-predicate to filter klib-related t…
Browse files Browse the repository at this point in the history
…asks
  • Loading branch information
fzhinkin committed Apr 5, 2024
1 parent 42abf7b commit 5a03695
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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")
}
}
Expand All @@ -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")
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/kotlin/-Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -51,5 +52,6 @@ public class KlibDumpMetadata(
* Path to a resulting dump file.
*/
@get:InputFiles
@get:SkipWhenEmpty
public val dumpFile: RegularFileProperty
) : Serializable
51 changes: 14 additions & 37 deletions src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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> {
Expand Down Expand Up @@ -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"
Expand Down
93 changes: 93 additions & 0 deletions src/main/kotlin/KotlinApiCompareTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
2 changes: 2 additions & 0 deletions src/main/kotlin/KotlinKlibAbiBuildTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -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

/**
Expand Down
3 changes: 2 additions & 1 deletion src/main/kotlin/KotlinKlibExtractAbiTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -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.")
}
Expand Down
16 changes: 11 additions & 5 deletions src/main/kotlin/CopyFile.kt → src/main/kotlin/SyncFile.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
}

0 comments on commit 5a03695

Please sign in to comment.