From 965b4199f4ae94dd77f3d59a7396175ec1b6013d Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 16 May 2017 13:04:10 +0300 Subject: [PATCH] Fix compilation against JRE 9 on JPS Write the modular JDK (9+) path to the module.xml file passed to the compiler from the JPS plugin. This path is then recorded in the compiler configuration in KotlinToJVMBytecodeCompiler.configureSourceRoots. This is needed because in JPS plugin, we pass "-no-jdk" and thus no JDK home path was recorded in the compiler configuration in K2JVMCompiler.setupJdkClasspathRoots. Presence of JDK home path in the configuration is crucial for JDK 9 support (see KotlinCoreEnvironment.Companion.createApplicationEnvironment), because classes there can only be loaded with the special "jrt" file system, not as .class files in .jar files #KT-17801 Fixed --- .../jetbrains/kotlin/incremental/buildUtil.kt | 6 +++- .../kotlin/modules/KotlinModuleXmlBuilder.kt | 5 +++ .../cli/common/modules/ModuleBuilder.kt | 9 ++--- .../cli/common/modules/ModuleXmlParser.java | 5 +++ .../compiler/KotlinToJVMBytecodeCompiler.kt | 10 ++++++ .../kotlin/test/KotlinTestUtils.java | 20 +++++++---- .../AbstractDiagnosticsWithJdk9Test.java | 5 +-- .../org/jetbrains/kotlin/modules/Module.kt | 2 ++ idea/testData/modules.xml/modularJdkRoot.xml | 8 +++++ .../kotlin/jps/build/KotlinJpsBuildTest.kt | 16 +++++++++ .../kotlin/jvm/compiler/ClasspathOrderTest.kt | 1 + .../modules/KotlinModuleXmlGeneratorTest.java | 36 ++++++++++++++----- .../KotlinBuilderModuleScriptGenerator.kt | 16 +++++++++ .../testData/general/Jre9/kotlinProject.iml | 12 +++++++ .../testData/general/Jre9/kotlinProject.ipr | 14 ++++++++ jps-plugin/testData/general/Jre9/src/test.kt | 14 ++++++++ 16 files changed, 158 insertions(+), 21 deletions(-) create mode 100644 idea/testData/modules.xml/modularJdkRoot.xml create mode 100644 jps-plugin/testData/general/Jre9/kotlinProject.iml create mode 100644 jps-plugin/testData/general/Jre9/kotlinProject.ipr create mode 100644 jps-plugin/testData/general/Jre9/src/test.kt diff --git a/build-common/src/org/jetbrains/kotlin/incremental/buildUtil.kt b/build-common/src/org/jetbrains/kotlin/incremental/buildUtil.kt index 1460048c7ba..c54d63976b5 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/buildUtil.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/buildUtil.kt @@ -21,7 +21,10 @@ package org.jetbrains.kotlin.incremental import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.text.StringUtil -import org.jetbrains.kotlin.build.* +import org.jetbrains.kotlin.build.GeneratedFile +import org.jetbrains.kotlin.build.GeneratedJvmClass +import org.jetbrains.kotlin.build.JvmSourceRoot +import org.jetbrains.kotlin.build.isModuleMappingFile import org.jetbrains.kotlin.compilerRunner.OutputItemsCollectorImpl import org.jetbrains.kotlin.config.IncrementalCompilation import org.jetbrains.kotlin.config.Services @@ -50,6 +53,7 @@ fun makeModuleFile(name: String, isTest: Boolean, outputDir: File, sourcesToComp sourcesToCompile, javaSourceRoots.map { JvmSourceRoot(it) }, classpath, + null, "java-production", isTest, // this excludes the output directories from the class path, to be removed for true incremental compilation diff --git a/build-common/src/org/jetbrains/kotlin/modules/KotlinModuleXmlBuilder.kt b/build-common/src/org/jetbrains/kotlin/modules/KotlinModuleXmlBuilder.kt index 864b73c286e..14e69b1bbd9 100644 --- a/build-common/src/org/jetbrains/kotlin/modules/KotlinModuleXmlBuilder.kt +++ b/build-common/src/org/jetbrains/kotlin/modules/KotlinModuleXmlBuilder.kt @@ -39,6 +39,7 @@ class KotlinModuleXmlBuilder { sourceFiles: Iterable, javaSourceRoots: Iterable, classpathRoots: Iterable, + modularJdkRoot: File?, targetTypeId: String, isTests: Boolean, directoriesToFilterOut: Set, @@ -64,6 +65,10 @@ class KotlinModuleXmlBuilder { processJavaSourceRoots(javaSourceRoots) processClasspath(classpathRoots, directoriesToFilterOut) + if (modularJdkRoot != null) { + p.println("<", MODULAR_JDK_ROOT, " ", PATH, "=\"", getEscapedPath(modularJdkRoot), "\"/>") + } + closeTag(p, MODULE) return this } diff --git a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleBuilder.kt b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleBuilder.kt index 8eeefdd27a5..dca2eb6f234 100644 --- a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleBuilder.kt +++ b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleBuilder.kt @@ -29,13 +29,14 @@ class ModuleBuilder( private val classpathRoots = ArrayList() private val javaSourceRoots = ArrayList() private val friendDirs = ArrayList() + override var modularJdkRoot: String? = null - fun addSourceFiles(pattern: String) { - sourceFiles.add(pattern) + fun addSourceFiles(path: String) { + sourceFiles.add(path) } - fun addClasspathEntry(name: String) { - classpathRoots.add(name) + fun addClasspathEntry(path: String) { + classpathRoots.add(path) } fun addJavaSourceRoot(rootPath: JavaRootPath) { diff --git a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleXmlParser.java b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleXmlParser.java index 0aae9c0a8a5..dd0b96203e4 100644 --- a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleXmlParser.java +++ b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleXmlParser.java @@ -52,6 +52,7 @@ public class ModuleXmlParser { public static final String JAVA_SOURCE_PACKAGE_PREFIX = "packagePrefix"; public static final String PATH = "path"; public static final String CLASSPATH = "classpath"; + public static final String MODULAR_JDK_ROOT = "modularJdkRoot"; @NotNull public static ModuleScriptData parseModuleScript( @@ -172,6 +173,10 @@ public class ModuleXmlParser { String packagePrefix = getNullableAttribute(attributes, JAVA_SOURCE_PACKAGE_PREFIX); moduleBuilder.addJavaSourceRoot(new JavaRootPath(path, packagePrefix)); } + else if (MODULAR_JDK_ROOT.equalsIgnoreCase(qName)) { + String path = getAttribute(attributes, PATH, qName); + moduleBuilder.setModularJdkRoot(path); + } else { throw createError(qName); } diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.kt index 892cd4340dd..10f742a8716 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.kt @@ -204,6 +204,16 @@ object KotlinToJVMBytecodeCompiler { } } + for (module in chunk) { + val modularJdkRoot = module.modularJdkRoot + if (modularJdkRoot != null) { + // We use the SDK of the first module in the chunk, which is not always correct because some other module in the chunk + // might depend on a different SDK + configuration.put(JVMConfigurationKeys.JDK_HOME, File(modularJdkRoot)) + break + } + } + configuration.addAll(JVMConfigurationKeys.MODULES, chunk) } diff --git a/compiler/tests-common/org/jetbrains/kotlin/test/KotlinTestUtils.java b/compiler/tests-common/org/jetbrains/kotlin/test/KotlinTestUtils.java index 4c0e4b37521..1966a91b71a 100644 --- a/compiler/tests-common/org/jetbrains/kotlin/test/KotlinTestUtils.java +++ b/compiler/tests-common/org/jetbrains/kotlin/test/KotlinTestUtils.java @@ -497,12 +497,9 @@ public class KotlinTestUtils { JvmContentRootsKt.addJvmClasspathRoots(configuration, PathUtil.getJdkClassesRootsFromJre(getJreHome(jdk6))); } else if (jdkKind == TestJdkKind.FULL_JDK_9) { - String jdk9 = System.getenv("JDK_19"); - if (jdk9 != null) { - configuration.put(JVMConfigurationKeys.JDK_HOME, new File(jdk9)); - } - else { - System.err.println("Environment variable JDK_19 is not set, the test will be skipped"); + File home = getJre9HomeIfPossible(); + if (home != null) { + configuration.put(JVMConfigurationKeys.JDK_HOME, home); } } else { @@ -527,6 +524,17 @@ public class KotlinTestUtils { return configuration; } + @Nullable + public static File getJre9HomeIfPossible() { + String jdk9 = System.getenv("JDK_19"); + if (jdk9 == null) { + // TODO: replace this with a failure as soon as Java 9 is installed on all TeamCity agents + System.err.println("Environment variable JDK_19 is not set, the test will be skipped"); + return null; + } + return new File(jdk9); + } + @NotNull private static String getJreHome(@NotNull String jdkHome) { File jre = new File(jdkHome, "jre"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsWithJdk9Test.java b/compiler/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsWithJdk9Test.java index 19d23cbc907..03a77c89e69 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsWithJdk9Test.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsWithJdk9Test.java @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.checkers; import org.jetbrains.annotations.NotNull; import org.jetbrains.kotlin.test.ConfigurationKind; +import org.jetbrains.kotlin.test.KotlinTestUtils; import org.jetbrains.kotlin.test.TestJdkKind; import java.io.File; @@ -43,8 +44,8 @@ public abstract class AbstractDiagnosticsWithJdk9Test extends AbstractDiagnostic @NotNull Map modules, @NotNull List testFiles ) { - if (System.getenv("JDK_19") == null) { - // Skip this test if no environment variable JDK_19 is set up + if (KotlinTestUtils.getJre9HomeIfPossible() == null) { + // Skip this test if no Java 9 is found return; } super.doMultiFileTest(file, modules, testFiles); diff --git a/compiler/util/src/org/jetbrains/kotlin/modules/Module.kt b/compiler/util/src/org/jetbrains/kotlin/modules/Module.kt index ec4b8639f42..84d0b9bf102 100644 --- a/compiler/util/src/org/jetbrains/kotlin/modules/Module.kt +++ b/compiler/util/src/org/jetbrains/kotlin/modules/Module.kt @@ -30,6 +30,8 @@ interface Module { fun getClasspathRoots(): List fun getJavaSourceRoots(): List + + val modularJdkRoot: String? } data class JavaRootPath(val path: String, val packagePrefix: String? = null) diff --git a/idea/testData/modules.xml/modularJdkRoot.xml b/idea/testData/modules.xml/modularJdkRoot.xml new file mode 100644 index 00000000000..ca7614f1d16 --- /dev/null +++ b/idea/testData/modules.xml/modularJdkRoot.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/KotlinJpsBuildTest.kt b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/KotlinJpsBuildTest.kt index 58bb145d6d9..6e371f6e4de 100644 --- a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/KotlinJpsBuildTest.kt +++ b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/KotlinJpsBuildTest.kt @@ -21,10 +21,12 @@ import com.intellij.openapi.util.Condition import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.io.FileUtil.toSystemIndependentName import com.intellij.openapi.util.io.FileUtilRt +import com.intellij.openapi.vfs.StandardFileSystems import com.intellij.testFramework.LightVirtualFile import com.intellij.testFramework.UsefulTestCase import com.intellij.util.ArrayUtil import com.intellij.util.containers.ContainerUtil +import com.intellij.util.io.URLUtil import com.intellij.util.io.ZipUtil import org.jetbrains.jps.ModuleChunk import org.jetbrains.jps.api.CanceledStatus @@ -45,6 +47,8 @@ import org.jetbrains.jps.model.JpsModuleRootModificationUtil import org.jetbrains.jps.model.java.JavaSourceRootType import org.jetbrains.jps.model.java.JpsJavaDependencyScope import org.jetbrains.jps.model.java.JpsJavaExtensionService +import org.jetbrains.jps.model.java.JpsJavaSdkType +import org.jetbrains.jps.model.library.JpsOrderRootType import org.jetbrains.jps.model.module.JpsModule import org.jetbrains.jps.util.JpsPathUtil import org.jetbrains.kotlin.codegen.AsmUtil @@ -899,6 +903,18 @@ class KotlinJpsBuildTest : AbstractKotlinJpsBuildTestCase() { KotlinTestUtils.assertEqualsToFile(expectedFile, actual.toString()) } + fun testJre9() { + val path = KotlinTestUtils.getJre9HomeIfPossible()?.absolutePath ?: return + + val jdk = myModel.global.addSdk(JDK_NAME, path, "9", JpsJavaSdkType.INSTANCE) + jdk.addRoot(StandardFileSystems.JRT_PROTOCOL_PREFIX + path + URLUtil.JAR_SEPARATOR + "java.base", JpsOrderRootType.COMPILED) + + loadProject(workDir.absolutePath + File.separator + PROJECT_NAME + ".ipr") + addKotlinRuntimeDependency() + + buildAllModules().assertSuccessful() + } + private fun BuildResult.checkErrors() { val actualErrors = getMessages(BuildMessage.Kind.ERROR) .map { it as CompilerMessage } diff --git a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jvm/compiler/ClasspathOrderTest.kt b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jvm/compiler/ClasspathOrderTest.kt index a3c1581b6b6..baea25e041b 100644 --- a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jvm/compiler/ClasspathOrderTest.kt +++ b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jvm/compiler/ClasspathOrderTest.kt @@ -46,6 +46,7 @@ class ClasspathOrderTest : TestCaseWithTmpdir() { listOf(sourceDir), listOf(JvmSourceRoot(sourceDir)), listOf(PathUtil.getKotlinPathsForDistDirectory().runtimePath), + null, JavaModuleBuildTargetType.PRODUCTION.typeId, JavaModuleBuildTargetType.PRODUCTION.isTests, setOf(), diff --git a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/modules/KotlinModuleXmlGeneratorTest.java b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/modules/KotlinModuleXmlGeneratorTest.java index b8fedb9f6fd..7aa57449b70 100644 --- a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/modules/KotlinModuleXmlGeneratorTest.java +++ b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/modules/KotlinModuleXmlGeneratorTest.java @@ -33,10 +33,11 @@ public class KotlinModuleXmlGeneratorTest extends TestCase { Arrays.asList(new File("s1"), new File("s2")), Collections.singletonList(new JvmSourceRoot(new File("java"), null)), Arrays.asList(new File("cp1"), new File("cp2")), + null, JavaModuleBuildTargetType.PRODUCTION.getTypeId(), JavaModuleBuildTargetType.PRODUCTION.isTests(), - Collections.emptySet(), - Collections.emptyList() + Collections.emptySet(), + Collections.emptyList() ).asText().toString(); KotlinTestUtils.assertEqualsToFile(new File("idea/testData/modules.xml/basic.xml"), actual); } @@ -46,12 +47,13 @@ public class KotlinModuleXmlGeneratorTest extends TestCase { "name", "output", Arrays.asList(new File("s1"), new File("s2")), - Collections.emptyList(), + Collections.emptyList(), Arrays.asList(new File("cp1"), new File("cp2")), + null, JavaModuleBuildTargetType.PRODUCTION.getTypeId(), JavaModuleBuildTargetType.PRODUCTION.isTests(), Collections.singleton(new File("cp1")), - Collections.emptyList() + Collections.emptyList() ).asText().toString(); KotlinTestUtils.assertEqualsToFile(new File("idea/testData/modules.xml/filtered.xml"), actual); } @@ -62,25 +64,43 @@ public class KotlinModuleXmlGeneratorTest extends TestCase { "name", "output", Arrays.asList(new File("s1"), new File("s2")), - Collections.emptyList(), + Collections.emptyList(), Arrays.asList(new File("cp1"), new File("cp2")), + null, JavaModuleBuildTargetType.PRODUCTION.getTypeId(), JavaModuleBuildTargetType.PRODUCTION.isTests(), Collections.singleton(new File("cp1")), - Collections.emptyList() + Collections.emptyList() ); builder.addModule( "name2", "output2", Arrays.asList(new File("s12"), new File("s22")), - Collections.emptyList(), + Collections.emptyList(), Arrays.asList(new File("cp12"), new File("cp22")), + null, JavaModuleBuildTargetType.TEST.getTypeId(), JavaModuleBuildTargetType.TEST.isTests(), Collections.singleton(new File("cp12")), - Collections.emptyList() + Collections.emptyList() ); String actual = builder.asText().toString(); KotlinTestUtils.assertEqualsToFile(new File("idea/testData/modules.xml/multiple.xml"), actual); } + + public void testModularJdkRoot() throws Exception { + String actual = new KotlinModuleXmlBuilder().addModule( + "name", + "output", + Collections.emptyList(), + Collections.emptyList(), + Collections.emptyList(), + new File("/path/to/modular/jdk"), + JavaModuleBuildTargetType.PRODUCTION.getTypeId(), + JavaModuleBuildTargetType.PRODUCTION.isTests(), + Collections.emptySet(), + Collections.emptyList() + ).asText().toString(); + KotlinTestUtils.assertEqualsToFile(new File("idea/testData/modules.xml/modularJdkRoot.xml"), actual); + } } diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilderModuleScriptGenerator.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilderModuleScriptGenerator.kt index f216c680c37..658eb9ee5a1 100644 --- a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilderModuleScriptGenerator.kt +++ b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilderModuleScriptGenerator.kt @@ -18,9 +18,11 @@ package org.jetbrains.kotlin.jps.build import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.text.StringUtil +import com.intellij.openapi.vfs.StandardFileSystems import com.intellij.util.SmartList import com.intellij.util.containers.ContainerUtil import com.intellij.util.containers.MultiMap +import com.intellij.util.io.URLUtil import org.jetbrains.jps.ModuleChunk import org.jetbrains.jps.builders.java.JavaModuleBuildTargetType import org.jetbrains.jps.incremental.CompileContext @@ -28,6 +30,7 @@ import org.jetbrains.jps.incremental.ModuleBuildTarget import org.jetbrains.jps.incremental.ProjectBuildException import org.jetbrains.jps.model.java.JpsJavaExtensionService import org.jetbrains.jps.model.module.JpsModule +import org.jetbrains.jps.model.module.JpsSdkDependency import org.jetbrains.kotlin.build.JvmSourceRoot import org.jetbrains.kotlin.config.IncrementalCompilation import org.jetbrains.kotlin.jps.build.JpsUtils.getAllDependencies @@ -113,6 +116,7 @@ object KotlinBuilderModuleScriptGenerator { moduleSources, findSourceRoots(context, target), findClassPathRoots(target), + findModularJdkRoot(target), targetId.type, (targetType as JavaModuleBuildTargetType).isTests, // this excludes the output directories from the class path, to be removed for true incremental compilation @@ -162,6 +166,18 @@ object KotlinBuilderModuleScriptGenerator { } } + private fun findModularJdkRoot(target: ModuleBuildTarget): File? { + // List of paths to JRE modules in the following format: + // jrt:///Library/Java/JavaVirtualMachines/jdk-9.jdk/Contents/Home!/java.base + val urls = JpsJavaExtensionService.dependencies(target.module) + .satisfying { dependency -> dependency is JpsSdkDependency } + .classes().urls + + val url = urls.firstOrNull { it.startsWith(StandardFileSystems.JRT_PROTOCOL_PREFIX) } ?: return null + + return File(url.substringAfter(StandardFileSystems.JRT_PROTOCOL_PREFIX).substringBeforeLast(URLUtil.JAR_SEPARATOR)) + } + private fun findSourceRoots(context: CompileContext, target: ModuleBuildTarget): List { val roots = context.projectDescriptor.buildRootIndex.getTargetRoots(target, context) val result = ContainerUtil.newArrayList() diff --git a/jps-plugin/testData/general/Jre9/kotlinProject.iml b/jps-plugin/testData/general/Jre9/kotlinProject.iml new file mode 100644 index 00000000000..a0cbe548242 --- /dev/null +++ b/jps-plugin/testData/general/Jre9/kotlinProject.iml @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/jps-plugin/testData/general/Jre9/kotlinProject.ipr b/jps-plugin/testData/general/Jre9/kotlinProject.ipr new file mode 100644 index 00000000000..90747786771 --- /dev/null +++ b/jps-plugin/testData/general/Jre9/kotlinProject.ipr @@ -0,0 +1,14 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/jps-plugin/testData/general/Jre9/src/test.kt b/jps-plugin/testData/general/Jre9/src/test.kt new file mode 100644 index 00000000000..a03a648dfd4 --- /dev/null +++ b/jps-plugin/testData/general/Jre9/src/test.kt @@ -0,0 +1,14 @@ +import java.nio.file.Path +import java.util.stream.IntStream +import java.util.function.Consumer + +class Foo : Consumer { + override fun accept(path: Path) {} +} + +fun foo(s: IntStream): List { + println(s.boxed()) + Any() + if (s.count() == 0L) throw Exception() + return object : ArrayList() {} +}