From f25e11cefa3d3ad812700974f124072b75717538 Mon Sep 17 00:00:00 2001 From: Dan Chao Date: Tue, 6 Jan 2026 15:42:19 -0800 Subject: [PATCH 1/5] Change loading of external readers This introduces breaking changes for external readers are loaded: 1. In PklProject, relative paths are resolved relative to the enclosing PklProject file (make behavior consistent with how other settings work) 2. Prevent external readers from being set for non-file based PklProject 3. Make CLI flags blow away any settings set on a PklProject The overall goal is to make this behavior consistent with how other settings work. For example, relative paths for other evaluator settings are already relative to the project directory. Additionally, in every other case, CLI flags will overwrite any setting set within PklProject. --- docs/modules/release-notes/pages/0.31.adoc | 10 ++- .../org/pkl/commons/cli/CliBaseOptions.kt | 6 +- .../kotlin/org/pkl/commons/cli/CliCommand.kt | 14 ++-- .../pkl/commons/cli/commands/BaseOptions.kt | 6 +- .../org/pkl/commons/cli/CliCommandTest.kt | 35 +++++++++ .../PklEvaluatorSettings.java | 18 +++-- .../org/pkl/core/project/ProjectTest.kt | 73 ++++++++++++++++++- stdlib/EvaluatorSettings.pkl | 2 +- stdlib/Project.pkl | 4 + 9 files changed, 147 insertions(+), 21 deletions(-) diff --git a/docs/modules/release-notes/pages/0.31.adoc b/docs/modules/release-notes/pages/0.31.adoc index cb62260de..64ce325f3 100644 --- a/docs/modules/release-notes/pages/0.31.adoc +++ b/docs/modules/release-notes/pages/0.31.adoc @@ -46,7 +46,15 @@ The following APIs have been added: Things to watch out for when upgrading. -=== XXX +=== Rule changes when loading external readers + +The following changes have been made when loading external readers: + +1. Executable paths declared inside a PklProject file are resolved relative to the enclosing directory, instead of the current working directory. +2. In a PklProject, the `externalModuleReaders` and `externalResourceReaders` properties of `evaluatorSettings` cannot be set in a non-file project. +3. The `--external-module-reader` and `--external-resource-reader` CLI flags will _replace_ any external readers otherwise configured within a PklProject, instead of add to it. + ++ +This makes this behavior consistent with how other settings work. == Miscellaneous [small]#🐸# diff --git a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliBaseOptions.kt b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliBaseOptions.kt index 0c06a8263..8306a167f 100644 --- a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliBaseOptions.kt +++ b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliBaseOptions.kt @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -145,10 +145,10 @@ data class CliBaseOptions( val httpRewrites: Map? = null, /** External module reader process specs */ - val externalModuleReaders: Map = mapOf(), + val externalModuleReaders: Map? = null, /** External resource reader process specs */ - val externalResourceReaders: Map = mapOf(), + val externalResourceReaders: Map? = null, /** Defines options for the formatting of calls to the trace() method. */ val traceMode: TraceMode? = null, diff --git a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt index fa694b1d7..f9f7e4e3c 100644 --- a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt +++ b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -186,14 +186,14 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) { ?: settings.http?.rewrites() } - private val externalModuleReaders: Map by lazy { - (project?.evaluatorSettings?.externalModuleReaders ?: emptyMap()) + - cliOptions.externalModuleReaders + protected val externalModuleReaders: Map by lazy { + cliOptions.externalModuleReaders ?: project?.evaluatorSettings?.externalModuleReaders ?: mapOf() } - private val externalResourceReaders: Map by lazy { - (project?.evaluatorSettings?.externalResourceReaders ?: emptyMap()) + - cliOptions.externalResourceReaders + protected val externalResourceReaders: Map by lazy { + cliOptions.externalResourceReaders + ?: project?.evaluatorSettings?.externalResourceReaders + ?: mapOf() } private val externalProcesses: diff --git a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt index cd7353258..3fe595d19 100644 --- a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt +++ b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -340,8 +340,8 @@ class BaseOptions : OptionGroup() { httpProxy = proxy, httpNoProxy = noProxy, httpRewrites = httpRewrites.ifEmpty { null }, - externalModuleReaders = externalModuleReaders, - externalResourceReaders = externalResourceReaders, + externalModuleReaders = externalModuleReaders.ifEmpty { null }, + externalResourceReaders = externalResourceReaders.ifEmpty { null }, traceMode = traceMode, ) } diff --git a/pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/CliCommandTest.kt b/pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/CliCommandTest.kt index d5a2eea57..33afbfea4 100644 --- a/pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/CliCommandTest.kt +++ b/pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/CliCommandTest.kt @@ -16,10 +16,15 @@ package org.pkl.commons.cli import com.github.ajalt.clikt.core.parse +import java.nio.file.Path +import kotlin.collections.mapOf import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir import org.pkl.commons.cli.commands.BaseCommand +import org.pkl.commons.writeString import org.pkl.core.SecurityManagers +import org.pkl.core.evaluatorSettings.PklEvaluatorSettings class CliCommandTest { @@ -28,6 +33,8 @@ class CliCommandTest { val myAllowedResources = allowedResources val myAllowedModules = allowedModules + val myExternalModuleReaders = externalModuleReaders + val myExternalResourceReaders = externalResourceReaders } private val cmd = @@ -68,4 +75,32 @@ class CliCommandTest { listOf("\\Qscheme1:\\E", "\\Qscheme2:\\E", "\\Qscheme+ext:\\E") ) } + + @Test + fun `--external-module-reader blows away PklProject externalModuleReaders`( + @TempDir tempDir: Path + ) { + tempDir + .resolve("PklProject") + .writeString( + // language=pkl + """ + amends "pkl:Project" + + evaluatorSettings { + externalModuleReaders { + ["foo"] { + executable = "foo" + } + } + } + """ + .trimIndent() + ) + cmd.parse(arrayOf("--external-module-reader", "bar=bar")) + val opts = cmd.baseOptions.baseOptions(emptyList(), null, true) + val cliTest = CliTest(opts) + assertThat(cliTest.myExternalModuleReaders) + .isEqualTo(mapOf("bar" to PklEvaluatorSettings.ExternalReader("bar", listOf()))) + } } diff --git a/pkl-core/src/main/java/org/pkl/core/evaluatorSettings/PklEvaluatorSettings.java b/pkl-core/src/main/java/org/pkl/core/evaluatorSettings/PklEvaluatorSettings.java index bcb196a91..6cc62ea2e 100644 --- a/pkl-core/src/main/java/org/pkl/core/evaluatorSettings/PklEvaluatorSettings.java +++ b/pkl-core/src/main/java/org/pkl/core/evaluatorSettings/PklEvaluatorSettings.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -17,6 +17,7 @@ import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; import java.util.HashMap; @@ -56,7 +57,7 @@ public record PklEvaluatorSettings( /** Initializes a {@link PklEvaluatorSettings} from a raw object representation. */ @SuppressWarnings("unchecked") public static PklEvaluatorSettings parse( - Value input, BiFunction pathNormalizer) { + Value input, BiFunction pathNormalizer) { if (!(input instanceof PObject pSettings)) { throw PklBugException.unreachableCode(); } @@ -95,7 +96,8 @@ public static PklEvaluatorSettings parse( : externalModuleReadersRaw.entrySet().stream() .collect( Collectors.toMap( - Entry::getKey, entry -> ExternalReader.parse(entry.getValue()))); + Entry::getKey, + entry -> ExternalReader.parse(entry.getValue(), pathNormalizer))); var externalResourceReadersRaw = (Map) pSettings.get("externalResourceReaders"); var externalResourceReaders = @@ -104,7 +106,8 @@ public static PklEvaluatorSettings parse( : externalResourceReadersRaw.entrySet().stream() .collect( Collectors.toMap( - Entry::getKey, entry -> ExternalReader.parse(entry.getValue()))); + Entry::getKey, + entry -> ExternalReader.parse(entry.getValue(), pathNormalizer))); var color = (String) pSettings.get("color"); var traceMode = (String) pSettings.get("traceMode"); @@ -190,10 +193,15 @@ public static Proxy create(@Nullable String address, @Nullable List noPr public record ExternalReader(String executable, @Nullable List arguments) { @SuppressWarnings("unchecked") - public static ExternalReader parse(Value input) { + public static ExternalReader parse( + Value input, BiFunction pathNormalizer) { if (input instanceof PObject externalReader) { var executable = (String) externalReader.getProperty("executable"); + var executablePath = pathNormalizer.apply(executable, "executable"); var arguments = (List) externalReader.get("arguments"); + if (Files.exists(executablePath)) { + return new ExternalReader(executablePath.toString(), arguments); + } return new ExternalReader(executable, arguments); } throw PklBugException.unreachableCode(); diff --git a/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt index b228ff61a..8d3b7e490 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -16,8 +16,12 @@ package org.pkl.core.project import java.net.URI +import java.nio.file.Files import java.nio.file.Path +import java.nio.file.attribute.PosixFilePermission import java.util.regex.Pattern +import kotlin.io.path.createDirectories +import kotlin.io.path.createFile import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatCode import org.junit.jupiter.api.Test @@ -31,6 +35,7 @@ import org.pkl.core.* import org.pkl.core.evaluatorSettings.PklEvaluatorSettings import org.pkl.core.http.HttpClient import org.pkl.core.packages.PackageUri +import org.pkl.core.util.IoUtils class ProjectTest { @Test @@ -261,4 +266,70 @@ class ProjectTest { .trimIndent() ) } + + @Test + fun `external readers -- executable path is relative to project dir`(@TempDir tempDir: Path) { + val projectDir = tempDir.resolve("project").also { it.createDirectories() } + val pklProject = + projectDir.resolve("PklProject").also { + it.writeString( + // language=pkl + """ + amends "pkl:Project" + + evaluatorSettings { + externalModuleReaders { + ["foo"] { + executable = "foo" + } + } + } + """ + .trimIndent() + ) + } + val executableName = if (IoUtils.isWindows()) "foo.cmd" else "foo" + val executable = + projectDir.resolve(executableName).also { + it.createFile() + if (!IoUtils.isWindows()) { + Files.setPosixFilePermissions(it, setOf(PosixFilePermission.OWNER_EXECUTE)) + } + } + val project = Project.loadFromPath(pklProject, SecurityManagers.defaultManager, null) + assertThat(project.evaluatorSettings.externalModuleReaders).hasSize(1) + assertThat(project.evaluatorSettings.externalModuleReaders?.get("foo")!!.executable()) + .isEqualTo(executable.toAbsolutePath().toString()) + } + + @Test + fun `external readers -- executable is unmodified if not resolvable relative to project dir`( + @TempDir tempDir: Path + ) { + // echo exists in POSIX and also Windows (both command prompt and PowerShell) + val projectDir = tempDir.resolve("project").also { it.createDirectories() } + val pklProject = + projectDir.resolve("PklProject").also { + it.writeString( + // language=pkl + """ + amends "pkl:Project" + + evaluatorSettings { + externalModuleReaders { + ["foo"] { + executable = "echo" + } + } + } + """ + .trimIndent() + ) + } + + val project = Project.loadFromPath(pklProject, SecurityManagers.defaultManager, null) + assertThat(project.evaluatorSettings.externalModuleReaders).hasSize(1) + assertThat(project.evaluatorSettings.externalModuleReaders?.get("foo")!!.executable()) + .isEqualTo("echo") + } } diff --git a/stdlib/EvaluatorSettings.pkl b/stdlib/EvaluatorSettings.pkl index a27fe3599..56c0ccfe2 100644 --- a/stdlib/EvaluatorSettings.pkl +++ b/stdlib/EvaluatorSettings.pkl @@ -228,7 +228,7 @@ class ExternalReader { /// On macOS, Linux, and Windows platforms, this may be: /// /// * An absolute path - /// * A relative path (to the current working directory) + /// * A relative path /// * The name of the executable, to be resolved against the `PATH` environment variable executable: String diff --git a/stdlib/Project.pkl b/stdlib/Project.pkl index d9e9b7fe1..2e3fc060b 100644 --- a/stdlib/Project.pkl +++ b/stdlib/Project.pkl @@ -196,12 +196,16 @@ local isFileBasedProject = projectFileUri.startsWith("file:") /// - [modulePath][EvaluatorSettings.modulePath] /// - [rootDir][EvaluatorSettings.rootDir] /// - [moduleCacheDir][EvaluatorSettings.moduleCacheDir] +/// - [externalModuleReaders][EvaluatorSettings.externalModuleReaders] +/// - [externalResourceReaders][EvaluatorSettings.externalResourceReaders] /// /// For each of these, relative paths are resolved against the project's enclosing directory. evaluatorSettings: EvaluatorSettingsModule( (modulePath != null).implies(isFileBasedProject), (rootDir != null).implies(isFileBasedProject), (moduleCacheDir != null).implies(isFileBasedProject), + (externalModuleReaders != null).implies(isFileBasedProject), + (externalResourceReaders != null).implies(isFileBasedProject), ) /// The URI of the PklProject file. From d63619c70590ebe91a328eb53f08a26f47d01614 Mon Sep 17 00:00:00 2001 From: Dan Chao Date: Wed, 7 Jan 2026 16:33:08 -0800 Subject: [PATCH 2/5] Resolve relative paths against project dir, simple name against PWD --- docs/modules/release-notes/pages/0.31.adoc | 3 +- .../kotlin/org/pkl/commons/cli/CliCommand.kt | 20 +-- .../java/org/pkl/core/EvaluatorBuilder.java | 4 +- .../PklEvaluatorSettings.java | 30 +---- .../ExternalReaderProcessImpl.java | 22 +++- .../java/org/pkl/core/project/Project.java | 45 +++---- .../main/java/org/pkl/core/util/IoUtils.java | 21 ++- .../org/pkl/core/project/ProjectTest.kt | 55 ++++---- stdlib/EvaluatorSettings.pkl | 124 +++++++++++++++++- stdlib/Project.pkl | 89 +++++++++++-- 10 files changed, 314 insertions(+), 99 deletions(-) diff --git a/docs/modules/release-notes/pages/0.31.adoc b/docs/modules/release-notes/pages/0.31.adoc index 64ce325f3..104baf749 100644 --- a/docs/modules/release-notes/pages/0.31.adoc +++ b/docs/modules/release-notes/pages/0.31.adoc @@ -51,8 +51,7 @@ Things to watch out for when upgrading. The following changes have been made when loading external readers: 1. Executable paths declared inside a PklProject file are resolved relative to the enclosing directory, instead of the current working directory. -2. In a PklProject, the `externalModuleReaders` and `externalResourceReaders` properties of `evaluatorSettings` cannot be set in a non-file project. -3. The `--external-module-reader` and `--external-resource-reader` CLI flags will _replace_ any external readers otherwise configured within a PklProject, instead of add to it. + +2. The `--external-module-reader` and `--external-resource-reader` CLI flags will _replace_ any external readers otherwise configured within a PklProject, instead of add to it. + + This makes this behavior consistent with how other settings work. diff --git a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt index f9f7e4e3c..9a4f30588 100644 --- a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt +++ b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt @@ -106,7 +106,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) { } private val evaluatorSettings: PklEvaluatorSettings? by lazy { - if (cliOptions.omitProjectSettings) null else project?.evaluatorSettings + if (cliOptions.omitProjectSettings) null else project?.resolvedEvaluatorSettings } protected val allowedModules: List by lazy { @@ -169,31 +169,25 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) { protected val useColor: Boolean by lazy { cliOptions.color?.hasColor() ?: false } private val proxyAddress: URI? by lazy { - cliOptions.httpProxy - ?: project?.evaluatorSettings?.http?.proxy?.address - ?: settings.http?.proxy?.address + cliOptions.httpProxy ?: evaluatorSettings?.http?.proxy?.address ?: settings.http?.proxy?.address } private val noProxy: List? by lazy { cliOptions.httpNoProxy - ?: project?.evaluatorSettings?.http?.proxy?.noProxy + ?: evaluatorSettings?.http?.proxy?.noProxy ?: settings.http?.proxy?.noProxy } private val httpRewrites: Map? by lazy { - cliOptions.httpRewrites - ?: project?.evaluatorSettings?.http?.rewrites - ?: settings.http?.rewrites() + cliOptions.httpRewrites ?: evaluatorSettings?.http?.rewrites ?: settings.http?.rewrites() } protected val externalModuleReaders: Map by lazy { - cliOptions.externalModuleReaders ?: project?.evaluatorSettings?.externalModuleReaders ?: mapOf() + cliOptions.externalModuleReaders ?: evaluatorSettings?.externalModuleReaders ?: mapOf() } protected val externalResourceReaders: Map by lazy { - cliOptions.externalResourceReaders - ?: project?.evaluatorSettings?.externalResourceReaders - ?: mapOf() + cliOptions.externalResourceReaders ?: evaluatorSettings?.externalResourceReaders ?: mapOf() } private val externalProcesses: @@ -207,7 +201,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) { } private val traceMode: TraceMode by lazy { - cliOptions.traceMode ?: project?.evaluatorSettings?.traceMode ?: TraceMode.COMPACT + cliOptions.traceMode ?: evaluatorSettings?.traceMode ?: TraceMode.COMPACT } private fun HttpClient.Builder.addDefaultCliCertificates() { diff --git a/pkl-core/src/main/java/org/pkl/core/EvaluatorBuilder.java b/pkl-core/src/main/java/org/pkl/core/EvaluatorBuilder.java index efc284f80..e338deec7 100644 --- a/pkl-core/src/main/java/org/pkl/core/EvaluatorBuilder.java +++ b/pkl-core/src/main/java/org/pkl/core/EvaluatorBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -475,7 +475,7 @@ public TraceMode getTraceMode() { */ public EvaluatorBuilder applyFromProject(Project project) { this.dependencies = project.getDependencies(); - var settings = project.getEvaluatorSettings(); + var settings = project.getResolvedEvaluatorSettings(); if (securityManager != null) { throw new IllegalStateException( "Cannot call both `setSecurityManager` and `setProject`, because both define security manager settings. Call `setProjectOnly` if the security manager is desired."); diff --git a/pkl-core/src/main/java/org/pkl/core/evaluatorSettings/PklEvaluatorSettings.java b/pkl-core/src/main/java/org/pkl/core/evaluatorSettings/PklEvaluatorSettings.java index 6cc62ea2e..17d519481 100644 --- a/pkl-core/src/main/java/org/pkl/core/evaluatorSettings/PklEvaluatorSettings.java +++ b/pkl-core/src/main/java/org/pkl/core/evaluatorSettings/PklEvaluatorSettings.java @@ -17,7 +17,6 @@ import java.net.URI; import java.net.URISyntaxException; -import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; import java.util.HashMap; @@ -25,7 +24,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; -import java.util.function.BiFunction; import java.util.regex.Pattern; import java.util.stream.Collectors; import org.pkl.core.Duration; @@ -56,17 +54,13 @@ public record PklEvaluatorSettings( /** Initializes a {@link PklEvaluatorSettings} from a raw object representation. */ @SuppressWarnings("unchecked") - public static PklEvaluatorSettings parse( - Value input, BiFunction pathNormalizer) { + public static PklEvaluatorSettings parse(Value input) { if (!(input instanceof PObject pSettings)) { throw PklBugException.unreachableCode(); } var moduleCacheDirStr = (String) pSettings.get("moduleCacheDir"); - var moduleCacheDir = - moduleCacheDirStr == null - ? null - : pathNormalizer.apply(moduleCacheDirStr, "moduleCacheDir"); + var moduleCacheDir = moduleCacheDirStr == null ? null : Path.of(moduleCacheDirStr).normalize(); var allowedModulesStrs = (List) pSettings.get("allowedModules"); var allowedModules = @@ -81,13 +75,10 @@ public static PklEvaluatorSettings parse( : allowedResourcesStrs.stream().map(Pattern::compile).toList(); var modulePathStrs = (List) pSettings.get("modulePath"); - var modulePath = - modulePathStrs == null - ? null - : modulePathStrs.stream().map(it -> pathNormalizer.apply(it, "modulePath")).toList(); + var modulePath = modulePathStrs == null ? null : modulePathStrs.stream().map(Path::of).toList(); var rootDirStr = (String) pSettings.get("rootDir"); - var rootDir = rootDirStr == null ? null : pathNormalizer.apply(rootDirStr, "rootDir"); + var rootDir = rootDirStr == null ? null : Path.of(rootDirStr).normalize(); var externalModuleReadersRaw = (Map) pSettings.get("externalModuleReaders"); var externalModuleReaders = @@ -96,8 +87,7 @@ public static PklEvaluatorSettings parse( : externalModuleReadersRaw.entrySet().stream() .collect( Collectors.toMap( - Entry::getKey, - entry -> ExternalReader.parse(entry.getValue(), pathNormalizer))); + Entry::getKey, entry -> ExternalReader.parse(entry.getValue()))); var externalResourceReadersRaw = (Map) pSettings.get("externalResourceReaders"); var externalResourceReaders = @@ -106,8 +96,7 @@ public static PklEvaluatorSettings parse( : externalResourceReadersRaw.entrySet().stream() .collect( Collectors.toMap( - Entry::getKey, - entry -> ExternalReader.parse(entry.getValue(), pathNormalizer))); + Entry::getKey, entry -> ExternalReader.parse(entry.getValue()))); var color = (String) pSettings.get("color"); var traceMode = (String) pSettings.get("traceMode"); @@ -193,15 +182,10 @@ public static Proxy create(@Nullable String address, @Nullable List noPr public record ExternalReader(String executable, @Nullable List arguments) { @SuppressWarnings("unchecked") - public static ExternalReader parse( - Value input, BiFunction pathNormalizer) { + public static ExternalReader parse(Value input) { if (input instanceof PObject externalReader) { var executable = (String) externalReader.getProperty("executable"); - var executablePath = pathNormalizer.apply(executable, "executable"); var arguments = (List) externalReader.get("arguments"); - if (Files.exists(executablePath)) { - return new ExternalReader(executablePath.toString(), arguments); - } return new ExternalReader(executable, arguments); } throw PklBugException.unreachableCode(); diff --git a/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java b/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java index de35f6b06..6f9f530ab 100644 --- a/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java +++ b/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -33,6 +33,7 @@ import org.pkl.core.messaging.MessageTransports; import org.pkl.core.messaging.ProtocolException; import org.pkl.core.util.ErrorMessages; +import org.pkl.core.util.IoUtils; import org.pkl.core.util.LateInit; import org.pkl.core.util.Nullable; @@ -85,6 +86,17 @@ public ExternalResourceResolver getResourceResolver(long evaluatorId) return ExternalResourceResolver.of(getTransport(), evaluatorId); } + private @Nullable String getExecutablePath(String executable) { + if (executable.contains("/") || executable.contains("\"")) { + return executable; + } + var resolved = IoUtils.findExecutableOnPath(executable); + if (resolved != null) { + return resolved.toAbsolutePath().toString(); + } + return null; + } + private MessageTransport getTransport() throws ExternalReaderProcessException { synchronized (lock) { if (closed) { @@ -100,9 +112,13 @@ private MessageTransport getTransport() throws ExternalReaderProcessException { } } - // This relies on Java/OS behavior around PATH resolution, absolute/relative paths, etc. var command = new ArrayList(); - command.add(spec.executable()); + var executable = getExecutablePath(spec.executable()); + if (executable == null) { + throw new ExternalReaderProcessException( + ErrorMessages.create("cannotFindCommand", spec.executable())); + } + command.add(executable); if (spec.arguments() != null) { command.addAll(spec.arguments()); } diff --git a/pkl-core/src/main/java/org/pkl/core/project/Project.java b/pkl-core/src/main/java/org/pkl/core/project/Project.java index 5757b4398..3d43c5446 100644 --- a/pkl-core/src/main/java/org/pkl/core/project/Project.java +++ b/pkl-core/src/main/java/org/pkl/core/project/Project.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -62,6 +62,7 @@ public final class Project { private final @Nullable Package pkg; private final DeclaredDependencies dependencies; private final PklEvaluatorSettings evaluatorSettings; + private final PklEvaluatorSettings resolvedEvaluatorSettings; private final URI projectFileUri; private final URI projectBaseUri; private final List tests; @@ -278,9 +279,14 @@ public static Project parseProject(PObject module) throws URISyntaxException { getProperty( module, "evaluatorSettings", - (settings) -> - PklEvaluatorSettings.parse( - (Value) settings, (it, name) -> resolveNullablePath(it, projectBaseUri, name))); + (settings) -> PklEvaluatorSettings.parse((Value) settings)); + + var resolvedEvaluatorSettings = + getProperty( + module, + "resolvedEvaluatorSettings", + (settings) -> PklEvaluatorSettings.parse((Value) settings)); + @SuppressWarnings("unchecked") var testPathStrs = (List) getProperty(module, "tests"); var tests = @@ -293,6 +299,7 @@ public static Project parseProject(PObject module) throws URISyntaxException { pkg, dependencies, evaluatorSettings, + resolvedEvaluatorSettings, projectFileUri, projectBaseUri, tests, @@ -348,24 +355,6 @@ private static T getProperty(PObject settings, String propertyName, Function return new URI((String) value); } - /** - * Resolve a path string against projectBaseUri. - * - * @throws PackageLoadError if projectBaseUri is not a {@code file:} URI. - */ - private static @Nullable Path resolveNullablePath( - @Nullable String path, URI projectBaseUri, String propertyName) { - if (path == null) { - return null; - } - try { - return Path.of(projectBaseUri).resolve(path).normalize(); - } catch (FileSystemNotFoundException e) { - throw new PackageLoadError( - "relativePathPropertyDefinedByProjectFromNonFileUri", projectBaseUri, propertyName); - } - } - @SuppressWarnings("unchecked") private static Package parsePackage(PObject pObj) throws URISyntaxException { var name = (String) pObj.getProperty("name"); @@ -407,6 +396,7 @@ private Project( @Nullable Package pkg, DeclaredDependencies dependencies, PklEvaluatorSettings evaluatorSettings, + PklEvaluatorSettings resolvedEvaluatorSettings, URI projectFileUri, URI projectBaseUri, List tests, @@ -415,6 +405,7 @@ private Project( this.pkg = pkg; this.dependencies = dependencies; this.evaluatorSettings = evaluatorSettings; + this.resolvedEvaluatorSettings = resolvedEvaluatorSettings; this.projectFileUri = projectFileUri; this.projectBaseUri = projectBaseUri; this.tests = tests; @@ -436,6 +427,15 @@ public PklEvaluatorSettings getEvaluatorSettings() { return evaluatorSettings; } + /** + * The evaluator settings whose paths have been resolved against the project dir. + * + * @since 0.31.0 + */ + public PklEvaluatorSettings getResolvedEvaluatorSettings() { + return resolvedEvaluatorSettings; + } + public URI getProjectFileUri() { return projectFileUri; } @@ -483,6 +483,7 @@ public Map getLocalProjectDependencies() { return localProjectDependencies; } + @SuppressWarnings("unused") public URI getProjectBaseUri() { return projectBaseUri; } diff --git a/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java b/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java index 362fa5dda..5b9c2a1bf 100644 --- a/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java +++ b/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -16,6 +16,7 @@ package org.pkl.core.util; import com.oracle.truffle.api.TruffleOptions; +import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -854,4 +855,22 @@ public static void validateRewriteRule(URI rewrite) { "Rewrite rule must end with '/', but was '%s'".formatted(rewrite)); } } + + public static @Nullable Path findExecutableOnPath(String executable) { + var pathEnvVar = System.getenv("PATH"); + if (pathEnvVar == null) { + return null; + } + var extensions = isWindows() ? List.of(".cmd", ".bat", ".exe", ".dll") : List.of(""); + var pathDirs = pathEnvVar.split(File.pathSeparator); + for (var dir : pathDirs) { + for (var extension : extensions) { + var candidate = Path.of(dir, executable + extension); + if (Files.exists(candidate) && Files.isExecutable(candidate)) { + return candidate; + } + } + } + return null; + } } diff --git a/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt index 8d3b7e490..45241602d 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt @@ -16,12 +16,9 @@ package org.pkl.core.project import java.net.URI -import java.nio.file.Files import java.nio.file.Path -import java.nio.file.attribute.PosixFilePermission import java.util.regex.Pattern import kotlin.io.path.createDirectories -import kotlin.io.path.createFile import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatCode import org.junit.jupiter.api.Test @@ -35,7 +32,6 @@ import org.pkl.core.* import org.pkl.core.evaluatorSettings.PklEvaluatorSettings import org.pkl.core.http.HttpClient import org.pkl.core.packages.PackageUri -import org.pkl.core.util.IoUtils class ProjectTest { @Test @@ -158,7 +154,7 @@ class ProjectTest { ) val project = Project.loadFromPath(projectPath) assertThat(project.`package`).isEqualTo(expectedPackage) - assertThat(project.evaluatorSettings).isEqualTo(expectedSettings) + assertThat(project.resolvedEvaluatorSettings).isEqualTo(expectedSettings) assertThat(project.annotations).isEqualTo(expectedAnnotations) assertThat(project.tests) .isEqualTo(listOf(path.resolve("test1.pkl"), path.resolve("test2.pkl"))) @@ -280,7 +276,7 @@ class ProjectTest { evaluatorSettings { externalModuleReaders { ["foo"] { - executable = "foo" + executable = "foo/bar/baz" } } } @@ -288,25 +284,14 @@ class ProjectTest { .trimIndent() ) } - val executableName = if (IoUtils.isWindows()) "foo.cmd" else "foo" - val executable = - projectDir.resolve(executableName).also { - it.createFile() - if (!IoUtils.isWindows()) { - Files.setPosixFilePermissions(it, setOf(PosixFilePermission.OWNER_EXECUTE)) - } - } val project = Project.loadFromPath(pklProject, SecurityManagers.defaultManager, null) - assertThat(project.evaluatorSettings.externalModuleReaders).hasSize(1) - assertThat(project.evaluatorSettings.externalModuleReaders?.get("foo")!!.executable()) - .isEqualTo(executable.toAbsolutePath().toString()) + assertThat(project.resolvedEvaluatorSettings.externalModuleReaders).hasSize(1) + assertThat(project.resolvedEvaluatorSettings.externalModuleReaders?.get("foo")!!.executable()) + .isEqualTo(projectDir.resolve("foo/bar/baz").toString()) } @Test - fun `external readers -- executable is unmodified if not resolvable relative to project dir`( - @TempDir tempDir: Path - ) { - // echo exists in POSIX and also Windows (both command prompt and PowerShell) + fun `external readers -- executable is unmodified simple name`(@TempDir tempDir: Path) { val projectDir = tempDir.resolve("project").also { it.createDirectories() } val pklProject = projectDir.resolve("PklProject").also { @@ -318,7 +303,7 @@ class ProjectTest { evaluatorSettings { externalModuleReaders { ["foo"] { - executable = "echo" + executable = "my-command" } } } @@ -330,6 +315,30 @@ class ProjectTest { val project = Project.loadFromPath(pklProject, SecurityManagers.defaultManager, null) assertThat(project.evaluatorSettings.externalModuleReaders).hasSize(1) assertThat(project.evaluatorSettings.externalModuleReaders?.get("foo")!!.executable()) - .isEqualTo("echo") + .isEqualTo("my-command") + } + + @Test + fun `cannot set relative executable in non-file project`() { + val src = + ModuleSource.text( + // language=pkl + """ + amends "pkl:Project" + + evaluatorSettings { + externalModuleReaders { + ["foo"] { + executable = "foo/bar" + } + } + } + """ + .trimIndent() + ) + assertThatCode { Project.load(src) } + .hasMessageContaining( + "Type constraint `(!relativeExecutables(externalModuleReaders).isEmpty).implies(isFileBasedProject)` violated" + ) } } diff --git a/stdlib/EvaluatorSettings.pkl b/stdlib/EvaluatorSettings.pkl index 56c0ccfe2..90c994b3e 100644 --- a/stdlib/EvaluatorSettings.pkl +++ b/stdlib/EvaluatorSettings.pkl @@ -19,6 +19,8 @@ @Since { version = "0.26.0" } module pkl.EvaluatorSettings +import "pkl:EvaluatorSettings" + /// The external properties available to Pkl, read using the `prop:` scheme. externalProperties: Mapping? @@ -118,6 +120,70 @@ externalResourceReaders: Mapping? @Since { version = "0.30.0" } traceMode: ("compact" | "pretty")? +/// The enclosing URI used to resolve relative paths. +@Since { version = "0.31.0" } +hidden enclosingUri: String + +local function resolvePath(base: String, path: String) = + if (path.startsWith("/") || path.startsWith(Regex(#"([a-z]:)?\\"#))) + path + else + "\(base)/\(path)" + +local function resolveExecutable(base: String, path: String) = + if (path.contains("/") || path.contains("\\")) + resolvePath(base, path) + else + path + +/// These evaluator settings, whose settings are resolved against [baseDir]. +/// +/// The following settings are resolved: +/// +/// * [modulePath] +/// * [rootDir] +/// * [moduleCacheDir] +/// * [externalResourceReaders] +/// * [externalModuleReaders] +@Since { version = "0.31.0" } +function resolve(baseDir: String): EvaluatorSettings = (module) { + when (module.modulePath != null) { + modulePath = new { + for (path in module.modulePath!!) { + resolvePath(baseDir, path) + } + } + } + + when (module.rootDir != null) { + rootDir = resolvePath(baseDir, module.rootDir!!) + } + + when (module.moduleCacheDir != null) { + moduleCacheDir = resolvePath(baseDir, module.moduleCacheDir!!) + } + + when (module.externalResourceReaders != null) { + externalResourceReaders { + for (readerName, reader in module.externalResourceReaders!!) { + [readerName] { + executable = resolveExecutable(baseDir, reader.executable) + } + } + } + } + + when (module.externalModuleReaders != null) { + externalModuleReaders { + for (readerName, reader in module.externalModuleReaders!!) { + [readerName] { + executable = resolveExecutable(baseDir, reader.executable) + } + } + } + } +} + local const hostnameRegex = Regex(#"https?://([^/?#]*)"#) local const hasNonEmptyHostname = (it: String) -> @@ -224,14 +290,68 @@ class ExternalReader { /// The external reader executable. /// /// Will be spawned with the same environment variables and working directory as the Pkl process. - /// Executable is resolved according to the operating system's process spawning rules. - /// On macOS, Linux, and Windows platforms, this may be: + /// This may be: /// /// * An absolute path /// * A relative path /// * The name of the executable, to be resolved against the `PATH` environment variable + /// + /// On Windows, it is acceptable to use either `\` or `/` as the path separator. + /// + /// A simple name without slashes (e.g. `"my-command"`) is resolved against the `PATH` + /// environment variable, rather than against the enclosing directory. + /// + /// To resolve this as a path, prefix it with `./` (or `.\` on Windows). executable: String /// Additional command line arguments passed to the external reader process. arguments: Listing? } + +/// Machiney for URI percent decoding. +// TODO: replace me when URI stdlib is introduced +@Unlisted +local class Impl { + const percentDecode = (str: String) -> + str.replaceAllMapped(PERCENT_REGEX, (match) -> + let (bytes = getBytes(match.value)) + doPercentDecode(bytes) + ) + + local const PERCENT_REGEX = Regex(#"(?:%[\da-fA-F]{2})+"#) + + local const hexDigits = "0123456789ABCDEF" + + local const function getBytes(str: String): List = + str + .split("%") + .drop(1) + .map((it) -> + let (msb = hexDigits.indexOf(it[0].toUpperCase())) + let (lsb = hexDigits.indexOf(it[1].toUpperCase())) + lsb + (msb * 16) + ) + + local const function doPercentDecode(bytes: List): String = _doPercentDecode(bytes, "") + + local const function _doPercentDecode(bytes: List, ret: String) = + if (bytes.length == 0) + ret + else if (bytes[0] < 0x80) + _doPercentDecode(bytes.drop(1), ret + bytes[0].toChar()) + else if (bytes[0] < 0xE0) + let (b0 = bytes[0].and(0x1f).shl(6)) + let (b1 = bytes[1].and(0x3f)) + _doPercentDecode(bytes.drop(2), ret + b0.or(b1).toChar()) + else if (bytes[0] < 0xF0) + let (b0 = bytes[0].and(0xf).shl(12)) + let (b1 = bytes[1].and(0x3f).shl(6)) + let (b2 = bytes[2].and(0x3f)) + _doPercentDecode(bytes.drop(3), ret + b0.or(b1).or(b2).toChar()) + else + let (b0 = bytes[0].and(0x7).shl(18)) + let (b1 = bytes[1].and(0x3f).shl(12)) + let (b2 = bytes[2].and(0x3f).shl(6)) + let (b3 = bytes[3].and(0x3f)) + _doPercentDecode(bytes.drop(4), ret + b0.or(b1).or(b2).or(b3).toChar()) +} diff --git a/stdlib/Project.pkl b/stdlib/Project.pkl index 2e3fc060b..e5699c733 100644 --- a/stdlib/Project.pkl +++ b/stdlib/Project.pkl @@ -193,21 +193,43 @@ local isFileBasedProject = projectFileUri.startsWith("file:") /// /// The following values can only be set if this is a file-based project. /// -/// - [modulePath][EvaluatorSettings.modulePath] -/// - [rootDir][EvaluatorSettings.rootDir] -/// - [moduleCacheDir][EvaluatorSettings.moduleCacheDir] -/// - [externalModuleReaders][EvaluatorSettings.externalModuleReaders] -/// - [externalResourceReaders][EvaluatorSettings.externalResourceReaders] +/// - [modulePath][EvaluatorSettingsModule.modulePath] +/// - [rootDir][EvaluatorSettingsModule.rootDir] +/// - [moduleCacheDir][EvaluatorSettingsModule.moduleCacheDir] /// -/// For each of these, relative paths are resolved against the project's enclosing directory. +/// The following paths are resolved against the project's enclosing directory: +/// +/// - [modulePath[]][EvaluatorSettingsModule.modulePath] +/// - [rootDir][EvaluatorSettingsModule.rootDir] +/// - [moduleCacheDir][EvaluatorSettingsModule.moduleCacheDir] +/// - [externalModuleReaders[].executable][EvaluatorSettingsModule.ExternalReader.executable] +/// - [externalResourceReaders[].executable][EvaluatorSettingsModule.ExternalReader.executable] evaluatorSettings: EvaluatorSettingsModule( (modulePath != null).implies(isFileBasedProject), (rootDir != null).implies(isFileBasedProject), (moduleCacheDir != null).implies(isFileBasedProject), - (externalModuleReaders != null).implies(isFileBasedProject), - (externalResourceReaders != null).implies(isFileBasedProject), + (!relativeExecutables(externalModuleReaders).isEmpty).implies(isFileBasedProject), + (!relativeExecutables(externalResourceReaders).isEmpty).implies(isFileBasedProject), ) +local function relativeExecutables( + it: Mapping?, +): List = + if (it == null) + List() + else + let (executables = it.toMap().values.map((it) -> it.executable)) + let (relativeExecutables = executables.filter((it) -> it.contains("/") || it.contains("\\"))) + relativeExecutables + +/// The evaluator settings, whose various settings are resolved against the project root dir. +@Since { version = "0.31.0" } +fixed resolvedEvaluatorSettings: EvaluatorSettingsModule = + if (isFileBasedProject) + evaluatorSettings.resolve(new Impl {}.getBasePath(projectFileUri)) + else + evaluatorSettings + /// The URI of the PklProject file. /// /// This value is used to resolve relative paths when importing another local project as a @@ -445,3 +467,54 @@ typealias CommonSpdxLicenseIdentifier = @Unlisted @Since { version = "0.27.0" } fixed annotations: List = reflect.moduleOf(this).annotations + +/// Machiney for URI percent decoding. +// TODO: replace me when URI stdlib is introduced +@Unlisted +local class Impl { + function getBasePath(uri: String) = + uri.replaceFirst("file://", "").split("/").dropLast(1).map(new Impl {}.percentDecode).join("/") + + const percentDecode = (str: String) -> + str.replaceAllMapped(PERCENT_REGEX, (match) -> + let (bytes = getBytes(match.value)) + doPercentDecode(bytes) + ) + + local const PERCENT_REGEX = Regex(#"(?:%[\da-fA-F]{2})+"#) + + local const hexDigits = "0123456789ABCDEF" + + local const function getBytes(str: String): List = + str + .split("%") + .drop(1) + .map((it) -> + let (msb = hexDigits.indexOf(it[0].toUpperCase())) + let (lsb = hexDigits.indexOf(it[1].toUpperCase())) + lsb + (msb * 16) + ) + + local const function doPercentDecode(bytes: List): String = _doPercentDecode(bytes, "") + + local const function _doPercentDecode(bytes: List, ret: String) = + if (bytes.length == 0) + ret + else if (bytes[0] < 0x80) + _doPercentDecode(bytes.drop(1), ret + bytes[0].toChar()) + else if (bytes[0] < 0xE0) + let (b0 = bytes[0].and(0x1f).shl(6)) + let (b1 = bytes[1].and(0x3f)) + _doPercentDecode(bytes.drop(2), ret + b0.or(b1).toChar()) + else if (bytes[0] < 0xF0) + let (b0 = bytes[0].and(0xf).shl(12)) + let (b1 = bytes[1].and(0x3f).shl(6)) + let (b2 = bytes[2].and(0x3f)) + _doPercentDecode(bytes.drop(3), ret + b0.or(b1).or(b2).toChar()) + else + let (b0 = bytes[0].and(0x7).shl(18)) + let (b1 = bytes[1].and(0x3f).shl(12)) + let (b2 = bytes[2].and(0x3f).shl(6)) + let (b3 = bytes[3].and(0x3f)) + _doPercentDecode(bytes.drop(4), ret + b0.or(b1).or(b2).or(b3).toChar()) +} From 09218e797396d76c6cd989e5c02eadebcff99f46 Mon Sep 17 00:00:00 2001 From: Dan Chao Date: Thu, 8 Jan 2026 15:27:24 -0800 Subject: [PATCH 3/5] Allow executables to be file URIs --- .../ExternalReaderProcessImpl.java | 26 +++++++- .../org/pkl/core/errorMessages.properties | 11 ++++ .../input/projects/badPklProject6/PklProject | 9 +++ .../input/projects/badPklProject6/bug.pkl | 1 + .../input/projects/badPklProject7/PklProject | 11 ++++ .../input/projects/badPklProject7/bug.pkl | 1 + .../output/projects/badPklProject6/bug.err | 27 ++++++++ .../output/projects/badPklProject7/bug.err | 11 ++++ .../ExternalReaderProcessTest.kt | 39 +++++++++++ .../org/pkl/core/project/ProjectTest.kt | 26 +++++++- stdlib/EvaluatorSettings.pkl | 65 +++---------------- stdlib/Project.pkl | 19 +++--- 12 files changed, 177 insertions(+), 69 deletions(-) create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject6/PklProject create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject6/bug.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject7/PklProject create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject7/bug.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/projects/badPklProject6/bug.err create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/projects/badPklProject7/bug.err create mode 100644 pkl-core/src/test/kotlin/org/pkl/core/externalreader/ExternalReaderProcessTest.kt diff --git a/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java b/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java index 6f9f530ab..b93feefd8 100644 --- a/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java +++ b/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java @@ -17,6 +17,8 @@ import java.io.IOException; import java.lang.ProcessBuilder.Redirect; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Duration; import java.util.ArrayList; import java.util.Map; @@ -86,8 +88,26 @@ public ExternalResourceResolver getResourceResolver(long evaluatorId) return ExternalResourceResolver.of(getTransport(), evaluatorId); } - private @Nullable String getExecutablePath(String executable) { - if (executable.contains("/") || executable.contains("\"")) { + private @Nullable String getExecutablePath(String executable) + throws ExternalReaderProcessException { + if (IoUtils.isUriLike(executable)) { + try { + var uri = new URI(executable); + if (!uri.getScheme().equalsIgnoreCase("file")) { + throw new ExternalReaderProcessException( + ErrorMessages.create("cannotSpawnNonFileExecutable", uri)); + } + if (!uri.getPath().startsWith("/")) { + throw new ExternalReaderProcessException( + ErrorMessages.create("invalidOpaqueFileUri", uri)); + } + return uri.getPath(); + } catch (URISyntaxException e) { + throw new ExternalReaderProcessException( + ErrorMessages.create("invalidReaderExecutableUri", executable)); + } + } + if (executable.contains("/")) { return executable; } var resolved = IoUtils.findExecutableOnPath(executable); @@ -116,7 +136,7 @@ private MessageTransport getTransport() throws ExternalReaderProcessException { var executable = getExecutablePath(spec.executable()); if (executable == null) { throw new ExternalReaderProcessException( - ErrorMessages.create("cannotFindCommand", spec.executable())); + ErrorMessages.create("cannotResolveExternalReaderCommand", spec.executable())); } command.add(executable); if (spec.arguments() != null) { diff --git a/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties b/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties index 1f463ce8a..73aa50f54 100644 --- a/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties +++ b/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties @@ -1067,6 +1067,17 @@ External {0} reader does not support scheme `{1}`. externalReaderAlreadyTerminated=\ External reader process has already terminated. +cannotSpawnNonFileExecutable=\ +Invalid external reader executable `{0}`.\n\ +\n\ +Executables must be files. + +cannotResolveExternalReaderCommand=\ +Cannot resolve external reader executable `{0}`. + +invalidReaderExecutableUri=\ +Exernal reader executable URI `{0}` has invalid syntax. + invalidOpaqueFileUri=\ File URIs must have a path that starts with `/` (e.g. file:/path/to/my_module.pkl).\n\ To resolve relative paths, remove the scheme prefix (remove "file:"). diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject6/PklProject b/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject6/PklProject new file mode 100644 index 000000000..9b521d5a2 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject6/PklProject @@ -0,0 +1,9 @@ +amends "pkl:Project" + +evaluatorSettings { + externalModuleReaders { + ["foo"] { + executable = "qux:/foo" + } + } +} diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject6/bug.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject6/bug.pkl new file mode 100644 index 000000000..9f46e971e --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject6/bug.pkl @@ -0,0 +1 @@ +foo = read("foo:bar") diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject7/PklProject b/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject7/PklProject new file mode 100644 index 000000000..0b16faa3d --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject7/PklProject @@ -0,0 +1,11 @@ +amends "pkl:Project" + +projectFileUri = "modulepath:/foo/bar/PklProject" + +evaluatorSettings { + externalModuleReaders { + ["foo"] { + executable = "foo/bar" + } + } +} diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject7/bug.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject7/bug.pkl new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/projects/badPklProject7/bug.pkl @@ -0,0 +1 @@ + diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/projects/badPklProject6/bug.err b/pkl-core/src/test/files/LanguageSnippetTests/output/projects/badPklProject6/bug.err new file mode 100644 index 000000000..f0ba67360 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/projects/badPklProject6/bug.err @@ -0,0 +1,27 @@ +–– Pkl Error –– +Type constraint `(this is AbsoluteUri).implies(startsWith("file:/"))` violated. +Value: "qux:/foo" + +xxx | executable: String((this is AbsoluteUri).implies(startsWith("file:/"))) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at pkl.EvaluatorSettings#ExternalReader.executable (pkl:EvaluatorSettings) + +x | executable = "qux:/foo" + ^^^^^^^^^^ +at PklProject#evaluatorSettings.externalModuleReaders["foo"].executable (file:///$snippetsDir/input/projects/badPklProject6/PklProject) + +xxx | ?.map((it) -> it.executable) + ^^^^^^^^^^^^^ +at pkl.Project#pathBasedExecutables. (pkl:Project) + +xxx | it + ^^ +at pkl.Project#pathBasedExecutables (pkl:Project) + +xxx | pathBasedExecutables(externalModuleReaders).isNotEmpty.implies(isFileBasedProject), + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at pkl.Project#evaluatorSettings (pkl:Project) + +x | evaluatorSettings { + ^^^^^^^^^^^^^^^^^^^ +at PklProject#evaluatorSettings (file:///$snippetsDir/input/projects/badPklProject6/PklProject) diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/projects/badPklProject7/bug.err b/pkl-core/src/test/files/LanguageSnippetTests/output/projects/badPklProject7/bug.err new file mode 100644 index 000000000..52f147f3f --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/projects/badPklProject7/bug.err @@ -0,0 +1,11 @@ +–– Pkl Error –– +Type constraint `pathBasedExecutables(externalModuleReaders).isNotEmpty.implies(isFileBasedProject)` violated. +Value: new ModuleClass { externalProperties = ?; env = ?; allowedModules = ?; allowe... + +xxx | pathBasedExecutables(externalModuleReaders).isNotEmpty.implies(isFileBasedProject), + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at pkl.Project#evaluatorSettings (pkl:Project) + +x | evaluatorSettings { + ^^^^^^^^^^^^^^^^^^^ +at PklProject#evaluatorSettings (file:///$snippetsDir/input/projects/badPklProject7/PklProject) diff --git a/pkl-core/src/test/kotlin/org/pkl/core/externalreader/ExternalReaderProcessTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/externalreader/ExternalReaderProcessTest.kt new file mode 100644 index 000000000..265e5cb30 --- /dev/null +++ b/pkl-core/src/test/kotlin/org/pkl/core/externalreader/ExternalReaderProcessTest.kt @@ -0,0 +1,39 @@ +/* + * Copyright © 2026 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. + */ +package org.pkl.core.externalreader + +import org.assertj.core.api.Assertions.assertThatCode +import org.junit.jupiter.api.Test +import org.pkl.core.evaluatorSettings.PklEvaluatorSettings + +class ExternalReaderProcessTest { + @Test + fun `invalid URI`() { + val process = + ExternalReaderProcess.of( + PklEvaluatorSettings.ExternalReader("qux:/path/to/execuable", listOf()) + ) + assertThatCode { process.getModuleResolver(1) } + .hasMessageContaining("Invalid external reader executable `qux:/path/to/execuable`") + } + + @Test + fun `simple name is resolved off path`() { + val process = ExternalReaderProcess.of(PklEvaluatorSettings.ExternalReader("foo", listOf())) + assertThatCode { process.getModuleResolver(1) } + .hasMessageContaining("Cannot resolve external reader executable `foo`.") + } +} diff --git a/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt index 45241602d..75c1abf4c 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt @@ -338,7 +338,31 @@ class ProjectTest { ) assertThatCode { Project.load(src) } .hasMessageContaining( - "Type constraint `(!relativeExecutables(externalModuleReaders).isEmpty).implies(isFileBasedProject)` violated" + "Type constraint `pathBasedExecutables(externalModuleReaders).isNotEmpty.implies(isFileBasedProject)` violated." + ) + } + + @Test + fun `cannot set non-file URI`() { + val src = + ModuleSource.text( + // language=pkl + """ + amends "pkl:Project" + + evaluatorSettings { + externalModuleReaders { + ["foo"] { + executable = "qux:///path/to/executable" + } + } + } + """ + .trimIndent() + ) + assertThatCode { Project.load(src) } + .hasMessageContaining( + "Type constraint `(this is AbsoluteUri).implies(startsWith(\"file:/\"))` violated." ) } } diff --git a/stdlib/EvaluatorSettings.pkl b/stdlib/EvaluatorSettings.pkl index 90c994b3e..0596650c0 100644 --- a/stdlib/EvaluatorSettings.pkl +++ b/stdlib/EvaluatorSettings.pkl @@ -125,13 +125,13 @@ traceMode: ("compact" | "pretty")? hidden enclosingUri: String local function resolvePath(base: String, path: String) = - if (path.startsWith("/") || path.startsWith(Regex(#"([a-z]:)?\\"#))) + if (path.startsWith("/") || path is AbsoluteUri) path else "\(base)/\(path)" local function resolveExecutable(base: String, path: String) = - if (path.contains("/") || path.contains("\\")) + if (path.contains("/")) resolvePath(base, path) else path @@ -285,6 +285,8 @@ class Proxy { noProxy: Listing(isDistinct) } +local typealias AbsoluteUri = String(matches(Regex(#"[\w+.-]+:[^\\].*"#))) + @Since { version = "0.27.0" } class ExternalReader { /// The external reader executable. @@ -292,66 +294,17 @@ class ExternalReader { /// Will be spawned with the same environment variables and working directory as the Pkl process. /// This may be: /// - /// * An absolute path - /// * A relative path + /// * An absolute or relative path, delineated with `/` + /// * A file URI /// * The name of the executable, to be resolved against the `PATH` environment variable /// - /// On Windows, it is acceptable to use either `\` or `/` as the path separator. + /// To represent a path within a drive on Windows, use a file URI (e.g. + /// `file:///C:/path/to/executable`). /// /// A simple name without slashes (e.g. `"my-command"`) is resolved against the `PATH` /// environment variable, rather than against the enclosing directory. - /// - /// To resolve this as a path, prefix it with `./` (or `.\` on Windows). - executable: String + executable: String((this is AbsoluteUri).implies(startsWith("file:/"))) /// Additional command line arguments passed to the external reader process. arguments: Listing? } - -/// Machiney for URI percent decoding. -// TODO: replace me when URI stdlib is introduced -@Unlisted -local class Impl { - const percentDecode = (str: String) -> - str.replaceAllMapped(PERCENT_REGEX, (match) -> - let (bytes = getBytes(match.value)) - doPercentDecode(bytes) - ) - - local const PERCENT_REGEX = Regex(#"(?:%[\da-fA-F]{2})+"#) - - local const hexDigits = "0123456789ABCDEF" - - local const function getBytes(str: String): List = - str - .split("%") - .drop(1) - .map((it) -> - let (msb = hexDigits.indexOf(it[0].toUpperCase())) - let (lsb = hexDigits.indexOf(it[1].toUpperCase())) - lsb + (msb * 16) - ) - - local const function doPercentDecode(bytes: List): String = _doPercentDecode(bytes, "") - - local const function _doPercentDecode(bytes: List, ret: String) = - if (bytes.length == 0) - ret - else if (bytes[0] < 0x80) - _doPercentDecode(bytes.drop(1), ret + bytes[0].toChar()) - else if (bytes[0] < 0xE0) - let (b0 = bytes[0].and(0x1f).shl(6)) - let (b1 = bytes[1].and(0x3f)) - _doPercentDecode(bytes.drop(2), ret + b0.or(b1).toChar()) - else if (bytes[0] < 0xF0) - let (b0 = bytes[0].and(0xf).shl(12)) - let (b1 = bytes[1].and(0x3f).shl(6)) - let (b2 = bytes[2].and(0x3f)) - _doPercentDecode(bytes.drop(3), ret + b0.or(b1).or(b2).toChar()) - else - let (b0 = bytes[0].and(0x7).shl(18)) - let (b1 = bytes[1].and(0x3f).shl(12)) - let (b2 = bytes[2].and(0x3f).shl(6)) - let (b3 = bytes[3].and(0x3f)) - _doPercentDecode(bytes.drop(4), ret + b0.or(b1).or(b2).or(b3).toChar()) -} diff --git a/stdlib/Project.pkl b/stdlib/Project.pkl index e5699c733..53aed0d8d 100644 --- a/stdlib/Project.pkl +++ b/stdlib/Project.pkl @@ -208,19 +208,20 @@ evaluatorSettings: EvaluatorSettingsModule( (modulePath != null).implies(isFileBasedProject), (rootDir != null).implies(isFileBasedProject), (moduleCacheDir != null).implies(isFileBasedProject), - (!relativeExecutables(externalModuleReaders).isEmpty).implies(isFileBasedProject), - (!relativeExecutables(externalResourceReaders).isEmpty).implies(isFileBasedProject), + pathBasedExecutables(externalModuleReaders).isNotEmpty.implies(isFileBasedProject), + pathBasedExecutables(externalResourceReaders).isNotEmpty.implies(isFileBasedProject), ) -local function relativeExecutables( +/// The executables that are path segments (those that contain `/` characters) +local function pathBasedExecutables( it: Mapping?, ): List = - if (it == null) - List() - else - let (executables = it.toMap().values.map((it) -> it.executable)) - let (relativeExecutables = executables.filter((it) -> it.contains("/") || it.contains("\\"))) - relativeExecutables + it + ?.toMap() + ?.values + ?.map((it) -> it.executable) + ?.filter((it) -> it.contains("/")) + ?? List() /// The evaluator settings, whose various settings are resolved against the project root dir. @Since { version = "0.31.0" } From 6b8cb336f159d67aaa515c598487b9ad8106b78f Mon Sep 17 00:00:00 2001 From: Dan Chao Date: Fri, 9 Jan 2026 09:59:26 -0800 Subject: [PATCH 4/5] Add test for spawning an external reader --- pkl-core/pkl-core.gradle.kts | 57 +++++++++++++- .../pkl/core/externalreaderfixture/Main.kt | 67 ++++++++++++++++ .../ExternalReaderMessagePackDecoder.java | 4 +- .../ExternalReaderMessagePackEncoder.java | 4 +- .../pkl/core/module/ModuleKeyFactoriesTest.kt | 77 ++++++++++++++++++- 5 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 pkl-core/src/externalReaderFixture/kotlin/org/pkl/core/externalreaderfixture/Main.kt diff --git a/pkl-core/pkl-core.gradle.kts b/pkl-core/pkl-core.gradle.kts index 005377304..5869b03dc 100644 --- a/pkl-core/pkl-core.gradle.kts +++ b/pkl-core/pkl-core.gradle.kts @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -24,7 +24,16 @@ plugins { idea } -val generatorSourceSet = sourceSets.register("generator") +val generatorSourceSet: NamedDomainObjectProvider = sourceSets.register("generator") + +val externalReaderFixtureSourceSet: NamedDomainObjectProvider = + sourceSets.register("externalReaderFixture") { + compileClasspath += sourceSets.test.get().output + sourceSets.test.get().compileClasspath + runtimeClasspath += sourceSets.test.get().output + sourceSets.test.get().runtimeClasspath + } + +val externalReaderFixtureImplementation: Configuration by + configurations.getting { extendsFrom(configurations.testImplementation.get()) } idea { module { @@ -110,8 +119,46 @@ tasks.compileJava { options.generatedSourceOutputDirectory.set(file("generated/t tasks.compileKotlin { enabled = false } +val externalReaderFixture by + tasks.registering { + group = "build" + dependsOn(tasks.named("compileExternalReaderFixtureJava")) + inputs.files(externalReaderFixtureSourceSet.map { it.output }) + val fileName = if (buildInfo.os.isWindows) "externalreader.bat" else "externalreader" + val outputFile = layout.buildDirectory.file("fixtures/$fileName") + outputs.file(outputFile) + doLast { + val classpath = externalReaderFixtureSourceSet.get().runtimeClasspath.asPath + val scriptContent = + if (buildInfo.os.isWindows) { + """ + @echo off + java -cp $classpath org.pkl.core.externalreaderfixture.Main + """ + .trimIndent() + } else { + """ + #!/usr/bin/env bash + + java -cp $classpath org.pkl.core.externalreaderfixture.Main + """ + .trimIndent() + } + + outputFile.get().asFile.writeText(scriptContent) + outputFile.get().asFile.setExecutable(true) + println("Created external reader ${outputFile.get().asFile.absolutePath}") + } + } + tasks.test { configureTest() + dependsOn(externalReaderFixture) + environment( + "PATH", + listOf(System.getenv("PATH"), layout.buildDirectory.dir("fixtures/").get()) + .joinToString(File.pathSeparator), + ) useJUnitPlatform { excludeEngines("MacAmd64LanguageSnippetTestsEngine") excludeEngines("MacAarch64LanguageSnippetTestsEngine") @@ -123,6 +170,12 @@ tasks.test { // testing very large lists requires more memory than the default 512m! maxHeapSize = "1g" + + dependsOn(externalReaderFixture) + systemProperty( + "org.pkl.core.testExternalReaderPath", + externalReaderFixture.map { it.outputs.files.singleFile.absolutePath }, + ) } val testJavaExecutable by diff --git a/pkl-core/src/externalReaderFixture/kotlin/org/pkl/core/externalreaderfixture/Main.kt b/pkl-core/src/externalReaderFixture/kotlin/org/pkl/core/externalreaderfixture/Main.kt new file mode 100644 index 000000000..0222ef794 --- /dev/null +++ b/pkl-core/src/externalReaderFixture/kotlin/org/pkl/core/externalreaderfixture/Main.kt @@ -0,0 +1,67 @@ +/* + * Copyright © 2026 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:JvmName("Main") + +package org.pkl.core.externalreaderfixture + +import java.net.URI +import org.pkl.core.externalreader.ExternalModuleReader +import org.pkl.core.externalreader.ExternalReaderClient +import org.pkl.core.externalreader.ExternalReaderMessagePackDecoder +import org.pkl.core.externalreader.ExternalReaderMessagePackEncoder +import org.pkl.core.externalreader.ExternalResourceReader +import org.pkl.core.messaging.MessageTransports +import org.pkl.core.module.PathElement + +object ModuleReader : ExternalModuleReader { + override val isLocal: Boolean = true + + override fun read(uri: URI): String = "hello" + + override val scheme: String = "foo" + + override val hasHierarchicalUris: Boolean = false + + override val isGlobbable: Boolean = false + + override fun listElements(uri: URI): List { + throw NotImplementedError() + } +} + +object ResourceReader : ExternalResourceReader { + override fun read(uri: URI): ByteArray = "hello".toByteArray() + + override val scheme: String = "foo" + + override val hasHierarchicalUris: Boolean = false + + override val isGlobbable: Boolean = false + + override fun listElements(uri: URI): List { + throw NotImplementedError() + } +} + +fun main() { + val transport = + MessageTransports.stream( + ExternalReaderMessagePackDecoder(System.`in`), + ExternalReaderMessagePackEncoder(System.out), + ) {} + val client = ExternalReaderClient(listOf(ModuleReader), listOf(ResourceReader), transport) + client.run() +} diff --git a/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderMessagePackDecoder.java b/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderMessagePackDecoder.java index 9578b6a3c..8483b5171 100644 --- a/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderMessagePackDecoder.java +++ b/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderMessagePackDecoder.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -28,7 +28,7 @@ import org.pkl.core.messaging.Message.Type; import org.pkl.core.util.Nullable; -final class ExternalReaderMessagePackDecoder extends BaseMessagePackDecoder { +public final class ExternalReaderMessagePackDecoder extends BaseMessagePackDecoder { public ExternalReaderMessagePackDecoder(MessageUnpacker unpacker) { super(unpacker); diff --git a/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderMessagePackEncoder.java b/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderMessagePackEncoder.java index b383b4ac6..44442f83f 100644 --- a/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderMessagePackEncoder.java +++ b/pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderMessagePackEncoder.java @@ -1,5 +1,5 @@ /* - * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -25,7 +25,7 @@ import org.pkl.core.messaging.ProtocolException; import org.pkl.core.util.Nullable; -final class ExternalReaderMessagePackEncoder extends BaseMessagePackEncoder { +public final class ExternalReaderMessagePackEncoder extends BaseMessagePackEncoder { public ExternalReaderMessagePackEncoder(MessagePacker packer) { super(packer); diff --git a/pkl-core/src/test/kotlin/org/pkl/core/module/ModuleKeyFactoriesTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/module/ModuleKeyFactoriesTest.kt index 58ada8659..6b9543316 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/module/ModuleKeyFactoriesTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/module/ModuleKeyFactoriesTest.kt @@ -1,5 +1,5 @@ /* - * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2024-2026 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. @@ -15,20 +15,51 @@ */ package org.pkl.core.module +import java.io.File import java.net.URI +import java.nio.file.Files import java.nio.file.Path +import java.util.regex.Pattern import kotlin.io.path.createDirectories import kotlin.io.path.createParentDirectories import kotlin.io.path.outputStream import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Assumptions.assumeTrue import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir +import org.pkl.commons.test.FileTestUtils import org.pkl.commons.toPath import org.pkl.commons.writeString +import org.pkl.core.EvaluatorBuilder +import org.pkl.core.ModuleSource import org.pkl.core.SecurityManagers -import org.pkl.core.externalreader.* +import org.pkl.core.evaluatorSettings.PklEvaluatorSettings +import org.pkl.core.externalreader.ExternalReaderProcess +import org.pkl.core.externalreader.TestExternalModuleReader +import org.pkl.core.externalreader.TestExternalReaderProcess +import org.pkl.core.resource.ResourceReaders class ModuleKeyFactoriesTest { + companion object { + private val externalReaderFixture by lazy { + FileTestUtils.rootProjectDir.resolve("pkl-core/build/fixtures/externalreader").also { path -> + if (!Files.exists(path)) { + throw AssertionError( + "Fixture `externalreader` not found. To fix this problem, first run" + + " `./gradlew pkl-core:externalReaderFixture`." + ) + } + } + } + + @JvmStatic + private fun pathEnvIsSet(): Boolean { + return System.getenv("PATH") + ?.split(File.pathSeparator) + ?.contains(externalReaderFixture.toAbsolutePath().toString()) ?: false + } + } + @Test fun `standard library`() { val factory = ModuleKeyFactories.standardLibrary @@ -146,4 +177,46 @@ class ModuleKeyFactoriesTest { proc.close() runtime.close() } + + @Test + fun `external process -- spawning an executable using a path`() { + val evaluator = + makeEvaluatorWithExternalReader(externalReaderFixture.toAbsolutePath().toString()) + val result = + evaluator.use { + evaluator.evaluateExpression(ModuleSource.uri("pkl:base"), "read(\"foo:foo\").text") + } + assertThat(result).isEqualTo("hello") + } + + @Test + fun `external process -- spawning an executable using a file URI`() { + val evaluator = makeEvaluatorWithExternalReader(externalReaderFixture.toUri().toString()) + val result = + evaluator.use { + evaluator.evaluateExpression(ModuleSource.uri("pkl:base"), "read(\"foo:foo\").text") + } + assertThat(result).isEqualTo("hello") + } + + @Test + fun `external process -- spawning an executable using a simple name off PATH`() { + assumeTrue(pathEnvIsSet(), "PATH contains fixtures dir") + val evaluator = makeEvaluatorWithExternalReader("externalreader") + val result = + evaluator.use { + evaluator.evaluateExpression(ModuleSource.uri("pkl:base"), "read(\"foo:foo\").text") + } + assertThat(result).isEqualTo("hello") + } + + private fun makeEvaluatorWithExternalReader(reader: String) = + with(EvaluatorBuilder.preconfigured()) { + val process = ExternalReaderProcess.of(PklEvaluatorSettings.ExternalReader(reader, listOf())) + addModuleKeyFactory(ModuleKeyFactories.externalProcess("foo", process)) + addResourceReader(ResourceReaders.externalProcess("foo", process)) + setAllowedModules(allowedModules + listOf(Pattern.compile("foo:"))) + setAllowedResources(allowedResources + listOf(Pattern.compile("foo:"))) + build() + } } From a56306763fca29efc25751514994cbed13fadc73 Mon Sep 17 00:00:00 2001 From: Dan Chao Date: Tue, 13 Jan 2026 15:14:54 -0800 Subject: [PATCH 5/5] Address PR comments --- stdlib/EvaluatorSettings.pkl | 4 ---- stdlib/Project.pkl | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/stdlib/EvaluatorSettings.pkl b/stdlib/EvaluatorSettings.pkl index 0596650c0..b2a513c07 100644 --- a/stdlib/EvaluatorSettings.pkl +++ b/stdlib/EvaluatorSettings.pkl @@ -120,10 +120,6 @@ externalResourceReaders: Mapping? @Since { version = "0.30.0" } traceMode: ("compact" | "pretty")? -/// The enclosing URI used to resolve relative paths. -@Since { version = "0.31.0" } -hidden enclosingUri: String - local function resolvePath(base: String, path: String) = if (path.startsWith("/") || path is AbsoluteUri) path diff --git a/stdlib/Project.pkl b/stdlib/Project.pkl index 53aed0d8d..4e49cdae4 100644 --- a/stdlib/Project.pkl +++ b/stdlib/Project.pkl @@ -469,7 +469,7 @@ typealias CommonSpdxLicenseIdentifier = @Since { version = "0.27.0" } fixed annotations: List = reflect.moduleOf(this).annotations -/// Machiney for URI percent decoding. +/// Machinery for URI percent decoding. // TODO: replace me when URI stdlib is introduced @Unlisted local class Impl {