Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid using project.name for ignored projects check #262

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import kotlinx.validation.api.runner
import kotlinx.validation.api.test
import org.assertj.core.api.Assertions
import org.junit.Test
import kotlin.test.assertContains
import kotlin.test.assertTrue

internal class SubprojectsWithPluginOnRootTests : BaseKotlinGradleTest() {
Expand Down Expand Up @@ -317,4 +318,39 @@ internal class SubprojectsWithPluginOnRootTests : BaseKotlinGradleTest() {
Assertions.assertThat(apiSub2.readText()).isEqualToIgnoringNewLines("")
}
}

/**
* https://github.com/Kotlin/binary-compatibility-validator/issues/257
*/
@Test
fun `using project name instead of path should not be ignored`() {
val runner = test {
createProjectHierarchyWithPluginOnRoot()
rootProjectDir.resolve("build.gradle.kts").writeText(
"""
apiValidation {
ignoredProjects += listOf(
"subsub1"
)
}
""".trimIndent()
)

runner {
arguments.add(":apiCheck")
}
}

try {
runner.build()
error("Should have failed.")
} catch (t: Throwable) {
assertContains(
t.stackTraceToString(),
"""
Cannot find excluded project subsub1 in all projects: [:, :sub1, :sub2, :sub1:subsub1, :sub1:subsub2]
""".trimIndent()
)
}
}
}
61 changes: 30 additions & 31 deletions src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import org.jetbrains.kotlin.gradle.plugin.*
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget
import org.jetbrains.kotlin.konan.target.HostManager
import org.jetbrains.kotlin.library.abi.ExperimentalLibraryAbiReader
import org.jetbrains.kotlin.library.abi.LibraryAbiReader
import java.io.*
import java.util.*

Expand All @@ -35,7 +34,7 @@ public class BinaryCompatibilityValidatorPlugin : Plugin<Project> {
private fun Project.validateExtension(extension: ApiValidationExtension) {
afterEvaluate {
val ignored = extension.ignoredProjects
val all = allprojects.map { it.name }
val all = allprojects.map { it.path }
for (project in ignored) {
require(project in all) { "Cannot find excluded project $project in all projects: $all" }
}
Expand All @@ -55,7 +54,7 @@ public class BinaryCompatibilityValidatorPlugin : Plugin<Project> {
extension: ApiValidationExtension,
action: Action<AppliedPlugin>
) = project.pluginManager.withPlugin(name) {
if (project.name in extension.ignoredProjects) return@withPlugin
if (project.path in extension.ignoredProjects) return@withPlugin
action.execute(it)
}

Expand All @@ -64,7 +63,7 @@ public class BinaryCompatibilityValidatorPlugin : Plugin<Project> {
extension: ApiValidationExtension,
jvmRuntimeClasspath: NamedDomainObjectProvider<Configuration>
) = configurePlugin("kotlin-multiplatform", project, extension) {
if (project.name in extension.ignoredProjects) return@configurePlugin
if (project.path in extension.ignoredProjects) return@configurePlugin
val kotlin = project.kotlinMultiplatform

// Create common tasks for multiplatform
Expand Down Expand Up @@ -208,16 +207,16 @@ private fun Project.configureKotlinCompilation(
commonApiCheck: TaskProvider<Task>? = null,
useOutput: Boolean = false,
) {
val projectName = project.name
val projectPath = project.path
val dumpFileName = project.jvmDumpFileName
val apiDirProvider = targetConfig.apiDir
val apiBuildDir = apiDirProvider.flatMap { f -> layout.buildDirectory.asFile.map { it.resolve(f) } }

val apiBuild = task<KotlinApiBuildTask>(targetConfig.apiTaskName("Build")) {
isEnabled = apiCheckEnabled(projectName, extension)
isEnabled = apiCheckEnabled(projectPath, extension)
// '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"
"Builds Kotlin API for 'main' compilations of $projectPath. Complementary task and shouldn't be called manually"
if (useOutput) {
// Workaround for #4
inputClassesDirs.from(compilation.output.classesDirs)
Expand All @@ -240,19 +239,19 @@ internal val Project.apiValidationExtensionOrNull: ApiValidationExtension?
.map { it.extensions.findByType(ApiValidationExtension::class.java) }
.firstOrNull { it != null }

private fun apiCheckEnabled(projectName: String, extension: ApiValidationExtension): Boolean =
projectName !in extension.ignoredProjects && !extension.validationDisabled
private fun apiCheckEnabled(projectPath: String, extension: ApiValidationExtension): Boolean =
projectPath !in extension.ignoredProjects && !extension.validationDisabled

@OptIn(ExperimentalBCVApi::class)
private fun klibAbiCheckEnabled(projectName: String, extension: ApiValidationExtension): Boolean =
projectName !in extension.ignoredProjects && !extension.validationDisabled && extension.klib.enabled
private fun klibAbiCheckEnabled(projectPath: String, extension: ApiValidationExtension): Boolean =
projectPath !in extension.ignoredProjects && !extension.validationDisabled && extension.klib.enabled

private fun Project.configureApiTasks(
extension: ApiValidationExtension,
targetConfig: TargetConfig = TargetConfig(this, extension),
jvmRuntimeClasspath: NamedDomainObjectProvider<Configuration>,
) {
val projectName = project.name
val projectPath = project.path
val dumpFileName = project.jvmDumpFileName
val apiBuildDir = targetConfig.apiDir.flatMap { f -> layout.buildDirectory.asFile.map { it.resolve(f) } }
val sourceSetsOutputsProvider = project.provider {
Expand All @@ -262,10 +261,10 @@ private fun Project.configureApiTasks(
}

val apiBuild = task<KotlinApiBuildTask>(targetConfig.apiTaskName("Build")) {
isEnabled = apiCheckEnabled(projectName, extension)
isEnabled = apiCheckEnabled(projectPath, extension)
// '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"
"Builds Kotlin API for 'main' compilations of $projectPath. Complementary task and shouldn't be called manually"
inputClassesDirs.from(sourceSetsOutputsProvider)
outputApiFile.fileProvider(apiBuildDir.map { it.resolve(dumpFileName) })
runtimeClasspath.from(jvmRuntimeClasspath)
Expand All @@ -281,25 +280,25 @@ private fun Project.configureCheckTasks(
commonApiDump: TaskProvider<Task>? = null,
commonApiCheck: TaskProvider<Task>? = null,
) {
val projectName = project.name
val projectPath = project.path
val apiCheckDir = targetConfig.apiDir.map {
projectDir.resolve(it).also { r ->
logger.debug("Configuring api for ${targetConfig.targetName ?: "jvm"} to $r")
}
}
val apiCheck = task<KotlinApiCompareTask>(targetConfig.apiTaskName("Check")) {
isEnabled = apiCheckEnabled(projectName, extension) && apiBuild.map { it.enabled }.getOrElse(true)
isEnabled = apiCheckEnabled(projectPath, extension) && apiBuild.map { it.enabled }.getOrElse(true)
group = "verification"
description = "Checks signatures of public API against the golden value in API folder for $projectName"
description = "Checks signatures of public API against the golden value in API folder for $projectPath"
projectApiFile.fileProvider(apiCheckDir.map { it.resolve(jvmDumpFileName) })
generatedApiFile.set(apiBuild.flatMap { it.outputApiFile })
}

val dumpFileName = project.jvmDumpFileName
val apiDump = task<SyncFile>(targetConfig.apiTaskName("Dump")) {
isEnabled = apiCheckEnabled(projectName, extension) && apiBuild.map { it.enabled }.getOrElse(true)
isEnabled = apiCheckEnabled(projectPath, extension) && apiBuild.map { it.enabled }.getOrElse(true)
group = "other"
description = "Syncs the API file for $projectName"
description = "Syncs the API file for $projectPath"
from.set(apiBuild.flatMap { it.outputApiFile })
to.fileProvider(apiCheckDir.map { it.resolve(dumpFileName) })
}
Expand Down Expand Up @@ -398,16 +397,16 @@ private class KlibValidationPipelineBuilder(

private fun Project.checkKlibsTask(klibDumpConfig: TargetConfig) =
project.task<KotlinApiCompareTask>(klibDumpConfig.apiTaskName("Check")) {
isEnabled = klibAbiCheckEnabled(project.name, extension)
isEnabled = klibAbiCheckEnabled(project.path, extension)
group = "verification"
description =
"Checks signatures of a public KLib ABI against the golden value in ABI folder for ${project.name}"
"Checks signatures of a public KLib ABI against the golden value in ABI folder for ${project.path}"
}

private fun Project.dumpKlibsTask(klibDumpConfig: TargetConfig) =
project.task<SyncFile>(klibDumpConfig.apiTaskName("Dump")) {
isEnabled = klibAbiCheckEnabled(project.name, extension)
description = "Syncs the KLib ABI file for ${project.name}"
isEnabled = klibAbiCheckEnabled(project.path, extension)
description = "Syncs the KLib ABI file for ${project.path}"
group = "other"
onlyIf {
it as SyncFile
Expand All @@ -424,7 +423,7 @@ private class KlibValidationPipelineBuilder(
klibDumpConfig.apiTaskName("ExtractForValidation")
)
{
isEnabled = klibAbiCheckEnabled(project.name, extension)
isEnabled = klibAbiCheckEnabled(project.path, extension)
description = "Prepare a reference KLib ABI file by removing all unsupported targets from " +
"the golden file stored in the project"
group = "other"
Expand All @@ -443,7 +442,7 @@ private class KlibValidationPipelineBuilder(
klibDumpConfig.apiTaskName("MergeInferred")
)
{
isEnabled = klibAbiCheckEnabled(project.name, extension)
isEnabled = klibAbiCheckEnabled(project.path, extension)
description = "Merges multiple KLib ABI dump files generated for " +
"different targets (including inferred dumps for unsupported targets) " +
"into a single merged KLib ABI dump"
Expand All @@ -456,7 +455,7 @@ private class KlibValidationPipelineBuilder(
klibMergeDir: Provider<File>,
runtimeClasspath: NamedDomainObjectProvider<Configuration>
) = project.task<KotlinKlibMergeAbiTask>(klibDumpConfig.apiTaskName("Merge")) {
isEnabled = klibAbiCheckEnabled(project.name, extension)
isEnabled = klibAbiCheckEnabled(project.path, extension)
description = "Merges multiple KLib ABI dump files generated for " +
"different targets into a single merged KLib ABI dump"
mergedApiFile.fileProvider(klibMergeDir.map { it.resolve(klibDumpFileName) })
Expand Down Expand Up @@ -577,11 +576,11 @@ private class KlibValidationPipelineBuilder(
apiBuildDir: Provider<File>,
runtimeClasspath: NamedDomainObjectProvider<Configuration>
): TaskProvider<KotlinKlibAbiBuildTask> {
val projectName = project.name
val projectPath = project.path
val buildTask = project.task<KotlinKlibAbiBuildTask>(targetConfig.apiTaskName("Build")) {
isEnabled = klibAbiCheckEnabled(projectName, extension)
isEnabled = klibAbiCheckEnabled(projectPath, extension)
// 'group' is not specified deliberately, so it will be hidden from ./gradlew tasks
description = "Builds Kotlin KLib ABI dump for 'main' compilations of $projectName. " +
description = "Builds Kotlin KLib ABI dump for 'main' compilations of $projectPath. " +
"Complementary task and shouldn't be called manually"
this.target.set(target)
klibFile.from(compilation.output.classesDirs)
Expand All @@ -594,7 +593,7 @@ private class KlibValidationPipelineBuilder(

private fun Project.mergeDependencyForUnsupportedTarget(targetConfig: TargetConfig): TaskProvider<DefaultTask> {
return project.task<DefaultTask>(targetConfig.apiTaskName("Build")) {
isEnabled = apiCheckEnabled(project.name, extension)
isEnabled = apiCheckEnabled(project.path, extension)

doLast {
logger.warn(
Expand All @@ -613,7 +612,7 @@ private class KlibValidationPipelineBuilder(
): TaskProvider<KotlinKlibInferAbiTask> {
val targetName = targetConfig.targetName!!
return project.task<KotlinKlibInferAbiTask>(targetConfig.apiTaskName("Infer")) {
isEnabled = klibAbiCheckEnabled(project.name, extension)
isEnabled = klibAbiCheckEnabled(project.path, extension)
description = "Try to infer the dump for unsupported target $targetName using dumps " +
"generated for supported targets."
group = "other"
Expand Down
3 changes: 0 additions & 3 deletions src/main/kotlin/BuildTaskBase.kt
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ public abstract class BuildTaskBase : WorkerAwareTaskBase() {
@get:Input
public val publicClasses: SetProperty<String> = stringSetProperty { publicClasses }

@get:Internal
internal val projectName = project.name

internal fun fillCommonParams(params: BuildParametersBase) {
params.ignoredPackages.set(ignoredPackages)
params.nonPublicMarkers.set(nonPublicMarkers)
Expand Down
8 changes: 3 additions & 5 deletions src/main/kotlin/KotlinApiCompareTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package kotlinx.validation
import com.github.difflib.DiffUtils
import com.github.difflib.UnifiedDiffUtils
import java.io.*
import javax.inject.Inject
import org.gradle.api.*
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.tasks.*
Expand All @@ -23,7 +22,7 @@ public open class KotlinApiCompareTask : DefaultTask() {
@get:PathSensitive(PathSensitivity.RELATIVE)
public val generatedApiFile: RegularFileProperty = project.objects.fileProperty()

private val projectName = project.name
private val projectPath = project.path

private val rootDir = project.rootDir

Expand Down Expand Up @@ -51,10 +50,9 @@ public open class KotlinApiCompareTask : DefaultTask() {
if (diff != null) diffSet.add(diff)
if (diffSet.isNotEmpty()) {
val diffText = diffSet.joinToString("\n\n")
val subject = projectName
error(
"API check failed for project $subject.\n$diffText\n\n" +
"You can run :$subject:apiDump task to overwrite API declarations"
"API check failed for project $projectPath.\n$diffText\n\n" +
"You can run $projectPath:apiDump task to overwrite API declarations"
)
}
}
Expand Down