Executor SPI changes

changes.diff

diff --git a/pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java b/pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java
index e1b6d236f..2fdb39b82 100644
--- a/pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java
+++ b/pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java
@@ -37,11 +37,9 @@ import org.pkl.core.resource.ResourceReaders;
 import org.pkl.executor.spi.v1.ExecutorSpi;
 import org.pkl.executor.spi.v1.ExecutorSpiException;
 import org.pkl.executor.spi.v1.ExecutorSpiOptions;
-import org.pkl.executor.spi.v2.ExecutorSpi2;
-import org.pkl.executor.spi.v2.ExecutorSpiException2;
-import org.pkl.executor.spi.v2.ExecutorSpiOptions2;
+import org.pkl.executor.spi.v1.ExecutorSpiOptions2;
 
-public class ExecutorSpiImpl implements ExecutorSpi, ExecutorSpi2 {
+public class ExecutorSpiImpl implements ExecutorSpi {
   private static final int MAX_HTTP_CLIENTS = 3;
 
   // Don't create a new HTTP client for every executor request.
@@ -70,94 +68,33 @@ public class ExecutorSpiImpl implements ExecutorSpi, ExecutorSpi2 {
 
   @Override
   public String evaluatePath(Path modulePath, ExecutorSpiOptions options) {
-    var builder =
-        createEvaluatorBuilder(
-            options.getAllowedModules(),
-            options.getAllowedResources(),
-            options.getRootDir(),
-            options.getModulePath(),
-            options.getEnvironmentVariables(),
-            options.getExternalProperties(),
-            options.getTimeout(),
-            options.getOutputFormat(),
-            options.getModuleCacheDir(),
-            options.getProjectDir(),
-            List.of(),
-            List.of());
 
-    try (var evaluator = builder.build()) {
-      return evaluator.evaluateOutputText(ModuleSource.path(modulePath));
-    } catch (PklException e) {
-      throw new ExecutorSpiException(e.getMessage(), e.getCause());
-    } finally {
-      ModuleKeyFactories.closeQuietly(builder.getModuleKeyFactories());
-    }
-  }
+    var allowedModules =
+        options.getAllowedModules().stream().map(Pattern::compile).collect(Collectors.toList());
 
-  @Override
-  public String evaluatePath(Path modulePath, ExecutorSpiOptions2 options) {
-    var builder =
-        createEvaluatorBuilder(
-            options.getAllowedModules(),
-            options.getAllowedResources(),
-            options.getRootDir(),
-            options.getModulePath(),
-            options.getEnvironmentVariables(),
-            options.getExternalProperties(),
-            options.getTimeout(),
-            options.getOutputFormat(),
-            options.getModuleCacheDir(),
-            options.getProjectDir(),
-            options.getCertificateFiles(),
-            options.getCertificateUris());
-
-    try (var evaluator = builder.build()) {
-      return evaluator.evaluateOutputText(ModuleSource.path(modulePath));
-    } catch (PklException e) {
-      throw new ExecutorSpiException2(e.getMessage(), e.getCause());
-    } finally {
-      ModuleKeyFactories.closeQuietly(builder.getModuleKeyFactories());
-    }
-  }
-
-  private EvaluatorBuilder createEvaluatorBuilder(
-      List<String> allowedModules,
-      List<String> allowedResources,
-      Path rootDir,
-      List<Path> modulePath,
-      Map<String, String> environmentVariables,
-      Map<String, String> externalProperties,
-      java.time.Duration timeout,
-      String outputFormat,
-      Path moduleCacheDir,
-      Path projectDir,
-      List<Path> certificateFiles,
-      List<URI> certificateUris) {
-    var allowedModulePatterns =
-        allowedModules.stream().map(Pattern::compile).collect(Collectors.toList());
-
-    var allowedResourcePatterns =
-        allowedResources.stream().map(Pattern::compile).collect(Collectors.toList());
+    var allowedResources =
+        options.getAllowedResources().stream().map(Pattern::compile).collect(Collectors.toList());
 
     var securityManager =
         SecurityManagers.standard(
-            allowedModulePatterns,
-            allowedResourcePatterns,
+            allowedModules,
+            allowedResources,
             SecurityManagers.defaultTrustLevels,
-            rootDir);
+            options.getRootDir());
 
     var transformer = StackFrameTransformers.defaultTransformer;
-    if (rootDir != null) {
+    if (options.getRootDir() != null) {
       transformer =
-          transformer.andThen(StackFrameTransformers.relativizeModuleUri(rootDir.toUri()));
+          transformer.andThen(
+              StackFrameTransformers.relativizeModuleUri(options.getRootDir().toUri()));
     }
 
-    var resolver = new ModulePathResolver(modulePath);
+    var resolver = new ModulePathResolver(options.getModulePath());
+
     var builder =
         EvaluatorBuilder.unconfigured()
             .setStackFrameTransformer(transformer)
             .setSecurityManager(securityManager)
-            .setHttpClient(getOrCreateHttpClient(certificateFiles, certificateUris))
             .addResourceReader(ResourceReaders.environmentVariable())
             .addResourceReader(ResourceReaders.externalProperty())
             .addResourceReader(ResourceReaders.modulePath(resolver))
@@ -173,18 +110,29 @@ public class ExecutorSpiImpl implements ExecutorSpi, ExecutorSpi2 {
             .addModuleKeyFactory(ModuleKeyFactories.projectpackage)
             .addModuleKeyFactory(ModuleKeyFactories.file)
             .addModuleKeyFactory(ModuleKeyFactories.genericUrl)
-            .setEnvironmentVariables(environmentVariables)
-            .setExternalProperties(externalProperties)
-            .setTimeout(timeout)
-            .setOutputFormat(outputFormat)
-            .setModuleCacheDir(moduleCacheDir);
-
-    if (projectDir != null) {
-      var project = Project.loadFromPath(projectDir.resolve(PKL_PROJECT_FILENAME));
+            .setEnvironmentVariables(options.getEnvironmentVariables())
+            .setExternalProperties(options.getExternalProperties())
+            .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());
     }
+    if (options instanceof ExecutorSpiOptions2) {
+      builder.setHttpClient(
+          getOrCreateHttpClient(
+              ((ExecutorSpiOptions2) options).getCertificateFiles(),
+              ((ExecutorSpiOptions2) options).getCertificateUris()));
+    }
 
-    return builder;
+    try (var evaluator = builder.build()) {
+      return evaluator.evaluateOutputText(ModuleSource.path(modulePath));
+    } catch (PklException e) {
+      throw new ExecutorSpiException(e.getMessage(), e.getCause());
+    } finally {
+      ModuleKeyFactories.closeQuietly(builder.getModuleKeyFactories());
+    }
   }
 
   private HttpClient getOrCreateHttpClient(List<Path> certificateFiles, List<URI> certificateUris) {
diff --git a/pkl-core/src/main/resources/META-INF/services/org.pkl.executor.spi.v2.ExecutorSpi2 b/pkl-core/src/main/resources/META-INF/services/org.pkl.executor.spi.v2.ExecutorSpi2
deleted file mode 100644
index 57c9344b3..000000000
--- a/pkl-core/src/main/resources/META-INF/services/org.pkl.executor.spi.v2.ExecutorSpi2
+++ /dev/null
@@ -1 +0,0 @@
-org.pkl.core.service.ExecutorSpiImpl
diff --git a/pkl-executor/src/main/java/org/pkl/executor/EmbeddedExecutor.java b/pkl-executor/src/main/java/org/pkl/executor/EmbeddedExecutor.java
index df7d8cc85..7f2e0df69 100644
--- a/pkl-executor/src/main/java/org/pkl/executor/EmbeddedExecutor.java
+++ b/pkl-executor/src/main/java/org/pkl/executor/EmbeddedExecutor.java
@@ -27,10 +27,7 @@ import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import org.pkl.executor.spi.v1.ExecutorSpi;
 import org.pkl.executor.spi.v1.ExecutorSpiException;
-import org.pkl.executor.spi.v1.ExecutorSpiOptions;
-import org.pkl.executor.spi.v2.ExecutorSpi2;
-import org.pkl.executor.spi.v2.ExecutorSpiException2;
-import org.pkl.executor.spi.v2.ExecutorSpiOptions2;
+import org.pkl.executor.spi.v1.ExecutorSpiOptions2;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -47,13 +44,8 @@ final class EmbeddedExecutor implements Executor {
    *     distribution
    */
   public EmbeddedExecutor(List<Path> pklFatJars) {
-    this(pklFatJars, 0);
-  }
-
-  // for testing only
-  EmbeddedExecutor(List<Path> pklFatJars, int spiVersion) {
     for (var jarFile : pklFatJars) {
-      pklDistributions.add(new PklDistribution(jarFile, spiVersion));
+      pklDistributions.add(new PklDistribution(jarFile));
     }
   }
 
@@ -172,53 +164,29 @@ final class EmbeddedExecutor implements Executor {
 
   private static final class PklDistribution implements AutoCloseable {
     final URLClassLoader pklDistributionClassLoader;
-    final /* @Nullable */ ExecutorSpi executorSpi;
-    final /* @Nullable */ ExecutorSpi2 executorSpi2;
+    final ExecutorSpi executorSpi;
     final Version version;
 
     /**
      * @throws IllegalArgumentException if the Jar file does not exist or is not a valid Pkl
      *     distribution
      */
-    PklDistribution(Path pklFatJar, /* 0 -> choose highest available */ int spiVersion) {
+    PklDistribution(Path pklFatJar) {
       if (!Files.isRegularFile(pklFatJar)) {
         throw new IllegalArgumentException(
             String.format("Invalid Pkl distribution: Cannot find Jar file `%s`.", pklFatJar));
       }
 
       pklDistributionClassLoader = new PklDistributionClassLoader(pklFatJar);
+      var serviceLoader = ServiceLoader.load(ExecutorSpi.class, pklDistributionClassLoader);
 
       try {
-        if (spiVersion == 0 || spiVersion == 2) {
-          var serviceLoader2 = ServiceLoader.load(ExecutorSpi2.class, pklDistributionClassLoader);
-          executorSpi2 = serviceLoader2.findFirst().orElse(null);
-          if (executorSpi2 != null) {
-            executorSpi = null;
-            // convert to normal to allow running with a dev version
-            version = Version.parse(executorSpi2.getPklVersion()).toNormal();
-            return;
-          }
-        } else {
-          executorSpi2 = null;
-        }
-        if (spiVersion == 0 || spiVersion == 1) {
-          var serviceLoader = ServiceLoader.load(ExecutorSpi.class, pklDistributionClassLoader);
-          executorSpi = serviceLoader.findFirst().orElse(null);
-          if (executorSpi != null) {
-            // convert to normal to allow running with a dev version
-            version = Version.parse(executorSpi.getPklVersion()).toNormal();
-            return;
-          }
-        } else {
-          executorSpi = null;
-        }
-        if (spiVersion == 0) {
-          throw new IllegalArgumentException(
-              String.format(
-                  "Invalid Pkl distribution: Cannot find service of type `%s` in Jar file `%s`.",
-                  ExecutorSpi.class.getTypeName(), pklFatJar));
-        }
-        throw new IllegalArgumentException("Invalid SPI version: " + spiVersion);
+        executorSpi = serviceLoader.iterator().next();
+      } catch (NoSuchElementException e) {
+        throw new IllegalArgumentException(
+            String.format(
+                "Invalid Pkl distribution: Cannot find service of type `%s` in Jar file `%s`.",
+                ExecutorSpi.class.getTypeName(), pklFatJar));
       } catch (ServiceConfigurationError e) {
         throw new IllegalArgumentException(
             String.format(
@@ -226,6 +194,8 @@ final class EmbeddedExecutor implements Executor {
                 ExecutorSpi.class.getTypeName(), pklFatJar),
             e);
       }
+      // convert to normal to allow running with a dev version
+      version = Version.parse(executorSpi.getPklVersion()).toNormal();
     }
 
     Version getVersion() {
@@ -238,10 +208,8 @@ final class EmbeddedExecutor implements Executor {
       // Truffle loads stuff from context class loader, so set it to our class loader
       currentThread.setContextClassLoader(pklDistributionClassLoader);
       try {
-        return executorSpi2 != null
-            ? executorSpi2.evaluatePath(modulePath, toSpiOptions2(options))
-            : executorSpi.evaluatePath(modulePath, toSpiOptions(options));
-      } catch (ExecutorSpiException | ExecutorSpiException2 e) {
+        return executorSpi.evaluatePath(modulePath, toSpiOptions(options));
+      } catch (ExecutorSpiException e) {
         throw new ExecutorException(e.getMessage(), e.getCause());
       } finally {
         currentThread.setContextClassLoader(prevContextClassLoader);
@@ -253,21 +221,7 @@ final class EmbeddedExecutor implements Executor {
       pklDistributionClassLoader.close();
     }
 
-    ExecutorSpiOptions toSpiOptions(ExecutorOptions options) {
-      return new ExecutorSpiOptions(
-          options.getAllowedModules(),
-          options.getAllowedResources(),
-          options.getEnvironmentVariables(),
-          options.getExternalProperties(),
-          options.getModulePath(),
-          options.getRootDir(),
-          options.getTimeout(),
-          options.getOutputFormat(),
-          options.getModuleCacheDir(),
-          options.getProjectDir());
-    }
-
-    ExecutorSpiOptions2 toSpiOptions2(ExecutorOptions options) {
+    ExecutorSpiOptions2 toSpiOptions(ExecutorOptions options) {
       return new ExecutorSpiOptions2(
           options.getAllowedModules(),
           options.getAllowedResources(),
diff --git a/pkl-executor/src/main/java/org/pkl/executor/spi/v1/ExecutorSpiOptions2.java b/pkl-executor/src/main/java/org/pkl/executor/spi/v1/ExecutorSpiOptions2.java
new file mode 100644
index 000000000..c4ac5e335
--- /dev/null
+++ b/pkl-executor/src/main/java/org/pkl/executor/spi/v1/ExecutorSpiOptions2.java
@@ -0,0 +1,64 @@
+/**
+ * Copyright © 2024 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.executor.spi.v1;
+
+import java.net.URI;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.List;
+import java.util.Map;
+
+public class ExecutorSpiOptions2 extends ExecutorSpiOptions {
+
+  private final List<Path> certificateFiles;
+  private final List<URI> certificateUris;
+
+  public ExecutorSpiOptions2(
+      List<String> allowedModules,
+      List<String> allowedResources,
+      List<Path> modulePath,
+      Path rootDir,
+      Duration timeout,
+      /* @Nullable */ String outputFormat,
+      /* @Nullable */ Path moduleCacheDir,
+      /* @Nullable */ Path projectDir,
+      /* @Nullable */ Map<String, String> environmentVariables,
+      /* @Nullable */ Map<String, String> externalProperties,
+      List<Path> certificateFiles,
+      List<URI> certificateUris) {
+    super(
+        allowedModules,
+        allowedResources,
+        environmentVariables,
+        externalProperties,
+        modulePath,
+        rootDir,
+        timeout,
+        outputFormat,
+        moduleCacheDir,
+        projectDir);
+    this.certificateFiles = certificateFiles;
+    this.certificateUris = certificateUris;
+  }
+
+  public List<Path> getCertificateFiles() {
+    return certificateFiles;
+  }
+
+  public List<URI> getCertificateUris() {
+    return certificateUris;
+  }
+}
diff --git a/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpi2.java b/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpi2.java
deleted file mode 100644
index c049afa96..000000000
--- a/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpi2.java
+++ /dev/null
@@ -1,29 +0,0 @@
-/**
- * Copyright © 2024 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.executor.spi.v2;
-
-import java.nio.file.Path;
-
-public interface ExecutorSpi2 {
-  String getPklVersion();
-
-  /**
-   * Evaluation the Pkl module with the given path, returning the module's output.
-   *
-   * <p>If evaluation fails, throws {@link ExecutorSpiException2} with a descriptive message.
-   */
-  String evaluatePath(Path modulePath, ExecutorSpiOptions2 options);
-}
diff --git a/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpiException2.java b/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpiException2.java
deleted file mode 100644
index 77317ea55..000000000
--- a/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpiException2.java
+++ /dev/null
@@ -1,22 +0,0 @@
-/**
- * Copyright © 2024 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.executor.spi.v2;
-
-public final class ExecutorSpiException2 extends RuntimeException {
-  public ExecutorSpiException2(String message, Throwable cause) {
-    super(message, cause);
-  }
-}
diff --git a/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpiOptions2.java b/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpiOptions2.java
deleted file mode 100644
index 2234d92ba..000000000
--- a/pkl-executor/src/main/java/org/pkl/executor/spi/v2/ExecutorSpiOptions2.java
+++ /dev/null
@@ -1,120 +0,0 @@
-/**
- * Copyright © 2024 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.executor.spi.v2;
-
-import java.net.URI;
-import java.nio.file.Path;
-import java.time.Duration;
-import java.util.List;
-import java.util.Map;
-
-public class ExecutorSpiOptions2 {
-  private final List<String> allowedModules;
-
-  private final List<String> allowedResources;
-
-  private final Map<String, String> environmentVariables;
-
-  private final Map<String, String> externalProperties;
-
-  private final List<Path> modulePath;
-
-  private final Path rootDir;
-
-  private final Duration timeout;
-
-  private final String outputFormat;
-  private final Path moduleCacheDir;
-  private final Path projectDir;
-
-  private final List<Path> certificateFiles;
-  private final List<URI> certificateUris;
-
-  public ExecutorSpiOptions2(
-      List<String> allowedModules,
-      List<String> allowedResources,
-      List<Path> modulePath,
-      Path rootDir,
-      Duration timeout,
-      /* @Nullable */ String outputFormat,
-      /* @Nullable */ Path moduleCacheDir,
-      /* @Nullable */ Path projectDir,
-      /* @Nullable */ Map<String, String> environmentVariables,
-      /* @Nullable */ Map<String, String> externalProperties,
-      List<Path> certificateFiles,
-      List<URI> certificateUris) {
-    this.allowedModules = allowedModules;
-    this.allowedResources = allowedResources;
-    this.environmentVariables = environmentVariables;
-    this.externalProperties = externalProperties;
-    this.modulePath = modulePath;
-    this.rootDir = rootDir;
-    this.timeout = timeout;
-    this.outputFormat = outputFormat;
-    this.moduleCacheDir = moduleCacheDir;
-    this.projectDir = projectDir;
-    this.certificateFiles = certificateFiles;
-    this.certificateUris = certificateUris;
-  }
-
-  public List<String> getAllowedModules() {
-    return allowedModules;
-  }
-
-  public List<String> getAllowedResources() {
-    return allowedResources;
-  }
-
-  public Map<String, String> getEnvironmentVariables() {
-    return environmentVariables;
-  }
-
-  public Map<String, String> getExternalProperties() {
-    return externalProperties;
-  }
-
-  public List<Path> getModulePath() {
-    return modulePath;
-  }
-
-  public /* @Nullable */ Path getRootDir() {
-    return rootDir;
-  }
-
-  public /* @Nullable */ Duration getTimeout() {
-    return timeout;
-  }
-
-  public /* @Nullable */ String getOutputFormat() {
-    return outputFormat;
-  }
-
-  public /* @Nullable */ Path getModuleCacheDir() {
-    return moduleCacheDir;
-  }
-
-  public Path getProjectDir() {
-    return projectDir;
-  }
-
-  public List<Path> getCertificateFiles() {
-    return certificateFiles;
-  }
-
-  public List<URI> getCertificateUris() {
-    return certificateUris;
-  }
-}
diff --git a/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt b/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt
index 797c46f64..5346a53bf 100644
--- a/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt
+++ b/pkl-executor/src/test/kotlin/org/pkl/executor/EmbeddedExecutorTest.kt
@@ -19,7 +19,9 @@ import kotlin.io.path.createDirectories
 class EmbeddedExecutorTest {
   companion object {
     @JvmStatic
-    fun allExecutors(): List<Executor> = listOf(executor.value, executor1.value)
+    fun allExecutors(): List<Executor> = listOf(
+      Executors.embedded(listOf(pklDistribution)), 
+      EmbeddedExecutor(listOf(pklDistribution)))
 
     @JvmStatic
     fun executorsSinceV2(): List<Executor> = listOf(executor.value)
@@ -36,7 +38,7 @@ class EmbeddedExecutorTest {
     }
     
     private val executor1: Lazy<Executor> = lazy {
-      EmbeddedExecutor(listOf(pklDistribution), 1)
+      EmbeddedExecutor(listOf(pklDistribution))
     }
     
     private val pklDistribution: Path by lazy {