Skip to content

pkl: Add native build using graalvm#404324

Closed
ealap wants to merge 1 commit intoNixOS:masterfrom
ealap:pkl/native
Closed

pkl: Add native build using graalvm#404324
ealap wants to merge 1 commit intoNixOS:masterfrom
ealap:pkl/native

Conversation

@ealap
Copy link
Contributor

@ealap ealap commented May 5, 2025

Continuing the work from #286658 and #402086 by adding support for building native pkl. On my machine, native build is on average twice as fast as the jpkl build.

I don't know much about building Java applications so I just pieced everything from patches in previous PRs.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@NixOSInfra NixOSInfra added the 12.first-time contribution This PR is the author's first one; please be gentle! label May 5, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 5, 2025
@nix-owners nix-owners bot requested a review from hugolgst May 5, 2025 10:30
Copy link
Contributor

@dtomvan dtomvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your first contribution, welcome to nixpkgs! I do however have some nitpicks, and then also a bug where graalvm-ce gets pulled into the derivation if nativeBuild == false.

@hugolgst
Copy link
Member

hugolgst commented May 5, 2025

aarch64-darwin

❌ 1 package failed to build:
  • pkl

@dtomvan
Copy link
Contributor

dtomvan commented May 5, 2025

Yup, same here. here are the logs for that failure

@dtomvan
Copy link
Contributor

dtomvan commented May 5, 2025

Also, I saw someone upstream package it so that the temurin requirement is not needed anymore, should we implement that in a seperate PR?

substituteInPlace buildSrc/build.gradle.kts \ 
--replace-fail "vendor = JvmVendorSpec.ADOPTIUM" "" 
substituteInPlace buildSrc/src/main/kotlin/pklJavaLibrary.gradle.kts \
--replace-fail "vendor = info.jdkVendor" "" 
substituteInPlace buildSrc/src/main/kotlin/BuildInfo.kt \
--replace-fail "vendor.set(jdkVendor)" ""

@ealap ealap force-pushed the pkl/native branch 2 times, most recently from 5f45769 to 655381a Compare May 5, 2025 17:57

This comment was marked as resolved.

@ealap
Copy link
Contributor Author

ealap commented May 5, 2025

Also, I saw someone upstream package it so that the temurin requirement is not needed anymore, should we implement that in a seperate PR?

substituteInPlace buildSrc/build.gradle.kts \ 
--replace-fail "vendor = JvmVendorSpec.ADOPTIUM" "" 
substituteInPlace buildSrc/src/main/kotlin/pklJavaLibrary.gradle.kts \
--replace-fail "vendor = info.jdkVendor" "" 
substituteInPlace buildSrc/src/main/kotlin/BuildInfo.kt \
--replace-fail "vendor.set(jdkVendor)" ""

I can confirm that this works! Could I include here on a separate commit or is it much preferred here to create separate PR?

@hugolgst
Copy link
Member

hugolgst commented May 6, 2025

I think it could be fine in this PR

@ealap
Copy link
Contributor Author

ealap commented May 8, 2025

I'm unable to build this now :(

[1/8] Initializing...                                                                                    (0.0s @ 0.15GB)
Error: Polyglot version compatibility check failed.
Your Java runtime '24.0.1+9-jvmci-b01' with native-image feature version '24.2.1' is incompatible with polyglot version '24.1.2'.
Update the org.graalvm.polyglot versions to at least '24.2.1' to resolve this.

I'm not quite sure but does this mean nixpkgs' graalvm-ce version is not compatible with pkl's lockfiles anymore? I see an update from 23 to 24 was recently merged.

cc: @bioball

@dtomvan
Copy link
Contributor

dtomvan commented May 8, 2025

I see an update from 23 to 24 was recently merged.

graalvm-ce doesn't seem to have versioned packages. I know it's unfree, but I got it building and working again by replacing graalvmPackages.graalvm-ce with graalvmPackages.graalvm-oracle_23. I don't know what would be best here, since in that PR most packages which depend on graalvm-ce seem to continue working, so keeping old ce-versions around as is done with the JDK might not be worth it.

There might also be value in using whatever GraalVM we use for the native step as the JDK we use for gradle/kotlinc, so jdk.version == graalvmDir.version? Because currently, we depend on a JDK 24 for GraalVM, but JDK 21 for Gradle.

@dtomvan
Copy link
Contributor

dtomvan commented May 8, 2025

One problem I found is that the current version of KGP they use in libs.versions.toml, 2.0.21, does not know what JDK 23 is. So using GraalVM 23 as JDK target doesn't seem possible as of now.

FWIW I can get it building with the following patch:

default-openjdk-graalvm-23.patch
diff --git i/pkgs/by-name/pk/pkl/package.nix w/pkgs/by-name/pk/pkl/package.nix
index da54a38d7fad..12d08ba2d469 100644
--- i/pkgs/by-name/pk/pkl/package.nix
+++ w/pkgs/by-name/pk/pkl/package.nix
@@ -3,18 +3,21 @@
   lib,
   fetchFromGitHub,
   gradle,
-  temurin-bin-21,
   kotlin,
   nix-update-script,
   replaceVars,
   makeWrapper,
   graalvmPackages,
+  openjdk,
   buildNative ? true,
 }:
 let
-  jdk = temurin-bin-21;
+  jdk = openjdk;
+  jdkMajorVersion = lib.versions.major jdk.version;
+  graalvmDir = graalvmPackages.graalvm-oracle_23;
   gradleOverlay = gradle.override { java = jdk; };
   kotlinOverlay = kotlin.override { jre = jdk; };
+
   binaries = {
     aarch64-darwin = "pkl-macos-aarch64";
     aarch64-linux = "pkl-linux-aarch64";
@@ -54,14 +57,24 @@ stdenv.mkDerivation (finalAttrs: {
     ++ (
       if nativeBuild then
         [
-          (replaceVars ./use_nix_graalvm_instead_of_download.patch {
-            graalvmDir = graalvmPackages.graalvm-ce;
-          })
+          (replaceVars ./use_nix_graalvm_instead_of_download.patch { inherit graalvmDir; })
         ]
       else
         [ ]
     );
 
+  postPatch = ''
+    substituteInPlace buildSrc/build.gradle.kts \
+      --replace-fail "vendor = JvmVendorSpec.ADOPTIUM" "" \
+      --replace-fail "toolchainVersion = 21" "toolchainVersion = ${jdkMajorVersion}"
+
+    substituteInPlace buildSrc/src/main/kotlin/pklJavaLibrary.gradle.kts \
+      --replace-fail "vendor = info.jdkVendor" ""
+
+    substituteInPlace buildSrc/src/main/kotlin/BuildInfo.kt \
+      --replace-fail "vendor.set(jdkVendor)" ""
+  '';
+
   nativeBuildInputs = [
     gradleOverlay
     jdk

Signed-off-by: Emmanuel Alap <15620712+ealap@users.noreply.github.com>
@dtomvan
Copy link
Contributor

dtomvan commented May 24, 2025

Any blocker for this PR? Shall I attract some committer to here via discourse?

@ealap
Copy link
Contributor Author

ealap commented May 27, 2025

We still have these build failure on darwin
https://github.com/ealap/nixpkgs-review-gha/actions/runs/15281953003/job/42982915260

which I don't understand, unfortunately.

@bioball
Copy link

bioball commented May 27, 2025

Looks like the error is happening when Truffle is trying to copy a native library onto the host machine.

Try adding these CLI flags when calling into ./gradlew:

-Dpkl.native-Dpolyglotimpl.AttachLibraryFailureAction=ignore -Dpkl.native-Dtruffle.UseFallbackRuntime=true

Might also be good to add -Dpkl.native-H:-CopyLanguageResources.

@ealap
Copy link
Contributor Author

ealap commented May 28, 2025

Looks like the error is happening when Truffle is trying to copy a native library onto the host machine.

Try adding these CLI flags when calling into ./gradlew:

-Dpkl.native-Dpolyglotimpl.AttachLibraryFailureAction=ignore -Dpkl.native-Dtruffle.UseFallbackRuntime=true

Might also be good to add -Dpkl.native-H:-CopyLanguageResources.

Thanks @bioball! Unfortunately, it still fails on the same error message. I wonder it only happens on macos.

@bioball
Copy link

bioball commented May 28, 2025

It has to do with filesystem permissions; the build is trying to copy files into /var/empty/Library:

pkl> Caused by: java.nio.file.AccessDeniedException: /var/empty/Library
pkl>    at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:90)
pkl>    at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
pkl>    at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
pkl>    at java.base/sun.nio.fs.UnixFileSystemProvider.createDirectory(UnixFileSystemProvider.java:463)
pkl>    at java.base/java.nio.file.Files.createDirectory(Files.java:700)
pkl>    at java.base/java.nio.file.Files.createAndCheckIsDirectory(Files.java:808)
pkl>    at java.base/java.nio.file.Files.createDirectories(Files.java:794)
pkl>    at org.graalvm.truffle/com.oracle.truffle.polyglot.InternalResourceCache.installResource(InternalResourceCache.java:233)
pkl>    at org.graalvm.truffle/com.oracle.truffle.polyglot.InternalResourceCache.installRuntimeResource(InternalResourceCache.java:190)
pkl>    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
pkl>    ... 23 more

Looking at this PR, I'm not sure where /var/empty is being set, but alternatively, you can also try giving it a directory that's writable.

@dtomvan dtomvan added the 9.needs: help on darwin Also consider pinging @NixOS/darwin-maintainers. label May 30, 2025
@ealap ealap closed this Jul 2, 2025
@dtomvan dtomvan mentioned this pull request Jul 25, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

9.needs: help on darwin Also consider pinging @NixOS/darwin-maintainers. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants