Skip to content

Commit

Permalink
[ABI Validation] Stop supporting case insensitive file names
Browse files Browse the repository at this point in the history
Filename case mismatch is no longer tolerated
on case-sensitive filesystems.

Fixes Kotlin/binary-compatibility-validator#231

Pull request Kotlin/binary-compatibility-validator#237
  • Loading branch information
fzhinkin authored and shanshin committed Oct 22, 2024
1 parent c47053b commit c4abaa3
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public class kotlinx/validation/KotlinApiBuildTask : kotlinx/validation/BuildTas
public class kotlinx/validation/KotlinApiCompareTask : org/gradle/api/DefaultTask {
public field generatedApiFile Ljava/io/File;
public field projectApiFile Ljava/io/File;
public fun <init> (Lorg/gradle/api/model/ObjectFactory;)V
public fun <init> ()V
public final fun getGeneratedApiFile ()Ljava/io/File;
public final fun getProjectApiFile ()Ljava/io/File;
public final fun setGeneratedApiFile (Ljava/io/File;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ package kotlinx.validation.test

import kotlinx.validation.api.*
import org.assertj.core.api.*
import org.junit.Assume
import org.junit.Test
import java.io.File
import java.nio.file.Files
import kotlin.test.*

internal class DefaultConfigTests : BaseKotlinGradleTest() {
Expand Down Expand Up @@ -90,7 +93,9 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() {
}

@Test
fun `apiCheck should succeed when public classes match api file ignoring case`() {
fun `apiCheck should fail when public classes match api file ignoring case`() {
Assume.assumeTrue(underlyingFsIsCaseSensitive())

val runner = test {
buildGradleKts {
resolve("/examples/gradle/base/withPlugin.gradle.kts")
Expand All @@ -107,8 +112,8 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() {
}
}

runner.build().apply {
assertTaskSuccess(":apiCheck")
runner.buildAndFail().apply {
assertTaskFailure(":apiCheck")
}
}

Expand Down Expand Up @@ -237,4 +242,16 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() {
assertTaskSuccess(":apiCheck")
}
}


private fun underlyingFsIsCaseSensitive(): Boolean {
val f = Files.createTempFile("UPPER", "UPPER").toFile()
f.deleteOnExit()
try {
val lower = File(f.absolutePath.lowercase())
return !lower.exists()
} finally {
f.delete()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,34 @@ internal class MultipleJvmTargetsTest : BaseKotlinGradleTest() {
}
}

// Scenario from #233: if there were two targets (and two dumps, correspondingly),
// removal of one of the targets should trigger validation failure.
@Test
fun testValidationAfterTargetRemoval() {
val runner = test {
buildGradleKts {
resolve("/examples/gradle/base/withPlugin.gradle.kts")
}
kotlin("AnotherBuildConfig.kt") {
resolve("/examples/classes/AnotherBuildConfig.kt")
}
// let's pretend, there were multiple targets before
for (tgtName in listOf("jvm", "anotherJvm")) {
dir("$API_DIR/$tgtName/") {
file("${rootProjectDir.name.lowercase()}.api") {
resolve("/examples/classes/AnotherBuildConfig.dump")
}
}
}
runner {
arguments.add(":apiCheck")
}
}
runner.buildAndFail().apply {
assertTaskFailure(":apiCheck")
}
}

private val jvmApiDump: File get() = rootProjectDir.resolve("$API_DIR/jvm/testproject.api")
private val anotherApiDump: File get() = rootProjectDir.resolve("$API_DIR/anotherJvm/testproject.api")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@ package kotlinx.validation
import com.github.difflib.DiffUtils
import com.github.difflib.UnifiedDiffUtils
import java.io.*
import java.util.TreeMap
import javax.inject.Inject
import org.gradle.api.*
import org.gradle.api.file.*
import org.gradle.api.model.ObjectFactory
import org.gradle.api.tasks.*

public open class KotlinApiCompareTask @Inject constructor(private val objects: ObjectFactory): DefaultTask() {
public open class KotlinApiCompareTask @Inject constructor(): DefaultTask() {

@InputFiles
@PathSensitive(PathSensitivity.RELATIVE)
Expand All @@ -41,43 +38,17 @@ public open class KotlinApiCompareTask @Inject constructor(private val objects:
}
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.name)) {
if (!projectApiFile.exists()) {
error("File ${projectApiFile.name} is missing from ${projectApiDir.relativeDirPath()}, please run " +
":$subject:apiDump task to generate one")
}
if (!apiBuildDirFiles.containsKey(generatedApiFile.name)) {
if (!generatedApiFile.exists()) {
error("File ${generatedApiFile.name} is missing from dump results.")
}

// Normalize case-sensitivity
val expectedApiDeclaration = expectedApiFiles.getValue(projectApiFile.name)
val actualApiDeclaration = apiBuildDirFiles.getValue(generatedApiFile.name)
val diffSet = mutableSetOf<String>()
val expectedFile = expectedApiDeclaration.getFile(projectApiDir)
val actualFile = actualApiDeclaration.getFile(buildApiDir)
val diff = compareFiles(expectedFile, actualFile)
val diff = compareFiles(projectApiFile, generatedApiFile)
if (diff != null) diffSet.add(diff)
if (diffSet.isNotEmpty()) {
val diffText = diffSet.joinToString("\n\n")
Expand Down

0 comments on commit c4abaa3

Please sign in to comment.