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

feature: remove limitations on [enabled_if] on libraries #10250

Merged
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/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
rgrinberg marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -587,13 +587,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 ->
jchavarri marked this conversation as resolved.
Show resolved Hide resolved
rgrinberg marked this conversation as resolved.
Show resolved Hide resolved
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
rgrinberg marked this conversation as resolved.
Show resolved Hide resolved

$ 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}
File "command line", line 1, characters 0-12:
Error: Library foo does not exist.
[1]
$ FOO=true dune build %{cma:./foo}
Loading