From 90b461aa586dbc0a9a504907fd91509cfcf67eea Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Fri, 12 Apr 2024 13:38:31 -0700 Subject: [PATCH] Support building with JDK 21 (#234) - Update google-java-format to a version compatible with JDK 21 and run "gw spotlessApply". - Fix wrong test assumption JavaCodeGenerator writes a properties file using java.util.Properties, which doesn't guarantee order of entries. - Fix most deprecation warnings - Add CI job for JDK 21 --- .circleci/config.pkl | 4 +++ .circleci/config.yml | 33 +++++++++++++++++++ .circleci/jobs/GradleCheckJob.pkl | 2 +- buildSrc/src/main/kotlin/BuildInfo.kt | 3 +- buildSrc/src/main/kotlin/GradleVersionInfo.kt | 8 ++--- gradle/libs.versions.toml | 2 +- patches/graalVm23.patch | 2 +- .../codegen/java/CliJavaCodeGeneratorTest.kt | 19 +++++------ .../pkl/core/resource/ResourceReaders.java | 3 +- .../main/java/org/pkl/core/util/IoUtils.java | 8 ++--- .../kotlin/org/pkl/core/util/HttpUtilsTest.kt | 9 +++-- .../kotlin/org/pkl/core/util/IoUtilsTest.kt | 8 ++--- .../src/dummy/java/org/pkl/tools/Empty.java | 3 +- 13 files changed, 69 insertions(+), 35 deletions(-) diff --git a/.circleci/config.pkl b/.circleci/config.pkl index 9f801cd8..10865388 100644 --- a/.circleci/config.pkl +++ b/.circleci/config.pkl @@ -148,6 +148,10 @@ local gradleCheckJobs: Mapping = new { javaVersion = "17.0" isRelease = false } + ["gradle-check-jdk21"] { + javaVersion = "21.0" + isRelease = false + } } jobs { diff --git a/.circleci/config.yml b/.circleci/config.yml index 88720e6e..762fe54b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -615,6 +615,24 @@ jobs: LANG: en_US.UTF-8 docker: - image: cimg/openjdk:17.0 + gradle-check-jdk21: + steps: + - checkout + - run: + command: ./gradlew --info --stacktrace check + name: gradle check + - run: + command: |- + mkdir ~/test-results/ + find . -type f -regex ".*/build/test-results/.*xml" -exec cp {} ~/test-results/ \; + name: Gather test results + when: always + - store_test_results: + path: ~/test-results + environment: + LANG: en_US.UTF-8 + docker: + - image: cimg/openjdk:21.0 bench: steps: - checkout @@ -765,6 +783,9 @@ workflows: - gradle-check-jdk17: requires: - hold + - gradle-check-jdk21: + requires: + - hold - check-patch-file: requires: - hold @@ -776,6 +797,7 @@ workflows: jobs: - gradle-check-jdk11 - gradle-check-jdk17 + - gradle-check-jdk21 - check-patch-file - bench - gradle-compatibility @@ -788,6 +810,7 @@ workflows: requires: - gradle-check-jdk11 - gradle-check-jdk17 + - gradle-check-jdk21 - check-patch-file - bench - gradle-compatibility @@ -820,6 +843,12 @@ workflows: ignore: /.*/ tags: only: /^v?\d+\.\d+\.\d+$/ + - gradle-check-jdk21: + filters: + branches: + ignore: /.*/ + tags: + only: /^v?\d+\.\d+\.\d+$/ - check-patch-file: filters: branches: @@ -872,6 +901,7 @@ workflows: requires: - gradle-check-jdk11 - gradle-check-jdk17 + - gradle-check-jdk21 - check-patch-file - bench - gradle-compatibility @@ -917,6 +947,9 @@ workflows: - gradle-check-jdk17: requires: - hold + - gradle-check-jdk21: + requires: + - hold - check-patch-file: requires: - hold diff --git a/.circleci/jobs/GradleCheckJob.pkl b/.circleci/jobs/GradleCheckJob.pkl index cf9b70b3..1e4937ea 100644 --- a/.circleci/jobs/GradleCheckJob.pkl +++ b/.circleci/jobs/GradleCheckJob.pkl @@ -17,7 +17,7 @@ extends "GradleJob.pkl" import "package://pkg.pkl-lang.org/pkl-pantry/com.circleci.v2@1.1.0#/Config.pkl" -javaVersion: "11.0"|"17.0" +javaVersion: "11.0"|"17.0"|"21.0" steps { new Config.RunStep { diff --git a/buildSrc/src/main/kotlin/BuildInfo.kt b/buildSrc/src/main/kotlin/BuildInfo.kt index 7edf206a..540ec79a 100644 --- a/buildSrc/src/main/kotlin/BuildInfo.kt +++ b/buildSrc/src/main/kotlin/BuildInfo.kt @@ -4,7 +4,6 @@ import java.io.File import org.gradle.api.Project import org.gradle.api.artifacts.VersionCatalog import org.gradle.api.artifacts.VersionCatalogsExtension -import org.gradle.api.artifacts.VersionConstraint import org.gradle.kotlin.dsl.getByType // `buildInfo` in main build scripts @@ -109,7 +108,7 @@ open class BuildInfo(project: Project) { // only run command once per build invocation if (project === project.rootProject) { Runtime.getRuntime() - .exec("git rev-parse --short HEAD", arrayOf(), project.rootDir) + .exec(arrayOf("git", "rev-parse", "--short", "HEAD"), arrayOf(), project.rootDir) .inputStream.reader().readText().trim() } else { project.rootProject.extensions.getByType(BuildInfo::class.java).commitId diff --git a/buildSrc/src/main/kotlin/GradleVersionInfo.kt b/buildSrc/src/main/kotlin/GradleVersionInfo.kt index 6bd1cf16..0327a7b8 100644 --- a/buildSrc/src/main/kotlin/GradleVersionInfo.kt +++ b/buildSrc/src/main/kotlin/GradleVersionInfo.kt @@ -1,6 +1,6 @@ -import java.net.URL import org.gradle.util.GradleVersion import groovy.json.JsonSlurper +import java.net.URI @Suppress("unused") class GradleVersionInfo(json: Map) { @@ -50,18 +50,18 @@ class GradleVersionInfo(json: Map) { private fun fetchSingle(url: String): GradleVersionInfo { @Suppress("UNCHECKED_CAST") - return GradleVersionInfo(JsonSlurper().parse(URL(url)) as Map) + return GradleVersionInfo(JsonSlurper().parse(URI(url).toURL()) as Map) } private fun fetchSingleOrNull(url: String): GradleVersionInfo? { @Suppress("UNCHECKED_CAST") - val json = JsonSlurper().parse(URL(url)) as Map + val json = JsonSlurper().parse(URI(url).toURL()) as Map return if (json.isEmpty()) null else GradleVersionInfo(json) } private fun fetchMultiple(url: String): List { @Suppress("UNCHECKED_CAST") - return (JsonSlurper().parse(URL(url)) as List>) + return (JsonSlurper().parse(URI(url).toURL()) as List>) .map { GradleVersionInfo(it) } } } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 76e20589..1f8436f2 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -6,7 +6,7 @@ clikt = "3.5.1" commonMark = "0.+" downloadTaskPlugin = "4.1.2" geantyref = "1.+" -googleJavaFormat = "1.15.0" +googleJavaFormat = "1.21.0" # must not use `+` because used in download URL graalVm = "22.3.3" # intentionally empty; replaced by patch file when building pkl-cli macos/aarch64 diff --git a/patches/graalVm23.patch b/patches/graalVm23.patch index fec041f0..7fca03d2 100644 --- a/patches/graalVm23.patch +++ b/patches/graalVm23.patch @@ -108,7 +108,7 @@ index f242210..e2e8ee8 100644 +++ b/gradle/libs.versions.toml @@ -8,11 +8,11 @@ downloadTaskPlugin = "4.1.2" geantyref = "1.+" - googleJavaFormat = "1.15.0" + googleJavaFormat = "1.21.0" # must not use `+` because used in download URL -graalVm = "22.3.3" -# intentionally empty; replaced by patch file when building pkl-cli macos/aarch64 diff --git a/pkl-codegen-java/src/test/kotlin/org/pkl/codegen/java/CliJavaCodeGeneratorTest.kt b/pkl-codegen-java/src/test/kotlin/org/pkl/codegen/java/CliJavaCodeGeneratorTest.kt index 2c949482..e89974fa 100644 --- a/pkl-codegen-java/src/test/kotlin/org/pkl/codegen/java/CliJavaCodeGeneratorTest.kt +++ b/pkl-codegen-java/src/test/kotlin/org/pkl/codegen/java/CliJavaCodeGeneratorTest.kt @@ -97,22 +97,21 @@ class CliJavaCodeGeneratorTest { val module1PropertiesFile = resourcesDir.resolve("org.mod1.properties") + val module1PropertiesString = module1PropertiesFile.readString() + // use two assertions because java.util.Properties doesn't guarantee order assertContains( - """ - org.pkl.config.java.mapper.org.mod1\#Person=org.Mod1${dollar}Person - org.pkl.config.java.mapper.org.mod1\#ModuleClass=org.Mod1 - """ - .trimIndent(), - module1PropertiesFile.readString() + """org.pkl.config.java.mapper.org.mod1\#Person=org.Mod1${dollar}Person""", + module1PropertiesString + ) + assertContains( + """org.pkl.config.java.mapper.org.mod1\#ModuleClass=org.Mod1""", + module1PropertiesString ) val module2PropertiesFile = resourcesDir.resolve("org.mod2.properties") assertContains( - """ - org.pkl.config.java.mapper.org.mod2\#ModuleClass=org.Mod2 - """ - .trimIndent(), + """org.pkl.config.java.mapper.org.mod2\#ModuleClass=org.Mod2""", module2PropertiesFile.readString() ) } diff --git a/pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java b/pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java index ab87648e..a33fdff0 100644 --- a/pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java +++ b/pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java @@ -304,8 +304,7 @@ public final class ResourceReaders { } try { - var url = IoUtils.toUrl(uri); - var content = IoUtils.readBytes(url); + var content = IoUtils.readBytes(uri); return Optional.of(new Resource(uri, content)); } catch (FileNotFoundException e) { return Optional.empty(); diff --git a/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java b/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java index f6e43851..44f3fafa 100644 --- a/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java +++ b/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java @@ -111,11 +111,11 @@ public final class IoUtils { return new String(inputStream.readAllBytes(), StandardCharsets.UTF_8); } - public static byte[] readBytes(URL url) throws IOException { - if (HttpUtils.isHttpUrl(url)) { - throw new IllegalArgumentException("Should use HTTP client to GET " + url); + public static byte[] readBytes(URI uri) throws IOException { + if (HttpUtils.isHttpUrl(uri)) { + throw new IllegalArgumentException("Should use HTTP client to GET " + uri); } - try (var stream = url.openStream()) { + try (var stream = IoUtils.toUrl(uri).openStream()) { return stream.readAllBytes(); } } diff --git a/pkl-core/src/test/kotlin/org/pkl/core/util/HttpUtilsTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/util/HttpUtilsTest.kt index 774e5795..705a8ca2 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/util/HttpUtilsTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/util/HttpUtilsTest.kt @@ -7,7 +7,6 @@ import org.junit.jupiter.api.assertThrows import org.pkl.commons.test.FakeHttpResponse import java.io.IOException import java.net.URI -import java.net.URL class HttpUtilsTest { @Test @@ -17,10 +16,10 @@ class HttpUtilsTest { assertThat(HttpUtils.isHttpUrl(URI("HtTpS://example.com"))).isTrue assertThat(HttpUtils.isHttpUrl(URI("file://example.com"))).isFalse - assertThat(HttpUtils.isHttpUrl(URL("http://example.com"))).isTrue - assertThat(HttpUtils.isHttpUrl(URL("https://example.com"))).isTrue - assertThat(HttpUtils.isHttpUrl(URL("HtTpS://example.com"))).isTrue - assertThat(HttpUtils.isHttpUrl(URL("file://example.com"))).isFalse + assertThat(HttpUtils.isHttpUrl(URI("http://example.com").toURL())).isTrue + assertThat(HttpUtils.isHttpUrl(URI("https://example.com").toURL())).isTrue + assertThat(HttpUtils.isHttpUrl(URI("HtTpS://example.com").toURL())).isTrue + assertThat(HttpUtils.isHttpUrl(URI("file://example.com").toURL())).isFalse } @Test diff --git a/pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt index 06787fe2..c795cc2c 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt @@ -415,20 +415,20 @@ class IoUtilsTest { @Test fun `readBytes(URL) does not support HTTP URLs`() { assertThrows { - IoUtils.readBytes(URL("https://example.com")) + IoUtils.readBytes(URI("https://example.com")) } assertThrows { - IoUtils.readBytes(URL("http://example.com")) + IoUtils.readBytes(URI("http://example.com")) } } @Test fun `readString(URL) does not support HTTP URLs`() { assertThrows { - IoUtils.readString(URL("https://example.com")) + IoUtils.readString(URI("https://example.com").toURL()) } assertThrows { - IoUtils.readString(URL("http://example.com")) + IoUtils.readString(URI("http://example.com").toURL()) } } } diff --git a/pkl-tools/src/dummy/java/org/pkl/tools/Empty.java b/pkl-tools/src/dummy/java/org/pkl/tools/Empty.java index 0e7d5a8e..6f070feb 100644 --- a/pkl-tools/src/dummy/java/org/pkl/tools/Empty.java +++ b/pkl-tools/src/dummy/java/org/pkl/tools/Empty.java @@ -20,5 +20,6 @@ package org.pkl.tools; */ // TODO: figure out how to generate javadoc for a shadow jar. @SuppressWarnings("unused") -public class Empty { +public final class Empty { + private Empty() {} }