Skip to content

Commit

Permalink
Fixes to get rules_pkl to pass for larger projects (#31)
Browse files Browse the repository at this point in the history
* 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

Co-authored-by: Kushal Pisavadia <[email protected]>
  • Loading branch information
shs96c and KushalP authored Dec 5, 2024
1 parent 167889c commit c45eccb
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 1 deletion.
3 changes: 3 additions & 0 deletions pkl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion pkl/private/org/pkl_lang/bazel/symlinks/Symlinks.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static void main(String[] args) throws IOException {
}

Files.createDirectories(link.getParent());
Files.createSymbolicLink(link, target);
Files.copy(target, link);
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions pkl/private/pkl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c45eccb

Please sign in to comment.