From 2a4a5bf6b904549dccd580904bff52e49a8ec8f2 Mon Sep 17 00:00:00 2001 From: Stephen Sherratt Date: Wed, 3 Apr 2024 15:08:36 +1100 Subject: [PATCH] Allow rules with directory targets to be disabled (#10367) Previously attempting to disable a rule with a directory target would cause dune to crash. Fixes https://github.com/ocaml/dune/issues/10310 Signed-off-by: Stephen Sherratt --- src/dune_rules/dir_status.ml | 58 +++++++++++++----------- src/dune_rules/dir_status.mli | 6 ++- src/dune_rules/gen_rules.ml | 5 +- test/blackbox-tests/test-cases/gh10310.t | 13 ++---- 4 files changed, 43 insertions(+), 39 deletions(-) diff --git a/src/dune_rules/dir_status.ml b/src/dune_rules/dir_status.ml index 77c92136486..b9735e4fd5e 100644 --- a/src/dune_rules/dir_status.ml +++ b/src/dune_rules/dir_status.ml @@ -85,32 +85,36 @@ let error_no_module_consumer ~loc (qualification : Include_subdirs.qualification ] ;; -let extract_directory_targets ~dir stanzas = +let extract_directory_targets ~dir ~expander stanzas = Memo.List.fold_left stanzas ~init:Path.Build.Map.empty ~f:(fun acc stanza -> match Stanza.repr stanza with - | Rule_conf.T { targets = Static { targets = l; _ }; loc = rule_loc; _ } -> - List.fold_left l ~init:acc ~f:(fun acc (target, kind) -> - let loc = String_with_vars.loc target in - match (kind : Targets_spec.Kind.t) with - | File -> acc - | Directory -> - (match String_with_vars.text_only target with - | None -> - User_error.raise - ~loc - [ Pp.text "Variables are not allowed in directory targets." ] - | Some target -> - let dir_target = Path.Build.relative ~error_loc:loc dir target in - if Path.Build.is_descendant dir_target ~of_:dir - then - (* We ignore duplicates here as duplicates are detected and - reported by [Load_rules]. *) - Path.Build.Map.set acc dir_target rule_loc - else - (* This will be checked when we interpret the stanza - completely, so just ignore this rule for now. *) - acc)) - |> Memo.return + | Rule_conf.T { targets = Static { targets = l; _ }; loc = rule_loc; enabled_if; _ } + -> + let+ available = Expander.eval_blang expander enabled_if in + if not available + then acc + else + List.fold_left l ~init:acc ~f:(fun acc (target, kind) -> + let loc = String_with_vars.loc target in + match (kind : Targets_spec.Kind.t) with + | File -> acc + | Directory -> + (match String_with_vars.text_only target with + | None -> + User_error.raise + ~loc + [ Pp.text "Variables are not allowed in directory targets." ] + | Some target -> + let dir_target = Path.Build.relative ~error_loc:loc dir target in + if Path.Build.is_descendant dir_target ~of_:dir + then + (* We ignore duplicates here as duplicates are detected and + reported by [Load_rules]. *) + Path.Build.Map.set acc dir_target rule_loc + else + (* This will be checked when we interpret the stanza + completely, so just ignore this rule for now. *) + acc)) | Coq_stanza.Theory.T m -> (* It's unfortunate that we need to pull in the coq rules here. But we don't have a generic mechanism for this yet. *) @@ -257,15 +261,15 @@ end = struct ;; end -let directory_targets t ~dir = +let directory_targets t ~dir ~expander = match t with | Lock_dir | Generated | Source_only _ | Is_component_of_a_group_but_not_the_root _ -> Memo.return Path.Build.Map.empty | Standalone (_, dune_file) -> - Dune_file.stanzas dune_file >>= extract_directory_targets ~dir + Dune_file.stanzas dune_file >>= extract_directory_targets ~dir ~expander | Group_root { components; dune_file; _ } -> let f ~dir stanzas acc = - extract_directory_targets ~dir stanzas >>| Path.Build.Map.superpose acc + extract_directory_targets ~dir ~expander stanzas >>| Path.Build.Map.superpose acc in let* init = let* stanzas = Dune_file.stanzas dune_file in diff --git a/src/dune_rules/dir_status.mli b/src/dune_rules/dir_status.mli index 0980d46911d..736cca93ed3 100644 --- a/src/dune_rules/dir_status.mli +++ b/src/dune_rules/dir_status.mli @@ -42,4 +42,8 @@ module DB : sig val get : dir:Path.Build.t -> t Memo.t end -val directory_targets : t -> dir:Path.Build.t -> Loc.t Path.Build.Map.t Memo.t +val directory_targets + : t + -> dir:Path.Build.t + -> expander:Expander.t + -> Loc.t Path.Build.Map.t Memo.t diff --git a/src/dune_rules/gen_rules.ml b/src/dune_rules/gen_rules.ml index 41dc463d4c2..402d34136cb 100644 --- a/src/dune_rules/gen_rules.ml +++ b/src/dune_rules/gen_rules.ml @@ -518,7 +518,10 @@ let gen_rules_regular_directory sctx ~src_dir ~components ~dir = in let+ rules = let+ make_rules = - let+ directory_targets = Dir_status.directory_targets dir_status ~dir in + let* expander = sctx >>= Super_context.expander ~dir in + let+ directory_targets = + Dir_status.directory_targets dir_status ~dir ~expander + in let allowed_subdirs = let automatic = Automatic_subdir.subdirs components in let toplevel = diff --git a/test/blackbox-tests/test-cases/gh10310.t b/test/blackbox-tests/test-cases/gh10310.t index e76d8606b7f..7ca695c2794 100644 --- a/test/blackbox-tests/test-cases/gh10310.t +++ b/test/blackbox-tests/test-cases/gh10310.t @@ -1,5 +1,5 @@ -Reproduces a bug where if a rule with a directory target is excluded with -enabled_if then dune crashes. +Make sure that dune can handle rules with directory targets that are disabled +with enabled_if. $ cat > dune-project < (lang dune 3.14) @@ -13,11 +13,4 @@ enabled_if then dune crashes. > (action (progn))) > EOF - $ dune build 2>&1 | head -n 7 - Internal error, please report upstream including the contents of _build/log. - Description: - ("gen_rules returned a set of directory targets that doesn't match the set of directory targets from returned rules", - { dir = In_build_dir "default" - ; mismatched_directories = - map { "default/x" : { message = "not generated"; loc = "dune:1" } } - }) + $ dune build