Skip to content

Commit

Permalink
[ABI Validation] Correctly handle compilations without sources
Browse files Browse the repository at this point in the history
* Correctly handle targets with a main compilation resulting in no compiled artifacts
* Postpone source directories check by wrapping it into a provider

Fixes Kotlin/binary-compatibility-validator#199 
Pull request Kotlin/binary-compatibility-validator#200
  • Loading branch information
fzhinkin authored and shanshin committed Dec 3, 2024
1 parent 8a3afbd commit 7d12c9f
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ internal fun BuildResult.assertTaskFailure(task: String) {
assertTaskOutcome(TaskOutcome.FAILED, task)
}

/**
* Helper `fun` for asserting a [TaskOutcome] to be equal to [TaskOutcome.SKIPPED]
*/
internal fun BuildResult.assertTaskSkipped(task: String) {
assertTaskOutcome(TaskOutcome.SKIPPED, task)
}

private fun BuildResult.assertTaskOutcome(taskOutcome: TaskOutcome, taskName: String) {
assertEquals(taskOutcome, task(taskName)?.outcome)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import org.junit.Test
import java.io.File
import java.nio.file.Files
import java.nio.file.Paths
import kotlin.test.assertFalse
import kotlin.test.assertTrue

internal const val BANNED_TARGETS_PROPERTY_NAME = "binary.compatibility.validator.klib.targets.disabled.for.testing"
Expand Down Expand Up @@ -633,4 +634,39 @@ internal class KlibVerificationTests : BaseKotlinGradleTest() {
)
}
}

@Test
fun `apiDump should not fail for empty project`() {
val runner = test {
baseProjectSetting()
addToSrcSet("/examples/classes/AnotherBuildConfig.kt", sourceSet = "commonTest")
runApiDump()
}

runner.build().apply {
assertTaskSkipped(":klibApiDump")
}
assertFalse(runner.projectDir.resolve("api").exists())
}

@Test
fun `apiDump should not fail if there is only one target`() {
val runner = test {
baseProjectSetting()
addToSrcSet("/examples/classes/AnotherBuildConfig.kt", sourceSet = "commonTest")
addToSrcSet("/examples/classes/AnotherBuildConfig.kt", sourceSet = "linuxX64Main")
runApiDump()
}
checkKlibDump(runner.build(), "/examples/classes/AnotherBuildConfig.klib.linuxX64Only.dump")
}

@Test
fun `apiCheck should not fail for empty project`() {
val runner = test {
baseProjectSetting()
addToSrcSet("/examples/classes/AnotherBuildConfig.kt", sourceSet = "commonTest")
runApiCheck()
}
runner.build()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Klib ABI Dump
// Targets: [linuxX64]
// Rendering settings:
// - Signature version: 2
// - Show manifest properties: true
// - Show declarations: true

// Library unique name: <testproject>
final class org.different.pack/BuildConfig { // org.different.pack/BuildConfig|null[0]
constructor <init>() // org.different.pack/BuildConfig.<init>|<init>(){}[0]
final fun f1(): kotlin/Int // org.different.pack/BuildConfig.f1|f1(){}[0]
final val p1 // org.different.pack/BuildConfig.p1|{}p1[0]
final fun <get-p1>(): kotlin/Int // org.different.pack/BuildConfig.p1.<get-p1>|<get-p1>(){}[0]
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public class BinaryCompatibilityValidatorPlugin : Plugin<Project> {
kotlin.targets.matching { it.jvmBased }.all { target ->
val targetConfig = TargetConfig(project, extension, target.name, jvmDirConfig)
if (target.platformType == KotlinPlatformType.jvm) {
target.mainCompilations.all {
target.mainCompilationOrNull?.also {
project.configureKotlinCompilation(it, extension, targetConfig, commonApiDump, commonApiCheck)
}
} else if (target.platformType == KotlinPlatformType.androidJvm) {
Expand Down Expand Up @@ -219,11 +219,9 @@ private fun Project.configureKotlinCompilation(

val apiBuild = task<KotlinApiBuildTask>(targetConfig.apiTaskName("Build")) {
// Do not enable task for empty umbrella modules
isEnabled =
apiCheckEnabled(
projectName,
extension
) && compilation.allKotlinSourceSets.any { it.kotlin.srcDirs.any { it.exists() } }
isEnabled = apiCheckEnabled(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 API for 'main' compilations of $projectName. Complementary task and shouldn't be called manually"
Expand Down Expand Up @@ -419,6 +417,8 @@ private class KlibValidationPipelineBuilder(
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() }
}

private fun Project.dumpKlibsTask(
Expand All @@ -431,6 +431,8 @@ private class KlibValidationPipelineBuilder(
group = "other"
from = klibMergeDir.resolve(klibDumpFileName)
to = klibApiDir.get().resolve(klibDumpFileName)
val hasCompilableTargets = project.hasCompilableTargetsPredicate()
onlyIf("There are no klibs compiled for the project") { hasCompilableTargets.get() }
}

private fun Project.extractAbi(
Expand All @@ -449,6 +451,8 @@ private class KlibValidationPipelineBuilder(
supportedTargets = supportedTargets()
inputAbiFile = klibApiDir.get().resolve(klibDumpFileName)
outputAbiFile = klibOutputDir.resolve(klibDumpFileName)
val hasCompilableTargets = project.hasCompilableTargetsPredicate()
onlyIf("There are no klibs compiled for the project") { hasCompilableTargets.get() }
}

private fun Project.mergeInferredKlibsUmbrellaTask(
Expand All @@ -464,6 +468,8 @@ private class KlibValidationPipelineBuilder(
"into a single merged KLib ABI dump"
dumpFileName = klibDumpFileName
mergedFile = klibMergeDir.resolve(klibDumpFileName)
val hasCompilableTargets = project.hasCompilableTargetsPredicate()
onlyIf("There are no dumps to merge") { hasCompilableTargets.get() }
}

private fun Project.mergeKlibsUmbrellaTask(
Expand All @@ -475,6 +481,8 @@ private class KlibValidationPipelineBuilder(
"different targets into a single merged KLib ABI dump"
dumpFileName = klibDumpFileName
mergedFile = klibMergeDir.resolve(klibDumpFileName)
val hasCompilableTargets = project.hasCompilableTargetsPredicate()
onlyIf("There are no dumps to merge") { hasCompilableTargets.get() }
}

fun Project.bannedTargets(): Set<String> {
Expand All @@ -499,30 +507,22 @@ private class KlibValidationPipelineBuilder(

val supportedTargetsProvider = supportedTargets()
kotlin.targets.matching { it.emitsKlib }.configureEach { currentTarget ->
val mainCompilations = currentTarget.mainCompilations
if (mainCompilations.none()) {
return@configureEach
}
val mainCompilation = currentTarget.mainCompilationOrNull ?: return@configureEach

val targetName = currentTarget.targetName
val targetConfig = TargetConfig(project, extension, targetName, intermediateFilesConfig)
val apiBuildDir = targetConfig.apiDir.map { project.layout.buildDirectory.asFile.get().resolve(it) }.get()
val targetSupported = targetIsSupported(currentTarget)
// If a target is supported, the workflow is simple: create a dump, then merge it along with other dumps.
if (targetSupported) {
mainCompilations.all {
val buildTargetAbi = configureKlibCompilation(
it, extension, targetConfig,
apiBuildDir
)
mergeTask.configure {
it.addInput(targetName, apiBuildDir)
it.dependsOn(buildTargetAbi)
}
mergeInferredTask.configure {
it.addInput(targetName, apiBuildDir)
it.dependsOn(buildTargetAbi)
}
val buildTargetAbi = configureKlibCompilation(mainCompilation, extension, targetConfig, apiBuildDir)
mergeTask.configure {
it.addInput(targetName, apiBuildDir)
it.dependsOn(buildTargetAbi)
}
mergeInferredTask.configure {
it.addInput(targetName, apiBuildDir)
it.dependsOn(buildTargetAbi)
}
return@configureEach
}
Expand All @@ -534,9 +534,12 @@ private class KlibValidationPipelineBuilder(
}
// The actual merge will happen here, where we'll try to infer a dump for the unsupported target and merge
// it with other supported target dumps.
val proxy = unsupportedTargetDumpProxy(klibApiDir, targetConfig,
val proxy = unsupportedTargetDumpProxy(
mainCompilation,
klibApiDir, targetConfig,
extractUnderlyingTarget(currentTarget),
apiBuildDir, supportedTargetsProvider)
apiBuildDir, supportedTargetsProvider
)
mergeInferredTask.configure {
it.addInput(targetName, apiBuildDir)
it.dependsOn(proxy)
Expand All @@ -555,18 +558,20 @@ private class KlibValidationPipelineBuilder(

private fun Project.targetIsSupported(target: KotlinTarget): Boolean {
if (bannedTargets().contains(target.targetName)) return false
return when(target) {
return when (target) {
is KotlinNativeTarget -> HostManager().isEnabled(target.konanTarget)
else -> true
}
}

// Compilable targets supported by the host compiler
private fun Project.supportedTargets(): Provider<Set<String>> {
val banned = bannedTargets() // for testing only
return project.provider {
val hm = HostManager()
project.kotlinMultiplatform.targets.matching { it.emitsKlib }
.asSequence()
.filter { it.mainCompilationOrNull?.hasAnySources() == true }
.filter {
if (it is KotlinNativeTarget) {
hm.isEnabled(it.konanTarget) && it.targetName !in banned
Expand All @@ -579,6 +584,14 @@ private class KlibValidationPipelineBuilder(
}
}

// Returns a predicate that checks if there are any compilable targets
private fun Project.hasCompilableTargetsPredicate(): Provider<Boolean> {
return project.provider {
project.kotlinMultiplatform.targets.matching { it.emitsKlib }
.asSequence()
.any { it.mainCompilationOrNull?.hasAnySources() == true }
}
}

private fun Project.configureKlibCompilation(
compilation: KotlinCompilation<KotlinCommonOptions>,
Expand All @@ -590,11 +603,9 @@ private class KlibValidationPipelineBuilder(
val buildTask = project.task<KotlinKlibAbiBuildTask>(targetConfig.apiTaskName("Build")) {
target = targetConfig.targetName!!
// Do not enable task for empty umbrella modules
isEnabled =
klibAbiCheckEnabled(
projectName,
extension
) && compilation.allKotlinSourceSets.any { it.kotlin.srcDirs.any { it.exists() } }
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 All @@ -620,6 +631,7 @@ private class KlibValidationPipelineBuilder(
}

private fun Project.unsupportedTargetDumpProxy(
compilation: KotlinCompilation<KotlinCommonOptions>,
klibApiDir: Provider<File>,
targetConfig: TargetConfig,
underlyingTarget: String,
Expand All @@ -629,6 +641,8 @@ private class KlibValidationPipelineBuilder(
val targetName = targetConfig.targetName!!
return project.task<KotlinKlibInferAbiForUnsupportedTargetTask>(targetConfig.apiTaskName("Infer")) {
isEnabled = klibAbiCheckEnabled(project.name, extension)
val hasSourcesPredicate = compilation.hasAnySourcesPredicate()
onlyIf { hasSourcesPredicate.get() }
description = "Try to infer the dump for unsupported target $targetName using dumps " +
"generated for supported targets."
group = "other"
Expand Down Expand Up @@ -676,10 +690,18 @@ private fun extractUnderlyingTarget(target: KotlinTarget): String {
private val Project.kotlinMultiplatform
get() = extensions.getByName("kotlin") as KotlinMultiplatformExtension

private val KotlinTarget.mainCompilations
get() = compilations.matching { it.name == "main" }
private val KotlinTarget.mainCompilationOrNull: KotlinCompilation<KotlinCommonOptions>?
get() = compilations.firstOrNull { it.name == KotlinCompilation.MAIN_COMPILATION_NAME }

private val Project.jvmDumpFileName: String
get() = "$name.api"
private val Project.klibDumpFileName: String
get() = "$name.klib.api"

private fun KotlinCompilation<KotlinCommonOptions>.hasAnySources(): Boolean = allKotlinSourceSets.any {
it.kotlin.srcDirs.any(File::exists)
}

private fun KotlinCompilation<KotlinCommonOptions>.hasAnySourcesPredicate(): Provider<Boolean> = project.provider {
this.hasAnySources()
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ internal abstract class KotlinKlibMergeAbiTask : DefaultTask() {
internal fun merge() {
KlibDump().apply {
targetToFile.forEach { (targetName, dumpDir) ->
merge(dumpDir.resolve(dumpFileName), targetName)
val dumpFile = dumpDir.resolve(dumpFileName)
if (dumpFile.exists()) {
merge(dumpFile, targetName)
}
}
}.saveTo(mergedFile)
}
Expand Down

0 comments on commit 7d12c9f

Please sign in to comment.