Module cache directory propagation (#1900)

Currently the module cache directory in CLI commands (and thus in Gradle tasks, since they use CLI commands under the hood) is only used for the "primary" task. For example, for an eval command, the module cache directory will be used for the evaluation of the target module.

However, most commands also work with projects, and for projects the `PklProject` file must be evaluated. This is done via `Project.loadFromPath`, which internally builds an evaluator with the default module cache path. Therefore, all evaluations of project manifests always use default cache path instead of whatever is configured through the CLI option/Gradle task property. This, in turn, may result in unexpected behavior: in general, the user would expect that if they specify the cache directory, it will be used for all evaluations (e.g. when they pre-populate the cache and expect evaluations to work offline).

The same is true for settings evaluation, although for settings it is significantly less important since settings scripts can't have remote dependencies.

This PR propagates cache path into the project evaluator (as well as the settings evaluator, for consistency). This will result in the user-specified cache directory being used for `PklProject` evaluations as well.

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

@@ -69,9 +69,9 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
   protected val settings: PklSettings by lazy {
     try {
       if (cliOptions.normalizedSettingsModule != null) {
-        PklSettings.load(ModuleSource.uri(cliOptions.normalizedSettingsModule))
+        PklSettings.load(ModuleSource.uri(cliOptions.normalizedSettingsModule), cliModuleCacheDir)
       } else {
-        PklSettings.loadFromSystem()
+        PklSettings.loadFromSystem(cliModuleCacheDir)
       }
     } catch (e: PklException) {
       // do not use `errorRenderer` because it depends on `settings`
@@ -107,9 +107,19 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
       stackFrameTransformer,
       envVars,
       cliOptions.powerAssertionsEnabled,
+      cliModuleCacheDir,
     )
   }
 
+  /**
+   * The module cache dir as determined by CLI options alone. Used to evaluate the settings and
+   * project files, which are loaded before project evaluator settings are known.
+   */
+  private val cliModuleCacheDir: Path? by lazy {
+    if (cliOptions.noCache) null
+    else cliOptions.normalizedModuleCacheDir ?: IoUtils.getSystemModuleCacheDir()
+  }
+
   private val evaluatorSettings: PklEvaluatorSettings? by lazy {
     @Suppress("PklCliDirectProjectEvaluatorSettingsAccess")
     if (cliOptions.omitProjectSettings) null else project?.resolvedEvaluatorSettings

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

@@ -26,6 +26,7 @@ import org.junit.jupiter.api.condition.JRE
 import org.junit.jupiter.api.io.TempDir
 import org.pkl.commons.cli.commands.BaseCommand
 import org.pkl.commons.cli.commands.ProjectOptions
+import org.pkl.commons.test.PackageServer
 import org.pkl.commons.writeString
 import org.pkl.core.SecurityManagers
 import org.pkl.core.evaluatorSettings.PklEvaluatorSettings
@@ -134,7 +135,9 @@ class CliCommandTest {
   @DisabledOnJre(JRE.JAVA_22, JRE.JAVA_23, JRE.JAVA_24)
   fun `test that --omit-project-settings actually omits project settings`(@TempDir tempDir: Path) {
     val project = tempDir.resolve("PklProject").writeString(projectWithAllEvaluatorSettings)
-    cmd.parse(arrayOf("--working-dir=$tempDir", "--omit-project-settings"))
+    cmd.parse(
+      arrayOf("--working-dir=$tempDir", "--omit-project-settings", "--settings=pkl:settings")
+    )
     val opts =
       cmd.baseOptions.baseOptions(listOf(project.toUri()), cmd.projectOptions, testMode = true)
     val cliTest = CliTest(opts)
@@ -200,4 +203,31 @@ class CliCommandTest {
     assertThat(cliTest.myExternalModuleReaders)
       .isEqualTo(mapOf("bar" to PklEvaluatorSettings.ExternalReader("bar", listOf(), null)))
   }
+
+  @Test
+  fun `--cache-dir is used when loading the project`(@TempDir tempDir: Path) {
+    val cacheDir = tempDir.resolve("cache").also(PackageServer::populateCacheDir)
+    tempDir
+      .resolve("PklProject")
+      .writeString(
+        // language=pkl
+        """
+        amends "pkl:Project"
+
+        import "package://localhost:0/fruit@1.0.5#/catalog/apple.pkl"
+
+        evaluatorSettings {
+          externalProperties {
+            ["fruit"] = apple.name
+          }
+        }
+        """
+          .trimIndent()
+      )
+    cmd.parse(arrayOf("--working-dir=$tempDir", "--cache-dir=$cacheDir"))
+    val opts = cmd.baseOptions.baseOptions(emptyList(), cmd.projectOptions, testMode = true)
+    val cliTest = CliTest(opts)
+    assertThat(cliTest.myProjectEvaluatorSettings!!.externalProperties)
+      .containsEntry("fruit", "Apple")
+  }
 }

pkl-core/src/main/java/org/pkl/core/project/Project.java

@@ -84,6 +84,34 @@ public final class Project {
       StackFrameTransformer stackFrameTransformer,
       Map<String, String> envVars,
       boolean powerAssertionsEnabled) {
+    return loadFromPath(
+        path,
+        securityManager,
+        timeout,
+        stackFrameTransformer,
+        envVars,
+        powerAssertionsEnabled,
+        IoUtils.getSystemModuleCacheDir());
+  }
+
+  /**
+   * Loads Project data from the given {@link Path}.
+   *
+   * <p>Evaluates a module's {@code output.value} to allow for embedding a project within a
+   * template.
+   *
+   * @param moduleCacheDir the directory where packages imported by the project file are cached;
+   *     {@code null} disables caching
+   * @throws PklException if an error occurred while evaluating the project file.
+   */
+  public static Project loadFromPath(
+      Path path,
+      SecurityManager securityManager,
+      java.time.@Nullable Duration timeout,
+      StackFrameTransformer stackFrameTransformer,
+      Map<String, String> envVars,
+      boolean powerAssertionsEnabled,
+      @Nullable Path moduleCacheDir) {
     try (var evaluator =
         EvaluatorBuilder.unconfigured()
             .setSecurityManager(securityManager)
@@ -97,6 +125,7 @@ public final class Project {
             .addEnvironmentVariables(envVars)
             .setTimeout(timeout)
             .setPowerAssertionsEnabled(powerAssertionsEnabled)
+            .setModuleCacheDir(moduleCacheDir)
             .build()) {
       return load(evaluator, ModuleSource.path(path));
     }

pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java

@@ -125,7 +125,9 @@ public final class ExecutorSpiImpl implements ExecutorSpi {
                 securityManager,
                 null,
                 transformer,
-                options.getEnvironmentVariables());
+                options.getEnvironmentVariables(),
+                false,
+                options.getModuleCacheDir());
         builder.setProjectDependencies(project.getDependencies());
       }
 

pkl-core/src/main/java/org/pkl/core/settings/PklSettings.java

@@ -53,10 +53,20 @@ public record PklSettings(Editor editor, PklEvaluatorSettings.@Nullable Http htt
    * <p>If neither file exists, returns default settings defined by module {@code pkl.settings}.
    */
   public static PklSettings loadFromSystem() throws VmEvalException {
+    return loadFromSystem(IoUtils.getSystemModuleCacheDir());
+  }
+
+  /**
+   * Loads the user settings file, using the given module cache directory.
+   *
+   * @param moduleCacheDir the directory where packages are cached; {@code null} disables caching
+   * @see #loadFromSystem()
+   */
+  public static PklSettings loadFromSystem(@Nullable Path moduleCacheDir) throws VmEvalException {
     var file = IoUtils.getSystemSettingsFile();
     if (Files.exists(file)) {
       DebugLogger.log("Loading settings file from " + file.normalize().toAbsolutePath());
-      return load(ModuleSource.path(file));
+      return load(ModuleSource.path(file), moduleCacheDir);
     }
     return defaultInstance;
   }
@@ -89,6 +99,16 @@ public record PklSettings(Editor editor, PklEvaluatorSettings.@Nullable Http htt
 
   /** Loads a settings file from the given path. */
   public static PklSettings load(ModuleSource moduleSource) throws VmEvalException {
+    return load(moduleSource, IoUtils.getSystemModuleCacheDir());
+  }
+
+  /**
+   * Loads a settings file from the given path, using the given module cache directory.
+   *
+   * @param moduleCacheDir the directory where packages are cached; {@code null} disables caching
+   */
+  public static PklSettings load(ModuleSource moduleSource, @Nullable Path moduleCacheDir)
+      throws VmEvalException {
     try (var evaluator =
         EvaluatorBuilder.unconfigured()
             .setSecurityManager(
@@ -100,6 +120,7 @@ public record PklSettings(Editor editor, PklEvaluatorSettings.@Nullable Http htt
             .addResourceReader(ResourceReaders.environmentVariable())
             .addResourceReader(ResourceReaders.file())
             .addEnvironmentVariables(System.getenv())
+            .setModuleCacheDir(moduleCacheDir)
             .build()) {
       var module = evaluator.evaluateOutputValueAs(moduleSource, PClassInfo.Settings);
       return parseSettings(module, moduleSource);

pkl-core/src/test/kotlin/org/pkl/core/project/ProjectTest.kt

@@ -362,4 +362,56 @@ class ProjectTest {
     assertThat(project.evaluatorSettings.externalModuleReaders?.get("foo")!!.executable())
       .isEqualTo("my-command")
   }
+
+  @Test
+  fun `loadFromPath uses the given module cache dir`(@TempDir tempDir: Path) {
+    val cacheDir = tempDir.resolve("cache").also(PackageServer::populateCacheDir)
+    val pklProject = tempDir.resolve("PklProject").writeString(PROJECT_IMPORTING_PACKAGE)
+    val project =
+      Project.loadFromPath(
+        pklProject,
+        SecurityManagers.defaultManager,
+        null,
+        StackFrameTransformers.empty,
+        mapOf(),
+        false,
+        cacheDir,
+      )
+    assertThat(project.evaluatorSettings.externalProperties).containsEntry("fruit", "Apple")
+  }
+
+  @Test
+  fun `loadFromPath does not use a cache if module cache dir is null`(@TempDir tempDir: Path) {
+    val pklProject = tempDir.resolve("PklProject").writeString(PROJECT_IMPORTING_PACKAGE)
+    assertThrows<PklException> {
+      Project.loadFromPath(
+        pklProject,
+        SecurityManagers.defaultManager,
+        null,
+        StackFrameTransformers.empty,
+        mapOf(),
+        false,
+        null,
+      )
+    }
+  }
+
+  companion object {
+    // `localhost:0` is never reachable without a test HTTP client, so this package can only be
+    // resolved from a pre-populated cache dir.
+    private val PROJECT_IMPORTING_PACKAGE =
+      // language=pkl
+      """
+      amends "pkl:Project"
+
+      import "package://localhost:0/fruit@1.0.5#/catalog/apple.pkl"
+
+      evaluatorSettings {
+        externalProperties {
+          ["fruit"] = apple.name
+        }
+      }
+      """
+        .trimIndent()
+  }
 }