Skip to content

Commit 91b43f6

Browse files
yigittschuchortdev
authored andcommitted
Use in process java compilation unless jdk is explicitly set by the developer
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/kotlin-compile-testing#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: google#113 Test: As this is a bit difficult to test, I've added test that will check the logs ¯\_(ツ)_/¯
1 parent 25601ab commit 91b43f6

File tree

3 files changed

+68
-16
lines changed

3 files changed

+68
-16
lines changed

core/src/main/kotlin/com/tschuchort/compiletesting/KotlinCompilation.kt

+14-13
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class KotlinCompilation : AbstractKotlinCompilation<K2JVMCompilerArguments>() {
170170
* (on JDK8) or --system none (on JDK9+). This can be useful if all
171171
* the JDK classes you need are already on the (inherited) classpath.
172172
* */
173-
var jdkHome: File? by default { getJdkHome() }
173+
var jdkHome: File? by default { processJdkHome }
174174

175175
/**
176176
* Path to the kotlin-stdlib.jar
@@ -503,11 +503,11 @@ class KotlinCompilation : AbstractKotlinCompilation<K2JVMCompilerArguments>() {
503503
if(javaSources.isEmpty())
504504
return ExitCode.OK
505505

506-
if(jdkHome != null) {
506+
if(jdkHome != null && jdkHome!!.canonicalPath != processJdkHome.canonicalPath) {
507507
/* If a JDK home is given, try to run javac from there so it uses the same JDK
508508
as K2JVMCompiler. Changing the JDK of the system java compiler via the
509509
"--system" and "-bootclasspath" options is not so easy. */
510-
510+
log("compiling java in a sub-process because a jdkHome is specified")
511511
val jdkBinFile = File(jdkHome, "bin")
512512
check(jdkBinFile.exists()) { "No JDK bin folder found at: ${jdkBinFile.toPath()}" }
513513

@@ -531,22 +531,23 @@ class KotlinCompilation : AbstractKotlinCompilation<K2JVMCompilerArguments>() {
531531
}
532532
}
533533
else {
534-
/* If no JDK is given, we will use the host process' system java compiler
535-
and erase the bootclasspath. The user is then on their own to somehow
534+
/* If no JDK is given, we will use the host process' system java compiler.
535+
If it is set to `null`, we will erase the bootclasspath. The user is then on their own to somehow
536536
provide the JDK classes via the regular classpath because javac won't
537537
work at all without them */
538-
538+
log("jdkHome is not specified. Using system java compiler of the host process.")
539539
val isJavac9OrLater = isJdk9OrLater()
540540
val javacArgs = baseJavacArgs(isJavac9OrLater).apply {
541-
// erase bootclasspath or JDK path because no JDK was specified
542-
if (isJavac9OrLater)
543-
addAll("--system", "none")
544-
else
545-
addAll("-bootclasspath", "")
541+
if (jdkHome == null) {
542+
log("jdkHome is set to null, removing booth classpath from java compilation")
543+
// erase bootclasspath or JDK path because no JDK was specified
544+
if (isJavac9OrLater)
545+
addAll("--system", "none")
546+
else
547+
addAll("-bootclasspath", "")
548+
}
546549
}
547550

548-
log("jdkHome is null. Using system java compiler of the host process.")
549-
550551
val javac = SynchronizedToolProvider.systemJavaCompiler
551552
val javaFileManager = javac.getStandardFileManager(null, null, null)
552553
val diagnosticCollector = DiagnosticCollector<JavaFileObject>()

core/src/main/kotlin/com/tschuchort/compiletesting/Utils.kt

+3-2
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@ internal fun getJavaHome(): File {
1919
return File(path).also { check(it.isDirectory) }
2020
}
2121

22-
internal fun getJdkHome()
23-
= if(isJdk9OrLater())
22+
internal val processJdkHome by lazy {
23+
if(isJdk9OrLater())
2424
getJavaHome()
2525
else
2626
getJavaHome().parentFile
27+
}
2728

2829
/** Checks if the JDK of the host process is version 9 or later */
2930
internal fun isJdk9OrLater(): Boolean

core/src/test/kotlin/com/tschuchort/compiletesting/KotlinCompilationTests.kt

+51-1
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import org.assertj.core.api.Assertions.assertThat
77
import org.jetbrains.kotlin.compiler.plugin.AbstractCliOption
88
import org.jetbrains.kotlin.compiler.plugin.CliOption
99
import org.jetbrains.kotlin.compiler.plugin.CommandLineProcessor
10-
import org.jetbrains.kotlin.compiler.plugin.PluginCliOptionProcessingException
1110
import org.junit.Rule
1211
import org.junit.Test
1312
import org.junit.rules.TemporaryFolder
13+
import java.io.ByteArrayOutputStream
1414
import java.io.File
1515
import javax.annotation.processing.AbstractProcessor
1616
import javax.annotation.processing.RoundEnvironment
@@ -183,6 +183,9 @@ class KotlinCompilationTests {
183183

184184
assertThat(result.exitCode).isEqualTo(ExitCode.COMPILATION_ERROR)
185185
assertThat(result.messages).contains("Unable to find package java.lang")
186+
assertThat(result.messages).contains(
187+
"jdkHome is set to null, removing booth classpath from java compilation"
188+
)
186189
}
187190

188191
@Test
@@ -808,6 +811,53 @@ class KotlinCompilationTests {
808811
assertThat(result.outputDirectory.listFilesRecursively().map { it.name })
809812
.contains("JSource.class", "KSource.class")
810813
}
814+
815+
@Test
816+
fun `java compilation runs in process when no jdk is specified`() {
817+
val source = SourceFile.java("JSource.java",
818+
"""
819+
class JSource {}
820+
""".trimIndent())
821+
val result = defaultCompilerConfig().apply {
822+
sources = listOf(source)
823+
}.compile()
824+
assertThat(result.exitCode).isEqualTo(ExitCode.OK)
825+
assertThat(result.messages).contains(
826+
"jdkHome is not specified. Using system java compiler of the host process."
827+
)
828+
assertThat(result.messages).doesNotContain(
829+
"jdkHome is set to null, removing booth classpath from java compilation"
830+
)
831+
}
832+
833+
@Test
834+
fun `java compilation runs in a sub-process when jdk is specified`() {
835+
val source = SourceFile.java("JSource.java",
836+
"""
837+
class JSource {}
838+
""".trimIndent())
839+
val fakeJdkHome = temporaryFolder.newFolder("fake-jdk-home")
840+
fakeJdkHome.resolve("bin").mkdirs()
841+
val logsStream = ByteArrayOutputStream()
842+
val compiler = defaultCompilerConfig().apply {
843+
sources = listOf(source)
844+
jdkHome = fakeJdkHome
845+
messageOutputStream = logsStream
846+
}
847+
val compilation = runCatching {
848+
// jdk is fake so it won't compile
849+
compiler.compile()
850+
}
851+
// it should fail since we are passing a fake jdk
852+
assertThat(compilation.isFailure).isTrue()
853+
val logs = logsStream.toString(Charsets.UTF_8)
854+
assertThat(logs).contains(
855+
"compiling java in a sub-process because a jdkHome is specified"
856+
)
857+
assertThat(logs).doesNotContain(
858+
"jdkHome is set to null, removing booth classpath from java compilation"
859+
)
860+
}
811861

812862
class InheritedClass {}
813863
}

0 commit comments

Comments
 (0)