Skip to content

Stop supporting case insensitive file names #237

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

Merged
merged 3 commits into from
Jun 6, 2024
Merged
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
2 changes: 1 addition & 1 deletion api/binary-compatibility-validator.api
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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the tests run on different systems at once?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are running on Windows/Linux/MacOS from time to time, yes.


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
37 changes: 4 additions & 33 deletions src/main/kotlin/KotlinApiCompareTask.kt
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

/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hurray! )

* 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