Copy the lock directories to the build folder and read package rules from there #12394
Conversation
| ])))) | ||
| let dev_tool_lock_dir = Dune_rules.Lock_dir.dev_tool_source_lock_dir dev_tool in | ||
| let* lock_dir_exists = | ||
| Dune_engine.Fs_memo.dir_exists (In_source_dir dev_tool_lock_dir) |
There was a problem hiding this comment.
You should be using Source_tree (or at least Fs_memo) to inspect the source tree.
There was a problem hiding this comment.
But this is using Fs_memo?
| val parallel_map : 'a list -> f:('a -> 'b t) -> 'b list t | ||
| val readdir_with_kinds : Path.t -> (Filename.t * Unix.file_kind) list t | ||
| val with_lexbuf_from_file : Path.t -> f:(Lexing.lexbuf -> 'a) -> 'a t | ||
| val stats_kind : Path.t -> (File_kind.t, Unix_error.Detailed.t) result t |
There was a problem hiding this comment.
How come this function went away?
There was a problem hiding this comment.
It was only used in check_path which was used to get information about a lock dir path. But given lock dirs are read from the build directory and created by copy rules (so either they exist because the rule exists or they don't which makes the build fail), there is nothing useful that check_path could communicate to the user anymore.
| Fs_memo.dir_exists | ||
| (Path.as_outside_build_dir_exn | ||
| (Dune_pkg.Lock_dir.dev_tool_lock_dir_path dev_tool)) | ||
| Source_tree.find_dir (Lock_dir.dev_tool_source_lock_dir dev_tool) |
There was a problem hiding this comment.
If the dev-tool lock dir stops being in the source tree but ends up being read from the build folder, then as_outside_build_dir_exn will always fail with an exception. Hence I changed it, but then I realized that for now we need to check the existance of the dev tool lock dir in the source tree.
In any case, Source_tree.find_dir seems like the more appropriate function anyway, as it takes ignore-rules into account, whereas Fs_memo.dir_exists will happily read directories that were declared as to be ignored by the user.
| let readdir_with_kinds path = | ||
| Fs_memo.dir_contents (Path.as_outside_build_dir_exn path) | ||
| >>| function | ||
| Readdir.read_directory_with_kinds (Path.to_string path) |
There was a problem hiding this comment.
I don't see how this can work if the lock directory is still in the source directory. Without some sort of dependency tracking mechanism, dune will not know to invalidate the rules in watch mode if the lock directory changes.
There was a problem hiding this comment.
It doesn't read the source directory (hence why the PR this is spun off used Path.Build.t as path for a while). If the lock dir changes, the copy rule is retriggered, thus updating the files in the build folder.
Leonidas-from-XIV
force-pushed
the
lock-dir-copy-rule
branch
from
a139181 to
a9e9d3f
Compare
Leonidas-from-XIV
force-pushed
the
lock-dir-copy-rule
branch
2 times, most recently
from
884e272 to
abb1d61
Compare
Leonidas-from-XIV
force-pushed
the
lock-dir-copy-rule
branch
from
62ee30e to
27bc7d5
Compare
This is pulling out a change from ocaml#12394 to make it easier to control whether package management should be force-enabled, force-disabled or left at the default setting. Signed-off-by: Marek Kubica <[email protected]>
This is factored out of ocaml#12394 and makes sure that the dev tool flag is set when attempting to use dev-tool functionality. Signed-off-by: Marek Kubica <[email protected]>
This is factored out of ocaml#12394 and makes sure that the dev tool flag is set when attempting to use dev-tool functionality. Signed-off-by: Marek Kubica <[email protected]>
This is factored out of #12394 and makes sure that the dev tool flag is set when attempting to use dev-tool functionality. Signed-off-by: Marek Kubica <[email protected]>
This is pulling out a change from ocaml#12394 to make it easier to control whether package management should be force-enabled, force-disabled or left at the default setting. Signed-off-by: Marek Kubica <[email protected]>
This is pulling out a change from #12394 to make it easier to control whether package management should be force-enabled, force-disabled or left at the default setting. Signed-off-by: Marek Kubica <[email protected]>
Leonidas-from-XIV
force-pushed
the
lock-dir-copy-rule
branch
4 times, most recently
from
3c601d9 to
18c1f60
Compare
Leonidas-from-XIV
force-pushed
the
lock-dir-copy-rule
branch
8 times, most recently
from
554c924 to
e41b636
Compare
|
Looks good to me. Happy for the disabled tests to be fixed in a later change. Nice work! |
Signed-off-by: Marek Kubica <[email protected]>
Signed-off-by: Marek Kubica <[email protected]>
Signed-off-by: Marek Kubica <[email protected]>
Signed-off-by: Marek Kubica <[email protected]>
Signed-off-by: Marek Kubica <[email protected]>
Signed-off-by: Marek Kubica <[email protected]>
Alizter
force-pushed
the
lock-dir-copy-rule
branch
from
e41b636 to
54268d6
Compare
5 participants
This is a WIP PR to implement rules that copy the lock dirs (project lock dirs and dev-tool lock dirs) into the build folder before the package rules. It's a a prerequisite for #11775 which changes the logic to directly generate the lock directories in the build directory.