From 317c39f27d33c2ad05523c2a0a31eb78a07feaa5 Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov Date: Sat, 27 Jul 2019 19:00:38 +0300 Subject: [PATCH] Enable assertions in Kotlin Compile Daemon process #KT-32992 Fixed --- .../kotlin/daemon/common/DaemonParams.kt | 5 ++ .../kotlin/daemon/CompilerDaemonTest.kt | 48 +++++++++++++++++++ .../integration/CompilerDaemonTest.kt | 48 +++++++++++++++++++ .../jetbrains/kotlin/gradle/BaseGradleIT.kt | 1 - .../kotlin/gradle/ExecutionStrategyIT.kt | 36 +++++++++----- .../GradleKotlinCompilerRunner.kt | 6 +-- .../GradleKotlinCompilerWork.kt | 4 +- 7 files changed, 127 insertions(+), 21 deletions(-) diff --git a/compiler/daemon/daemon-common/src/org/jetbrains/kotlin/daemon/common/DaemonParams.kt b/compiler/daemon/daemon-common/src/org/jetbrains/kotlin/daemon/common/DaemonParams.kt index e8874b9c421..3343e829764 100644 --- a/compiler/daemon/daemon-common/src/org/jetbrains/kotlin/daemon/common/DaemonParams.kt +++ b/compiler/daemon/daemon-common/src/org/jetbrains/kotlin/daemon/common/DaemonParams.kt @@ -314,9 +314,14 @@ fun configureDaemonJVMOptions(opts: DaemonJVMOptions, System.getProperty(COMPILE_DAEMON_LOG_PATH_PROPERTY)?.let { opts.jvmParams.add("D$COMPILE_DAEMON_LOG_PATH_PROPERTY=\"$it\"") } System.getProperty(KOTLIN_COMPILER_ENVIRONMENT_KEEPALIVE_PROPERTY)?.let { opts.jvmParams.add("D$KOTLIN_COMPILER_ENVIRONMENT_KEEPALIVE_PROPERTY") } } + + if (opts.jvmParams.none { it.matches(jvmAssertArgsRegex) }) { + opts.jvmParams.add("ea") + } return opts } +private val jvmAssertArgsRegex = "(es?a|ds?a|(enable|disable)(system)?assertions)(${'$'}|:)".toRegex() fun configureDaemonJVMOptions(vararg additionalParams: String, inheritMemoryLimits: Boolean, diff --git a/compiler/tests/org/jetbrains/kotlin/daemon/CompilerDaemonTest.kt b/compiler/tests/org/jetbrains/kotlin/daemon/CompilerDaemonTest.kt index ed8ffce29a3..437c5038517 100644 --- a/compiler/tests/org/jetbrains/kotlin/daemon/CompilerDaemonTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/daemon/CompilerDaemonTest.kt @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.daemon import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.io.FileUtilRt +import junit.framework.Assert import junit.framework.TestCase import org.jetbrains.kotlin.cli.AbstractCliTest import org.jetbrains.kotlin.cli.common.messages.MessageRenderer @@ -187,6 +188,53 @@ class CompilerDaemonTest : KotlinIntegrationTestBase() { } } + fun testDaemonAssertsOptions() { + val allAssetionsArgs = setOf( + "-ea", "-enableassertions", + "-da", "-disableassertions", + "-esa", "-enablesystemassertions", + "-dsa", "-disablesystemassertions" + ) + + fun assertionsJvmArgs() = configureDaemonJVMOptions( + inheritMemoryLimits = true, + inheritOtherJvmOptions = false, + inheritAdditionalProperties = true + ).mappers.flatMap { it.toArgs("-") }.filter { it in allAssetionsArgs }.joinToString(", ") + + for (assertArgValue in allAssetionsArgs) { + withDaemonJvmOptionsSetTo(assertArgValue) { + Assert.assertEquals(assertArgValue, assertionsJvmArgs()) + } + } + + withDaemonJvmOptionsSetTo(null) { + Assert.assertEquals("-ea", assertionsJvmArgs()) + } + } + + private fun withDaemonJvmOptionsSetTo(newValue: String?, fn: () -> Unit) { + val backup = getAndSetSystemProperty(COMPILE_DAEMON_JVM_OPTIONS_PROPERTY, newValue) + + try { + fn() + } finally { + getAndSetSystemProperty(COMPILE_DAEMON_JVM_OPTIONS_PROPERTY, backup) + } + } + + private fun getAndSetSystemProperty(property: String, newValue: String?): String? { + val oldValue = System.getProperty(property) + + if (newValue != null) { + System.setProperty(property, newValue) + } else { + System.clearProperty(property) + } + + return oldValue + } + fun testDaemonOptionsParsing() { val backupOptions = System.getProperty(COMPILE_DAEMON_OPTIONS_PROPERTY) try { diff --git a/compiler/tests/org/jetbrains/kotlin/daemon/experimental/integration/CompilerDaemonTest.kt b/compiler/tests/org/jetbrains/kotlin/daemon/experimental/integration/CompilerDaemonTest.kt index 804fdacd0a1..9076316cd7e 100644 --- a/compiler/tests/org/jetbrains/kotlin/daemon/experimental/integration/CompilerDaemonTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/daemon/experimental/integration/CompilerDaemonTest.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.daemon.experimental.integration +import junit.framework.Assert import junit.framework.TestCase import kotlinx.coroutines.* import org.jetbrains.kotlin.cli.AbstractCliTest @@ -264,6 +265,53 @@ class CompilerDaemonTest : KotlinIntegrationTestBase() { } } + fun ignore_testDaemonAssertsOptions() { + val allAssetionsArgs = setOf( + "-ea", "-enableassertions", + "-da", "-disableassertions", + "-esa", "-enablesystemassertions", + "-dsa", "-disablesystemassertions" + ) + + fun assertionsJvmArgs() = configureDaemonJVMOptions( + inheritMemoryLimits = true, + inheritOtherJvmOptions = false, + inheritAdditionalProperties = true + ).mappers.flatMap { it.toArgs("-") }.filter { it in allAssetionsArgs }.joinToString(", ") + + for (assertArgValue in allAssetionsArgs) { + withDaemonJvmOptionsSetTo(assertArgValue) { + Assert.assertEquals(assertArgValue, assertionsJvmArgs()) + } + } + + withDaemonJvmOptionsSetTo(null) { + Assert.assertEquals("-ea", assertionsJvmArgs()) + } + } + + private fun withDaemonJvmOptionsSetTo(newValue: String?, fn: () -> Unit) { + val backup = getAndSetSystemProperty(COMPILE_DAEMON_JVM_OPTIONS_PROPERTY, newValue) + + try { + fn() + } finally { + getAndSetSystemProperty(COMPILE_DAEMON_JVM_OPTIONS_PROPERTY, backup) + } + } + + private fun getAndSetSystemProperty(property: String, newValue: String?): String? { + val oldValue = System.getProperty(property) + + if (newValue != null) { + System.setProperty(property, newValue) + } else { + System.clearProperty(property) + } + + return oldValue + } + fun ignore_testDaemonOptionsParsing() { val backupOptions = System.getProperty(COMPILE_DAEMON_OPTIONS_PROPERTY) try { 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 03d84c7d7fa..5bbefcfb4e5 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 @@ -734,7 +734,6 @@ abstract class BaseGradleIT { // Workaround: override a console type set in the user machine gradle.properties (since Gradle 4.3): add("--console=plain") - add("-Dkotlin.daemon.ea=true") addAll(options.freeCommandLineArgs) } diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/ExecutionStrategyIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/ExecutionStrategyIT.kt index aa295f966cc..a4bef833667 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/ExecutionStrategyIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/ExecutionStrategyIT.kt @@ -1,6 +1,5 @@ package org.jetbrains.kotlin.gradle -import org.gradle.util.GradleVersion import org.jetbrains.kotlin.gradle.util.checkedReplace import org.jetbrains.kotlin.gradle.util.getFileByName import org.jetbrains.kotlin.gradle.util.modify @@ -66,23 +65,14 @@ abstract class ExecutionStrategyIT : BaseGradleIT() { val strategyCLIArg = "-Dkotlin.compiler.execution.strategy=$executionStrategy" val finishMessage = "Finished executing kotlin compiler using $executionStrategy strategy" - val isGradleAtLeast50 = project.testGradleVersionAtLeast("5.0") - project.build("build", strategyCLIArg) { assertSuccessful() assertContains(finishMessage) checkOutput() assertNoWarnings() - if (executionStrategy == "daemon" && isGradleAtLeast50) { - val m = "Kotlin compile daemon JVM options: \\[(.*?)\\]".toRegex().find(output) - ?: error("Could not find Kotlin compile daemon JVM options in Gradle's output") - val kotlinDaemonJvmArgs = m.groupValues[1].split(",").map { it.trim() } - val maxMetaspaceArg = "-XX:MaxMetaspaceSize=256m" - Assert.assertTrue( - "Kotlin daemon JVM args do not contain '$maxMetaspaceArg': $kotlinDaemonJvmArgs", - maxMetaspaceArg in kotlinDaemonJvmArgs - ) + if (executionStrategy == "daemon") { + checkCompileDaemon() } } @@ -98,6 +88,28 @@ abstract class ExecutionStrategyIT : BaseGradleIT() { } } + private fun CompiledProject.checkCompileDaemon() { + val isGradleAtLeast50 = project.testGradleVersionAtLeast("5.0") + + val m = "Kotlin compile daemon JVM options: \\[(.*?)\\]".toRegex().find(output) + ?: error("Could not find Kotlin compile daemon JVM options in Gradle's output") + val kotlinDaemonJvmArgs = m.groupValues[1].split(",").mapTo(LinkedHashSet()) { it.trim() } + + fun assertDaemonArgsContain(arg: String) { + Assert.assertTrue( + "Expected '$arg' in kotlin daemon JVM args, got: $kotlinDaemonJvmArgs", + arg in kotlinDaemonJvmArgs + ) + } + + if (isGradleAtLeast50) { + // 256m is the default value for Gradle 5.0+ + assertDaemonArgsContain("-XX:MaxMetaspaceSize=256m") + } + + assertDaemonArgsContain("-ea") + } + protected open fun setupProject(project: Project) { project.setupWorkingDir() File(project.projectDir, "app/build.gradle").appendText( diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerRunner.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerRunner.kt index e76d0c2ba20..139424d5913 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerRunner.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerRunner.kt @@ -153,14 +153,10 @@ internal open class GradleCompilerRunner(protected val task: Task) { sessionIsAliveFlagFile: File, compilerFullClasspath: List, messageCollector: MessageCollector, - isDebugEnabled: Boolean, - enableAssertions: Boolean + isDebugEnabled: Boolean ): CompileServiceSession? { val compilerId = CompilerId.makeCompilerId(compilerFullClasspath) val additionalJvmParams = arrayListOf() - if (enableAssertions) { - additionalJvmParams.add("ea") - } return KotlinCompilerRunnerUtils.newDaemonConnection( compilerId, clientIsAliveFlagFile, sessionIsAliveFlagFile, messageCollector = messageCollector, 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 942e6ae0dfb..4ebf85b37cd 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 @@ -150,7 +150,6 @@ internal class GradleKotlinCompilerWork @Inject constructor( private fun compileWithDaemon(messageCollector: MessageCollector): ExitCode? { val isDebugEnabled = log.isDebugEnabled || System.getProperty("kotlin.daemon.debug.log")?.toBoolean() ?: true - val enableAssertions = System.getProperty("kotlin.daemon.ea")?.toBoolean() ?: false val daemonMessageCollector = if (isDebugEnabled) messageCollector else MessageCollector.NONE @@ -161,8 +160,7 @@ internal class GradleKotlinCompilerWork @Inject constructor( sessionFlagFile, compilerFullClasspath, daemonMessageCollector, - isDebugEnabled = isDebugEnabled, - enableAssertions = enableAssertions + isDebugEnabled = isDebugEnabled ) } catch (e: Throwable) { log.error("Caught an exception trying to connect to Kotlin Daemon:")