Address PR comments

Author: bioball

gradle/libs.versions.toml

@@ -2,6 +2,7 @@
 antlr = "4.+"
 assertj = "3.+"
 checksumPlugin = "1.4.0"
+clangfmt = "10.0.1"
 clikt = "5.+"
 commonMark = "0.+"
 downloadTaskPlugin = "5.6.0"

libpkl/libpkl.gradle.kts

@@ -1,29 +1,27 @@
 /*
- * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+* Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+*
+*     https://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+@file:Suppress("unused")
+
 plugins {
   pklAllProjects
   pklGraalVm
   pklJavaLibrary
-  pklJavaExecutable
   pklNativeLifecycle
 }
 
-// assumes that `pklJavaExecutable` is also applied
-val executableSpec = project.extensions.getByType<ExecutableSpec>()
-
 val stagedMacAmd64NativeLibrary: Configuration by configurations.creating
 val stagedMacAarch64NativeLibrary: Configuration by configurations.creating
 val stagedLinuxAmd64NativeLibrary: Configuration by configurations.creating
@@ -57,18 +55,6 @@ dependencies {
   stagedWindowsAmd64NativeLibrary(sharedLibrary("windows-amd64.exe"))
 }
 
-executable {
-  name = "libpkl_internal"
-
-  // TODO(kushal): Why is all of this necessary now? Can it be stripped back?
-  javaName = "libpkl"
-  documentationName = "Pkl Native Library"
-  publicationName = "libpkl"
-  javaPublicationName = "libpkl"
-  mainClass = "org.pkl.libpkl.LibPkl"
-  website = "TODO"
-}
-
 private fun extension(osAndArch: String) =
   when (osAndArch.split("-").dropWhile { it == "alpine" }.first()) {
     "linux" -> "so"
@@ -85,7 +71,7 @@ private fun extension(osAndArch: String) =
 private fun nativeLibraryOutputFiles(osAndArch: String) =
   project.layout.buildDirectory.dir("libs/$osAndArch").map { outputDir ->
     // TODO(kushal): dashes/underscores for library files? C convention assumes underscores.
-    val libraryName = executableSpec.name
+    val libraryName = "libpkl_internal"
     val libraryOutputFiles =
       listOf(
         "lib${libraryName}.${extension(osAndArch)}",
@@ -125,8 +111,8 @@ private fun NativeImageBuild.setClasspath() {
 val macNativeLibraryAmd64 by
   tasks.registering(NativeImageBuild::class) {
     outputDir = project.layout.buildDirectory.dir("libs/macos-amd64")
-    imageName = executableSpec.name
-    mainClass = executableSpec.mainClass
+    imageName = "libpkl_internal"
+    mainClass = "org.pkl.libpkl.LibPkl"
     amd64()
     setClasspath()
     extraNativeImageArgs = listOf("--shared")
@@ -137,8 +123,8 @@ val macNativeLibraryAmd64 by
 val macNativeLibraryAarch64 by
   tasks.registering(NativeImageBuild::class) {
     outputDir = project.layout.buildDirectory.dir("libs/macos-aarch64")
-    imageName = executableSpec.name
-    mainClass = executableSpec.mainClass
+    imageName = "libpkl_internal"
+    mainClass = "org.pkl.libpkl.LibPkl"
     aarch64()
     setClasspath()
     extraNativeImageArgs = listOf("--shared")
@@ -149,8 +135,8 @@ val macNativeLibraryAarch64 by
 val linuxNativeLibraryAmd64 by
   tasks.registering(NativeImageBuild::class) {
     outputDir = project.layout.buildDirectory.dir("libs/linux-amd64")
-    imageName = executableSpec.name
-    mainClass = executableSpec.mainClass
+    imageName = "libpkl_internal"
+    mainClass = "org.pkl.libpkl.LibPkl"
     amd64()
     setClasspath()
     extraNativeImageArgs = listOf("--shared")
@@ -161,8 +147,8 @@ val linuxNativeLibraryAmd64 by
 val linuxNativeLibraryAarch64 by
   tasks.registering(NativeImageBuild::class) {
     outputDir = project.layout.buildDirectory.dir("libs/linux-aarch64")
-    imageName = executableSpec.name
-    mainClass = executableSpec.mainClass
+    imageName = "libpkl_internal"
+    mainClass = "org.pkl.libpkl.LibPkl"
     aarch64()
     setClasspath()
 
@@ -180,8 +166,8 @@ val linuxNativeLibraryAarch64 by
 val alpineNativeLibraryAmd64 by
   tasks.registering(NativeImageBuild::class) {
     outputDir = project.layout.buildDirectory.dir("libs/alpine-linux-amd64")
-    imageName = executableSpec.name
-    mainClass = executableSpec.mainClass
+    imageName = "libpkl_internal"
+    mainClass = "org.pkl.libpkl.LibPkl"
     amd64()
     setClasspath()
 
@@ -198,8 +184,8 @@ val alpineNativeLibraryAmd64 by
 val windowsNativeLibraryAmd64 by
   tasks.registering(NativeImageBuild::class) {
     outputDir = project.layout.buildDirectory.dir("libs/windows-amd64")
-    imageName = executableSpec.name
-    mainClass = executableSpec.mainClass
+    imageName = "libpkl_internal"
+    mainClass = "org.pkl.libpkl.LibPkl"
     amd64()
     setClasspath()
     extraNativeImageArgs = listOf("--shared", "-Dfile.encoding=UTF-8")
@@ -317,3 +303,14 @@ tasks.withType<Test> {
 
   useJUnitPlatform()
 }
+
+private val licenseHeaderFile by lazy {
+  rootProject.file("buildSrc/src/main/resources/license-header.star-block.txt")
+}
+
+spotless {
+  cpp {
+    licenseHeaderFile(licenseHeaderFile, "// ")
+    target("src/main/c/*.c", "src/main/c/*.h")
+  }
+}

libpkl/src/main/c/pkl.c

@@ -1,3 +1,20 @@
+/*
+ * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+// pkl.c
+
 #include <stdlib.h>
 #include <stdio.h>
 #include <pthread.h>
@@ -14,7 +31,7 @@
 pthread_mutex_t graal_mutex;
 graal_isolatethread_t *isolatethread = NULL;
 
-int pkl_init(PklMessageResponseHandler handler) {
+int pkl_init(PklMessageResponseHandler handler, void *payload) {
   if (isolatethread != NULL) {
     perror("pkl_init: isolatethread is already initialised");
     return -1;
@@ -30,20 +47,19 @@ int pkl_init(PklMessageResponseHandler handler) {
   }
 
   isolatethread = pkl_internal_init();
-  pkl_internal_register_response_handler(isolatethread, handler);
+  pkl_internal_register_response_handler(isolatethread, handler, payload);
   pkl_internal_server_start(isolatethread);
   pthread_mutex_unlock(&graal_mutex);
 
   return 0;
 };
 
-
-int pkl_send_message(int length, char *message, void *handlerContext) {
+int pkl_send_message(int length, char *message) {
   if (pthread_mutex_lock(&graal_mutex) != 0) {
     return -1;
   }
 
-  pkl_internal_send_message(isolatethread, length, message, handlerContext);
+  pkl_internal_send_message(isolatethread, length, message);
   pthread_mutex_unlock(&graal_mutex);
 
   return 0;

libpkl/src/main/c/pkl.h

@@ -1,30 +1,52 @@
+/*
+ * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+// pkl.h
+
 /**
-* @brief The Pkl Message Response Handler that a user should implement.
-*
-* The resulting messages from `pkl_send` will be sent to this handler using a callback style.
-*/
-typedef void (*PklMessageResponseHandler)(int length, char *message, void *handlerContext);
+ * The callback that gets called when a message is received from Pkl.
+ *
+ * @param length    The length the message bytes
+ * @param message   The message itself
+ * @param payload   User-defined data passed to pkl_init.
+ */
+typedef void (*PklMessageResponseHandler)(int length, char *message, void *payload);
 
 /**
-* @brief Initialises and allocates a Pkl executor.
-*
-* @return -1 on failure.
-* @return 0 on success.
-*/
-int pkl_init(PklMessageResponseHandler handler);
+ * Initialises and allocates a Pkl executor.
+ *
+ * @param handler   The callback that gets called when a message is received from Pkl.
+ * @param payload   User-defined data that gets passed to handler.
+ *
+ * @return -1 on failure, 0 on success.
+ */
+int pkl_init(PklMessageResponseHandler handler, void *payload);
 
 /**
-* @brief Send a message to Pkl, providing the length and a pointer to the first byte.
-*
-* @return -1 on failure.
-* @return 0 on success.
-*/
-int pkl_send_message(int length, char *message, void *handlerContext);
+ * Send a message to Pkl, providing the length and a pointer to the first byte.
+ *
+ * @param length    The length of the message, in bytes.
+ * @param message   The message to send to Pkl.
+ *
+ * @return -1 on failure, 0 on success.
+ */
+int pkl_send_message(int length, char *message);
 
 /**
-* @brief Cleans up any resources that were created as part of the `pkl_init` process.
-*
-* @return -1 on failure.
-* @return 0 on success.
-*/
+ * Cleans up any resources that were created as part of the `pkl_init` process.
+ *
+ * @return -1 on failure, 0 on success.
+ */
 int pkl_close();

libpkl/src/main/java/org/pkl/libpkl/LibPkl.java

@@ -15,7 +15,6 @@
  */
 package org.pkl.libpkl;
 
-import java.io.IOException;
 import org.graalvm.nativeimage.IsolateThread;
 import org.graalvm.nativeimage.PinnedObject;
 import org.graalvm.nativeimage.c.function.CEntryPoint;
@@ -24,42 +23,40 @@
 import org.graalvm.nativeimage.c.function.InvokeCFunctionPointer;
 import org.graalvm.nativeimage.c.type.CCharPointer;
 import org.graalvm.nativeimage.c.type.VoidPointer;
-import org.graalvm.word.WordFactory;
 import org.pkl.core.messaging.MessageTransports.Logger;
-import org.pkl.core.messaging.ProtocolException;
 import org.pkl.server.Server;
 
 @SuppressWarnings("unused")
 public class LibPkl {
   public interface MessageCallbackFunctionPointer extends CFunctionPointer {
     @InvokeCFunctionPointer(transition = Transition.TO_NATIVE)
-    void invoke(int length, CCharPointer msg, VoidPointer handlerContext);
+    void invoke(int length, CCharPointer msg, VoidPointer userData);
   }
 
   private static final Logger logger = new LibPklLogger();
   private static final NativeTransport transport =
       new NativeTransport(logger, LibPkl::handleSendMessageToNative);
   private static Server server;
   private static MessageCallbackFunctionPointer cb;
+  private static VoidPointer userData;
 
   private LibPkl() {}
 
   @CEntryPoint(name = "pkl_internal_init", builtin = CEntryPoint.Builtin.CREATE_ISOLATE)
   static native IsolateThread pklInternalInit();
 
   @CEntryPoint(name = "pkl_internal_send_message")
-  public static void pklInternalSendMessage(
-      IsolateThread thread, int length, CCharPointer ptr, VoidPointer handlerContext)
-      throws ProtocolException, IOException {
+  public static void pklInternalSendMessage(IsolateThread thread, int length, CCharPointer ptr) {
     logger.log("Got message from native");
-    transport.sendMessage(length, ptr, handlerContext);
+    transport.sendMessage(length, ptr);
   }
 
   @CEntryPoint(name = "pkl_internal_register_response_handler")
   public static void pklInternalRegisterResponseHandler(
-      IsolateThread thread, LibPkl.MessageCallbackFunctionPointer cb) {
+      IsolateThread thread, LibPkl.MessageCallbackFunctionPointer cb, VoidPointer userData) {
     logger.log("Got handler to call from Pkl");
     LibPkl.cb = cb;
+    LibPkl.userData = userData;
   }
 
   @CEntryPoint(name = "pkl_internal_close", builtin = CEntryPoint.Builtin.TEAR_DOWN_ISOLATE)
@@ -76,13 +73,10 @@ public static void pklInternalServerStop(IsolateThread thread) {
     server.close();
   }
 
-  public static void handleSendMessageToNative(byte[] bytes, Object handlerContext) {
+  public static void handleSendMessageToNative(byte[] bytes) {
     try (var pin = PinnedObject.create(bytes)) {
       // TODO: Provide a meaningful error the user if they haven't run `pkl_init`.
-      if (cb != null) {
-        // TODO: Propagate `handlerContext` through instead of `nullPointer`.
-        cb.invoke(bytes.length, pin.addressOfArrayElement(0), WordFactory.nullPointer());
-      }
+      cb.invoke(bytes.length, pin.addressOfArrayElement(0), LibPkl.userData);
     }
   }
 

libpkl/src/main/java/org/pkl/libpkl/NativeInputStream.java

@@ -23,21 +23,24 @@ public class NativeInputStream extends InputStream {
   private final int length;
   private final CCharPointer ptr;
 
+  public NativeInputStream(int length, CCharPointer ptr) {
+    super();
+    this.length = length;
+    this.ptr = ptr;
+  }
+
   @Override
   public int read() {
+    if (available() <= 0) {
+      return -1;
+    }
     var result = ptr.read(offset);
     offset++;
-    return result;
+    return result & 0xFF;
   }
 
   @Override
   public int available() {
     return length - offset;
   }
-
-  public NativeInputStream(int length, CCharPointer ptr) {
-    super();
-    this.length = length;
-    this.ptr = ptr;
-  }
 }