Skip to content

Commit

Permalink
feature: remove limitations on [enabled_if] on libraries
Browse files Browse the repository at this point in the history
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

<!-- ps-id: 2eb4a6ec-7903-421c-a370-9bbc0136e47a -->
  • Loading branch information
rgrinberg committed Mar 13, 2024
1 parent a262a63 commit eb86a85
Show file tree
Hide file tree
Showing 19 changed files with 169 additions and 63 deletions.
2 changes: 2 additions & 0 deletions doc/changes/10250.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Remove limitations on percent forms in the `(enabled_if ..)` field of
libraries (#10250, @rgrinberg)
6 changes: 4 additions & 2 deletions src/dune_rules/artifacts_obj.ml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ let empty = { libraries = Lib_name.Map.empty; modules = Module_name.Map.empty }
let lookup_module { modules; libraries = _ } = Module_name.Map.find modules
let lookup_library { libraries; modules = _ } = Lib_name.Map.find libraries

let make ~dir ~lib_config ~libs ~exes =
let make ~dir ~expander ~lib_config ~libs ~exes =
let+ libraries =
Memo.List.map libs ~f:(fun ((lib : Library.t), _, _) ->
let+ lib_config = lib_config in
let name = Lib_name.of_local lib.name in
let info = Library.to_lib_info lib ~dir ~lib_config in
let info =
Library.to_lib_info lib ~expander:(Memo.return expander) ~dir ~lib_config
in
name, info)
>>| Lib_name.Map.of_list_exn
in
Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/artifacts_obj.mli
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ val empty : t

val make
: dir:Path.Build.t
-> expander:Expander0.t
-> lib_config:Lib_config.t Memo.t
-> libs:(Library.t * Modules.t * Path.Build.t Obj_dir.t) list
-> exes:(Modules.t * Path.Build.t Obj_dir.t) list
Expand Down
5 changes: 2 additions & 3 deletions src/dune_rules/enabled_if.ml
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,9 @@ let emit_warning allowed_vars is_error var =
~loc
~is_error
[ Pp.textf
"Only %s variables are allowed in this 'enabled_if' field. If you think that %s \
should also be allowed, please file an issue about it."
"Only %s variables are allowed in this 'enabled_if' field. Please upgrade your \
dune language to at least 3.15."
(String.enumerate_and var_names)
(Dune_lang.Template.Pform.name var)
]
;;

Expand Down
10 changes: 10 additions & 0 deletions src/dune_rules/expander.ml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type t =

let artifacts t = t.artifacts_host
let dir t = t.dir
let project t = t.project
let context t = Context.name t.context

let set_local_env_var t ~var ~value =
Expand Down Expand Up @@ -908,3 +909,12 @@ let expand_locks t (locks : Locks.t) =
Memo.List.map locks ~f:(fun (Lock x) -> No_deps.expand_path t x)
|> Action_builder.of_memo
;;

module M = struct
type nonrec t = t

let project = project
let eval_blang = eval_blang
end

let to_expander0 t = Expander0.create (Memo.return t) (module M)
2 changes: 2 additions & 0 deletions src/dune_rules/expander.mli
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ type t

val dir : t -> Path.Build.t
val context : t -> Context_name.t
val project : t -> Dune_project.t

val make_root
: project:Dune_project.t
Expand Down Expand Up @@ -123,3 +124,4 @@ val foreign_flags
Fdecl.t

val lookup_artifacts : (dir:Path.Build.t -> Artifacts_obj.t Memo.t) Fdecl.t
val to_expander0 : t -> Expander0.t
23 changes: 23 additions & 0 deletions src/dune_rules/expander0.ml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,26 @@ let as_in_build_dir ~what ~loc p =
(Path.to_string_maybe_quoted p)
]
;;

module type S = sig
type t

val project : t -> Dune_project.t
val eval_blang : t -> Blang.t -> bool Memo.t
end

open Memo.O

type t = E : 'a Memo.t * (module S with type t = 'a) -> t

let db = Fdecl.create Dyn.opaque
let set_db = Fdecl.set db
let create e m = E (e, m)
let project (E (e, (module E))) = Memo.map e ~f:E.project

let eval_blang (E (e, (module E))) blang =
let* e = e in
E.eval_blang e blang
;;

let get ~dir = (Fdecl.get db) ~dir
14 changes: 14 additions & 0 deletions src/dune_rules/expander0.mli
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,17 @@ open Import

val isn't_allowed_in_this_position : source:Dune_lang.Template.Pform.t -> 'a
val as_in_build_dir : what:string -> loc:Loc.t -> Path.t -> Path.Build.t

module type S = sig
type t

val project : t -> Dune_project.t
val eval_blang : t -> Blang.t -> bool Memo.t
end

include S

val set_db : (dir:Path.Build.t -> t Memo.t) -> unit
val get : dir:Path.Build.t -> t Memo.t
val project : t -> Dune_project.t Memo.t
val create : 'a Memo.t -> (module S with type t = 'a) -> t
47 changes: 26 additions & 21 deletions src/dune_rules/install_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,14 @@ end = struct
ocaml.lib_config
in
let make_entry ?(loc = loc) = make_entry lib_subdir ~loc in
let info = Library.to_lib_info lib ~dir ~lib_config in
let* expander = Super_context.expander sctx ~dir in
let info =
Library.to_lib_info
lib
~expander:(Memo.return (Expander.to_expander0 expander))
~dir
~lib_config
in
let lib_name = Library.best_name lib in
let* installable_modules =
let+ modules =
Expand Down Expand Up @@ -210,26 +217,24 @@ end = struct
in
make_entry ?sub_dir Lib source ?dst))
in
let* additional_deps =
let+ expander = Super_context.expander sctx ~dir:lib_src_dir in
fun (loc, deps) ->
Lib_file_deps.eval deps ~expander ~loc ~paths:(Disallow_external lib_name)
>>| Path.Set.to_list_map ~f:(fun path ->
let path =
let path = path |> Path.as_in_build_dir_exn in
check_runtime_deps_relative_path ~lib_info:info ~loc (Path.Build.local path);
path
in
let sub_dir =
let src_dir = Path.Build.parent_exn path in
match Path.Build.equal lib_src_dir src_dir with
| true -> None
| false ->
Path.Build.local src_dir
|> Path.Local.descendant ~of_:(Path.Build.local lib_src_dir)
|> Option.map ~f:Path.Local.to_string
in
make_entry ?sub_dir Lib path)
let additional_deps (loc, deps) =
Lib_file_deps.eval deps ~expander ~loc ~paths:(Disallow_external lib_name)
>>| Path.Set.to_list_map ~f:(fun path ->
let path =
let path = path |> Path.as_in_build_dir_exn in
check_runtime_deps_relative_path ~lib_info:info ~loc (Path.Build.local path);
path
in
let sub_dir =
let src_dir = Path.Build.parent_exn path in
match Path.Build.equal lib_src_dir src_dir with
| true -> None
| false ->
Path.Build.local src_dir
|> Path.Local.descendant ~of_:(Path.Build.local lib_src_dir)
|> Option.map ~f:Path.Local.to_string
in
make_entry ?sub_dir Lib path)
in
let { Lib_config.has_native; ext_obj; _ } = lib_config in
let { Lib_mode.Map.ocaml = { Mode.Dict.byte; native } as ocaml; melange } =
Expand Down
10 changes: 8 additions & 2 deletions src/dune_rules/lib_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -580,13 +580,19 @@ let library_rules
let* () =
Memo.Option.iter vimpl ~f:(Virtual_rules.setup_copy_rules_for_impl ~sctx ~dir)
in
let* expander = Super_context.expander sctx ~dir in
let* () = Check_rules.add_cycle_check sctx ~dir top_sorted_modules in
let* () = gen_wrapped_compat_modules lib cctx
and* () = Module_compilation.build_all cctx
and* expander = Super_context.expander sctx ~dir
and* lib_info =
let lib_config = ocaml.lib_config in
let info = Library.to_lib_info lib ~dir ~lib_config in
let info =
Library.to_lib_info
lib
~expander:(Memo.return (Expander.to_expander0 expander))
~dir
~lib_config
in
let mode = Lib_mode.Map.Set.for_merlin (Lib_info.modes info) in
let+ () = Check_rules.add_obj_dir sctx ~obj_dir mode in
info
Expand Down
7 changes: 6 additions & 1 deletion src/dune_rules/ml_sources.ml
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,12 @@ let make
List.map modules_of_stanzas.executables ~f:(fun (part : _ Modules.group_part) ->
part.modules, part.obj_dir)
in
Artifacts_obj.make ~dir ~lib_config ~libs ~exes)
Artifacts_obj.make
~dir
~expander:(Expander.to_expander0 expander)
~lib_config
~libs
~exes)
in
{ modules; artifacts; include_subdirs }
;;
18 changes: 11 additions & 7 deletions src/dune_rules/scope.ml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ module DB = struct
end

let create_db_from_stanzas ~instrument_with ~parent ~lib_config stanzas =
let (map : Found_or_redirect.t Lib_name.Map.t) =
let map =
List.map stanzas ~f:(fun stanza ->
match (stanza : Library_related_stanza.t) with
| Library_redirect s ->
Expand All @@ -62,7 +62,10 @@ module DB = struct
let old_public_name = Deprecated_library_name.old_public_name s in
Found_or_redirect.redirect old_public_name s.new_public_name
| Library (dir, (conf : Library.t)) ->
let info = Library.to_lib_info conf ~dir ~lib_config |> Lib_info.of_local in
let info =
let expander = Expander0.get ~dir in
Library.to_lib_info conf ~expander ~dir ~lib_config |> Lib_info.of_local
in
Library.best_name conf, Found_or_redirect.found info)
|> Lib_name.Map.of_list_reducei ~f:(fun name (v1 : Found_or_redirect.t) v2 ->
let res =
Expand Down Expand Up @@ -100,11 +103,12 @@ module DB = struct
~parent:(Some parent)
~resolve:(fun name ->
Memo.return
(match Lib_name.Map.find map name with
| None -> Lib.DB.Resolve_result.not_found
| Some (Redirect lib) -> Lib.DB.Resolve_result.redirect_in_the_same_db lib
| Some (Found lib) -> Lib.DB.Resolve_result.found lib))
~all:(fun () -> Lib_name.Map.keys map |> Memo.return)
@@
match Lib_name.Map.find map name with
| None -> Lib.DB.Resolve_result.not_found
| Some (Redirect lib) -> Lib.DB.Resolve_result.redirect_in_the_same_db lib
| Some (Found lib) -> Lib.DB.Resolve_result.found lib)
~all:(fun () -> Memo.return @@ Lib_name.Map.keys map)
~lib_config
~instrument_with
;;
Expand Down
43 changes: 26 additions & 17 deletions src/dune_rules/stanzas/library.ml
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,13 @@ let decode =
and+ enabled_if =
let open Enabled_if in
let allowed_vars =
Only
(("context_name", (2, 8))
:: ("profile", (2, 5))
:: Lib_config.allowed_in_enabled_if)
if Dune_project.dune_version project >= (3, 15)
then Any
else
Only
(("context_name", (2, 8))
:: ("profile", (2, 5))
:: Lib_config.allowed_in_enabled_if)
in
decode ~allowed_vars ~since:(Some (1, 10)) ()
and+ instrumentation_backend =
Expand Down Expand Up @@ -402,6 +405,7 @@ let main_module_name t : Lib_info.Main_module_name.t =

let to_lib_info
conf
~expander
~dir
~lib_config:
({ Lib_config.has_native; ext_lib; ext_dll; natdynlink_supported; _ } as lib_config)
Expand Down Expand Up @@ -474,19 +478,24 @@ let to_lib_info
let name = best_name conf in
let enabled =
let+ enabled_if_result =
Blang_expand.eval conf.enabled_if ~dir:(Path.build dir) ~f:(fun ~source:_ pform ->
let+ value =
match pform with
| Var Context_name ->
let context, _ = Path.Build.extract_build_context_exn dir in
Memo.return context
| Var Profile ->
let context, _ = Path.Build.extract_build_context_exn dir in
let+ profile = Per_context.profile (Context_name.of_string context) in
Profile.to_string profile
| _ -> Memo.return @@ Lib_config.get_for_enabled_if lib_config pform
in
[ Value.String value ])
let* expander = expander in
let* project = Expander0.project expander in
if Dune_project.dune_version project >= (3, 15)
then Expander0.eval_blang expander conf.enabled_if
else
Blang_expand.eval conf.enabled_if ~dir:(Path.build dir) ~f:(fun ~source:_ pform ->
let+ value =
match pform with
| Var Context_name ->
let context, _ = Path.Build.extract_build_context_exn dir in
Memo.return context
| Var Profile ->
let context, _ = Path.Build.extract_build_context_exn dir in
let+ profile = Per_context.profile (Context_name.of_string context) in
Profile.to_string profile
| _ -> Memo.return @@ Lib_config.get_for_enabled_if lib_config pform
in
[ Value.String value ])
in
if not enabled_if_result
then Lib_info.Enabled_status.Disabled_because_of_enabled_if
Expand Down
8 changes: 7 additions & 1 deletion src/dune_rules/stanzas/library.mli
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,10 @@ val is_virtual : t -> bool
val is_impl : t -> bool
val obj_dir : dir:Path.Build.t -> t -> Path.Build.t Obj_dir.t
val main_module_name : t -> Lib_info.Main_module_name.t
val to_lib_info : t -> dir:Path.Build.t -> lib_config:Lib_config.t -> Lib_info.local

val to_lib_info
: t
-> expander:Expander0.t Memo.t
-> dir:Path.Build.t
-> lib_config:Lib_config.t
-> Lib_info.local
8 changes: 7 additions & 1 deletion src/dune_rules/super_context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,13 @@ let () =
let* ctx = Context.DB.by_dir dir in
let* t = find_exn (Context.name ctx) in
let* expander = expander t ~dir in
Expander.expand_str expander sw |> Action_builder.evaluate_and_collect_facts >>| fst)
Expander.expand_str expander sw |> Action_builder.evaluate_and_collect_facts >>| fst);
Expander0.set_db (fun ~dir ->
Context.DB.by_dir dir
>>| Context.name
>>= find_exn
>>= expander ~dir
>>| Expander.to_expander0)
;;

let context t = t.context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ The next ones use forbidden variables For dune 2.3 -> 2.5 it is a warning
^^^^^^^^^^^^^^^
Warning: Only architecture, system, model, os_type, ccomp_type, profile,
ocaml_version, context_name and arch_sixtyfour variables are allowed in this
'enabled_if' field. If you think that project_root should also be allowed,
please file an issue about it.
'enabled_if' field. Please upgrade your dune language to at least 3.15.
bar

For dune >= 2.6 it is an error
Expand All @@ -23,6 +22,5 @@ For dune >= 2.6 it is an error
^^^^^^^^^^^^^^^
Error: Only architecture, system, model, os_type, ccomp_type, profile,
ocaml_version, context_name and arch_sixtyfour variables are allowed in this
'enabled_if' field. If you think that project_root should also be allowed,
please file an issue about it.
'enabled_if' field. Please upgrade your dune language to at least 3.15.
[1]
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ This one uses forbidden variables
^^^^^^^^^^^^^^^
Error: Only context_name, profile, architecture, system, model, os_type,
ccomp_type and ocaml_version variables are allowed in this 'enabled_if'
field. If you think that project_root should also be allowed, please file an
issue about it.
field. Please upgrade your dune language to at least 3.15.
[1]
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ Tests for enabled_if in install stanza using forbidden variable.
^^^^^^^^^^^^^^^
Error: Only architecture, system, model, os_type, ccomp_type, profile,
ocaml_version, context_name and arch_sixtyfour variables are allowed in this
'enabled_if' field. If you think that project_root should also be allowed,
please file an issue about it.
'enabled_if' field. Please upgrade your dune language to at least 3.15.
[1]
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Here we demonstrate that we expand any variable in enabled_if on libraries

$ cat >dune-project <<EOF
> (lang dune 3.15)
> EOF

$ cat >dune <<EOF
> (library
> (name foo)
> (enabled_if %{env:FOO=false}))
> EOF
$ dune build %{cma:./foo}
Error: No rule found for foo.cma
-> required by %{cma:./foo} at command line:1
[1]
$ FOO=true dune build %{cma:./foo}

0 comments on commit eb86a85

Please sign in to comment.