Skip to content

Commit a83d418

Browse files
committed
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.
1 parent e34c3e8 commit a83d418

49 files changed

Lines changed: 1859 additions & 96 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.

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: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,21 @@ 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}#workingDirectory[`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

@@ -37,11 +47,22 @@ The following APIs have been removed without replacement.
3747

3848
* `org.pkl.config.java.Config#makeConfig` (pr:https://github.com/apple/pkl/pull/1531[])
3949

40-
.XXX
41-
[%collapsible]
42-
====
43-
XXX
44-
====
50+
=== Loading rule changes in `pkl:EvaluatorSettings`
51+
52+
Breaking changes have been made to how evaluator settings are loaded (https://github.com/apple/pkl/pull/1394[#1394]).
53+
54+
==== Loading rule changes for the external reader executable
55+
56+
The following changes have been made for the `executable` property in an external reader:
57+
58+
* If the executable does not contain a slash (`/`) character, it is always resolved against the `PATH` environment variable.
59+
* If it does contain a slash, it is resolved relative to the enclosing PklProject directory, instead of the current working directory.
60+
61+
=== Changes to `--external-module-reader` and `--external-resource-reader` CLI flags
62+
63+
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]).
64+
65+
This makes this behavior consistent with how other settings work.
4566

4667
== Work In Progress [small]#🚆#
4768

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
@@ -149,10 +149,10 @@ data class CliBaseOptions(
149149
val httpHeaders: List<Pair<Pattern, List<Pair<String, String>>>>? = null,
150150

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

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

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

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

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

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

116115
protected val allowedModules: List<Pattern> by lazy {
@@ -193,11 +192,11 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
193192
}
194193

195194
protected val externalModuleReaders: Map<String, PklEvaluatorSettings.ExternalReader> by lazy {
196-
(evaluatorSettings?.externalModuleReaders ?: emptyMap()) + cliOptions.externalModuleReaders
195+
cliOptions.externalModuleReaders ?: evaluatorSettings?.externalModuleReaders ?: mapOf()
197196
}
198197

199198
protected val externalResourceReaders: Map<String, PklEvaluatorSettings.ExternalReader> by lazy {
200-
(evaluatorSettings?.externalResourceReaders ?: emptyMap()) + cliOptions.externalResourceReaders
199+
cliOptions.externalResourceReaders ?: evaluatorSettings?.externalResourceReaders ?: mapOf()
201200
}
202201

203202
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
@@ -56,7 +56,8 @@ class BaseOptions : OptionGroup() {
5656
if (IoUtils.isUriLike(moduleName)) URI(moduleName)
5757
// Can't just use URI constructor, because URI(null, null, "C:/foo/bar", null) turns
5858
// into `URI("C", null, "/foo/bar", null)`.
59-
else if (IoUtils.isWindowsAbsolutePath(moduleName)) Path.of(moduleName).toUri()
59+
else if (IoUtils.isWindows() && IoUtils.isWindowsAbsolutePath(moduleName))
60+
Path.of(moduleName).toUri()
6061
else URI(null, null, IoUtils.toNormalizedPathString(Path.of(moduleName)), null)
6162
} catch (e: URISyntaxException) {
6263
val message = buildString {
@@ -92,7 +93,7 @@ class BaseOptions : OptionGroup() {
9293
> {
9394
return splitPair(delimiter).convert {
9495
val cmd = shlex(it.second)
95-
Pair(it.first, ExternalReader(cmd.first(), cmd.drop(1)))
96+
Pair(it.first, ExternalReader(cmd.first(), cmd.drop(1), null))
9697
}
9798
}
9899
}
@@ -385,8 +386,8 @@ class BaseOptions : OptionGroup() {
385386
httpNoProxy = noProxy,
386387
httpRewrites = httpRewrites.ifEmpty { null },
387388
httpHeaders = httpHeaders.ifEmpty { null },
388-
externalModuleReaders = externalModuleReaders,
389-
externalResourceReaders = externalResourceReaders,
389+
externalModuleReaders = externalModuleReaders.ifEmpty { null },
390+
externalResourceReaders = externalResourceReaders.ifEmpty { null },
390391
traceMode = traceMode,
391392
powerAssertionsEnabled = powerAssertionsEnabled,
392393
)

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
@@ -24,7 +24,17 @@ plugins {
2424
idea
2525
}
2626

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

2939
idea {
3040
module {
@@ -109,8 +119,45 @@ tasks.compileJava { options.generatedSourceOutputDirectory.set(file("generated/t
109119

110120
tasks.compileKotlin { enabled = false }
111121

122+
val externalReaderFixture by tasks.registering {
123+
group = "build"
124+
dependsOn(tasks.named("compileExternalReaderFixtureJava"))
125+
inputs.files(externalReaderFixtureSourceSet.map { it.output })
126+
val fileName = if (buildInfo.os.isWindows) "externalreader.bat" else "externalreader"
127+
val outputFile = layout.buildDirectory.file("fixtures/$fileName")
128+
outputs.file(outputFile)
129+
doLast {
130+
val classpath = externalReaderFixtureSourceSet.get().runtimeClasspath.asPath
131+
val scriptContent =
132+
if (buildInfo.os.isWindows) {
133+
"""
134+
@echo off
135+
java -cp $classpath org.pkl.core.externalreaderfixture.Main
136+
"""
137+
.trimIndent()
138+
} else {
139+
"""
140+
#!/usr/bin/env bash
141+
142+
java -cp $classpath org.pkl.core.externalreaderfixture.Main
143+
"""
144+
.trimIndent()
145+
}
146+
147+
outputFile.get().asFile.writeText(scriptContent)
148+
outputFile.get().asFile.setExecutable(true)
149+
println("Created external reader ${outputFile.get().asFile.absolutePath}")
150+
}
151+
}
152+
112153
tasks.test {
113154
configureTest()
155+
dependsOn(externalReaderFixture)
156+
environment(
157+
"PATH",
158+
listOf(System.getenv("PATH"), layout.buildDirectory.dir("fixtures/").get())
159+
.joinToString(File.pathSeparator),
160+
)
114161
useJUnitPlatform {
115162
excludeEngines("MacAmd64LanguageSnippetTestsEngine")
116163
excludeEngines("MacAarch64LanguageSnippetTestsEngine")
@@ -122,6 +169,11 @@ tasks.test {
122169

123170
// testing very large lists requires more memory than the default 512m!
124171
maxHeapSize = "1g"
172+
173+
systemProperty(
174+
"org.pkl.core.testExternalReaderPath",
175+
externalReaderFixture.map { it.outputs.files.singleFile.absolutePath },
176+
)
125177
}
126178

127179
val testJavaExecutable by
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
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+
@file:JvmName("Main")
17+
18+
package org.pkl.core.externalreaderfixture
19+
20+
import java.net.URI
21+
import org.pkl.core.externalreader.ExternalModuleReader
22+
import org.pkl.core.externalreader.ExternalReaderClient
23+
import org.pkl.core.externalreader.ExternalReaderMessagePackDecoder
24+
import org.pkl.core.externalreader.ExternalReaderMessagePackEncoder
25+
import org.pkl.core.externalreader.ExternalResourceReader
26+
import org.pkl.core.messaging.MessageTransports
27+
import org.pkl.core.module.PathElement
28+
29+
object ModuleReader : ExternalModuleReader {
30+
override val isLocal: Boolean = true
31+
32+
override fun read(uri: URI): String = "hello"
33+
34+
override val scheme: String = "foo"
35+
36+
override val hasHierarchicalUris: Boolean = false
37+
38+
override val isGlobbable: Boolean = false
39+
40+
override fun listElements(uri: URI): List<PathElement> {
41+
throw NotImplementedError()
42+
}
43+
}
44+
45+
object ResourceReader : ExternalResourceReader {
46+
override fun read(uri: URI): ByteArray = "hello".toByteArray()
47+
48+
override val scheme: String = "foo"
49+
50+
override val hasHierarchicalUris: Boolean = false
51+
52+
override val isGlobbable: Boolean = false
53+
54+
override fun listElements(uri: URI): List<PathElement> {
55+
throw NotImplementedError()
56+
}
57+
}
58+
59+
fun main() {
60+
val transport =
61+
MessageTransports.stream(
62+
ExternalReaderMessagePackDecoder(System.`in`),
63+
ExternalReaderMessagePackEncoder(System.out),
64+
) {}
65+
val client = ExternalReaderClient(listOf(ModuleReader), listOf(ResourceReader), transport)
66+
client.run()
67+
}

pkl-core/src/main/java/org/pkl/core/EvaluatorBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ public boolean getPowerAssertionsEnabled() {
488488
*/
489489
public EvaluatorBuilder applyFromProject(Project project) {
490490
this.dependencies = project.getDependencies();
491-
var settings = project.getEvaluatorSettings();
491+
var settings = project.getResolvedEvaluatorSettings();
492492
if (securityManager != null) {
493493
throw new IllegalStateException(
494494
"Cannot call both `setSecurityManager` and `setProject`, because both define security manager settings. Call `setProjectOnly` if the security manager is desired.");

0 commit comments

Comments
 (0)