pkl-executor: fix loading projects and fix incorrectly thrown PklException

* Load `PklProject` using the same security settings and env vars passed provided via ExecutorSPIOptions
* Handle thrown `PklException` from loading `PklProject` and throw `ExecutorSPIException`
* Handle thrown `PklException` of older Pkl distributions, converting them into `ExecutorSpiException`
This commit is contained in:
Josh B
2024-10-29 23:29:19 -07:00
committed by GitHub
parent acd2222534
commit dd16f7469e
3 changed files with 82 additions and 7 deletions

View File

@@ -116,13 +116,22 @@ public final class ExecutorSpiImpl implements ExecutorSpi {
.setTimeout(options.getTimeout())
.setOutputFormat(options.getOutputFormat())
.setModuleCacheDir(options.getModuleCacheDir());
if (options.getProjectDir() != null) {
var project = Project.loadFromPath(options.getProjectDir().resolve(PKL_PROJECT_FILENAME));
builder.setProjectDependencies(project.getDependencies());
}
try (var evaluator = builder.build()) {
return evaluator.evaluateOutputText(ModuleSource.path(modulePath));
try {
if (options.getProjectDir() != null) {
var project =
Project.loadFromPath(
options.getProjectDir().resolve(PKL_PROJECT_FILENAME),
securityManager,
null,
transformer,
options.getEnvironmentVariables());
builder.setProjectDependencies(project.getDependencies());
}
try (var evaluator = builder.build()) {
return evaluator.evaluateOutputText(ModuleSource.path(modulePath));
}
} catch (PklException e) {
throw new ExecutorSpiException(e.getMessage(), e.getCause());
} finally {

View File

@@ -227,6 +227,14 @@ final class EmbeddedExecutor implements Executor {
return executorSpi.evaluatePath(modulePath, options.toSpiOptions());
} catch (ExecutorSpiException e) {
throw new ExecutorException(e.getMessage(), e.getCause(), executorSpi.getPklVersion());
} catch (RuntimeException e) {
// This branch would ideally never be hit, but older Pkl releases (<0.27) erroneously throw
// PklException in some cases.
// Can't catch PklException directly because pkl-executor cannot depend on pkl-core.
if (e.getClass().getName().equals("org.pkl.core.PklException")) {
throw new ExecutorException(e.getMessage(), e.getCause());
}
throw e;
} finally {
currentThread.setContextClassLoader(prevContextClassLoader);
}

View File

@@ -415,6 +415,64 @@ class EmbeddedExecutorTest {
.doesNotContain(tempDir.toString())
}
@ParameterizedTest
@MethodSource("getAllTestExecutors")
fun `evaluate a module whose project evaluation fails`(
executor: TestExecutor,
@TempDir tempDir: Path
) {
// the toRealPath is important here or the failure reason can change
// this happens on macOS where /tmp is a symlink to /private/tmp
// it's related to how SecurityManagers.Standard handles canonicalizing paths that don't exist
val rootDir = tempDir.toRealPath()
val innerDir = rootDir.resolve("inner").createDirectories()
innerDir
.resolve("PklProject")
.toFile()
.writeText(
"""
amends "pkl:Project"
dependencies {
["myDep"] = import("../nonexistent/PklProject")
}
"""
.trimIndent()
)
val pklFile = innerDir.resolve("test.pkl")
pklFile
.toFile()
.writeText(
"""
@ModuleInfo { minPklVersion = "0.11.0" }
module test
foo = "bar"
"""
.trimIndent()
)
val e =
assertThrows<ExecutorException> {
executor.evaluatePath(pklFile) {
allowedModules("file:", "pkl:")
allowedResources("prop:")
projectDir(innerDir)
rootDir(rootDir)
}
}
assertThat(e.message).contains("Cannot find module").contains("/nonexistent/PklProject")
// ensure module file paths are relativized
// legacy distribution does not handle these errors with the correct stack frame transformer
// only assert on this for newer distributions
if (!executor.toString().contains("Distribution1")) {
assertThat(e.message).doesNotContain(innerDir.toString())
}
}
@ParameterizedTest
@MethodSource("getAllTestExecutors")
fun `time out a module`(executor: TestExecutor, @TempDir tempDir: Path) {
@@ -556,7 +614,7 @@ class EmbeddedExecutorTest {
)
val result =
executor.evaluatePath(pklFile) {
allowedModules("file:", "package:", "projectpackage:", "https:")
allowedModules("file:", "package:", "projectpackage:", "https:", "pkl:")
allowedResources("file:", "prop:", "package:", "projectpackage:", "https:")
moduleCacheDir(cacheDir)
projectDir(projectDir)