Change loading of external readers (#1394)

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. Make CLI flags blow away any settings set on a PklProject
3. Introduce a new `workingDir` property, which defaults to the
PklProject dir

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.
This commit is contained in:
Daniel Chao
2026-06-02 11:02:52 -07:00
committed by GitHub
parent c9f3823952
commit 035ef0a789
51 changed files with 1880 additions and 92 deletions
@@ -148,10 +148,10 @@ data class CliBaseOptions(
val httpHeaders: Map<String, Map<String, List<String>>>? = null,
/** External module reader process specs */
val externalModuleReaders: Map<String, ExternalReader> = mapOf(),
val externalModuleReaders: Map<String, ExternalReader>? = null,
/** External resource reader process specs */
val externalResourceReaders: Map<String, ExternalReader> = mapOf(),
val externalResourceReaders: Map<String, ExternalReader>? = null,
/** Defines options for the formatting of calls to the trace() method. */
val traceMode: TraceMode? = null,
@@ -110,7 +110,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
private val evaluatorSettings: PklEvaluatorSettings? by lazy {
@Suppress("PklCliDirectProjectEvaluatorSettingsAccess")
if (cliOptions.omitProjectSettings) null else project?.evaluatorSettings
if (cliOptions.omitProjectSettings) null else project?.resolvedEvaluatorSettings
}
protected val allowedModules: List<Pattern> by lazy {
@@ -193,11 +193,11 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
}
protected val externalModuleReaders: Map<String, PklEvaluatorSettings.ExternalReader> by lazy {
(evaluatorSettings?.externalModuleReaders ?: emptyMap()) + cliOptions.externalModuleReaders
cliOptions.externalModuleReaders ?: evaluatorSettings?.externalModuleReaders ?: mapOf()
}
protected val externalResourceReaders: Map<String, PklEvaluatorSettings.ExternalReader> by lazy {
(evaluatorSettings?.externalResourceReaders ?: emptyMap()) + cliOptions.externalResourceReaders
cliOptions.externalResourceReaders ?: evaluatorSettings?.externalResourceReaders ?: mapOf()
}
private val externalProcesses:
@@ -55,7 +55,8 @@ class BaseOptions : OptionGroup() {
if (IoUtils.isUriLike(moduleName)) URI(moduleName)
// Can't just use URI constructor, because URI(null, null, "C:/foo/bar", null) turns
// into `URI("C", null, "/foo/bar", null)`.
else if (IoUtils.isWindowsAbsolutePath(moduleName)) Path.of(moduleName).toUri()
else if (IoUtils.isWindows() && IoUtils.isWindowsAbsolutePath(moduleName))
Path.of(moduleName).toUri()
else URI(null, null, IoUtils.toNormalizedPathString(Path.of(moduleName)), null)
} catch (e: URISyntaxException) {
val message = buildString {
@@ -91,7 +92,7 @@ class BaseOptions : OptionGroup() {
> {
return splitPair(delimiter).convert {
val cmd = shlex(it.second)
Pair(it.first, ExternalReader(cmd.first(), cmd.drop(1)))
Pair(it.first, ExternalReader(cmd.first(), cmd.drop(1), null))
}
}
@@ -386,8 +387,8 @@ class BaseOptions : OptionGroup() {
httpNoProxy = noProxy,
httpRewrites = httpRewrites.ifEmpty { null },
httpHeaders = httpHeaders.ifEmpty { null },
externalModuleReaders = externalModuleReaders,
externalResourceReaders = externalResourceReaders,
externalModuleReaders = externalModuleReaders.ifEmpty { null },
externalResourceReaders = externalResourceReaders.ifEmpty { null },
traceMode = traceMode,
powerAssertionsEnabled = powerAssertionsEnabled,
)
@@ -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.
@@ -98,17 +98,17 @@ class BaseCommandTest {
assertThat(cmd.baseOptions.externalModuleReaders)
.isEqualTo(
mapOf(
"scheme3" to ExternalReader("reader3", emptyList()),
"scheme4" to ExternalReader("reader4", listOf("with", "args")),
"scheme+ext" to ExternalReader("reader5", listOf("with", "args")),
"scheme3" to ExternalReader("reader3", emptyList(), null),
"scheme4" to ExternalReader("reader4", listOf("with", "args"), null),
"scheme+ext" to ExternalReader("reader5", listOf("with", "args"), null),
)
)
assertThat(cmd.baseOptions.externalResourceReaders)
.isEqualTo(
mapOf(
"scheme1" to ExternalReader("reader1", emptyList()),
"scheme2" to ExternalReader("reader2", listOf("with", "args")),
"scheme+ext" to ExternalReader("reader5", listOf("with", "args")),
"scheme1" to ExternalReader("reader1", emptyList(), null),
"scheme2" to ExternalReader("reader2", listOf("with", "args"), null),
"scheme+ext" to ExternalReader("reader5", listOf("with", "args"), null),
)
)
}
@@ -28,12 +28,12 @@ import org.pkl.commons.cli.commands.BaseCommand
import org.pkl.commons.cli.commands.ProjectOptions
import org.pkl.commons.writeString
import org.pkl.core.SecurityManagers
import org.pkl.core.evaluatorSettings.PklEvaluatorSettings
import org.pkl.core.evaluatorSettings.TraceMode
import org.pkl.core.util.IoUtils
@OptIn(ExperimentalPathApi::class)
class CliCommandTest {
private val cmd =
object : BaseCommand("test", "") {
val projectOptions: ProjectOptions by ProjectOptions()
@@ -172,4 +172,32 @@ class CliCommandTest {
.isNotNull
}
}
@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(), null)))
}
}