Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: --ignore-promoted-rules should work on internal rules #8518

Merged
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
2 changes: 2 additions & 0 deletions doc/changes/8518.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Ignore internal promote rules when `--ignore-promoted-rules` is set (#8518,
fix #8417, @rgrinberg)
1 change: 1 addition & 0 deletions otherlibs/dune-site/test/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ Test with an opam like installation
["dune" "install" "-p" name "--create-install-files" name]
]

$ dune build b/b.opam c/c.opam d/d.opam
$ dune build -p a --promote-install-files=false @install

$ test -e a/a.install
Expand Down
1 change: 1 addition & 0 deletions otherlibs/dune-site/test/run_2_9.t
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ Test with an opam like installation
["dune" "install" "-p" name "--create-install-files" name]
]

$ dune build b/b.opam c/c.opam d/d.opam
$ dune build -p a --promote-install-files="false" @install

$ test -e a/a.install
Expand Down
21 changes: 6 additions & 15 deletions src/dune_rules/dune_file.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2463,21 +2463,12 @@ type t =
; stanzas : Stanzas.t
}

let is_promoted_rule =
let is_promoted_mode version = function
| Rule.Mode.Promote { only = None; lifetime; _ } ->
if version >= (3, 5)
then (
match lifetime with
| Unlimited -> true
| Until_clean -> false)
else true
| _ -> false
in
fun version rule ->
match rule with
| Rule { mode; _ } | Menhir_stanza.T { mode; _ } -> is_promoted_mode version mode
| _ -> false
let is_promoted_rule version rule =
match rule with
| Rule { mode; _ } | Menhir_stanza.T { mode; _ } ->
let until_clean = if version >= (3, 5) then `Keep else `Ignore in
Rule_mode_decoder.is_ignored mode ~until_clean
| _ -> false
;;

let parse sexps ~dir ~file ~project =
Expand Down
12 changes: 12 additions & 0 deletions src/dune_rules/rule_mode_decoder.ml
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,15 @@ end

let decode = sum mode_decoders
let field = field "mode" decode ~default:Rule.Mode.Standard

let is_ignored (mode : Rule.Mode.t) ~until_clean =
!Clflags.ignore_promoted_rules
&&
match mode with
| Promote { only = None; lifetime = Unlimited; _ } -> true
| Promote { only = None; lifetime = Until_clean; _ } ->
(match until_clean with
| `Ignore -> true
| `Keep -> false)
| _ -> false
;;
8 changes: 8 additions & 0 deletions src/dune_rules/rule_mode_decoder.mli
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,11 @@ end

val decode : Rule.Mode.t Dune_lang.Decoder.t
val field : Rule.Mode.t Dune_lang.Decoder.fields_parser

(** [is_ignored mode ~until_clean] will return if a rule with [mode] should be
ignored whenever [--ignored-promoted-rules] is set.

[until_clean] is used to set if [(promote (until-clean))] is ignored as
considered by this function. Old versions of dune would incorrectly ignore
this, so we need to maintain the old behavior for now. *)
val is_ignored : Rule.Mode.t -> until_clean:[ `Ignore | `Keep ] -> bool
rgrinberg marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 2 additions & 5 deletions src/dune_rules/simple_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,7 @@ let user_rule sctx ?extra_bindings ~dir ~expander (rule : Rule.t) =
else action
in
(match rule_kind ~rule ~action with
| No_alias ->
let+ targets = add_user_rule sctx ~dir ~rule ~action ~expander in
Some targets
| No_alias -> add_user_rule sctx ~dir ~rule ~action ~expander
| Aliases_with_targets (aliases, alias_target) ->
let* () =
let aliases = List.map ~f:(Alias.make ~dir) aliases in
Expand All @@ -145,8 +143,7 @@ let user_rule sctx ?extra_bindings ~dir ~expander (rule : Rule.t) =
alias
(Action_builder.path (Path.build alias_target)))
in
let+ targets = add_user_rule sctx ~dir ~rule ~action ~expander in
Some targets
add_user_rule sctx ~dir ~rule ~action ~expander
| Aliases_only aliases ->
let aliases = List.map ~f:(Alias.make ~dir) aliases in
let* action = interpret_and_add_locks ~expander rule.locks action.build in
Expand Down
31 changes: 19 additions & 12 deletions src/dune_rules/super_context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -255,24 +255,31 @@ let extend_action t ~dir build =
;;

let make_rule t ?mode ?loc ~dir { Action_builder.With_targets.build; targets } =
let build = extend_action t build ~dir in
Rule.make
?mode
~info:(Rule.Info.of_loc_opt loc)
~context:(Some (Context.build_context (Env_tree.context t)))
~targets
build
match mode with
| Some mode when Rule_mode_decoder.is_ignored mode ~until_clean:`Keep -> None
| _ ->
let build = extend_action t build ~dir in
Some
(Rule.make
?mode
~info:(Rule.Info.of_loc_opt loc)
~context:(Some (Context.build_context (Env_tree.context t)))
~targets
build)
;;

let add_rule t ?mode ?loc ~dir build =
let rule = make_rule t ?mode ?loc ~dir build in
Rules.Produce.rule rule
match make_rule t ?mode ?loc ~dir build with
| None -> Memo.return ()
| Some rule -> Rules.Produce.rule rule
;;

let add_rule_get_targets t ?mode ?loc ~dir build =
let rule = make_rule t ?mode ?loc ~dir build in
let+ () = Rules.Produce.rule rule in
rule.targets
match make_rule t ?mode ?loc ~dir build with
| None -> Memo.return None
| Some rule ->
let+ () = Rules.Produce.rule rule in
Some rule.targets
;;

let add_rules t ?loc ~dir builds = Memo.parallel_iter builds ~f:(add_rule ?loc t ~dir)
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 @@ -69,7 +69,7 @@ val add_rule_get_targets
-> ?loc:Loc.t
-> dir:Path.Build.t
-> Action.Full.t Action_builder.With_targets.t
-> Targets.Validated.t Memo.t
-> Targets.Validated.t option Memo.t

val add_rules
: t
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ This should not modify the file now

$ dune build --ignore-promoted-rules foo.opam
$ grep extra foo.opam
[1]
foobar_extra