Skip to content

Commit b3abadd

Browse files
authored
refactor(lock_rules): small improvements (ocaml#12534)
Small tweaks from a reading of the code. Some specific points to keep in mind: * There's no point to use [Rules.collect] if we aren't using dynamic scoping * Re-ordering of arguments to reduce line lengths when it's "free" * Pass in the workspace instead of recomputing. Calling memoized functions is OK, but passing the values we already computed is usually better. Signed-off-by: Rudi Grinberg <[email protected]> <!-- ps-id: 8615efbf-0524-4cd5-b3c9-a6d2b091f590 -->
1 parent fdedb23 commit b3abadd

File tree

1 file changed

+18
-30
lines changed

1 file changed

+18
-30
lines changed

src/dune_rules/lock_rules.ml

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,6 @@ open Import
22
open Memo.O
33
module Gen_rules = Build_config.Gen_rules
44

5-
let rule ?loc { Action_builder.With_targets.build; targets } =
6-
Rule.make ~info:(Rule.Info.of_loc_opt loc) ~targets build |> Rules.Produce.rule
7-
;;
8-
95
let action_builder_with_dir_targets ~directory_targets =
106
let targets =
117
Targets.create
@@ -33,23 +29,21 @@ let setup_copy_rules ~dir:target ~lock_dir =
3329
let+ deps, files = Source_deps.files (Path.source lock_dir) in
3430
let directory_targets, rules =
3531
match Path.Set.is_empty files with
36-
| true -> Path.Build.Map.empty, Memo.return Rules.empty
32+
| true -> Path.Build.Map.empty, Rules.empty
3733
| false ->
3834
let directory_targets = Path.Build.Map.singleton target Loc.none in
39-
let rules =
40-
Rules.collect_unit (fun () ->
41-
let copy_rule = copy_lock_dir ~target ~lock_dir ~deps ~files in
42-
rule ~loc:Loc.none copy_rule)
35+
let { Action_builder.With_targets.build; targets } =
36+
copy_lock_dir ~target ~lock_dir ~deps ~files
4337
in
44-
directory_targets, rules
38+
let rule = Rule.make ~targets build in
39+
directory_targets, Rules.of_rules [ rule ]
4540
in
46-
Gen_rules.make ~directory_targets rules
41+
Gen_rules.make ~directory_targets (Memo.return rules)
4742
;;
4843

49-
let setup_lock_rules ~dir ~lock_dir =
50-
let* workspace = Workspace.workspace () in
51-
let dir = Path.Build.relative dir lock_dir in
52-
let lock_dir = Path.Source.relative workspace.dir lock_dir in
44+
let setup_lock_rules (workspace : Workspace.t) ~dir ~lock_dir =
45+
let dir = Path.Build.append_local dir lock_dir in
46+
let lock_dir = Path.Source.append_local workspace.dir lock_dir in
5347
setup_copy_rules ~dir ~lock_dir
5448
;;
5549

@@ -86,22 +80,16 @@ let setup_rules ~components ~dir =
8680
match components with
8781
| [ ".lock" ] ->
8882
let* workspace = Workspace.workspace () in
89-
let* lock_dir_paths = lock_dirs_of_workspace workspace in
90-
lock_dir_paths
91-
|> Path.Source.Set.to_list
92-
|> Memo.List.fold_left
93-
~f:(fun rules lock_dir_path ->
94-
let lock_dir = Path.Source.to_string lock_dir_path in
95-
let+ lock_rule = setup_lock_rules ~dir ~lock_dir in
96-
Gen_rules.combine rules lock_rule)
97-
~init:empty
83+
lock_dirs_of_workspace workspace
84+
>>| Path.Source.Set.to_list
85+
>>= Memo.List.fold_left ~init:empty ~f:(fun rules lock_dir_path ->
86+
let lock_dir = Path.Source.to_local lock_dir_path in
87+
let+ lock_rule = setup_lock_rules workspace ~dir ~lock_dir in
88+
Gen_rules.combine rules lock_rule)
9889
| [ ".dev-tool-locks" ] ->
99-
Memo.List.fold_left
100-
Dune_pkg.Dev_tool.all
101-
~f:(fun rules dev_tool ->
102-
let+ dev_tool_rules = setup_dev_tool_lock_rules ~dir dev_tool in
103-
Gen_rules.combine rules dev_tool_rules)
104-
~init:empty
90+
Memo.List.fold_left Dune_pkg.Dev_tool.all ~init:empty ~f:(fun rules dev_tool ->
91+
let+ dev_tool_rules = setup_dev_tool_lock_rules ~dir dev_tool in
92+
Gen_rules.combine rules dev_tool_rules)
10593
| [] ->
10694
let sub_dirs = [ ".lock"; ".dev-tool-locks" ] in
10795
let build_dir_only_sub_dirs =

0 commit comments

Comments
 (0)