fix(pkg): resolve symlinks from (install) action #13239
Conversation
Signed-off-by: Ali Caglayan <alizter@gmail.com>
| > (version 0.0.1) | ||
| > (build | ||
| > (progn | ||
| > (run mkdir -p %{lib}/%{pkg-self:name}) |
There was a problem hiding this comment.
Here's a suggestion to improve the readability:
Instead of adding a run for every single command, why not a single bash action with a multi line string? They were made for exactly this purpose.
There was a problem hiding this comment.
We don't support (bash) in lock files.
There was a problem hiding this comment.
Do you want them in a single sh -c or line by line?
There was a problem hiding this comment.
If it was line by line, I think it would defeat the purpose of the change. Since you're already using the shell, use it all the way and invoke it only once do all the work.
There was a problem hiding this comment.
Similarly, in the actions where you don't use the shell (I think there's a couple?) you can leave things as is and use the dune language for actions.
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Alizter
force-pushed
the
push-stqwvouvzytt
branch
from
cc94c92 to
c2de6fc
Compare
| Async.async (fun () -> | ||
| if Path.Untracked.exists (Path.build target_dir) | ||
| then resolve_symlinks_in (Path.Build.to_string target_dir)) | ||
| in |
There was a problem hiding this comment.
I think you should combine this Async.async call with the one below it. There's a bunch of overhead for using the thread pool, so if it's to be used, the work should be batched as much as possible.
2 participants
The reloctable compiler, like the other OCaml compilers, has symlinks in it's installation layout. These need to be resolved before we declare them inside the directory target or else the reloctable compiler cannot be cached. This work is tracked in #13229.
In the second commit we introduce a general mechanism to comb through the targets of the install layout for a package and resolve these symlinks by replacing them with a hardlink, falling back to a copy if that is not possible.
We add quite a few tests to exercise the numerous edge cases that come up.