Skip to content

Commit

Permalink
Use in process java compilation unless jdk is explicitly set by the d…
Browse files Browse the repository at this point in the history
…eveloper

This PR updates KCT to use in process compilation unless the developer
explicitly provides a jdkHome that does not match the java home of the
current process.

In most cases, it won't be provided and because we have a default,
current implementation makes KCT spin off another process to compile java
sources, causing a significant compilation time penalty.
tschuchortdev#113 (comment)

With this change, KCT won't create another process unless jdkHome is
set to a value that does not match the jdk home inherited from the
current process.

Issue: tschuchortdev#113
Test: As this is a bit difficult to test, I've added test that will
check the logs ¯\_(ツ)_/¯
  • Loading branch information
yigit committed Feb 19, 2021
1 parent 4ef88b2 commit 54e6cec
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class KotlinCompilation : AbstractKotlinCompilation<K2JVMCompilerArguments>() {
* (on JDK8) or --system none (on JDK9+). This can be useful if all
* the JDK classes you need are already on the (inherited) classpath.
* */
var jdkHome: File? by default { getJdkHome() }
var jdkHome: File? by default { processJdkHome }

/**
* Path to the kotlin-stdlib.jar
Expand Down Expand Up @@ -503,11 +503,11 @@ class KotlinCompilation : AbstractKotlinCompilation<K2JVMCompilerArguments>() {
if(javaSources.isEmpty())
return ExitCode.OK

if(jdkHome != null) {
if(jdkHome != null && jdkHome!!.canonicalPath != processJdkHome.canonicalPath) {
/* If a JDK home is given, try to run javac from there so it uses the same JDK
as K2JVMCompiler. Changing the JDK of the system java compiler via the
"--system" and "-bootclasspath" options is not so easy. */

log("compiling java in a sub-process because a jdkHome is specified")
val jdkBinFile = File(jdkHome, "bin")
check(jdkBinFile.exists()) { "No JDK bin folder found at: ${jdkBinFile.toPath()}" }

Expand All @@ -531,22 +531,23 @@ class KotlinCompilation : AbstractKotlinCompilation<K2JVMCompilerArguments>() {
}
}
else {
/* If no JDK is given, we will use the host process' system java compiler
and erase the bootclasspath. The user is then on their own to somehow
/* If no JDK is given, we will use the host process' system java compiler.
If it is set to `null`, we will erase the bootclasspath. The user is then on their own to somehow
provide the JDK classes via the regular classpath because javac won't
work at all without them */

log("jdkHome is not specified. Using system java compiler of the host process.")
val isJavac9OrLater = isJdk9OrLater()
val javacArgs = baseJavacArgs(isJavac9OrLater).apply {
// erase bootclasspath or JDK path because no JDK was specified
if (isJavac9OrLater)
addAll("--system", "none")
else
addAll("-bootclasspath", "")
if (jdkHome == null) {
log("jdkHome is set to null, removing booth classpath from java compilation")
// erase bootclasspath or JDK path because no JDK was specified
if (isJavac9OrLater)
addAll("--system", "none")
else
addAll("-bootclasspath", "")
}
}

log("jdkHome is null. Using system java compiler of the host process.")

val javac = SynchronizedToolProvider.systemJavaCompiler
val javaFileManager = javac.getStandardFileManager(null, null, null)
val diagnosticCollector = DiagnosticCollector<JavaFileObject>()
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/kotlin/com/tschuchort/compiletesting/Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ internal fun getJavaHome(): File {
return File(path).also { check(it.isDirectory) }
}

internal fun getJdkHome()
= if(isJdk9OrLater())
internal val processJdkHome by lazy {
if(isJdk9OrLater())
getJavaHome()
else
getJavaHome().parentFile
}

/** Checks if the JDK of the host process is version 9 or later */
internal fun isJdk9OrLater(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.compiler.plugin.AbstractCliOption
import org.jetbrains.kotlin.compiler.plugin.CliOption
import org.jetbrains.kotlin.compiler.plugin.CommandLineProcessor
import org.jetbrains.kotlin.compiler.plugin.PluginCliOptionProcessingException
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
import java.io.ByteArrayOutputStream
import java.io.File
import javax.annotation.processing.AbstractProcessor
import javax.annotation.processing.RoundEnvironment
Expand Down Expand Up @@ -183,6 +183,9 @@ class KotlinCompilationTests {

assertThat(result.exitCode).isEqualTo(ExitCode.COMPILATION_ERROR)
assertThat(result.messages).contains("Unable to find package java.lang")
assertThat(result.messages).contains(
"jdkHome is set to null, removing booth classpath from java compilation"
)
}

@Test
Expand Down Expand Up @@ -808,6 +811,53 @@ class KotlinCompilationTests {
assertThat(result.outputDirectory.listFilesRecursively().map { it.name })
.contains("JSource.class", "KSource.class")
}

@Test
fun `java compilation runs in process when no jdk is specified`() {
val source = SourceFile.java("JSource.java",
"""
class JSource {}
""".trimIndent())
val result = defaultCompilerConfig().apply {
sources = listOf(source)
}.compile()
assertThat(result.exitCode).isEqualTo(ExitCode.OK)
assertThat(result.messages).contains(
"jdkHome is not specified. Using system java compiler of the host process."
)
assertThat(result.messages).doesNotContain(
"jdkHome is set to null, removing booth classpath from java compilation"
)
}

@Test
fun `java compilation runs in a sub-process when jdk is specified`() {
val source = SourceFile.java("JSource.java",
"""
class JSource {}
""".trimIndent())
val fakeJdkHome = temporaryFolder.newFolder("fake-jdk-home")
fakeJdkHome.resolve("bin").mkdirs()
val logsStream = ByteArrayOutputStream()
val compiler = defaultCompilerConfig().apply {
sources = listOf(source)
jdkHome = fakeJdkHome
messageOutputStream = logsStream
}
val compilation = runCatching {
// jdk is fake so it won't compile
compiler.compile()
}
// it should fail since we are passing a fake jdk
assertThat(compilation.isFailure).isTrue()
val logs = logsStream.toString(Charsets.UTF_8)
assertThat(logs).contains(
"compiling java in a sub-process because a jdkHome is specified"
)
assertThat(logs).doesNotContain(
"jdkHome is set to null, removing booth classpath from java compilation"
)
}

class InheritedClass {}
}

0 comments on commit 54e6cec

Please sign in to comment.