Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/changes/fixed/15581.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Fix a target-less rule attached to multiple aliases (`(rule (aliases a b)
...)`) leaking unrelated contributions between those aliases. Building one
alias no longer pulls in actions or dependencies that were added only to
another of the rule's aliases. (#15581, @NatKarmios)
Comment thread
NatKarmios marked this conversation as resolved.
31 changes: 20 additions & 11 deletions src/dune_engine/build_system.ml
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ module Internal = struct

(* The current version of the rule digest scheme. We should increment it when
making any changes to the scheme, to avoid collisions. *)
let rule_digest_version = 29
let rule_digest_version = 30

let compute_rule_digest
(rule : Rule.t)
Expand Down Expand Up @@ -735,14 +735,21 @@ module Internal = struct
in
let d = Digest.to_string digest in
let basename =
match act.alias with
| None -> d
| Some a -> Alias.Name.to_string a ^ "-" ^ d
match act.aliases with
| [] -> d
| aliases ->
let a =
aliases
|> List.map ~f:Alias.Name.to_string
|> List.min ~f:String.compare
|> Option.value_exn
in
a ^ "-" ^ d
in
Path.Build.relative dir basename
in
let rule =
let { Rule.Anonymous_action.action = _; loc; dir = _; alias = _ } = act in
let { Rule.Anonymous_action.action = _; loc; dir = _; aliases = _ } = act in
Rule.make
~info:(if Loc.is_none loc then Internal else From_dune_file loc)
~targets:(Targets.File.create target)
Expand All @@ -754,7 +761,7 @@ module Internal = struct
rule
~rule_kind:
(Anonymous_action
{ attached_to_alias = Option.is_some act.alias
{ attached_to_alias = List.is_non_empty act.aliases
; capture_stdout
; stamp_file = target
})
Expand Down Expand Up @@ -796,7 +803,7 @@ module Internal = struct
{ action; env; locks; can_go_in_shared_cache; sandbox; corrections }
; loc = _
; dir
; alias
; aliases
}
=
act
Expand All @@ -817,10 +824,12 @@ module Internal = struct
Action.digest d action;
digest_locks d locks;
Digest.Manual.string d (Path.Build.to_string dir);
Digest.Manual.option
d
~f:(fun d alias -> Digest.Manual.string d (Alias.Name.to_string alias))
alias;
let alias_names =
aliases
|> List.map ~f:Alias.Name.to_string
|> List.sort_uniq ~compare:String.compare
in
Digest.Manual.list d ~f:Digest.Manual.string alias_names;
Digest.Manual.bool d capture_stdout;
Digest.Manual.bool d can_go_in_shared_cache;
digest_sandbox_config d sandbox;
Expand Down
2 changes: 1 addition & 1 deletion src/dune_engine/rule.ml
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,6 @@ module Anonymous_action = struct
{ action : Action.Full.t
; loc : Loc.t
; dir : Path.Build.t
; alias : Alias.Name.t option
; aliases : Alias.Name.t list
}
end
3 changes: 2 additions & 1 deletion src/dune_engine/rule.mli
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ module Anonymous_action : sig
; dir : Path.Build.t
(** Directory the action is attached to. This is the directory where
the outcome of the action will be cached. *)
; alias : Alias.Name.t option (** For better error messages *)
; aliases : Alias.Name.t list
(** The aliases this action is attached to. For better error messages. *)
}
end
25 changes: 17 additions & 8 deletions src/dune_engine/rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -150,20 +150,29 @@ module Produce = struct
}
;;

let add_action t ~loc action =
(* All aliases in [ts] are expected to share a directory: the shared
anonymous action is created in the representative's directory. *)
let add_action ts ~loc action =
let representative =
match ts with
| [] -> Code_error.raise "Rules.Produce.Alias.add_action: empty list" []
| r :: _ -> r
in
let action =
let open Action_builder.O in
let+ action = action in
let+ action in
{ Rule.Anonymous_action.action
; loc
; dir = Alias.dir t
; alias = Some (Alias.name t)
; dir = Alias.dir representative
; aliases = List.map ts ~f:Alias.name
}
in
alias
t
{ expansions = Appendable_list.singleton (loc, Dir_rules.Alias_spec.Action action)
}
Memo.parallel_iter ts ~f:(fun t ->
alias
t
{ expansions =
Appendable_list.singleton (loc, Dir_rules.Alias_spec.Action action)
})
;;
end
end
Expand Down
6 changes: 3 additions & 3 deletions src/dune_engine/rules.mli
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ module Produce : sig
[alias]. *)
val add_deps : t -> ?loc:Stdune.Loc.t -> unit Action_builder.t -> unit Memo.t

(** [add_action alias ~loc action] arrange things so that [action]
is executed as part of the build of alias [alias]. *)
val add_action : t -> loc:Loc.t -> Action.Full.t Action_builder.t -> unit Memo.t
(** [add_action aliases ~loc action] arrange things so that [action]
is executed as part of the build of aliases [aliases]. *)
val add_action : t list -> loc:Loc.t -> Action.Full.t Action_builder.t -> unit Memo.t
end
end

Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/cc_flags.ml
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ let cc_vendor_action (ctx : Build_context.t) =
|> Action.Full.make
|> Action.Full.add_env env
in
{ Rule.Anonymous_action.action; loc = Loc.none; dir = ctx.build_dir; alias = None }
{ Rule.Anonymous_action.action; loc = Loc.none; dir = ctx.build_dir; aliases = [] }
;;

let check_warn = function
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/cinaps.ml
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ let gen_rules sctx t ~dir ~scope =
])
|> Action.Full.add_env env
in
Super_context.add_alias_action sctx ~dir ~loc cinaps_alias action
Super_context.add_alias_action sctx ~dir ~loc [ cinaps_alias ] action
in
match t.alias with
| Some _ -> Memo.return ()
Expand Down
4 changes: 2 additions & 2 deletions src/dune_rules/cram/cram_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ let test_rule
(* We error out on invalid tests even if they are disabled. *)
let* () = add_extra_aliases_deps () in
Action_builder.fail { fail = (fun () -> missing_run_t test) }
|> Alias_rules.add sctx ~alias ~loc
|> Alias_rules.add sctx ~aliases:[ alias ] ~loc
| Ok test ->
(* enabled_if controls whether this test is included in @runtest (and other aliases),
but the test can always be run explicitly via its own alias. *)
Expand Down Expand Up @@ -160,7 +160,7 @@ let test_rule
|> Action_builder.with_file_targets ~file_targets:[ output ]
|> Super_context.add_rule sctx ~dir ~loc
in
Alias_rules.add sctx ~alias ~loc
Alias_rules.add sctx ~aliases:[ alias ] ~loc
@@
let open Action_builder.O in
let+ () = List.map ~f:Path.build [ script; output ] |> Action_builder.paths in
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/dep_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ let merge_deps ~dir ~transitive ~immediate =
|> Action.Full.make ~sandbox:Sandbox_config.no_sandboxing
; loc = Loc.none
; dir
; alias = None
; aliases = []
}
in
Build_system.execute_action_stdout action |> Action_builder.of_memo
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/format_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ let formatter_diff_action =
{ Rule.Anonymous_action.action
; loc
; dir = Alias.dir alias
; alias = Some (Alias.name alias)
; aliases = [ Alias.name alias ]
}
in
Build_system.dep_on_alias_definition (Rules.Dir_rules.Alias_spec.Action action)
Expand Down
4 changes: 2 additions & 2 deletions src/dune_rules/inline_tests.ml
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ include Sub_system.Register_end_point (struct
sctx
~dir
~loc:info.loc
alias
[ alias ]
(let open Action_builder.O in
let promotion_targets =
List.concat_map source_modules ~f:Module.sources_without_pp
Expand Down Expand Up @@ -479,7 +479,7 @@ include Sub_system.Register_end_point (struct
| true -> gen_rules c ~expander ~info ~backends
| false ->
let alias = Alias.make Alias0.runtest ~dir in
Simple_rules.Alias_rules.add_empty sctx ~alias ~loc:info.loc
Simple_rules.Alias_rules.add_empty sctx ~aliases:[ alias ] ~loc:info.loc
;;
end)

Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/mdx.ml
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ let gen_rules_for_single_file stanza ~sctx ~dir ~expander ~mdx_prog ~mdx_prog_ge
in
Super_context.add_alias_action
sctx
(Alias.make Alias0.runtest ~dir)
[ Alias.make Alias0.runtest ~dir ]
mdx_action
~loc
~dir
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/ocamldep.ml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ let ocamldep_action ~sandbox ~sctx ~dir ~ml_kind unit =
; Dep (Module.File.path source)
]
in
{ Rule.Anonymous_action.action; loc = Loc.none; dir; alias = None }
{ Rule.Anonymous_action.action; loc = Loc.none; dir; aliases = [] }
;;

(* Top-level cache per (source path, ml_kind). Without it, each caller's
Expand Down
4 changes: 2 additions & 2 deletions src/dune_rules/ocamlobjinfo.mll
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ let rules (ocaml : Ocaml_toolchain.t) ~dir ~sandbox ~units =
{ Rule.Anonymous_action.action
; loc = Loc.none
; dir
; alias = None
; aliases = []
}
in
Dune_engine.Build_system.execute_action_stdout action
Expand All @@ -97,7 +97,7 @@ let archive_rules (ocaml : Ocaml_toolchain.t) ~dir ~sandbox ~archive =
{ Rule.Anonymous_action.action
; loc = Loc.none
; dir
; alias = None
; aliases = []
}
in
Dune_engine.Build_system.execute_action_stdout action
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/opam_create.ml
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ let add_alias_rule (ctx : Build_context.t) ~profile ~project ~pkg =
match Package.has_opam_file pkg with
| Generated_with_diff when not use_source_opam ->
Rules.Produce.Alias.add_action
opam_alias
[ opam_alias ]
~loc:(Loc.in_file source_opam_path)
(let open Action_builder.O in
let+ () = Action_builder.path (Path.build generated_opam_path)
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/pp_spec_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ let driver_flags expander ~corrected_suffix ~driver_flags ~standard =
let lint_module sctx ~sandbox ~dir ~expander ~lint ~lib_name ~scope =
let open Action_builder.O in
let add_alias build =
Super_context.add_alias_action sctx (Alias.make Alias0.lint ~dir) build ~dir
Super_context.add_alias_action sctx [ Alias.make Alias0.lint ~dir ] build ~dir
in
let lint =
Module_name.Per_item.map lint ~f:(function
Expand Down
3 changes: 1 addition & 2 deletions src/dune_rules/rocq/rocq_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -820,11 +820,10 @@ let setup_output_diff_rule ~loc ~dir ~sctx ~rocq_lang_version ~rocq_sources rocq
; directory_diffs = true
}
in
let alias = Alias.make ~dir Alias0.runtest in
Simple_rules.Alias_rules.add
sctx
~loc
~alias
~aliases:[ Alias.make ~dir Alias0.runtest ]
(let open Action_builder.O in
let+ () = Action_builder.paths [ diff.file1; Path.build diff.file2 ] in
Action.Full.make (Action.Diff diff))
Expand Down
31 changes: 15 additions & 16 deletions src/dune_rules/simple_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,19 @@ module Alias_rules = struct
else Memo.return ()
;;

let add sctx ~alias ~loc build =
let dir = Alias.dir alias in
check_empty ~loc ~dir alias
>>> Super_context.add_alias_action sctx alias ~dir ~loc build
let add sctx ~aliases ~loc build =
match aliases with
| [] -> Code_error.raise "Alias_rules.add: empty list of aliases" []
| representative :: _ ->
let dir = Alias.dir representative in
Memo.parallel_iter aliases ~f:(fun alias ->
check_empty ~loc ~dir:(Alias.dir alias) alias)
>>> Super_context.add_alias_action sctx aliases ~dir ~loc build
;;

let add_empty sctx ~loc ~alias =
let add_empty sctx ~loc ~aliases =
let action = Action_builder.return (Action.Full.make Action.empty) in
add sctx ~loc ~alias action
add sctx ~loc ~aliases action
;;
end

Expand Down Expand Up @@ -138,7 +142,7 @@ let user_rule sctx ~dir ~expander (rule : Rule_conf.t) =
let+ () =
Memo.parallel_iter rule.aliases ~f:(fun alias ->
let alias = Alias.make ~dir alias in
Alias_rules.add_empty sctx ~loc:rule.loc ~alias)
Alias_rules.add_empty sctx ~loc:rule.loc ~aliases:[ alias ])
in
None
| true ->
Expand Down Expand Up @@ -203,14 +207,9 @@ let user_rule sctx ~dir ~expander (rule : Rule_conf.t) =
let+ () =
match List.map ~f:(Alias.make ~dir) aliases with
| [] -> Code_error.raise "empty list of aliases" []
| alias :: extra_aliases ->
let loc = rule.loc in
| aliases ->
interpret_and_add_locks ~expander rule.locks action.build
|> Alias_rules.add sctx ~alias ~loc
>>> Memo.parallel_iter extra_aliases ~f:(fun extra_alias ->
Dep.alias alias
|> Action_builder.dep
|> Rules.Produce.Alias.add_deps ~loc extra_alias)
|> Alias_rules.add sctx ~aliases ~loc:rule.loc
in
None)
;;
Expand Down Expand Up @@ -341,7 +340,7 @@ let alias sctx ~dir ~expander (alias_conf : Alias_conf.t) =
Alias_rules.check_empty ~loc ~dir alias
>>> Expander.eval_blang expander alias_conf.enabled_if
>>= function
| false -> Alias_rules.add_empty sctx ~loc ~alias
| false -> Alias_rules.add_empty sctx ~loc ~aliases:[ alias ]
| true ->
(match alias_conf.action with
| None ->
Expand Down Expand Up @@ -369,5 +368,5 @@ let alias sctx ~dir ~expander (alias_conf : Alias_conf.t) =
~what:"aliases"
in
interpret_and_add_locks ~expander alias_conf.locks action
|> Alias_rules.add sctx ~loc ~alias)
|> Alias_rules.add sctx ~loc ~aliases:[ alias ])
;;
8 changes: 6 additions & 2 deletions src/dune_rules/simple_rules.mli
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ open Import
module Alias_rules : sig
val add
: Super_context.t
-> alias:Alias.t
-> aliases:Alias.t list
-> loc:Loc.t
-> Action.Full.t Action_builder.t
-> unit Memo.t

val add_empty : Super_context.t -> loc:Stdune.Loc.t -> alias:Alias.t -> unit Memo.t
val add_empty
: Super_context.t
-> loc:Stdune.Loc.t
-> aliases:Alias.t list
-> unit Memo.t
end

(** Interpret a [(rule ...)] stanza and return the targets it produces, if any. *)
Expand Down
6 changes: 3 additions & 3 deletions src/dune_rules/super_context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ let extend_action_env t ~dir action =
let execute_action_stdout t ?alias ~loc ~dir action =
let open Action_builder.O in
(let+ action = extend_action_env t ~dir action in
{ Rule.Anonymous_action.action; loc; dir; alias })
{ Rule.Anonymous_action.action; loc; dir; aliases = Option.to_list alias })
|> Build_system.execute_action_stdout
;;

Expand Down Expand Up @@ -190,9 +190,9 @@ let add_rule_get_targets t ?mode ?loc ~dir build =

let add_rules t ?loc ~dir builds = Memo.parallel_iter builds ~f:(add_rule ?loc t ~dir)

let add_alias_action t alias ~dir ~loc action =
let add_alias_action t aliases ~dir ~loc action =
let build = extend_action t action ~dir in
Rules.Produce.Alias.add_action alias ~loc build
Rules.Produce.Alias.add_action aliases ~loc build
;;

let resolve_program_memo t ~dir ?where ?hint ~loc bin =
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/super_context.mli
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ val add_rules

val add_alias_action
: t
-> Alias.t
-> Alias.t list
-> dir:Path.Build.t
-> loc:Loc.t
-> Action.Full.t Action_builder.t
Expand Down
Loading
Loading