From b42f190a6d7b01900c415c4526933e9a4e91786a Mon Sep 17 00:00:00 2001 From: Goooler Date: Wed, 24 Jul 2024 18:03:20 +0800 Subject: [PATCH] Avoid using project.name for ignored projects check > The project's name is not necessarily unique within a project hierarchy. You should use the getPath() method for a unique identifier for the project. See https://docs.gradle.org/current/javadoc/org/gradle/api/Project.html#getName(). --- .../test/SubprojectsWithPluginOnRootTests.kt | 36 +++++++++++ .../BinaryCompatibilityValidatorPlugin.kt | 61 +++++++++---------- src/main/kotlin/BuildTaskBase.kt | 3 - src/main/kotlin/KotlinApiCompareTask.kt | 8 +-- 4 files changed, 69 insertions(+), 39 deletions(-) diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt index 096d6b14..1617d20e 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt @@ -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() { @@ -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() + ) + } + } } diff --git a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt index 7cd2af31..df2c142f 100644 --- a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt +++ b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt @@ -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.* @@ -35,7 +34,7 @@ public class BinaryCompatibilityValidatorPlugin : Plugin { 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" } } @@ -55,7 +54,7 @@ public class BinaryCompatibilityValidatorPlugin : Plugin { extension: ApiValidationExtension, action: Action ) = project.pluginManager.withPlugin(name) { - if (project.name in extension.ignoredProjects) return@withPlugin + if (project.path in extension.ignoredProjects) return@withPlugin action.execute(it) } @@ -64,7 +63,7 @@ public class BinaryCompatibilityValidatorPlugin : Plugin { extension: ApiValidationExtension, jvmRuntimeClasspath: NamedDomainObjectProvider ) = 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 @@ -208,16 +207,16 @@ private fun Project.configureKotlinCompilation( commonApiCheck: TaskProvider? = 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(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) @@ -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, ) { - 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 { @@ -262,10 +261,10 @@ private fun Project.configureApiTasks( } val apiBuild = task(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) @@ -281,25 +280,25 @@ private fun Project.configureCheckTasks( commonApiDump: TaskProvider? = null, commonApiCheck: TaskProvider? = 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(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(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) }) } @@ -398,16 +397,16 @@ private class KlibValidationPipelineBuilder( private fun Project.checkKlibsTask(klibDumpConfig: TargetConfig) = project.task(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(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 @@ -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" @@ -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" @@ -456,7 +455,7 @@ private class KlibValidationPipelineBuilder( klibMergeDir: Provider, runtimeClasspath: NamedDomainObjectProvider ) = project.task(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) }) @@ -577,11 +576,11 @@ private class KlibValidationPipelineBuilder( apiBuildDir: Provider, runtimeClasspath: NamedDomainObjectProvider ): TaskProvider { - val projectName = project.name + val projectPath = project.path val buildTask = project.task(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) @@ -594,7 +593,7 @@ private class KlibValidationPipelineBuilder( private fun Project.mergeDependencyForUnsupportedTarget(targetConfig: TargetConfig): TaskProvider { return project.task(targetConfig.apiTaskName("Build")) { - isEnabled = apiCheckEnabled(project.name, extension) + isEnabled = apiCheckEnabled(project.path, extension) doLast { logger.warn( @@ -613,7 +612,7 @@ private class KlibValidationPipelineBuilder( ): TaskProvider { val targetName = targetConfig.targetName!! return project.task(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" diff --git a/src/main/kotlin/BuildTaskBase.kt b/src/main/kotlin/BuildTaskBase.kt index 8e93bd61..5145ade4 100644 --- a/src/main/kotlin/BuildTaskBase.kt +++ b/src/main/kotlin/BuildTaskBase.kt @@ -48,9 +48,6 @@ public abstract class BuildTaskBase : WorkerAwareTaskBase() { @get:Input public val publicClasses: SetProperty = stringSetProperty { publicClasses } - @get:Internal - internal val projectName = project.name - internal fun fillCommonParams(params: BuildParametersBase) { params.ignoredPackages.set(ignoredPackages) params.nonPublicMarkers.set(nonPublicMarkers) diff --git a/src/main/kotlin/KotlinApiCompareTask.kt b/src/main/kotlin/KotlinApiCompareTask.kt index 37845f4b..75b074fe 100644 --- a/src/main/kotlin/KotlinApiCompareTask.kt +++ b/src/main/kotlin/KotlinApiCompareTask.kt @@ -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.* @@ -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 @@ -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" ) } }