From 3789ed710d0bdce7dc3cd3bf3bca9a88df38eb51 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 We can also use `getPath()` to replace `getName()` here. > 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(). Avoid using project.name for ignored projects check We can also use `getPath()` to replace `getName()` here. > 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(). Avoid using project.name for ignored projects check We can also use `getPath()` to replace `getName()` here. > 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(). 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 | 32 ++++++++--------- src/main/kotlin/BuildTaskBase.kt | 2 +- src/main/kotlin/KotlinApiCompareTask.kt | 2 +- 4 files changed, 54 insertions(+), 18 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..a7ba0c9f 100644 --- a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt +++ b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt @@ -35,7 +35,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 +55,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 +64,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,7 +208,7 @@ private fun Project.configureKotlinCompilation( commonApiCheck: TaskProvider? = null, useOutput: Boolean = false, ) { - val projectName = project.name + val projectName = project.path val dumpFileName = project.jvmDumpFileName val apiDirProvider = targetConfig.apiDir val apiBuildDir = apiDirProvider.flatMap { f -> layout.buildDirectory.asFile.map { it.resolve(f) } } @@ -252,7 +252,7 @@ private fun Project.configureApiTasks( targetConfig: TargetConfig = TargetConfig(this, extension), jvmRuntimeClasspath: NamedDomainObjectProvider, ) { - val projectName = project.name + val projectName = project.path val dumpFileName = project.jvmDumpFileName val apiBuildDir = targetConfig.apiDir.flatMap { f -> layout.buildDirectory.asFile.map { it.resolve(f) } } val sourceSetsOutputsProvider = project.provider { @@ -281,7 +281,7 @@ private fun Project.configureCheckTasks( commonApiDump: TaskProvider? = null, commonApiCheck: TaskProvider? = null, ) { - val projectName = project.name + val projectName = project.path val apiCheckDir = targetConfig.apiDir.map { projectDir.resolve(it).also { r -> logger.debug("Configuring api for ${targetConfig.targetName ?: "jvm"} to $r") @@ -398,16 +398,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 +424,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 +443,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 +456,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,7 +577,7 @@ private class KlibValidationPipelineBuilder( apiBuildDir: Provider, runtimeClasspath: NamedDomainObjectProvider ): TaskProvider { - val projectName = project.name + val projectName = project.path val buildTask = project.task(targetConfig.apiTaskName("Build")) { isEnabled = klibAbiCheckEnabled(projectName, extension) // 'group' is not specified deliberately, so it will be hidden from ./gradlew tasks @@ -594,7 +594,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 +613,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..9bfdc4d2 100644 --- a/src/main/kotlin/BuildTaskBase.kt +++ b/src/main/kotlin/BuildTaskBase.kt @@ -49,7 +49,7 @@ public abstract class BuildTaskBase : WorkerAwareTaskBase() { public val publicClasses: SetProperty = stringSetProperty { publicClasses } @get:Internal - internal val projectName = project.name + internal val projectName = project.path internal fun fillCommonParams(params: BuildParametersBase) { params.ignoredPackages.set(ignoredPackages) diff --git a/src/main/kotlin/KotlinApiCompareTask.kt b/src/main/kotlin/KotlinApiCompareTask.kt index 37845f4b..668aceee 100644 --- a/src/main/kotlin/KotlinApiCompareTask.kt +++ b/src/main/kotlin/KotlinApiCompareTask.kt @@ -23,7 +23,7 @@ public open class KotlinApiCompareTask : DefaultTask() { @get:PathSensitive(PathSensitivity.RELATIVE) public val generatedApiFile: RegularFileProperty = project.objects.fileProperty() - private val projectName = project.name + private val projectName = project.path private val rootDir = project.rootDir