pkl
pkl copied to clipboard
Add Gradle toolchain support
fixes #184
With that we can get rid of the java 11 check and the information around it. Also, we can "on the fly" update the jdk version the project will build on. We only need "a" java version installed to start Gradle.
As discussed in the ticket, I also had to update Gradle to version 7.6 which (seems) brings flaky tests. But we can't use toolchains before that version. So maybe this PR wil hang here until this is fixed π
Questions / Todos
-
I guess this is not going to work anymore (or has to be adjusted) π https://github.com/apple/pkl/blob/2499e2c4937fc228e49e94e41fdaac065c203168/.circleci/config.pkl#L98C7-L107
-
I guess this is not going to work anymore (or has to be adjusted) π https://github.com/apple/pkl/blob/2499e2c4937fc228e49e94e41fdaac065c203168/patches/graalVm23.patch#L25-L30
Let me know if there is more like these π
In my opinion, itβs important that the Java version/vendor can be changed in a single place. For example, I often use this to reproduce a failed CI build.
Any suggestion/recommendation how to do that? Maybe an optional gradle property? π€
Perhaps you could add pklJvmProject.gradle.kts
and apply it in the other files.
How about renaming DEFAULT_JVM_VERSION
to JDK_VERSION
? This val
is what developers will want to find/edit when troubleshooting. I'd also add val JDK_VENDOR
just in case.
I think the project property is only useful for command-line invocations, so it could be a system property.
Do your changes guarantee that source/targetCompatibility remains fixed at 11 (or soon 17) regardless of which JDK version is selected?
Do your changes guarantee that source/targetCompatibility remains fixed at 11 (or soon 17) regardless of which JDK version is selected?
Yes, that is the case π Shameless self advertisement: Read my blog about this topic π
How about renaming DEFAULT_JVM_VERSION to JDK_VERSION? This val is what developers will want to find/edit when troubleshooting.
Are you sure we wannt go with JDK
instead of JVM
? π€
The whole (Gradle) feature is named "JVM Toolchain".
Using JDK in the middle of somewhere feels a bit strange to me.
I'd also add val JDK_VENDOR just in case
We could do this, but not as a const
as only primitves are allowed to be constants.
Using this without const
looks a bit odd IMHO.
I think JDK_VERSION
is correct, but mainly I'd like to get rid of DEFAULT
.
const
is only an optimization, but a vendor val
isn't essential.
System property would be a small improvement because setting the project property in a build script will likely not work correctly.
Hmm.. unfortunately I'm unable to update the dependency lock files π€ (for the graal23.patch file) This is my change:
index 445bfc4..bdb5c2a 100644
--- a/buildSrc/src/main/kotlin/JvmToolchain.kt
+++ b/buildSrc/src/main/kotlin/JvmToolchain.kt
@@ -2,7 +2,7 @@ import org.gradle.jvm.toolchain.JavaLanguageVersion
import org.gradle.jvm.toolchain.JvmVendorSpec
import org.gradle.api.Project
-private const val JDK_VERSION = 11
+private const val JDK_VERSION = 17
val jvmToolchainVersion: JavaLanguageVersion
get() {
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
index 62fc47f..cce9276 100644
--- a/gradle/libs.versions.toml
+++ b/gradle/libs.versions.toml
@@ -8,11 +8,11 @@ downloadTaskPlugin = "4.1.2"
geantyref = "1.+"
googleJavaFormat = "1.15.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
-graalVM23JdkVersion = "replace-me"
+graalVm = "23.0.2"
+graalVM23JdkVersion = "17.0.9"
# slightly hacky but convenient place so we remember to update the checksum
graalVmSha256-darwin-amd64 = "cab6a1436626adc28ec0f72791772315678e7c758e2fbae2cb6758a38f27c56a"
+graalVmSha256-macos-aarch64 = "2214b6ecb32faacc84dffcbfae930450abe77c31730c4b6310e22d8f743959a5"
graalVmSha256-linux-amd64 = "959154e1248108dcd6eb279032cecdd2a6076bdebb345de9532681185231d255"
graalVmSha256-linux-aarch64 = "177d682f3455d00fa2491246e809d9c9b743187a82115ad3f578998b4935d5a1"
ideaExtPlugin = "1.1"
When running ./gradlew updateDependencyLocks
I will get some erros like:
* What went wrong:
Execution failed for task ':pkl-tools:updateDependencyLocks'.
> Could not resolve all files for configuration ':pkl-tools:compileClasspath'.
> Could not resolve project :pkl-codegen-java.
Required by:
project :pkl-tools
> No matching variant of project :pkl-codegen-java was found. The consumer was configured to find an API of a library compatible with Java 11, preferably in the form of class files, preferably optimized for standard JVMs, and its dependencies declared externally but:
- Variant 'apiElements' capability org.pkl-lang:pkl-codegen-java:0.26.0-SNAPSHOT declares an API of a library, packaged as a jar, preferably optimized for standard JVMs, and its dependencies declared externally:
- Incompatible because this component declares a component compatible with Java 17 and the consumer needed a component compatible with Java 11
- Variant 'javadocElements' capability org.pkl-lang:pkl-codegen-java:0.26.0-SNAPSHOT declares a runtime of a component, and its dependencies declared externally:
- Incompatible because this component declares documentation and the consumer needed a library
- Other compatible attributes:
- Doesn't say anything about its target Java environment (preferred optimized for standard JVMs)
- Doesn't say anything about its target Java version (required compatibility with Java 11)
- Doesn't say anything about its elements (required them preferably in the form of class files)
- Variant 'mainSourceElements' capability org.pkl-lang:pkl-codegen-java:0.26.0-SNAPSHOT declares a component, and its dependencies declared externally:
- Incompatible because this component declares a component of category 'verification' and the consumer needed a library
- Other compatible attributes:
- Doesn't say anything about its target Java environment (preferred optimized for standard JVMs)
- Doesn't say anything about its target Java version (required compatibility with Java 11)
- Doesn't say anything about its elements (required them preferably in the form of class files)
- Doesn't say anything about its usage (required an API)
- Variant 'runtimeElements' capability org.pkl-lang:pkl-codegen-java:0.26.0-SNAPSHOT declares a runtime of a library, packaged as a jar, preferably optimized for standard JVMs, and its dependencies declared externally:
- Incompatible because this component declares a component compatible with Java 17 and the consumer needed a component compatible with Java 11
- Variant 'sourcesElements' capability org.pkl-lang:pkl-codegen-java:0.26.0-SNAPSHOT declares a runtime of a component, and its dependencies declared externally:
- Incompatible because this component declares documentation and the consumer needed a library
- Other compatible attributes:
- Doesn't say anything about its target Java environment (preferred optimized for standard JVMs)
- Doesn't say anything about its target Java version (required compatibility with Java 11)
- Doesn't say anything about its elements (required them preferably in the form of class files)
- Variant 'testResultsElementsForTest' capability org.pkl-lang:pkl-codegen-java:0.26.0-SNAPSHOT:
- Incompatible because this component declares a component of category 'verification' and the consumer needed a library
- Other compatible attributes:
- Doesn't say anything about how its dependencies are found (required its dependencies declared externally)
- Doesn't say anything about its target Java environment (preferred optimized for standard JVMs)
- Doesn't say anything about its target Java version (required compatibility with Java 11)
- Doesn't say anything about its elements (required them preferably in the form of class files)
- Doesn't say anything about its usage (required an API)
Seems we run into a few other Gradle 7x toolchain issues π https://github.com/gradle/gradle/issues/22796 https://github.com/gradle/gradle/issues/19382
Lets wait for #245
Hey @bioball and/or @stackoverflow I might need your help here:
- For clarification. The CI checks
gradle-check-jdk11
andgradle-check-jdk17
are meant to be check if the code got compiled/works with Java 11 and Java 17, right?
If this is the case, then my changes are correct.
Because before we rely on the installed Java version on the machine.
Since Gradle handles this now, we have to put the Java version into the build script (using the system property PKL_JVM_VERSION
).
For now I left using the respective Docker images (11 and 17), however, they are not required anymore.
Gradle toolchains make sure that the correct JDK will be installed before compiling anything.
We only require an java image that can "Start" Gradle π
- Can someone help me out to fix the
graalVm23.patch
file? π
I tried various things, but non of the results look like it should (or maybe? π€· )
I tried to run updateDependencyLocks
, but this seems to update too many updates.
I tried to run buildNative classes --update-locks org.graalvm.compiler:\*,org.graalvm.sdk:\*,org.graalvm.js:\*,org.graalvm.nativeimage:\*,org.graalvm.truffle:\*
, but this seems not to update all of the dependencies even if I mentioning them π
The new patch file contains only 266 lines (instead of the orignal 530 lines).
For example, the whole stdlib/gradle.lockfile
is not included in the patch at all.
So what I'm doing wrong? How can I update it? (Maybe these changes because of the Gradle 7.6 update? π€· I don't know) This is the patchfile I end up with: https://pastebin.com/xFVt1XRa
Hey @StefMa! Just getting around to looking at this.
To fix the patch file: you can apply the patch on a file-by-file basis using git apply --reject patches/graalVm23.patch
. This will apply changes to files where there are no conflicts, and create .rej
files for those where conflicts are found.
In this case, I think what you need to do is:
-
git apply --reject patches/graalVm23.patch
- Delete all
.rej
files (we don't need those changes anymore thanks to this PR) - Create the new patch file
-
git diff > patches/graalVm23.patch
-
- Commit the new changed patch file
- Cleanup; reset the rest of the un-staged changes
The CI tests are failing on task :pkl-config-java:generateTestConfigClasses
for gradle-check-jdk17.
Cause:
Caused by: org.gradle.process.internal.ExecException: Process 'command '/usr/local/jdk-11.0.22/bin/java'' finished with non-zero exit value 1
at org.gradle.process.internal.DefaultExecHandle$ExecResultImpl.assertNormalExitValue(DefaultExecHandle.java:431)
at org.gradle.process.internal.DefaultJavaExecAction.execute(DefaultJavaExecAction.java:52)
at org.gradle.api.tasks.JavaExec.exec(JavaExec.java:165)
I tried running this locally in docker using openjdk:11
, and am seeing this:
> Task :pkl-config-java:generateTestConfigClasses FAILED
Error: LinkageError occurred while loading main class org.pkl.codegen.java.Main
java.lang.UnsupportedClassVersionError: org/pkl/codegen/java/Main has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 55.0
So, it seems like Gradle toolchains might affect built-in Java compile and run tasks, but for some reason it doesn't affect JavaExec
. This looks related: https://github.com/gradle/gradle/issues/16791
Seems like we can fix it by adding this to pklJavaLibrary.gradle.kts
:
tasks.withType<JavaExec>().configureEach {
javaLauncher.set(javaToolchains.launcherFor(java.toolchain))
}
Thanks for you comment here @bioball !
I fixed it for the :pkl-config-java:generateTestConfigClasses
tasks. See https://github.com/apple/pkl/pull/262/commits/ad5df187ab28698ccb992a8724455e8fd3f5c3dc
But now I'm running in another (same? similar?) issue for :pkl-cli:testStartJavaExecutable
:
Error: LinkageError occurred while loading main class org.pkl.cli.Main
java.lang.UnsupportedClassVersionError: org/pkl/cli/Main has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 55.0
Setting the same code in the pklKotlinLibrary.gradle.kts
doesn't change that.
Trying to setting the launcher in the custom task testJavaExecutable
and testStartJavaExecutable
(as well as changing from Exec
to JavaExec
) leads to another strange error:
Toolchain from
executable
property does not match toolchain fromjavaLauncher
property.
I tried different things but run out of ideas here π