From 444faf4ef53f86b984277a03c32f18c1b8f2c890 Mon Sep 17 00:00:00 2001
From: Kushal Pisavadia <kushal.p@apple.com>
Date: Mon, 22 Jul 2024 13:24:03 +0100
Subject: [PATCH] Fixes to get `rules_pkl` to pass for larger projects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Correct cache directory for `pkl_test` and `pkl_test_suite` targets

These two targets appear to receive an incorrect (relative) path for
the cache directory.

They're prefixed with the `ctx.genfiles_dir.path` and this leads to an
error when providing a `pkl_project` repository alongside any number
of `pkl_library` targets as dependencoes.

* Copy files, don't symlink them so that Pkl can pick them up

We have lots of symlinks within the `--working-dir` that we pass to
Pkl. This means that any code that's using globbed reads `read*`,
or globbed imports `import*` can’t find the files that it requires.

This has been something that Pkl (and Pcl before it) haven’t
supported, so I’m curious how we ended up with this regression or what
we did. Here’s the bit of code in Pkl where symlinks are ignored:
https://github.com/apple/pkl/blob/e81a47a03807739f8ab69eb8c595b53ec0afb434/pkl-core/src/main/java/org/pkl/core/module/FileResolver.java#L39-L42

Not having a solution for this will be a deal breaker for some
projects as they will have a number of `pkl_library` targets that
they progressively depend upon and expect to have present when we
`pkl_eval` or `pkl_test`.

My understanding is that supporting this in Pkl is non-trivial right
now, and requires work on cycle detection.

I’ve provided a patch to the `Symlinks.java` code which copies files
over rather than symlinking them.

* Allow up to 80% of RAM to be used
---
 pkl/BUILD.bazel                                       | 3 +++
 pkl/private/org/pkl_lang/bazel/symlinks/Symlinks.java | 2 +-
 pkl/private/pkl.bzl                                   | 4 ++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/pkl/BUILD.bazel b/pkl/BUILD.bazel
index efdf8d2..ec20925 100644
--- a/pkl/BUILD.bazel
+++ b/pkl/BUILD.bazel
@@ -99,6 +99,9 @@ toolchain(
 
 java_binary(
     name = "pkl_cli_java",
+    jvm_flags = [
+        "-XX:MaxRAMPercentage=80.0",
+    ],
     main_class = "org.pkl.cli.Main",
     runtime_deps = [
         artifact(
diff --git a/pkl/private/org/pkl_lang/bazel/symlinks/Symlinks.java b/pkl/private/org/pkl_lang/bazel/symlinks/Symlinks.java
index 2a94b43..ef9adb5 100644
--- a/pkl/private/org/pkl_lang/bazel/symlinks/Symlinks.java
+++ b/pkl/private/org/pkl_lang/bazel/symlinks/Symlinks.java
@@ -60,7 +60,7 @@ public static void main(String[] args) throws IOException {
                 }
 
                 Files.createDirectories(link.getParent());
-                Files.createSymbolicLink(link, target);
+                Files.copy(target, link);
             }
         }
     }
diff --git a/pkl/private/pkl.bzl b/pkl/private/pkl.bzl
index 9749199..2e6b24c 100644
--- a/pkl/private/pkl.bzl
+++ b/pkl/private/pkl.bzl
@@ -62,6 +62,10 @@ def _prepare_pkl_script(ctx, is_test_target):
                 path_to_symlink_target[file.path] = path
 
     cache_root_path, caches, cache_deps = root_caches_and_dependencies(ctx.attr.srcs + ctx.attr.deps)
+
+    if is_test_target and cache_root_path != None:
+        cache_root_path = cache_root_path.removeprefix(ctx.genfiles_dir.path)[1:]
+
     if len(caches):
         path_to_symlink_target[caches[0].pkl_project.path] = "%s/PklProject" % working_dir
         path_to_symlink_target[caches[0].pkl_project_deps.path] = "%s/PklProject.deps.json" % working_dir