From 7dcdba7e1aa9af5ceb94638da459fdf0170e1f1a Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov Date: Thu, 17 Jan 2019 00:12:15 +0300 Subject: [PATCH] Restore outputs when Kotlin task fails #KT-12295 Fixed Gradle forces non-incremetnal builds, when a task failed, but its outputs were changed. This is an issue for Kotlin incremental compilation, because it also forces all dependent modules to be rebuilt. This change fixes the issue by reading all output and restoring it on a compile error/exception. This change does not affect non-incremental builds. --- .../jetbrains/kotlin/gradle/BaseGradleIT.kt | 10 ++++ .../IncrementalCompilationMultiProjectIT.kt | 34 +++++++++--- .../kotlin/gradle/KaptIncrementalIT.kt | 26 +++++++++ .../GradleKotlinCompilerWork.kt | 17 +----- .../internal/kapt/KaptWithKotlincTask.kt | 2 + .../jetbrains/kotlin/gradle/tasks/Tasks.kt | 54 +++++++++++++++++-- 6 files changed, 116 insertions(+), 27 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/BaseGradleIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/BaseGradleIT.kt index 422c1411a6b..130f43090a3 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/BaseGradleIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/BaseGradleIT.kt @@ -451,6 +451,16 @@ abstract class BaseGradleIT { } } + fun CompiledProject.assertTasksFailed(vararg tasks: String) { + assertTasksFailed(tasks.toList()) + } + + fun CompiledProject.assertTasksFailed(tasks: Iterable) { + for (task in tasks) { + assertContains("$task FAILED") + } + } + fun CompiledProject.assertTasksUpToDate(vararg tasks: String) { assertTasksUpToDate(tasks.toList()) } diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt index fde8bb330b2..64bb62ad340 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt @@ -27,12 +27,18 @@ class IncrementalCompilationJsMultiProjectIT : BaseIncrementalCompilationMultiPr override val additionalLibDependencies: String = "implementation \"org.jetbrains.kotlin:kotlin-test-js:${'$'}kotlin_version\"" + + override val compileKotlinTaskName: String + get() = "compileKotlin2Js" } class IncrementalCompilationJvmMultiProjectIT : BaseIncrementalCompilationMultiProjectIT() { override val additionalLibDependencies: String = "implementation \"org.jetbrains.kotlin:kotlin-stdlib:${'$'}kotlin_version\"" + override val compileKotlinTaskName: String + get() = "compileKotlin" + override fun defaultProject(): Project = Project("incrementalMultiproject") @@ -203,6 +209,7 @@ open class A { } protected abstract val additionalLibDependencies: String + protected abstract val compileKotlinTaskName: String @Test fun testAddDependencyToLib() { @@ -251,20 +258,31 @@ open class A { } val bKt = project.projectDir.getFileByName("B.kt") + val bKtContent = bKt.readText() bKt.delete() - project.build("build") { - assertFailed() + fun runFailingBuild() { + project.build("build") { + assertFailed() + assertContains("B.kt has been removed") + assertTasksFailed(":lib:$compileKotlinTaskName") + val affectedFiles = project.projectDir.getFilesByNames("barUseAB.kt", "barUseB.kt") + assertCompiledKotlinSources(project.relativize(affectedFiles)) + } } - project.projectDir.getFileByName("barUseB.kt").delete() - project.projectDir.getFileByName("barUseAB.kt").delete() + runFailingBuild() + runFailingBuild() + + bKt.writeText(bKtContent.replace("fun b", "open fun b")) project.build("build") { - assertFailed() - val affectedSources = project.projectDir.allKotlinFiles() - val relativePaths = project.relativize(affectedSources) - assertCompiledKotlinSources(relativePaths) + assertSuccessful() + val affectedFiles = project.projectDir.getFilesByNames( + "B.kt", "barUseAB.kt", "barUseB.kt", + "BB.kt", "fooUseB.kt" + ) + assertCompiledKotlinSources(project.relativize(affectedFiles)) } } } diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalIT.kt index 7608a091946..9caf1998f6b 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalIT.kt @@ -71,6 +71,32 @@ class KaptIncrementalIT : BaseGradleIT() { } } + @Test + fun testCompileError() { + val project = getProject() + + project.build("build") { + assertSuccessful() + } + + val bKt = project.projectDir.getFileByName("B.kt") + val errorKt = bKt.resolveSibling("error.kt") + errorKt.writeText("") + + project.build("build") { + assertFailed() + assertTasksFailed(":kaptGenerateStubsKotlin") + } + + errorKt.delete() + bKt.modify { "$it\n" } + project.build("build") { + assertSuccessful() + assertCompiledKotlinSources(project.relativize(bKt)) + assertTasksExecuted(":kaptGenerateStubsKotlin", ":compileKotlin") + } + } + @Test fun testChangeFunctionBodyWithoutChangingSignature() { val project = getProject() diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerWork.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerWork.kt index 982005ba0ca..a4270b83c04 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerWork.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerWork.kt @@ -91,30 +91,17 @@ internal class GradleKotlinCompilerWork @Inject constructor( get() = incrementalCompilationEnvironment != null override fun run() { - if (!isIncremental) { - clearLocalStateDirectories(log, localStateDirectories, "IC is disabled") - } - val messageCollector = GradlePrintingMessageCollector(log) val exitCode = try { compileWithDaemonOrFallbackImpl(messageCollector) - } catch (e: Throwable) { - clearLocalStateDirectories(log, localStateDirectories, "exception when running compiler") - throw e } finally { if (buildFile != null && System.getProperty(DELETE_MODULE_FILE_PROPERTY) != "false") { buildFile.delete() } } - if (incrementalCompilationEnvironment != null) { - if (incrementalCompilationEnvironment.disableMultiModuleIC) { - incrementalCompilationEnvironment.multiModuleICSettings.buildHistoryFile.delete() - } - - if (exitCode != ExitCode.OK) { - clearLocalStateDirectories(log, localStateDirectories, "exit code: $exitCode") - } + if (incrementalCompilationEnvironment?.disableMultiModuleIC == true) { + incrementalCompilationEnvironment.multiModuleICSettings.buildHistoryFile.delete() } throwGradleExceptionIfError(exitCode) diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt index 2b0b55c51cc..595b346b735 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt @@ -20,6 +20,7 @@ import org.jetbrains.kotlin.compilerRunner.OutputItemsCollectorImpl import org.jetbrains.kotlin.gradle.plugin.PLUGIN_CLASSPATH_CONFIGURATION_NAME import org.jetbrains.kotlin.gradle.tasks.CompilerPluginOptions import org.jetbrains.kotlin.gradle.logging.GradlePrintingMessageCollector +import org.jetbrains.kotlin.gradle.tasks.clearLocalStateDirectories import org.jetbrains.kotlin.gradle.tasks.localStateDirectories import org.jetbrains.kotlin.gradle.utils.toSortedPathsArray @@ -55,6 +56,7 @@ open class KaptWithKotlincTask : KaptTask(), CompilerArgumentAwareWithInput() : AbstractKo @TaskAction fun execute(inputs: IncrementalTaskInputs) { + // If task throws exception, but its outputs are changed during execution, + // then Gradle forces next build to be non-incremental (see Gradle's DefaultTaskArtifactStateRepository#persistNewOutputs) + // To prevent this, we backup outputs before incremental build and restore when exception is thrown + val prevOutputs: Map? = if (incremental && inputs.isIncremental) { + val outputFiles = HashSet() + outputsCompatible.files.forEach { + if (it.isDirectory) { + it.walk().filterTo(outputFiles, File::isFile) + } else if (it.isFile) { + outputFiles.add(it) + } + } + + outputFiles.keysToMap { it.readBytes() } + } else null + + if (!incremental) { + clearLocalStateDirectories("IC is disabled") + } + + try { + executeImpl(inputs) + } catch (t: Throwable) { + outputsCompatible.files.forEach { + if (it.isDirectory) { + it.deleteRecursively() + } else if (it.isFile) { + it.delete() + } + } + + if (prevOutputs != null) { + val dirs = HashSet() + + for ((file, bytes) in prevOutputs) { + val dir = file.parentFile + if (dirs.add(dir)) { + dir.mkdirs() + } + file.writeBytes(bytes) + } + } + + throw t + } + } + + private fun executeImpl(inputs: IncrementalTaskInputs) { // Check that the JDK tools are available in Gradle (fail-fast, instead of a fail during the compiler run): findToolsJar()