Add support for Windows (#492)

This adds support for Windows. The in-language path separator is still `/`, to ensure Pkl programs are cross-platform. Log lines are written using CRLF endings on Windows. Modules that are combined with `--module-output-separator` uses LF endings to ensure consistent rendering across platforms. `jpkl` does not work on Windows as a direct executable. However, it can work with `java -jar jpkl`. Additional details: * Adjust git settings for Windows * Add native executable for pkl cli * Add jdk17 windows Gradle check in CI * Adjust CI test reports to be staged within Gradle rather than by shell script. * Fix: encode more characters that are not safe Windows paths * Skip running tests involving symbolic links on Windows (these require administrator privileges to run). * Introduce custom implementation of `IoUtils.relativize` * Allow Gradle to initialize ExecutableJar `Property` values * Add Gradle flag to enable remote JVM debugging Co-authored-by: Philip K.F. Hölzenspies <holzensp@gmail.com>
2026-03-24 18:11:27 +01:00 · 2024-05-28 15:56:20 -07:00
parent 5e4ccfd4e8
commit 8ec06e631f
76 changed files with 905 additions and 402 deletions
--- a/pkl-cli/src/main/kotlin/org/pkl/cli/CliEvaluator.kt
+++ b/pkl-cli/src/main/kotlin/org/pkl/cli/CliEvaluator.kt
@@ -111,7 +111,9 @@ constructor(

    return moduleUris.associateWith { uri ->
      val moduleDir: String? =
-        IoUtils.toPath(uri)?.let { workingDir.relativize(it.parent).toString().ifEmpty { "." } }
+        IoUtils.toPath(uri)?.let {
+          IoUtils.relativize(it.parent, workingDir).toString().ifEmpty { "." }
+        }
      val moduleKey =
        try {
          moduleResolver.resolve(uri)
@@ -158,7 +160,7 @@ constructor(
          } else {
            if (output.isNotEmpty()) {
              outputFile.writeString(
-                options.moduleOutputSeparator + IoUtils.getLineSeparator(),
+                options.moduleOutputSeparator + '\n',
                Charsets.UTF_8,
                StandardOpenOption.WRITE,
                StandardOpenOption.APPEND
@@ -192,6 +194,14 @@ constructor(
    if (uri == VmUtils.REPL_TEXT_URI) ModuleSource.create(uri, reader.readText())
    else ModuleSource.uri(uri)

+  private fun checkPathSpec(pathSpec: String) {
+    val illegal = pathSpec.indexOfFirst { IoUtils.isReservedFilenameChar(it) && it != '/' }
+    if (illegal == -1) {
+      return
+    }
+    throw CliException("Path spec `$pathSpec` contains illegal character `${pathSpec[illegal]}`.")
+  }
+
  /**
   * Renders each module's `output.files`, writing each entry as a file into the specified output
   * directory.
@@ -207,6 +217,7 @@ constructor(
      val moduleSource = toModuleSource(moduleUri, consoleReader)
      val output = evaluator.evaluateOutputFiles(moduleSource)
      for ((pathSpec, fileOutput) in output) {
+        checkPathSpec(pathSpec)
        val resolvedPath = outputDir.resolve(pathSpec).normalize()
        val realPath = if (resolvedPath.exists()) resolvedPath.toRealPath() else resolvedPath
        if (!realPath.startsWith(outputDir)) {
@@ -228,7 +239,10 @@ constructor(
        writtenFiles[realPath] = OutputFile(pathSpec, moduleUri)
        realPath.createParentDirectories()
        realPath.writeString(fileOutput.text)
-        consoleWriter.write(currentWorkingDir.relativize(resolvedPath).toString() + "\n")
+        consoleWriter.write(
+          IoUtils.relativize(resolvedPath, currentWorkingDir).toString() +
+            IoUtils.getLineSeparator()
+        )
        consoleWriter.flush()
      }
    }
--- a/pkl-cli/src/main/kotlin/org/pkl/cli/CliPackageDownloader.kt
+++ b/pkl-cli/src/main/kotlin/org/pkl/cli/CliPackageDownloader.kt
@@ -42,7 +42,11 @@ class CliPackageDownloader(
    }
    when (errors.size) {
      0 -> return
-      1 -> throw CliException(errors.values.single().message!!)
+      1 ->
+        throw CliException(
+          errors.values.single().message
+            ?: ("An unexpected error occurred: " + errors.values.single())
+        )
      else ->
        throw CliException(
          buildString {
--- a/pkl-cli/src/main/kotlin/org/pkl/cli/commands/TestCommand.kt
+++ b/pkl-cli/src/main/kotlin/org/pkl/cli/commands/TestCommand.kt
@@ -22,6 +22,7 @@ import com.github.ajalt.clikt.parameters.groups.provideDelegate
 import java.net.URI
 import org.pkl.cli.CliTestRunner
 import org.pkl.commons.cli.commands.BaseCommand
+import org.pkl.commons.cli.commands.BaseOptions
 import org.pkl.commons.cli.commands.ProjectOptions
 import org.pkl.commons.cli.commands.TestOptions

@@ -29,7 +30,7 @@ class TestCommand(helpLink: String) :
  BaseCommand(name = "test", help = "Run tests within the given module(s)", helpLink = helpLink) {
  val modules: List<URI> by
    argument(name = "<modules>", help = "Module paths or URIs to evaluate.")
-      .convert { parseModuleName(it) }
+      .convert { BaseOptions.parseModuleName(it) }
      .multiple()

  private val projectOptions by ProjectOptions()
--- a/pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt
+++ b/pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt
@@ -28,6 +28,9 @@ import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
+import org.junit.jupiter.api.condition.DisabledOnOs
+import org.junit.jupiter.api.condition.EnabledOnOs
+import org.junit.jupiter.api.condition.OS
 import org.junit.jupiter.api.io.TempDir
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.EnumSource
@@ -424,7 +427,10 @@ result = someLib.x
    checkOutputFile(outputFiles[0], "result.pcf", contents)
  }

+  // Can't reliably create symlinks on Windows.
+  // Might get errors like "A required privilege is not held by the client".
  @Test
+  @DisabledOnOs(OS.WINDOWS)
  fun `moduleDir is relative to workingDir even through symlinks`() {
    val contents = "foo = 42"
    val realWorkingDir = tempDir.resolve("workingDir").createDirectories()
@@ -978,6 +984,56 @@ result = someLib.x
      .hasMessageContaining("resolve to the same file path")
  }

+  @Test
+  @EnabledOnOs(OS.WINDOWS)
+  fun `multiple-file output throws when using invalid Windows characters`() {
+    val moduleUri =
+      writePklFile(
+        "test.pkl",
+        """
+      output {
+        files {
+          ["foo:bar"] { text = "bar" }
+        }
+      }
+    """
+          .trimIndent()
+      )
+
+    val options =
+      CliEvaluatorOptions(
+        CliBaseOptions(sourceModules = listOf(moduleUri), workingDir = tempDir),
+        multipleFileOutputPath = ".output"
+      )
+    assertThatCode { evalToConsole(options) }
+      .hasMessageContaining("Path spec `foo:bar` contains illegal character `:`.")
+  }
+
+  @Test
+  @EnabledOnOs(OS.WINDOWS)
+  fun `multiple-file output - cannot use backslash as dir separator on Windows`() {
+    val moduleUri =
+      writePklFile(
+        "test.pkl",
+        """
+      output {
+        files {
+          ["foo\\bar"] { text = "bar" }
+        }
+      }
+    """
+          .trimIndent()
+      )
+
+    val options =
+      CliEvaluatorOptions(
+        CliBaseOptions(sourceModules = listOf(moduleUri), workingDir = tempDir),
+        multipleFileOutputPath = ".output"
+      )
+    assertThatCode { evalToConsole(options) }
+      .hasMessageContaining("Path spec `foo\\bar` contains illegal character `\\`.")
+  }
+
  @Test
  fun `evaluate output expression`() {
    val moduleUri =
--- a/pkl-cli/src/test/kotlin/org/pkl/cli/CliMainTest.kt
+++ b/pkl-cli/src/test/kotlin/org/pkl/cli/CliMainTest.kt
@@ -24,6 +24,8 @@ import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.AssertionsForClassTypes.assertThatCode
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
+import org.junit.jupiter.api.condition.DisabledOnOs
+import org.junit.jupiter.api.condition.OS
 import org.junit.jupiter.api.io.TempDir
 import org.pkl.cli.commands.EvalCommand
 import org.pkl.cli.commands.RootCommand
@@ -54,7 +56,10 @@ class CliMainTest {
    assertThatCode { cmd.parse(arrayOf("eval")) }.hasMessage("""Missing argument "<modules>"""")
  }

+  // Can't reliably create symlinks on Windows.
+  // Might get errors like "A required privilege is not held by the client".
  @Test
+  @DisabledOnOs(OS.WINDOWS)
  fun `output to symlinked directory works`(@TempDir tempDir: Path) {
    val code =
      """
--- a/pkl-cli/src/test/kotlin/org/pkl/cli/CliProjectPackagerTest.kt
+++ b/pkl-cli/src/test/kotlin/org/pkl/cli/CliProjectPackagerTest.kt
@@ -15,6 +15,7 @@
 */
 package org.pkl.cli

+import java.io.File
 import java.io.StringWriter
 import java.net.URI
 import java.nio.file.FileSystems
@@ -27,6 +28,8 @@ import org.assertj.core.api.Assertions.assertThatCode
 import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
+import org.junit.jupiter.api.condition.DisabledOnOs
+import org.junit.jupiter.api.condition.OS
 import org.junit.jupiter.api.io.TempDir
 import org.pkl.commons.cli.CliBaseOptions
 import org.pkl.commons.cli.CliException
@@ -35,6 +38,7 @@ import org.pkl.commons.readString
 import org.pkl.commons.test.FileTestUtils
 import org.pkl.commons.test.PackageServer
 import org.pkl.commons.writeString
+import org.pkl.core.util.IoUtils

 class CliProjectPackagerTest {
  companion object {
@@ -690,7 +694,11 @@ class CliProjectPackagerTest {
      )
  }

+  // Absolute path imports on Windows must use an absolute URI (e.g. file:///C:/Foo/Bar), because
+  // they must contain drive letters, which conflict with schemes.
+  // We skip validation for absolute URIs, so effectively we skip this check on Windows.
  @Test
+  @DisabledOnOs(OS.WINDOWS)
  fun `import path verification -- absolute import from root dir`(@TempDir tempDir: Path) {
    tempDir.writeFile(
      "main.pkl",
@@ -738,6 +746,7 @@ class CliProjectPackagerTest {
  }

  @Test
+  @DisabledOnOs(OS.WINDOWS)
  fun `import path verification -- absolute read from root dir`(@TempDir tempDir: Path) {
    tempDir.writeFile(
      "main.pkl",
@@ -858,17 +867,18 @@ class CliProjectPackagerTest {
        consoleWriter = out
      )
      .run()
+    val sep = File.separatorChar
    assertThat(out.toString())
-      .isEqualTo(
+      .isEqualToNormalizingNewlines(
        """
-      .out/project1@1.0.0/project1@1.0.0.zip
-      .out/project1@1.0.0/project1@1.0.0.zip.sha256
-      .out/project1@1.0.0/project1@1.0.0
-      .out/project1@1.0.0/project1@1.0.0.sha256
-      .out/project2@2.0.0/project2@2.0.0.zip
-      .out/project2@2.0.0/project2@2.0.0.zip.sha256
-      .out/project2@2.0.0/project2@2.0.0
-      .out/project2@2.0.0/project2@2.0.0.sha256
+      .out${sep}project1@1.0.0${sep}project1@1.0.0.zip
+      .out${sep}project1@1.0.0${sep}project1@1.0.0.zip.sha256
+      .out${sep}project1@1.0.0${sep}project1@1.0.0
+      .out${sep}project1@1.0.0${sep}project1@1.0.0.sha256
+      .out${sep}project2@2.0.0${sep}project2@2.0.0.zip
+      .out${sep}project2@2.0.0${sep}project2@2.0.0.zip.sha256
+      .out${sep}project2@2.0.0${sep}project2@2.0.0
+      .out${sep}project2@2.0.0${sep}project2@2.0.0.sha256

    """
          .trimIndent()
@@ -956,13 +966,14 @@ class CliProjectPackagerTest {
        consoleWriter = out
      )
      .run()
+    val sep = File.separatorChar
    assertThat(out.toString())
-      .isEqualTo(
+      .isEqualToNormalizingNewlines(
        """
-      .out/mangos@1.0.0/mangos@1.0.0.zip
-      .out/mangos@1.0.0/mangos@1.0.0.zip.sha256
-      .out/mangos@1.0.0/mangos@1.0.0
-      .out/mangos@1.0.0/mangos@1.0.0.sha256
+      .out${sep}mangos@1.0.0${sep}mangos@1.0.0.zip
+      .out${sep}mangos@1.0.0${sep}mangos@1.0.0.zip.sha256
+      .out${sep}mangos@1.0.0${sep}mangos@1.0.0
+      .out${sep}mangos@1.0.0${sep}mangos@1.0.0.sha256

    """
          .trimIndent()
@@ -971,7 +982,7 @@ class CliProjectPackagerTest {

  private fun Path.zipFilePaths(): List<String> {
    return FileSystems.newFileSystem(URI("jar:${toUri()}"), emptyMap<String, String>()).use { fs ->
-      Files.walk(fs.getPath("/")).map { it.toString() }.collect(Collectors.toList())
+      Files.walk(fs.getPath("/")).map(IoUtils::toNormalizedPathString).collect(Collectors.toList())
    }
  }
 }
--- a/pkl-cli/src/test/kotlin/org/pkl/cli/CliProjectResolverTest.kt
+++ b/pkl-cli/src/test/kotlin/org/pkl/cli/CliProjectResolverTest.kt
@@ -15,6 +15,7 @@
 */
 package org.pkl.cli

+import java.io.File
 import java.io.StringWriter
 import java.nio.file.Path
 import org.assertj.core.api.Assertions.assertThat
@@ -26,6 +27,7 @@ import org.pkl.commons.cli.CliBaseOptions
 import org.pkl.commons.cli.CliException
 import org.pkl.commons.test.FileTestUtils
 import org.pkl.commons.test.PackageServer
+import org.pkl.core.util.IoUtils

 class CliProjectResolverTest {
  companion object {
@@ -354,7 +356,7 @@ class CliProjectResolverTest {
      )
    assertThat(errOut.toString())
      .isEqualTo(
-        "WARN: local dependency `package://localhost:0/fruit@1.0.0` was overridden to remote dependency `package://localhost:0/fruit@1.0.5`.\n"
+        "WARN: local dependency `package://localhost:0/fruit@1.0.0` was overridden to remote dependency `package://localhost:0/fruit@1.0.5`.${IoUtils.getLineSeparator()}"
      )
  }

@@ -401,11 +403,12 @@ class CliProjectResolverTest {
        errWriter = errOut
      )
      .run()
+    val sep = File.separatorChar
    assertThat(consoleOut.toString())
-      .isEqualTo(
+      .isEqualToNormalizingNewlines(
        """
-      $tempDir/project1/PklProject.deps.json
-      $tempDir/project2/PklProject.deps.json
+      $tempDir${sep}project1${sep}PklProject.deps.json
+      $tempDir${sep}project2${sep}PklProject.deps.json
    
    """
          .trimIndent()