pkl: init at 0.28.2 by hugolgst · Pull Request #402086 · NixOS/nixpkgs

hugolgst · 2025-04-26T17:55:41Z

Since #286658 was not moving, tried initiating a new PR

Things done

Add a 👍 reaction to pull requests you find important.

dtomvan

Otherwise LGTM. (just some nitpicks and some new style changes like in d1b9886)

pkgs/by-name/pk/pkl/package.nix

dtomvan · 2025-04-27T13:07:28Z

Seems like somethings messed up with the foojay entry in the lockfile:

Relevant logs: here and here

`nixpkgs-review` result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 402086

Logs: https://github.com/dtomvan/nixpkgs-review-gha/actions/runs/14692110820

`x86_64-linux`

❌ 1 package failed to build:

pkl

`aarch64-linux`

❌ 1 package failed to build:

pkl

`x86_64-darwin`

❌ 1 package failed to build:

pkl

`aarch64-darwin`

❌ 1 package failed to build:

pkl

dtomvan · 2025-04-27T13:52:16Z

nix-shell maintainers/scripts/update.nix --argstr package pkl.mitmCache seems to fail (only relevant bit):

Cannot find a Java installation on your machine (Linux 6.12.24 amd64) matching: {languageVersion=21, vendor=Eclipse Temurin, implementation=vendor-specific}. 
Some toolchain resolvers had provisioning failures: foojay (Unable to download toolchain matching the requirements ({languageVersion=21, vendor=Eclipse Temurin, implementation=vendor-specific}) from 'https://api.foojay.io/disco/v3.0/ids/4c4f879899012ff0a8b2e2117df03b0e/redirect', due to: 
Unpacked JDK archive does not contain a Java home: /tmp/nix-shell-2-0/tmp.LjhZvKMrxU/.tmp/jdks/OpenJDK21U-jdk_x64_linux_hotspot_2115592472458233801916.tmp).

This foojay disco-api thing (found here) seems to be used to install a specific wanted version of openjdk21 automatically (I think specified here), which isn't something we want, we should probably just build with in-tree temurin-bin-21 (because they so desperately want to be built by temurin)...

The following patch to your branch makes it work I think:

use-temurin-instead.patch

From 634671fcbc9201c724f51621376cdd4b465a8bdf Mon Sep 17 00:00:00 2001
From: Tom van Dijk <18gatenmaker6@gmail.com>
Date: Sun, 27 Apr 2025 15:50:46 +0200
Subject: [PATCH] pkl: use temurin-bin-21 instead

---
 pkgs/by-name/pk/pkl/deps.json   |  7 -------
 pkgs/by-name/pk/pkl/package.nix | 10 +++++-----
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/pkgs/by-name/pk/pkl/deps.json b/pkgs/by-name/pk/pkl/deps.json
index a8c3ae6bdf85..af9adaa00978 100644
--- a/pkgs/by-name/pk/pkl/deps.json
+++ b/pkgs/by-name/pk/pkl/deps.json
@@ -1,13 +1,6 @@
 {
   "!comment": "This is a nixpkgs Gradle dependency lockfile. For more details, refer to the Gradle section in the nixpkgs manual.",
   "!version": 1,
-  "https://api.foojay.io": {
-    "disco/v3": {
-      "0/distributions": "sha256-Rp8uUFqKgDZZzFuikOVIsHVgg4+U3LLrecTM8E1E8+Q=",
-      "0/ids/73bcfb608d1fde9fb62e462f834a3299/redirect": "sha256-b8sl8/caX/JF3sTr6L1cZD8XmlzQphwI5YqMZZFNL5c=",
-      "0/packages": "sha256-AW0nLYHR4yQcJ9TIw6xdDIMUByoYUo+Vn+1YKMst0ec="
-    }
-  },
   "https://plugins.gradle.org/m2": {
     "com/github/johnrengelman#shadow/8.1.1": {
       "jar": "sha256-CEGXVVWQpTuyG1lQijMwVZ9TbdtEjq/R7GdfVGIDb88=",
diff --git a/pkgs/by-name/pk/pkl/package.nix b/pkgs/by-name/pk/pkl/package.nix
index fc7c91474c3c..2605dfc56e7c 100644
--- a/pkgs/by-name/pk/pkl/package.nix
+++ b/pkgs/by-name/pk/pkl/package.nix
@@ -3,12 +3,12 @@
   lib,
   fetchFromGitHub,
   gradle,
-  jdk21,
+  temurin-bin-21,
   kotlin,
   nix-update-script,
 }:
 let
-  jdk = jdk21;
+  jdk = temurin-bin-21;
   gradleOverlay = gradle.override { java = jdk; };
   kotlinOverlay = kotlin.override { jre = jdk; };
   commitHash = "8bd738e";
@@ -43,8 +43,8 @@ stdenv.mkDerivation (finalAttrs: {
     "-DreleaseBuild=true"
     "-Dorg.gradle.java.home=${jdk}"
     "-DcommitId=${commitHash}"
-    "-Dorg.gradle.java.toolchains.autoProvision=false"
-    "-Dorg.gradle.java.toolchains.auto-detect=false"
+    "-Porg.gradle.java.installations.auto-download=false"
+    "-Porg.gradle.java.installations.auto-detect=false"
   ];
 
   JAVA_TOOL_OPTIONS = "-Dfile.encoding=utf-8";
@@ -61,7 +61,7 @@ stdenv.mkDerivation (finalAttrs: {
   '';
 
   passthru.updateScript = nix-update-script { };
-  
+
   meta = {
     description = "A configuration as code language with rich validation and tooling.";
     homepage = "https://pkl-lang.org";
-- 
2.49.0

Built on x86_64-linux.

Note that this triggers a kotlinc and gradle rebuild, but those aren't from source so that cost almost no time. Also I manually removed that foojay.io entry, but it might re-appear after an update, so this is just a bodge.

hugolgst · 2025-04-27T14:55:39Z

Thank you @dtomvan. I had forgotten to mention it failed to build

dtomvan · 2025-04-27T15:19:08Z

#402115 just got merged into master, so nixpkgs-review currently needs to rebuild all reverse dependents of file... So I cannot test the build with nixpkgs-review-gha anymore... I guess wait until hydra built it all?

hugolgst · 2025-04-27T15:33:40Z

I guess so

hugolgst · 2025-04-27T17:19:54Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 402086

`aarch64-darwin`

✅ 1 package built:

pkl

dtomvan

Only nit I have. Happy to see pkl finally packaged 🎉

In the previous PR someone wanted a different path for "pkldoc" I think. Is that still needed?

pkgs/by-name/pk/pkl/package.nix

dtomvan · 2025-04-27T21:17:33Z

Also, it could be nice to have support for a graalvm native build for its better performance compared to jpkl. Here's how @rafaelrc7 initially did it, might come in useful here: https://github.com/rafaelrc7/nixpkgs/blob/547687c5e341c9b0a9290bb277ef6cc81d1a3554/pkgs/by-name/pk/pkl/package.nix

hugolgst · 2025-04-27T21:25:11Z

don't have that much time anymore to spend actually building on this PR. how big of an improvement are we talking here?
is that worth it? seems like a patch is needed

dtomvan · 2025-04-27T21:30:00Z

That's alright, I'll give it a try tomorrow then. Just a suggestion anyways but I think I'd be helpful to nixos users (otherwise some people might say "pkl is always just slower on nix for no reason" for example)

nyukuru

Please add the tests to the build, #286658 (comment) should still be useful.

pkgs/by-name/pk/pkl/package.nix

hugolgst · 2025-04-28T06:17:23Z

@dtomvan good point. i can have a go and will see if it takes too much time i will update you

hugolgst · 2025-04-28T09:07:50Z

Tried implementing tests but it fails and I do not have that much time left to spend on this PR. If anybody's willing to help me figure out why it fails it would be greatly appreciated
(#286658 (comment))

nix build .#pkl --keep-failed

marcusramberg · 2025-04-30T17:39:12Z

Shouldn't this be wrapping java? When I build and try to run it I get

[nix-shell:~/.cache/nixpkgs-review/pr-402086]$ ./results/pkl-aarch64-linux/bin/pkl
./results/pkl-aarch64-linux/bin/pkl: line 3: exec: java: not found

dtomvan · 2025-05-01T09:29:31Z

Something must've slipped through. I have definitely checked that the jpkl worked before. Now I get:

$ nix-build -A pkl
[...]
$ file ./result/bin/pkl
./result/bin/pkl: data

dtomvan

That file, albeit a weird format, seems to work when run with nix-shell -p temurin-bin-21 (which I accidentally must've done while reviewing), so maybe, instead of that sed command, which doesn't seems to work consistently, rather something like wrapProgram $out/bin/pkl --prefix PATH : "${lib.makeBinPath [ jdk ]}" should be used?

hugolgst · 2025-05-01T11:30:18Z

pushed your suggestion + dynamically getting the commit hash to feed to -DcommitId

pkgs/by-name/pk/pkl/package.nix

hugolgst · 2025-05-01T13:46:26Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 402086

`aarch64-darwin`

✅ 1 package built:

pkl

hugolgst · 2025-05-01T13:47:34Z

any kind souls willing to test on other platforms?

pkgs/by-name/pk/pkl/package.nix

FliegendeWurst · 2025-05-01T14:24:20Z

The wrapper seems to be broken on Linux, I'm looking into a fix right now.

dtomvan · 2025-05-01T14:32:53Z

By the way this can all be worked around when built as a native executable. That way it's also faster. The drv I made for that is here if you are interested in copying some parts of that.

hugolgst · 2025-05-01T14:34:06Z

@dtomvan didnt you say last time you had trouble running the tests with that setup?

dtomvan · 2025-05-01T14:36:01Z

You are right. We could just disable tests for the native build, but that kind-of defeats the point of all this effort to make the tests pass....

hugolgst · 2025-05-01T14:36:54Z

We'll see if @FliegendeWurst manages to fix the Linux build
Otherwise we can explore your solution

FliegendeWurst

With these changes the Linux build works

pkgs/by-name/pk/pkl/package.nix

hugolgst · 2025-05-01T14:59:04Z

builds & runs on darwin
thank you @FliegendeWurst

hugolgst · 2025-05-01T15:00:07Z

unless anybody has something else to point out we should probably move forward with this pr

FliegendeWurst

Works on Linux and Darwin

dtomvan

The following is probably redundant, but since I still have a "changes requested" next to my name, here you go:

`nixpkgs-review` result for #402086

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 402086

Logs: https://github.com/dtomvan/nixpkgs-review-gha/actions/runs/14777679232

`x86_64-linux`

✅ 1 package built:

pkl

`aarch64-linux`

✅ 1 package built:

pkl

`x86_64-darwin` (sandbox = false)

✅ 1 package built:

pkl

`aarch64-darwin` (sandbox = false)

✅ 1 package built:

pkl

hugolgst · 2025-05-01T16:42:09Z

Thanks a lot guys, really appreciate all the help. Great work

hugolgst force-pushed the pkl/0.28.2 branch from 335c724 to 4d32db6 Compare April 26, 2025 18:03

hugolgst force-pushed the pkl/0.28.2 branch from 4d32db6 to bdc156d Compare April 26, 2025 18:15

hugolgst mentioned this pull request Apr 26, 2025

Request to Add pkl (Configuration as Code Language) Package to NixOS apple/pkl#113

Open

dtomvan reviewed Apr 27, 2025

View reviewed changes

hugolgst force-pushed the pkl/0.28.2 branch from 4a54995 to 45c174a Compare April 27, 2025 12:41

hugolgst force-pushed the pkl/0.28.2 branch from 45c174a to 6080e9a Compare April 27, 2025 14:55

hugolgst marked this pull request as ready for review April 27, 2025 15:33

hugolgst self-assigned this Apr 27, 2025

hugolgst force-pushed the pkl/0.28.2 branch from 6080e9a to 06886ee Compare April 27, 2025 17:12

dtomvan reviewed Apr 27, 2025

View reviewed changes

pkgs/by-name/pk/pkl/package.nix Outdated Show resolved Hide resolved

nyukuru suggested changes Apr 28, 2025

View reviewed changes

pkgs/by-name/pk/pkl/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/pk/pkl/package.nix Outdated Show resolved Hide resolved

hugolgst requested a review from nyukuru April 28, 2025 06:46

hugolgst mentioned this pull request Apr 28, 2025

pkl: init at 0.26.2 #286658

Closed

13 tasks

hugolgst force-pushed the pkl/0.28.2 branch from 06886ee to ebffe37 Compare April 28, 2025 09:08

hugolgst requested a review from dtomvan April 28, 2025 09:08

wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Apr 30, 2025

dtomvan suggested changes May 1, 2025

View reviewed changes

hugolgst force-pushed the pkl/0.28.2 branch from 66563e1 to 6bd72d2 Compare May 1, 2025 11:29

FliegendeWurst reviewed May 1, 2025

View reviewed changes

pkgs/by-name/pk/pkl/package.nix Outdated Show resolved Hide resolved

hugolgst force-pushed the pkl/0.28.2 branch from 6bd72d2 to 50abfd0 Compare May 1, 2025 13:37

hugolgst requested a review from dtomvan May 1, 2025 13:47

FliegendeWurst reviewed May 1, 2025

View reviewed changes

pkgs/by-name/pk/pkl/package.nix Outdated Show resolved Hide resolved

hugolgst force-pushed the pkl/0.28.2 branch from 1e98ac6 to b37d2b4 Compare May 1, 2025 14:27

FliegendeWurst reviewed May 1, 2025

View reviewed changes

pkgs/by-name/pk/pkl/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/pk/pkl/package.nix Outdated Show resolved Hide resolved

pkl: init at 0.28.2

90da7eb

hugolgst force-pushed the pkl/0.28.2 branch from bbcfdf0 to 90da7eb Compare May 1, 2025 14:56

hugolgst requested a review from FliegendeWurst May 1, 2025 14:59

FliegendeWurst approved these changes May 1, 2025

View reviewed changes

dtomvan approved these changes May 1, 2025

View reviewed changes

FliegendeWurst merged commit 383b556 into NixOS:master May 1, 2025
25 of 27 checks passed

hugolgst mentioned this pull request May 1, 2025

Package request: pkl #286104

Closed

ealap mentioned this pull request May 5, 2025

pkl: Add native build using graalvm #404324

Closed

13 tasks

Uh oh!

Conversation

hugolgst commented Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

dtomvan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dtomvan commented Apr 27, 2025

nixpkgs-review result

x86_64-linux

aarch64-linux

x86_64-darwin

aarch64-darwin

Uh oh!

dtomvan commented Apr 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hugolgst commented Apr 27, 2025

Uh oh!

dtomvan commented Apr 27, 2025

Uh oh!

hugolgst commented Apr 27, 2025

Uh oh!

hugolgst commented Apr 27, 2025

nixpkgs-review result

aarch64-darwin

Uh oh!

dtomvan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dtomvan commented Apr 27, 2025

Uh oh!

hugolgst commented Apr 27, 2025

Uh oh!

dtomvan commented Apr 27, 2025

Uh oh!

nyukuru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hugolgst commented Apr 28, 2025

Uh oh!

hugolgst commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcusramberg commented Apr 30, 2025

Uh oh!

dtomvan commented May 1, 2025

Uh oh!

dtomvan left a comment

Choose a reason for hiding this comment

Uh oh!

hugolgst commented May 1, 2025

Uh oh!

Uh oh!

hugolgst commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

nixpkgs-review result

aarch64-darwin

Uh oh!

hugolgst commented May 1, 2025

Uh oh!

Uh oh!

FliegendeWurst commented May 1, 2025

Uh oh!

dtomvan commented May 1, 2025

Uh oh!

hugolgst commented May 1, 2025

hugolgst commented Apr 26, 2025 •

edited

Loading

dtomvan left a comment •

edited

Loading

`nixpkgs-review` result

`x86_64-linux`

`aarch64-linux`

`x86_64-darwin`

`aarch64-darwin`

dtomvan commented Apr 27, 2025 •

edited

Loading

`nixpkgs-review` result

`aarch64-darwin`

hugolgst commented Apr 28, 2025 •

edited

Loading

hugolgst commented May 1, 2025 •

edited

Loading

`nixpkgs-review` result

`aarch64-darwin`

`nixpkgs-review` result for #402086

`x86_64-linux`

`aarch64-linux`

`x86_64-darwin` (sandbox = false)

`aarch64-darwin` (sandbox = false)