From 0a2674250146113a034415b45481ac8ccfb49998 Mon Sep 17 00:00:00 2001 From: Ivan Gavrilovic Date: Wed, 7 Aug 2019 20:12:01 +0900 Subject: [PATCH] Run KAPT non-incrementally if annotation processor classpath changes Annotation processor classpath for a KAPT run is recorded, and the current run is compared against the previous one. If those differ, KAPT should run non-incrementally. Test: KaptIncrementalWithIsolatingApt.testChangingAnnotationProcessorClasspath --- .../gradle/KaptIncrementalWithIsolatingApt.kt | 23 ++++++++++++- .../kotlin/gradle/internal/kapt/KaptTask.kt | 7 +++- .../kapt/incremental/ClasspathSnapshot.kt | 29 ++++++++++++---- .../kapt/incremental/ClasspathSnapshotTest.kt | 33 ++++++++++++++----- 4 files changed, 75 insertions(+), 17 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalWithIsolatingApt.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalWithIsolatingApt.kt index e8285d00f0e..7065f7461a4 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalWithIsolatingApt.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalWithIsolatingApt.kt @@ -8,7 +8,6 @@ package org.jetbrains.kotlin.gradle import org.jetbrains.kotlin.gradle.incapt.IncrementalProcessor import org.jetbrains.kotlin.gradle.util.modify import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue import org.junit.Test import java.io.File import java.util.zip.ZipEntry @@ -65,6 +64,28 @@ class KaptIncrementalWithIsolatingApt : KaptIncrementalIT() { ) } } + + @Test + fun testChangingAnnotationProcessorClasspath() { + val project = getProject() + + project.build("clean", "build") { + assertSuccessful() + } + + project.gradleBuildScript().appendText( + """ + + dependencies { + kapt 'com.google.guava:guava:12.0' + } + """.trimIndent() + ) + project.build("build") { + assertSuccessful() + assertContains("Unable to use existing data, re-initializing classpath information for KAPT.") + } + } } private const val patternApt = "Processing java sources with annotation processors:" diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt index b83d0b4cdb1..b7a609c9591 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt @@ -201,7 +201,12 @@ abstract class KaptTask : ConventionTask(), TaskWithLocalState { ClasspathSnapshot.ClasspathSnapshotFactory.getEmptySnapshot() } val currentSnapshot = - ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(incAptCacheDir, classpath.files.toList(), allDataFiles) + ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent( + incAptCacheDir, + classpath.files.toList(), + kaptClasspath.files.toList(), + allDataFiles + ) val classpathChanges = currentSnapshot.diff(previousSnapshot, changedFiles) if (classpathChanges == KaptClasspathChanges.Unknown) { diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshot.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshot.kt index 6509abc7218..88c0c7aec43 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshot.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshot.kt @@ -10,18 +10,21 @@ import java.util.* import kotlin.collections.HashMap private const val CLASSPATH_ENTRIES_FILE = "classpath-entries.bin" +private const val ANNOTATION_PROCESSOR_CLASSPATH_ENTRIES_FILE = "ap-classpath-entries.bin" private const val CLASSPATH_STRUCTURE_FILE = "classpath-structure.bin" open class ClasspathSnapshot protected constructor( private val cacheDir: File, private val classpath: List, + private val annotationProcessorClasspath: List, private val dataForFiles: MutableMap ) { object ClasspathSnapshotFactory { fun loadFrom(cacheDir: File): ClasspathSnapshot { val classpathEntries = cacheDir.resolve(CLASSPATH_ENTRIES_FILE) val classpathStructureData = cacheDir.resolve(CLASSPATH_STRUCTURE_FILE) - if (!classpathEntries.exists() || !classpathStructureData.exists()) { + val annotationProcessorClasspathEntries= cacheDir.resolve(ANNOTATION_PROCESSOR_CLASSPATH_ENTRIES_FILE) + if (!classpathEntries.exists() || !classpathStructureData.exists() || !annotationProcessorClasspathEntries.exists()) { return UnknownSnapshot } @@ -30,25 +33,34 @@ open class ClasspathSnapshot protected constructor( it.readObject() as List } + val annotationProcessorClasspathFiles = + ObjectInputStream(BufferedInputStream(annotationProcessorClasspathEntries.inputStream())).use { + @Suppress("UNCHECKED_CAST") + it.readObject() as List + } + val dataForFiles = ObjectInputStream(BufferedInputStream(classpathStructureData.inputStream())).use { @Suppress("UNCHECKED_CAST") it.readObject() as MutableMap } - return ClasspathSnapshot(cacheDir, classpathFiles, dataForFiles) + return ClasspathSnapshot(cacheDir, classpathFiles, annotationProcessorClasspathFiles, dataForFiles) } - fun createCurrent(cacheDir: File, classpath: List, allStructureData: Set): ClasspathSnapshot { + fun createCurrent(cacheDir: File, classpath: List, annotationProcessorClasspath: List, allStructureData: Set): ClasspathSnapshot { val data = allStructureData.associateTo(HashMap(allStructureData.size)) { it to null } - return ClasspathSnapshot(cacheDir, classpath, data) + return ClasspathSnapshot(cacheDir, classpath, annotationProcessorClasspath, data) } fun getEmptySnapshot() = UnknownSnapshot } private fun isCompatible(snapshot: ClasspathSnapshot) = - this != UnknownSnapshot && snapshot != UnknownSnapshot && classpath == snapshot.classpath + this != UnknownSnapshot + && snapshot != UnknownSnapshot + && classpath == snapshot.classpath + && annotationProcessorClasspath == snapshot.annotationProcessorClasspath /** Compare this snapshot with the specified one only for the specified files. */ fun diff(previousSnapshot: ClasspathSnapshot, changedFiles: Set): KaptClasspathChanges { @@ -128,6 +140,11 @@ open class ClasspathSnapshot protected constructor( it.writeObject(classpath) } + val annotationProcessorClasspathEntries = cacheDir.resolve(ANNOTATION_PROCESSOR_CLASSPATH_ENTRIES_FILE) + ObjectOutputStream(BufferedOutputStream(annotationProcessorClasspathEntries.outputStream())).use { + it.writeObject(annotationProcessorClasspath) + } + val classpathStructureData = cacheDir.resolve(CLASSPATH_STRUCTURE_FILE) ObjectOutputStream(BufferedOutputStream(classpathStructureData.outputStream())).use { it.writeObject(dataForFiles) @@ -179,7 +196,7 @@ open class ClasspathSnapshot protected constructor( } } -object UnknownSnapshot : ClasspathSnapshot(File(""), emptyList(), mutableMapOf()) +object UnknownSnapshot : ClasspathSnapshot(File(""), emptyList(), emptyList(), mutableMapOf()) sealed class KaptIncrementalChanges { object Unknown : KaptIncrementalChanges() diff --git a/libraries/tools/kotlin-gradle-plugin/src/test/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshotTest.kt b/libraries/tools/kotlin-gradle-plugin/src/test/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshotTest.kt index ab658d9410d..0dedfb408e7 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/test/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshotTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/test/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshotTest.kt @@ -23,7 +23,7 @@ class ClasspathSnapshotTest { ) val snapshotDir = tmp.newFolder() - val currentSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(snapshotDir, listOf(), setOf(data)) + val currentSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(snapshotDir, listOf(), listOf(), setOf(data)) assertEquals(KaptClasspathChanges.Unknown, currentSnapshot.diff(UnknownSnapshot, setOf(data))) currentSnapshot.writeToCache() @@ -35,9 +35,23 @@ class ClasspathSnapshotTest { @Test fun testIncompatibleClasspaths() { - val firstSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(File(""), listOf(File("a.jar")), emptySet()) + val firstSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(File(""), listOf(File("a.jar")), listOf(), emptySet()) val secondSnapshot = - ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(File(""), listOf(File("b.jar")), setOf(File(""))) + ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(File(""), listOf(File("b.jar")), listOf(), setOf(File(""))) + assertEquals(KaptClasspathChanges.Unknown, firstSnapshot.diff(secondSnapshot, emptySet())) + } + + @Test + fun testIncompatibleAnnotationProcessorClasspaths() { + val firstSnapshot = + ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(File(""), listOf(File("a.jar")), listOf(File("ap2.jar")), emptySet()) + val secondSnapshot = + ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent( + File(""), + listOf(File("a.jar")), + listOf(File("ap1.jar")), + setOf(File("")) + ) assertEquals(KaptClasspathChanges.Unknown, firstSnapshot.diff(secondSnapshot, emptySet())) } @@ -47,7 +61,7 @@ class ClasspathSnapshotTest { ClassData("first/A"), ClassData("first/B").also { it.withAbiDependencies("first/A") } ) - val firstSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(tmp.newFolder(), listOf(), setOf(dataFile)) + val firstSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(tmp.newFolder(), listOf(), listOf(), setOf(dataFile)) firstSnapshot.writeToCache() generateStructureData( @@ -55,7 +69,7 @@ class ClasspathSnapshotTest { ClassData("first/B").also { it.withAbiDependencies("first/A") }, outputFile = dataFile ) - val changedSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(tmp.newFolder(), listOf(), setOf(dataFile)) + val changedSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(tmp.newFolder(), listOf(), listOf(), setOf(dataFile)) val diff = changedSnapshot.diff(firstSnapshot, setOf(dataFile)) as KaptClasspathChanges.Known assertEquals(setOf("first/A", "first/B"), diff.names) @@ -72,12 +86,12 @@ class ClasspathSnapshotTest { val cacheDir = tmp.newFolder() val firstSnapshot = - ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(cacheDir, listOf(), setOf(dataFile, libraryDataFile)) + ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(cacheDir, listOf(), listOf(), setOf(dataFile, libraryDataFile)) firstSnapshot.writeToCache() generateStructureData(ClassData("library/C", ByteArray(1)), outputFile = libraryDataFile) val changedSnapshot = - ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(cacheDir, listOf(), setOf(dataFile, libraryDataFile)) + ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(cacheDir, listOf(), listOf(), setOf(dataFile, libraryDataFile)) val diff = changedSnapshot.diff(firstSnapshot, setOf(libraryDataFile)) as KaptClasspathChanges.Known assertEquals(setOf("library/C", "first/A", "first/B"), diff.names) @@ -89,11 +103,12 @@ class ClasspathSnapshotTest { ClassData("first/A"), ClassData("first/B").also { it.withAbiDependencies("first/A") } ) - val firstSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(tmp.newFolder(), listOf(), setOf(dataFile)) + val firstSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(tmp.newFolder(), listOf(), listOf(), setOf(dataFile)) firstSnapshot.writeToCache() val copyOfDataFile = dataFile.copyTo(tmp.newFile(), overwrite = true) - val secondSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(tmp.newFolder(), listOf(), setOf(copyOfDataFile)) + val secondSnapshot = + ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(tmp.newFolder(), listOf(), listOf(), setOf(copyOfDataFile)) val diff = secondSnapshot.diff(firstSnapshot, setOf()) as KaptClasspathChanges.Known assertEquals(emptySet(), diff.names)