Skip to content

Commit 035ef0a

Browse files
authored
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.
1 parent c9f3823 commit 035ef0a

51 files changed

Lines changed: 1880 additions & 92 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.idea/inspectionProfiles/Project_Default.xml

Lines changed: 7 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/modules/ROOT/partials/component-attributes.adoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,10 @@ endif::[]
7878
:uri-stdlib-pklbinaryModule: {uri-pkl-stdlib-docs}/pklbinary
7979
:uri-stdlib-yamlModule: {uri-pkl-stdlib-docs}/yaml
8080
:uri-stdlib-YamlParser: {uri-stdlib-yamlModule}/Parser
81+
:uri-stdlib-projectModule: {uri-pkl-stdlib-docs}/Project
8182
:uri-stdlib-evaluatorSettingsModule: {uri-pkl-stdlib-docs}/EvaluatorSettings
8283
:uri-stdlib-evaluatorSettingsHttpClass: {uri-stdlib-evaluatorSettingsModule}/Http
84+
:uri-stdlib-evaluatorSettingsExternalReaderClass: {uri-stdlib-evaluatorSettingsModule}/ExternalReader
8385
:uri-stdlib-Boolean: {uri-stdlib-baseModule}/Boolean
8486
:uri-stdlib-xor: {uri-stdlib-baseModule}/Boolean#xor()
8587
:uri-stdlib-implies: {uri-stdlib-baseModule}/Boolean#implies()

docs/modules/release-notes/pages/0.32.adoc

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,31 @@ XXX
2121

2222
Ready when you need them.
2323

24-
.XXX
25-
[%collapsible]
26-
====
27-
XXX
28-
====
24+
=== Standard Library Changes
25+
26+
==== `pkl:EvaluatorSettings`
27+
28+
**Additions**
29+
30+
* New method: link:{uri-stdlib-evaluatorSettingsModule}#resolve()[`EvaluatorSettings.resolve()`]
31+
* New method: link:{uri-stdlib-evaluatorSettingsModule}#resolveForOs()[`EvaluatorSettings.resolveForOs()`]
32+
* New property: link:{uri-stdlib-evaluatorSettingsExternalReaderClass}#workingDir[`EvaluatorSettings#ExternalReader.workingDir`]
33+
34+
==== `pkl:Project`
35+
36+
**Additions**
37+
38+
* New property: link:{uri-stdlib-projectModule}#resolvedEvaluatorSettings[`Project.resolvedEvaluatorSettings`]
2939

3040
== Breaking Changes [small]#💔#
3141

3242
Things to watch out for when upgrading.
3343

34-
=== Removed Java APIs
44+
=== Java API changes
45+
46+
Changes have been made to the Java API.
47+
48+
==== Removals and deprecations
3549

3650
The following APIs have been removed without replacement.
3751

@@ -41,11 +55,26 @@ The following APIs have been deprecated for removal.
4155

4256
* `org.pkl.config.java.mapper.NonNull` (https://github.com/apple/pkl/pull/1607[#1607]).
4357

44-
.XXX
45-
[%collapsible]
46-
====
47-
XXX
48-
====
58+
==== Changes
59+
60+
* `org.pkl.core.evaluatorSettings.PklEvaluatorSettings.parse` no longer accepts a `pathNormalizer` argument.
61+
62+
=== Loading rule changes in `pkl:EvaluatorSettings`
63+
64+
Breaking changes have been made to how evaluator settings are loaded (https://github.com/apple/pkl/pull/1394[#1394]).
65+
66+
==== Loading rule changes for the external reader executable
67+
68+
The following changes have been made for the `executable` property in an external reader:
69+
70+
* If the executable does not contain a slash (`/` on POSIX, `\` on Windows) character, it is always resolved against the `PATH` environment variable.
71+
* If it does contain a slash, it is resolved relative to the enclosing PklProject directory, instead of the current working directory.
72+
73+
=== Changes to `--external-module-reader` and `--external-resource-reader` CLI flags
74+
75+
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 (https://github.com/apple/pkl/pull/1394[#1394]).
76+
77+
This makes this behavior consistent with how other settings work.
4978

5079
== Work In Progress [small]#🚆#
5180

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliBaseOptions.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,10 @@ data class CliBaseOptions(
148148
val httpHeaders: Map<String, Map<String, List<String>>>? = null,
149149

150150
/** External module reader process specs */
151-
val externalModuleReaders: Map<String, ExternalReader> = mapOf(),
151+
val externalModuleReaders: Map<String, ExternalReader>? = null,
152152

153153
/** External resource reader process specs */
154-
val externalResourceReaders: Map<String, ExternalReader> = mapOf(),
154+
val externalResourceReaders: Map<String, ExternalReader>? = null,
155155

156156
/** Defines options for the formatting of calls to the trace() method. */
157157
val traceMode: TraceMode? = null,

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
110110

111111
private val evaluatorSettings: PklEvaluatorSettings? by lazy {
112112
@Suppress("PklCliDirectProjectEvaluatorSettingsAccess")
113-
if (cliOptions.omitProjectSettings) null else project?.evaluatorSettings
113+
if (cliOptions.omitProjectSettings) null else project?.resolvedEvaluatorSettings
114114
}
115115

116116
protected val allowedModules: List<Pattern> by lazy {
@@ -193,11 +193,11 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
193193
}
194194

195195
protected val externalModuleReaders: Map<String, PklEvaluatorSettings.ExternalReader> by lazy {
196-
(evaluatorSettings?.externalModuleReaders ?: emptyMap()) + cliOptions.externalModuleReaders
196+
cliOptions.externalModuleReaders ?: evaluatorSettings?.externalModuleReaders ?: mapOf()
197197
}
198198

199199
protected val externalResourceReaders: Map<String, PklEvaluatorSettings.ExternalReader> by lazy {
200-
(evaluatorSettings?.externalResourceReaders ?: emptyMap()) + cliOptions.externalResourceReaders
200+
cliOptions.externalResourceReaders ?: evaluatorSettings?.externalResourceReaders ?: mapOf()
201201
}
202202

203203
private val externalProcesses:

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ class BaseOptions : OptionGroup() {
5555
if (IoUtils.isUriLike(moduleName)) URI(moduleName)
5656
// Can't just use URI constructor, because URI(null, null, "C:/foo/bar", null) turns
5757
// into `URI("C", null, "/foo/bar", null)`.
58-
else if (IoUtils.isWindowsAbsolutePath(moduleName)) Path.of(moduleName).toUri()
58+
else if (IoUtils.isWindows() && IoUtils.isWindowsAbsolutePath(moduleName))
59+
Path.of(moduleName).toUri()
5960
else URI(null, null, IoUtils.toNormalizedPathString(Path.of(moduleName)), null)
6061
} catch (e: URISyntaxException) {
6162
val message = buildString {
@@ -91,7 +92,7 @@ class BaseOptions : OptionGroup() {
9192
> {
9293
return splitPair(delimiter).convert {
9394
val cmd = shlex(it.second)
94-
Pair(it.first, ExternalReader(cmd.first(), cmd.drop(1)))
95+
Pair(it.first, ExternalReader(cmd.first(), cmd.drop(1), null))
9596
}
9697
}
9798

@@ -386,8 +387,8 @@ class BaseOptions : OptionGroup() {
386387
httpNoProxy = noProxy,
387388
httpRewrites = httpRewrites.ifEmpty { null },
388389
httpHeaders = httpHeaders.ifEmpty { null },
389-
externalModuleReaders = externalModuleReaders,
390-
externalResourceReaders = externalResourceReaders,
390+
externalModuleReaders = externalModuleReaders.ifEmpty { null },
391+
externalResourceReaders = externalResourceReaders.ifEmpty { null },
391392
traceMode = traceMode,
392393
powerAssertionsEnabled = powerAssertionsEnabled,
393394
)

pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/BaseCommandTest.kt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
2+
* Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -98,17 +98,17 @@ class BaseCommandTest {
9898
assertThat(cmd.baseOptions.externalModuleReaders)
9999
.isEqualTo(
100100
mapOf(
101-
"scheme3" to ExternalReader("reader3", emptyList()),
102-
"scheme4" to ExternalReader("reader4", listOf("with", "args")),
103-
"scheme+ext" to ExternalReader("reader5", listOf("with", "args")),
101+
"scheme3" to ExternalReader("reader3", emptyList(), null),
102+
"scheme4" to ExternalReader("reader4", listOf("with", "args"), null),
103+
"scheme+ext" to ExternalReader("reader5", listOf("with", "args"), null),
104104
)
105105
)
106106
assertThat(cmd.baseOptions.externalResourceReaders)
107107
.isEqualTo(
108108
mapOf(
109-
"scheme1" to ExternalReader("reader1", emptyList()),
110-
"scheme2" to ExternalReader("reader2", listOf("with", "args")),
111-
"scheme+ext" to ExternalReader("reader5", listOf("with", "args")),
109+
"scheme1" to ExternalReader("reader1", emptyList(), null),
110+
"scheme2" to ExternalReader("reader2", listOf("with", "args"), null),
111+
"scheme+ext" to ExternalReader("reader5", listOf("with", "args"), null),
112112
)
113113
)
114114
}

pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/CliCommandTest.kt

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ import org.pkl.commons.cli.commands.BaseCommand
2828
import org.pkl.commons.cli.commands.ProjectOptions
2929
import org.pkl.commons.writeString
3030
import org.pkl.core.SecurityManagers
31+
import org.pkl.core.evaluatorSettings.PklEvaluatorSettings
3132
import org.pkl.core.evaluatorSettings.TraceMode
3233
import org.pkl.core.util.IoUtils
3334

3435
@OptIn(ExperimentalPathApi::class)
3536
class CliCommandTest {
36-
3737
private val cmd =
3838
object : BaseCommand("test", "") {
3939
val projectOptions: ProjectOptions by ProjectOptions()
@@ -172,4 +172,32 @@ class CliCommandTest {
172172
.isNotNull
173173
}
174174
}
175+
176+
@Test
177+
fun `--external-module-reader blows away PklProject externalModuleReaders`(
178+
@TempDir tempDir: Path
179+
) {
180+
tempDir
181+
.resolve("PklProject")
182+
.writeString(
183+
// language=pkl
184+
"""
185+
amends "pkl:Project"
186+
187+
evaluatorSettings {
188+
externalModuleReaders {
189+
["foo"] {
190+
executable = "foo"
191+
}
192+
}
193+
}
194+
"""
195+
.trimIndent()
196+
)
197+
cmd.parse(arrayOf("--external-module-reader", "bar=bar"))
198+
val opts = cmd.baseOptions.baseOptions(emptyList(), null, true)
199+
val cliTest = CliTest(opts)
200+
assertThat(cliTest.myExternalModuleReaders)
201+
.isEqualTo(mapOf("bar" to PklEvaluatorSettings.ExternalReader("bar", listOf(), null)))
202+
}
175203
}

pkl-core/pkl-core.gradle.kts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,17 @@ plugins {
2727
idea
2828
}
2929

30-
val generatorSourceSet = sourceSets.register("generator")
30+
val generatorSourceSet: NamedDomainObjectProvider<SourceSet> = sourceSets.register("generator")
31+
32+
val externalReaderFixtureSourceSet: NamedDomainObjectProvider<SourceSet> =
33+
sourceSets.register("externalReaderFixture") {
34+
compileClasspath += sourceSets.test.get().output + sourceSets.test.get().compileClasspath
35+
runtimeClasspath += sourceSets.test.get().output + sourceSets.test.get().runtimeClasspath
36+
}
37+
38+
val externalReaderFixtureImplementation: Configuration by configurations.getting {
39+
extendsFrom(configurations.testImplementation.get())
40+
}
3141

3242
idea {
3343
module {
@@ -126,8 +136,45 @@ tasks.withType<JavaCompile>().configureEach {
126136

127137
tasks.compileKotlin { enabled = false }
128138

139+
val externalReaderFixture by tasks.registering {
140+
group = "build"
141+
dependsOn(tasks.named("compileExternalReaderFixtureJava"))
142+
inputs.files(externalReaderFixtureSourceSet.map { it.output })
143+
val fileName = if (buildInfo.os.isWindows) "externalreader.bat" else "externalreader"
144+
val outputFile = layout.buildDirectory.file("fixtures/$fileName")
145+
outputs.file(outputFile)
146+
doLast {
147+
val classpath = externalReaderFixtureSourceSet.get().runtimeClasspath.asPath
148+
val scriptContent =
149+
if (buildInfo.os.isWindows) {
150+
"""
151+
@echo off
152+
java -cp $classpath org.pkl.core.externalreaderfixture.Main
153+
"""
154+
.trimIndent()
155+
} else {
156+
"""
157+
#!/usr/bin/env bash
158+
159+
java -cp $classpath org.pkl.core.externalreaderfixture.Main
160+
"""
161+
.trimIndent()
162+
}
163+
164+
outputFile.get().asFile.writeText(scriptContent)
165+
outputFile.get().asFile.setExecutable(true)
166+
println("Created external reader ${outputFile.get().asFile.absolutePath}")
167+
}
168+
}
169+
129170
tasks.test {
130171
configureTest()
172+
dependsOn(externalReaderFixture)
173+
environment(
174+
"PATH",
175+
listOf(System.getenv("PATH"), layout.buildDirectory.dir("fixtures/").get())
176+
.joinToString(File.pathSeparator),
177+
)
131178
useJUnitPlatform {
132179
excludeEngines("MacAmd64LanguageSnippetTestsEngine")
133180
excludeEngines("MacAarch64LanguageSnippetTestsEngine")
@@ -139,6 +186,11 @@ tasks.test {
139186

140187
// testing very large lists requires more memory than the default 512m!
141188
maxHeapSize = "1g"
189+
190+
systemProperty(
191+
"org.pkl.core.testExternalReaderPath",
192+
externalReaderFixture.map { it.outputs.files.singleFile.absolutePath },
193+
)
142194
}
143195

144196
val generateBaseModuleMembers by
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright © 2026 Apple Inc. and the Pkl project authors. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.pkl.core.externalreader
17+
18+
import org.pkl.core.messaging.MessageTransports
19+
20+
private val systemInDecoder = ExternalReaderMessagePackDecoder(System.`in`)
21+
22+
private val systemOutEncoder = ExternalReaderMessagePackEncoder(System.out)
23+
24+
val stdioTransport = MessageTransports.stream(systemInDecoder, systemOutEncoder) {}

0 commit comments

Comments
 (0)