[IC] Handle custom source set located outside project directory
To support build cache relocatability, we need to convert absolute paths into relative paths before storing them in IC caches. Before this commit, we computed relative paths based on the (root) project directory and FAIL if some source files are located outside the project directory. With this commit, we will NOT FAIL in that case. This means relative paths may start with "../". It's not "clean", but it can work. Test: New test in BuildCacheRelocationIT ^KT-61852 Fixed
This commit is contained in:
+2
-5
@@ -16,12 +16,9 @@ import java.io.File
|
||||
*/
|
||||
class RelocatableFileToPathConverter(private val baseDir: File) : FileToPathConverter {
|
||||
|
||||
private val unixStyleBaseDirPathPrefix = "${baseDir.invariantSeparatorsPath}/"
|
||||
|
||||
override fun toPath(file: File): String {
|
||||
check(file.invariantSeparatorsPath.startsWith(unixStyleBaseDirPathPrefix)) {
|
||||
"The given file '${file.path}' is located outside the base directory '${baseDir.path}'"
|
||||
}
|
||||
// Note: If the given file is located outside `baseDir`, the relative path will start with "../". It's not "clean", but it can work.
|
||||
// TODO: Re-design the code such that `baseDir` always contains the given file (also add a precondition check here).
|
||||
return file.relativeTo(baseDir).invariantSeparatorsPath
|
||||
}
|
||||
|
||||
|
||||
+24
-15
@@ -5,34 +5,43 @@
|
||||
|
||||
package org.jetbrains.kotlin.incremental.storage
|
||||
|
||||
import org.jetbrains.kotlin.TestWithWorkingDir
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Before
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import kotlin.test.assertFailsWith
|
||||
import org.junit.rules.TemporaryFolder
|
||||
import java.io.File
|
||||
|
||||
class RelocatableFileToPathConverterTest : TestWithWorkingDir() {
|
||||
class RelocatableFileToPathConverterTest {
|
||||
|
||||
@get:Rule
|
||||
val tmpDir = TemporaryFolder()
|
||||
|
||||
private lateinit var baseDir: File
|
||||
private lateinit var pathConverter: RelocatableFileToPathConverter
|
||||
|
||||
override fun setUp() {
|
||||
super.setUp()
|
||||
pathConverter = RelocatableFileToPathConverter(workingDir)
|
||||
@Before
|
||||
fun setUp() {
|
||||
baseDir = tmpDir.newFolder("baseDir")
|
||||
pathConverter = RelocatableFileToPathConverter(baseDir)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testToPath() {
|
||||
assertEquals("com/example/Foo.kt", pathConverter.toPath(workingDir.resolve("com/example/Foo.kt")))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testToPathFails() {
|
||||
assertFailsWith(IllegalStateException::class) {
|
||||
pathConverter.toPath(workingDir.resolve("../outsideWorkingDir/com/example/Foo.kt").normalize())
|
||||
}
|
||||
assertEquals("com/example/Foo.kt", pathConverter.toPath(baseDir.resolve("com/example/Foo.kt")))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testToFile() {
|
||||
assertEquals(workingDir.resolve("com/example/Foo.kt"), pathConverter.toFile("com/example/Foo.kt"))
|
||||
assertEquals(baseDir.resolve("com/example/Foo.kt"), pathConverter.toFile("com/example/Foo.kt"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testFileOutsideBaseDirectory() {
|
||||
val fileOutsideBaseDir = baseDir.resolve("../outsideBaseDir/com/example/Foo.kt").normalize()
|
||||
|
||||
assertEquals("../outsideBaseDir/com/example/Foo.kt", pathConverter.toPath(fileOutsideBaseDir))
|
||||
assertEquals(fileOutsideBaseDir, pathConverter.toFile("../outsideBaseDir/com/example/Foo.kt"))
|
||||
}
|
||||
|
||||
}
|
||||
+2
-2
@@ -84,7 +84,7 @@ class SourceToOutputFilesMapTest {
|
||||
|
||||
@Test
|
||||
fun testSetRelativeFails() {
|
||||
assertFailsWith<IllegalStateException> {
|
||||
assertFailsWith<IllegalArgumentException> {
|
||||
stofMap[fooDotKt] = listOf(File("relativePath"))
|
||||
}
|
||||
}
|
||||
@@ -93,7 +93,7 @@ class SourceToOutputFilesMapTest {
|
||||
fun testGetRelativeFails() {
|
||||
stofMap[fooDotKt] = listOf(fooDotClass)
|
||||
|
||||
assertFailsWith<IllegalStateException> {
|
||||
assertFailsWith<IllegalArgumentException> {
|
||||
stofMap[fooDotKt.relativeTo(srcDir)]
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -269,7 +269,7 @@ abstract class BaseGradleIT {
|
||||
val hierarchicalMPPStructureSupport: Boolean? = null,
|
||||
val withReports: List<BuildReportType> = emptyList(),
|
||||
val enableKpmModelMapping: Boolean? = null,
|
||||
val useDaemonFallbackStrategy: Boolean = false,
|
||||
val useDaemonFallbackStrategy: Boolean? = null,
|
||||
val useParsableDiagnosticsFormatting: Boolean = true,
|
||||
val showDiagnosticsStacktrace: Boolean? = false, // false by default to not clutter the testdata + stacktraces change often
|
||||
val stacktraceMode: String? = StacktraceOption.FULL_STACKTRACE_LONG_OPTION,
|
||||
@@ -916,7 +916,7 @@ abstract class BaseGradleIT {
|
||||
add("-Pkotlin.build.report.output=${options.withReports.joinToString { it.name }}")
|
||||
}
|
||||
|
||||
add("-Pkotlin.daemon.useFallbackStrategy=${options.useDaemonFallbackStrategy}")
|
||||
options.useDaemonFallbackStrategy?.let { add("-Pkotlin.daemon.useFallbackStrategy=$it") }
|
||||
|
||||
add("-Dorg.gradle.unsafe.configuration-cache=${options.configurationCache}")
|
||||
add("-Dorg.gradle.unsafe.configuration-cache-problems=${options.configurationCacheProblems.name.lowercase(Locale.getDefault())}")
|
||||
|
||||
+53
-70
@@ -22,10 +22,7 @@ import org.gradle.util.GradleVersion
|
||||
import org.jetbrains.kotlin.gradle.testbase.*
|
||||
import org.junit.jupiter.api.DisplayName
|
||||
import java.io.File
|
||||
import kotlin.io.path.createDirectory
|
||||
import kotlin.io.path.isRegularFile
|
||||
import kotlin.io.path.readText
|
||||
import kotlin.io.path.walk
|
||||
import kotlin.io.path.*
|
||||
|
||||
@DisplayName("Build cache relocation")
|
||||
class BuildCacheRelocationIT : KGPBaseTest() {
|
||||
@@ -312,63 +309,6 @@ class BuildCacheRelocationIT : KGPBaseTest() {
|
||||
}
|
||||
}
|
||||
|
||||
@JvmGradlePluginTests
|
||||
@DisplayName("Kotlin incremental compilation should work correctly after cache hint")
|
||||
@GradleTest
|
||||
fun testKotlinIncrementalCompilation(gradleVersion: GradleVersion) {
|
||||
val options = defaultBuildOptions.copy(useGradleClasspathSnapshot = false)
|
||||
checkKotlinIncrementalCompilationAfterCacheHit(gradleVersion, options) {
|
||||
assertNonIncrementalCompilation()
|
||||
}
|
||||
}
|
||||
|
||||
@JvmGradlePluginTests
|
||||
@DisplayName("Kotlin incremental compilation with `kotlin.incremental.useClasspathSnapshot` feature should work correctly")
|
||||
@GradleTest
|
||||
fun testKotlinIncrementalCompilation_withGradleClasspathSnapshot(gradleVersion: GradleVersion) {
|
||||
checkKotlinIncrementalCompilationAfterCacheHit(gradleVersion, defaultBuildOptions.copy(useGradleClasspathSnapshot = true)) {
|
||||
assertIncrementalCompilation(listOf("src/main/kotlin/foo.kt", "src/main/kotlin/fooUsage.kt").toPaths())
|
||||
}
|
||||
}
|
||||
|
||||
@JvmGradlePluginTests
|
||||
@DisplayName("Kotlin incremental compilation with `kotlin.incremental.classpath.snapshot.enabled` feature should work correctly")
|
||||
@GradleTest
|
||||
fun testKotlinIncrementalCompilation_withICClasspathSnapshot(gradleVersion: GradleVersion) {
|
||||
checkKotlinIncrementalCompilationAfterCacheHit(gradleVersion, defaultBuildOptions.copy(useICClasspathSnapshot = true)) {
|
||||
assertIncrementalCompilation(listOf("src/main/kotlin/foo.kt", "src/main/kotlin/fooUsage.kt").toPaths())
|
||||
assertOutputContains("Incremental compilation with ABI snapshot enabled")
|
||||
}
|
||||
}
|
||||
|
||||
private fun checkKotlinIncrementalCompilationAfterCacheHit(
|
||||
gradleVersion: GradleVersion,
|
||||
buildOptions: BuildOptions = defaultBuildOptions,
|
||||
assertions: BuildResult.() -> Unit
|
||||
) {
|
||||
val (firstProject, secondProject) = prepareTestProjects("buildCacheSimple", gradleVersion, buildOptions)
|
||||
|
||||
// First build, should be stored into the build cache:
|
||||
firstProject.build("assemble") {
|
||||
assertTasksPackedToCache(":compileKotlin")
|
||||
}
|
||||
|
||||
// A cache hit: a clean build without any changes to the project
|
||||
secondProject.build("clean", "assemble") {
|
||||
assertTasksFromCache(":compileKotlin")
|
||||
}
|
||||
|
||||
// Check whether compilation after a cache hit is incremental (KT-34862)
|
||||
val fooKtSourceFile = secondProject.kotlinSourcesDir().resolve("foo.kt")
|
||||
fooKtSourceFile.modify { it.replace("Int = 1", "String = \"abc\"") }
|
||||
secondProject.build("assemble", buildOptions = buildOptions.copy(logLevel = LogLevel.DEBUG), assertions = assertions)
|
||||
// Revert the change to the return type of foo(), and check if we get a cache hit
|
||||
fooKtSourceFile.modify { it.replace("String = \"abc\"", "Int = 1") }
|
||||
secondProject.build("clean", "assemble") {
|
||||
assertTasksFromCache(":compileKotlin")
|
||||
}
|
||||
}
|
||||
|
||||
private fun checkKaptCachingIncrementalBuild(
|
||||
firstProject: TestProject,
|
||||
secondProject: TestProject
|
||||
@@ -410,27 +350,70 @@ class BuildCacheRelocationIT : KGPBaseTest() {
|
||||
}
|
||||
|
||||
@JvmGradlePluginTests
|
||||
@DisplayName("test relocatability for projects using custom build directory") // Regression test for KT-58547
|
||||
@DisplayName("Kotlin incremental compilation should work correctly after cache hint")
|
||||
@GradleTest
|
||||
fun testCustomBuildDirectory(gradleVersion: GradleVersion) {
|
||||
val (firstProject, secondProject) = prepareTestProjects("buildCacheSimple", gradleVersion)
|
||||
firstProject.buildGradle.append("buildDir = \"../BUILD_DIR_1\"")
|
||||
secondProject.buildGradle.append("buildDir = \"../BUILD_DIR_2\"")
|
||||
fun testKotlinIncrementalCompilationAfterCacheHit(gradleVersion: GradleVersion) {
|
||||
checkKotlinIncrementalCompilationAfterCacheHit(gradleVersion)
|
||||
}
|
||||
|
||||
firstProject.build(":compileKotlin")
|
||||
@JvmGradlePluginTests
|
||||
@DisplayName("test custom source set and build directory located outside project directory") // Regression test for KT-61852 and KT-58547
|
||||
@GradleTest
|
||||
fun testCustomSourceSetAndBuildDirectory(gradleVersion: GradleVersion) {
|
||||
val projects = mutableListOf<TestProject>()
|
||||
|
||||
checkKotlinIncrementalCompilationAfterCacheHit(
|
||||
gradleVersion,
|
||||
buildOptions = defaultBuildOptions.copy(useDaemonFallbackStrategy = false)
|
||||
) { project ->
|
||||
project.projectPath.toFile().resolve("../SOURCE_SET_OUTSIDE_PROJECT/src/main/kotlin").let {
|
||||
it.mkdirs()
|
||||
it.resolve("SomeClass.kt").createNewFile()
|
||||
}
|
||||
project.buildGradle.append("sourceSets { main.java.srcDirs += \"../SOURCE_SET_OUTSIDE_PROJECT/src/main/kotlin\" }")
|
||||
project.buildGradle.append("buildDir = \"../BUILD_DIR_OUTSIDE_PROJECT\"")
|
||||
|
||||
projects.add(project)
|
||||
}
|
||||
|
||||
// Also check that output files do not contain non-relocatable paths
|
||||
val projectPath = projects.first().projectPath
|
||||
val outputFilesContainingNonRelocatablePaths =
|
||||
firstProject.projectPath.resolve("../BUILD_DIR_1/kotlin/compileKotlin").walk()
|
||||
.filter {
|
||||
projectPath.resolve("../BUILD_DIR_OUTSIDE_PROJECT/kotlin/compileKotlin").walk().filter {
|
||||
// Use readText() even for binary files as we don't have a better way for now
|
||||
it.isRegularFile() && it.readText().contains("BUILD_DIR_1")
|
||||
it.isRegularFile() && it.readText().let { text ->
|
||||
text.contains(projectPath.parent.name) || text.contains("BUILD_DIR_OUTSIDE_PROJECT")
|
||||
}
|
||||
}.toList()
|
||||
assert(outputFilesContainingNonRelocatablePaths.isEmpty()) {
|
||||
"The following output files contain non-relocatable paths:\n" + outputFilesContainingNonRelocatablePaths.joinToString("\n")
|
||||
}
|
||||
}
|
||||
|
||||
private fun checkKotlinIncrementalCompilationAfterCacheHit(
|
||||
gradleVersion: GradleVersion,
|
||||
buildOptions: BuildOptions = defaultBuildOptions,
|
||||
configureProject: (TestProject) -> Unit = {},
|
||||
) {
|
||||
val (firstProject, secondProject) =
|
||||
prepareTestProjects("buildCacheSimple", gradleVersion, buildOptions, buildJdk = null, configureProject)
|
||||
|
||||
// Build the first project -- It should be a cache miss
|
||||
firstProject.build(":compileKotlin") {
|
||||
assertTasksPackedToCache(":compileKotlin")
|
||||
}
|
||||
|
||||
// Build the second project -- It should be a cache hit
|
||||
secondProject.build(":compileKotlin") {
|
||||
assertTasksFromCache(":compileKotlin")
|
||||
}
|
||||
|
||||
// Make a change to the second project and build again -- Compilation should be incremental
|
||||
secondProject.kotlinSourcesDir().resolve("foo.kt").modify {
|
||||
it.replace("Int = 1", "String = \"abc\"")
|
||||
}
|
||||
secondProject.build("assemble", buildOptions = buildOptions.copy(logLevel = LogLevel.DEBUG)) {
|
||||
assertIncrementalCompilation(listOf("src/main/kotlin/foo.kt", "src/main/kotlin/fooUsage.kt").toPaths())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user