Skip to content

Commit 4d116a8

Browse files
committed
Revert Reflection / dune rules changes
Signed-off-by: Nat Karmios <nat@karmios.com>
1 parent ba35683 commit 4d116a8

5 files changed

Lines changed: 40 additions & 215 deletions

File tree

bin/print_rules.ml

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -222,40 +222,24 @@ let targets_repr =
222222
;;
223223

224224
let rule_context (rule : Dune_engine.Reflection.Rule.t) =
225-
match
226-
Option.bind rule.targets ~f:(fun ts ->
227-
Path.Build.extract_build_context ts.Targets.Validated.root)
228-
with
225+
match Path.Build.extract_build_context rule.targets.Targets.Validated.root with
229226
| None -> None
230227
| Some (context, _) -> Some (Filename.to_string context)
231228
;;
232229

233-
let rule_loc ~with_locs (rule : Dune_engine.Reflection.Rule.t) =
234-
if (not with_locs) || Loc.is_none rule.loc
235-
then None
236-
else (
237-
let start = Loc.start rule.loc in
238-
Some (sprintf "%s:%d" start.pos_fname start.pos_lnum))
239-
;;
240-
241-
let rule_repr ~with_locs =
230+
let rule_repr =
242231
Repr.record
243232
"rule"
244233
[ Repr.field "deps" deps_repr ~get:(fun rule -> rule.Dune_engine.Reflection.Rule.deps)
245-
; Repr.field "targets" (Repr.option targets_repr) ~get:(fun rule ->
234+
; Repr.field "targets" targets_repr ~get:(fun rule ->
246235
rule.Dune_engine.Reflection.Rule.targets)
247236
; Repr.field "context" (Repr.option Repr.string) ~get:rule_context
248237
; Repr.field "action" action_repr ~get:(fun rule ->
249238
rule.Dune_engine.Reflection.Rule.action)
250-
; Repr.field
251-
"aliases"
252-
(Repr.option (Repr.list Alias_name.repr))
253-
~get:(fun rule -> rule.Dune_engine.Reflection.Rule.aliases)
254-
; Repr.field "loc" (Repr.option Repr.string) ~get:(rule_loc ~with_locs)
255239
]
256240
;;
257241

258-
let rules_repr ~with_locs = Repr.list (rule_repr ~with_locs)
242+
let rules_repr = Repr.list rule_repr
259243
let deps_only_repr = Repr.list deps_repr
260244

261245
let dep_sets_of_rules rules =
@@ -308,13 +292,10 @@ let print_json oc json =
308292
output_char oc '\n'
309293
;;
310294

311-
let print_rules_sexp ~with_locs ppf rules =
295+
let print_rules_sexp ppf rules =
312296
Syntax.prepare_formatter ppf;
313297
Format.pp_open_vbox ppf 0;
314-
Format.pp_print_list
315-
(fun ppf rule -> print_sexp_repr ppf (rule_repr ~with_locs) rule)
316-
ppf
317-
rules;
298+
Format.pp_print_list (fun ppf rule -> print_sexp_repr ppf rule_repr rule) ppf rules;
318299
Format.pp_print_flush ppf ()
319300
;;
320301

@@ -356,15 +337,8 @@ let term =
356337
value
357338
& opt (enum Output_format.all) Output_format.Sexp
358339
& info [ "format" ] ~docv:"FORMAT" ~doc:(Some doc))
359-
and+ with_locs =
360-
Arg.(value & flag & info [ "with-locs" ] ~doc:(Some "Include locations of rules"))
361340
(* CR-someday Alizter: document this option *)
362341
and+ targets = Arg.(value & pos_all dep [] & Arg.info [] ~docv:"TARGET" ~doc:None) in
363-
let targets =
364-
match targets with
365-
| [] -> [ Common.Builder.default_target builder ]
366-
| _ :: _ -> targets
367-
in
368342
let common, config = Common.init builder in
369343
let out = Option.map ~f:Path.of_string out in
370344
Scheduler_setup.go_with_rpc_server ~common ~config (fun () ->
@@ -383,10 +357,9 @@ let term =
383357
let print oc =
384358
let ppf = Format.formatter_of_out_channel oc in
385359
match format, deps_only with
386-
| Output_format.Sexp, false -> print_rules_sexp ~with_locs ppf rules
360+
| Output_format.Sexp, false -> print_rules_sexp ppf rules
387361
| Output_format.Sexp, true -> print_rule_deps_only_sexp ppf rules
388-
| Json, false ->
389-
print_json oc (Json.of_dyn (dyn_of_repr (rules_repr ~with_locs) rules))
362+
| Json, false -> print_json oc (Json.of_dyn (dyn_of_repr rules_repr rules))
390363
| Json, true ->
391364
print_json
392365
oc

doc/changes/changed/15581.md

Lines changed: 0 additions & 3 deletions
This file was deleted.

src/dune_engine/reflection.ml

Lines changed: 31 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,62 @@
11
open Import
22
module Non_evaluated_rule = Rule
3-
module Anon_rule = Rule.Anonymous_action.Rule
43
open Memo.O
54

65
module Rule = struct
76
type t =
87
{ id : Rule.Id.t
98
; deps : Dep.Set.t
109
; expanded_deps : Path.Set.t
11-
; targets : Targets.Validated.t option
10+
; targets : Targets.Validated.t
1211
; action : Action.t
13-
; aliases : Alias_name.t list option
14-
; loc : Loc.t
1512
}
1613
end
1714

1815
module Rule_top_closure = Top_closure.Make (Non_evaluated_rule.Id.Set) (Memo)
1916

2017
module rec Expand : sig
21-
type expanded := Path.Set.t * Anon_rule.Set.t
22-
23-
val alias : Alias.t -> expanded Memo.t
24-
val deps : Dep.Set.t -> expanded Memo.t
18+
val alias : Alias.t -> Path.Set.t Memo.t
19+
val deps : Dep.Set.t -> Path.Set.t Memo.t
2520
end = struct
26-
let combine_both fa fb (a1, b1) (a2, b2) = fa a1 a2, fb b1 b2
27-
2821
let alias =
2922
let memo =
3023
Memo.create
3124
"expand-alias"
3225
~input:(module Alias)
3326
(fun alias ->
34-
let* deps, anons =
27+
let* deps =
3528
Load_rules.get_alias_definition alias
3629
>>= Memo.map_reduce
37-
~empty:(Dep.Set.empty, Anon_rule.Set.empty)
38-
~combine:(combine_both Dep.Set.union Anon_rule.Set.union)
30+
~empty:Dep.Set.empty
31+
~combine:Dep.Set.union
3932
~f:(fun (loc, definition) ->
4033
Memo.push_stack_frame
4134
(fun () ->
42-
match (definition : Rules.Dir_rules.Alias_spec.item) with
43-
| Deps x ->
44-
let+ (), deps = Action_builder.evaluate_and_collect_deps x in
45-
deps, Anon_rule.Set.empty
46-
| Action x ->
47-
Memo.return (Dep.Set.empty, Anon_rule.Set.singleton x))
35+
Action_builder.evaluate_and_collect_deps
36+
(Build_system.dep_on_alias_definition definition)
37+
>>| snd)
4838
~human_readable_description:(fun () -> Alias.describe alias ~loc))
4939
in
50-
let+ expanded_deps, anons' = Expand.deps deps in
51-
expanded_deps, Anon_rule.Set.union anons anons')
40+
Expand.deps deps)
5241
in
5342
Memo.exec memo
5443
;;
5544

5645
let deps deps =
5746
Memo.map_reduce
5847
(Dep.Set.to_list deps)
59-
~empty:(Path.Set.empty, Anon_rule.Set.empty)
60-
~combine:(combine_both Path.Set.union Anon_rule.Set.union)
48+
~empty:Path.Set.empty
49+
~combine:Path.Set.union
6150
~f:(fun (dep : Dep.t) ->
6251
match dep with
63-
| File p -> Memo.return (Path.Set.singleton p, Anon_rule.Set.empty)
52+
| File p -> Memo.return (Path.Set.singleton p)
6453
| File_selector g ->
6554
let+ filenames = Build_system.eval_pred g in
6655
(* Alas, we can't use filename sets here because we end up putting paths coming
6756
from different directories together. *)
68-
Path.Set.of_list (Filename_set.to_list filenames), Anon_rule.Set.empty
57+
Path.Set.of_list (Filename_set.to_list filenames)
6958
| Alias a -> Expand.alias a
70-
| Env _ | Universe -> Memo.return (Path.Set.empty, Anon_rule.Set.empty))
59+
| Env _ | Universe -> Memo.return Path.Set.empty)
7160
;;
7261
end
7362

@@ -78,71 +67,29 @@ let evaluate_rule =
7867
~input:(module Non_evaluated_rule)
7968
(fun rule ->
8069
let* action, deps = Action_builder.evaluate_and_collect_deps rule.action in
81-
let* expanded_deps, _ = Expand.deps deps in
70+
let* expanded_deps = Expand.deps deps in
8271
Memo.return
8372
{ Rule.id = rule.id
8473
; deps
8574
; expanded_deps
86-
; targets = Some rule.targets
75+
; targets = rule.targets
8776
; action = action.action
88-
; aliases = None
89-
; loc = rule.loc
9077
})
9178
in
9279
Memo.exec memo
9380
;;
9481

95-
let evaluate_anonymous_action =
96-
let memo =
97-
Memo.create
98-
"evaluate-anonymous-action"
99-
~input:(module Anon_rule)
100-
(fun anon_action ->
101-
let* action, deps =
102-
Action_builder.evaluate_and_collect_deps anon_action.action
103-
in
104-
let* expanded_deps, _ = Expand.deps deps in
105-
Memo.return
106-
{ Rule.id = anon_action.id
107-
; deps
108-
; expanded_deps
109-
; targets = None
110-
; action = action.action
111-
; aliases =
112-
(match anon_action.aliases with
113-
| [] -> None
114-
| aliases -> Some aliases)
115-
; loc = anon_action.loc
116-
})
117-
in
118-
Memo.exec memo
119-
;;
120-
121-
let rules_of_dep_paths paths =
122-
Path.Set.to_list paths
123-
|> Memo.parallel_map ~f:(fun p ->
124-
Load_rules.get_rule p
125-
>>= function
126-
| None -> Memo.return None
127-
| Some rule -> evaluate_rule rule >>| Option.some)
128-
>>| List.filter_opt
129-
;;
130-
131-
let rules_of_anon_actions anons =
132-
Anon_rule.Set.to_list anons |> Memo.parallel_map ~f:evaluate_anonymous_action
133-
;;
134-
135-
let rules_of_deps deps =
136-
let* dep_paths, anons = Expand.deps deps in
137-
let+ dep_rules, anon_rules =
138-
Memo.fork_and_join
139-
(fun () -> rules_of_dep_paths dep_paths)
140-
(fun () -> rules_of_anon_actions anons)
141-
in
142-
dep_rules @ anon_rules
143-
;;
144-
14582
let eval ~recursive ~request =
83+
let rules_of_deps deps =
84+
Expand.deps deps
85+
>>| Path.Set.to_list
86+
>>= Memo.parallel_map ~f:(fun p ->
87+
Load_rules.get_rule p
88+
>>= function
89+
| None -> Memo.return None
90+
| Some rule -> evaluate_rule rule >>| Option.some)
91+
>>| List.filter_opt
92+
in
14693
let* (), deps = Action_builder.evaluate_and_collect_deps request in
14794
let* root_rules = rules_of_deps deps in
14895
Rule_top_closure.top_closure
@@ -154,18 +101,9 @@ let eval ~recursive ~request =
154101
| Error cycle ->
155102
User_error.raise
156103
[ Pp.text "Dependency cycle detected:"
157-
; Pp.chain cycle ~f:(fun { Rule.targets; loc; _ } ->
158-
match targets with
159-
| Some targets ->
160-
Pp.verbatim
161-
(Path.to_string_maybe_quoted (Path.build (Targets.Validated.head targets)))
162-
| None ->
163-
(* Anonymous actions have no targets; describe them by their location instead. *)
164-
if Loc.is_none loc
165-
then Pp.verbatim "<anonymous action>"
166-
else (
167-
let start = Loc.start loc in
168-
Pp.verbatim
169-
(sprintf "<anonymous action at %s:%d>" start.pos_fname start.pos_lnum)))
104+
; Pp.chain cycle ~f:(fun rule ->
105+
Pp.verbatim
106+
(Path.to_string_maybe_quoted
107+
(Path.build (Targets.Validated.head rule.targets))))
170108
]
171109
;;

src/dune_engine/reflection.mli

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@ module Rule : sig
88
; (* [expanded_deps] skips over non-file dependencies, such as: environment
99
variables, universe, glob listings, sandbox requirements *)
1010
expanded_deps : Path.Set.t
11-
; targets : Targets.Validated.t option
11+
; targets : Targets.Validated.t
1212
; action : Action.t
13-
; aliases : Alias_name.t list option
14-
; loc : Loc.t
1513
}
1614
end
1715

test/blackbox-tests/test-cases/print-rules.t

Lines changed: 0 additions & 81 deletions
This file was deleted.

0 commit comments

Comments
 (0)