From 286a3c0ee9c3fa21ac55350d25510a5bcf6d6309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Tue, 5 Mar 2024 11:45:09 +0000 Subject: [PATCH 01/17] eif: error on dups in ml_sources if both are enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/artifacts_obj.ml | 6 +- src/dune_rules/artifacts_obj.mli | 4 +- src/dune_rules/ml_sources.ml | 91 ++++++++++++++++--- src/dune_rules/modules.ml | 10 ++ src/dune_rules/modules.mli | 5 + .../eif-exe-name-collision-same-folder.t | 13 ++- ...-melange-emit-name-collision-same-folder.t | 13 ++- 7 files changed, 115 insertions(+), 27 deletions(-) diff --git a/src/dune_rules/artifacts_obj.ml b/src/dune_rules/artifacts_obj.ml index 51798f86048..1a4b8b4778b 100644 --- a/src/dune_rules/artifacts_obj.ml +++ b/src/dune_rules/artifacts_obj.ml @@ -12,7 +12,7 @@ let lookup_library { libraries; modules = _ } = Lib_name.Map.find libraries let make ~dir ~lib_config ~libs ~exes = let+ libraries = - Memo.List.map libs ~f:(fun ((lib : Library.t), _, _, _) -> + 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 @@ -28,9 +28,9 @@ let make ~dir ~lib_config ~libs ~exes = List.fold_left exes ~init:Module_name.Map.empty - ~f:(fun modules (_, _, m, obj_dir) -> by_name modules obj_dir m) + ~f:(fun modules (_, _, m, obj_dir, _) -> by_name modules obj_dir m) in - List.fold_left libs ~init ~f:(fun modules (_, _, m, obj_dir) -> + List.fold_left libs ~init ~f:(fun modules (_, _, m, obj_dir, _) -> by_name modules obj_dir m) in { libraries; modules } diff --git a/src/dune_rules/artifacts_obj.mli b/src/dune_rules/artifacts_obj.mli index d09a956155d..000b43b8229 100644 --- a/src/dune_rules/artifacts_obj.mli +++ b/src/dune_rules/artifacts_obj.mli @@ -7,8 +7,8 @@ val empty : t val make : dir:Path.Build.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 + -> libs:(Library.t * _ * Modules.t * Path.Build.t Obj_dir.t * Modules.enabled) list + -> exes:(_ * _ * Modules.t * Path.Build.t Obj_dir.t * Modules.enabled) list -> t Memo.t val lookup_module : t -> Module_name.t -> (Path.Build.t Obj_dir.t * Module.t) option diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index e348a6a6bfd..99e16639dda 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -45,6 +45,7 @@ module Modules = struct * (Loc.t * Module.Source.t) Module_trie.t * Modules_group.t * Path.Build.t Obj_dir.t + * Modules.enabled type groups = { libraries : Library.t group_part list @@ -52,42 +53,102 @@ module Modules = struct ; melange_emits : Melange_stanzas.Emit.t group_part list } + let enabled_of_bool = Modules.enabled_of_bool + let make { libraries = libs; executables = exes; melange_emits = emits } = let libraries = match - Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir) -> + Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> Library.best_name lib, (m, obj_dir)) with | Ok x -> x - | Error (name, _, (lib2, _, _, _)) -> + | Error (name, (_, _, _, _, Enabled), (lib2, _, _, _, Enabled)) -> User_error.raise ~loc:lib2.buildable.loc [ Pp.textf "Library %S appears for the second time in this directory" (Lib_name.to_string name) ] + | Error _ -> + (* In this case, there are two libraries with the same name in the same + folder, but at least one of them is disabled under the current context, + so we filter all duplicates out *) + (match + libs + |> List.filter ~f:(fun (_, _, _, _, enabled) -> + match enabled with + | Modules.Enabled -> true + | Disabled -> false) + |> Lib_name.Map.of_list_map ~f:(fun (lib, _, m, obj_dir, _) -> + Library.best_name lib, (m, obj_dir)) + with + | Ok x -> x + | Error (name, _, (lib2, _, _, _, _)) -> + User_error.raise + ~loc:lib2.buildable.loc + [ Pp.textf + "Library %S appears for the second time in this directory" + (Lib_name.to_string name) + ]) in let executables = match - String.Map.of_list_map exes ~f:(fun ((exes : Executables.t), _, m, obj_dir) -> + String.Map.of_list_map exes ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> snd (List.hd exes.names), (m, obj_dir)) with | Ok x -> x - | Error (name, _, (exes2, _, _, _)) -> + | Error (name, (_, _, _, _, Enabled), (exes2, _, _, _, Enabled)) -> User_error.raise ~loc:exes2.buildable.loc [ Pp.textf "Executable %S appears for the second time in this directory" name ] + | Error _ -> + (* In this case, there are two exes with the same name in the same + folder, but at least one of them is disabled under the current context, + so we filter all duplicates out *) + (match + exes + |> List.filter ~f:(fun (_, _, _, _, enabled) -> + match enabled with + | Modules.Enabled -> true + | Disabled -> false) + |> String.Map.of_list_map ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> + snd (List.hd exes.names), (m, obj_dir)) + with + | Ok x -> x + | Error (name, _, (exes2, _, _, _, _)) -> + User_error.raise + ~loc:exes2.buildable.loc + [ Pp.textf "Executable %S appears for the second time in this directory" name + ]) in let melange_emits = match - String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir) -> + String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> mel.target, (m, obj_dir)) with | Ok x -> x - | Error (name, _, (mel, _, _, _)) -> + | Error (name, (_, _, _, _, Enabled), (mel, _, _, _, Enabled)) -> User_error.raise ~loc:mel.loc [ Pp.textf "Target %S appears for the second time in this directory" name ] + | Error _ -> + (* In this case, there are two exes with the same name in the same + folder, but at least one of them is disabled under the current context, + so we filter all duplicates out *) + (match + emits + |> List.filter ~f:(fun (_, _, _, _, enabled) -> + match enabled with + | Modules.Enabled -> true + | Disabled -> false) + |> String.Map.of_list_map ~f:(fun (mel, _, m, obj_dir, _) -> + mel.Melange_stanzas.Emit.target, (m, obj_dir)) + with + | Ok x -> x + | Error (name, _, (mel, _, _, _, _)) -> + User_error.raise + ~loc:mel.loc + [ Pp.textf "Target %S appears for the second time in this directory" name ]) in let rev_map = let modules = @@ -96,9 +157,9 @@ module Modules = struct (Module.Source.path m, origin) :: acc) in List.concat - [ List.concat_map libs ~f:(fun (l, m, _, _) -> by_path (Library l) m) - ; List.concat_map exes ~f:(fun (e, m, _, _) -> by_path (Executables e) m) - ; List.concat_map emits ~f:(fun (l, m, _, _) -> by_path (Melange l) m) + [ List.concat_map libs ~f:(fun (l, m, _, _, _) -> by_path (Library l) m) + ; List.concat_map exes ~f:(fun (e, m, _, _, _) -> by_path (Executables e) m) + ; List.concat_map emits ~f:(fun (l, m, _, _, _) -> by_path (Melange l) m) ] in match Module_name.Path.Map.of_list modules with @@ -393,6 +454,8 @@ let modules_of_stanzas = Memo.parallel_map stanzas ~f:(fun stanza -> match Stanza.repr stanza with | Library.T lib -> + let* enabled = Expander.eval_blang expander lib.enabled_if in + let enabled = Modules.enabled_of_bool enabled in (* jeremiedimino: this [Resolve.get] means that if the user writes an invalid [implements] field, we will get an error immediately even if the library is not built. We should change this to carry the @@ -411,9 +474,11 @@ let modules_of_stanzas = >>= Resolve.read_memo in let obj_dir = Library.obj_dir lib ~dir in - `Library (lib, sources, modules, obj_dir) + `Library (lib, sources, modules, obj_dir, enabled) | Executables.T exes | Tests.T { exes; _ } -> let obj_dir = Executables.obj_dir ~dir exes in + let* enabled = Expander.eval_blang expander exes.enabled_if in + let enabled = Modules.enabled_of_bool enabled in let+ sources, modules = let { Buildable.loc = stanza_loc; modules = modules_settings; _ } = exes.buildable @@ -434,9 +499,11 @@ let modules_of_stanzas = then Modules_group.make_wrapped ~obj_dir ~modules `Exe else Modules_group.exe_unwrapped modules ~obj_dir in - `Executables (exes, sources, modules, obj_dir) + `Executables (exes, sources, modules, obj_dir, enabled) | Melange_stanzas.Emit.T mel -> let obj_dir = Obj_dir.make_melange_emit ~dir ~name:mel.target in + let* enabled = Expander.eval_blang expander mel.enabled_if in + let enabled = Modules.enabled_of_bool enabled in let+ sources, modules = Modules_field_evaluator.eval ~expander @@ -451,7 +518,7 @@ let modules_of_stanzas = let modules = Modules_group.make_wrapped ~obj_dir:(Obj_dir.obj_dir obj_dir) ~modules `Melange in - `Melange_emit (mel, sources, modules, obj_dir) + `Melange_emit (mel, sources, modules, obj_dir, enabled) | _ -> Memo.return `Skip) >>| filter_partition_map ;; diff --git a/src/dune_rules/modules.ml b/src/dune_rules/modules.ml index d67466740af..1029f98f8ef 100644 --- a/src/dune_rules/modules.ml +++ b/src/dune_rules/modules.ml @@ -1264,3 +1264,13 @@ let canonical_path t (group : Group.t) m = | Impl { impl = { modules = Wrapped w; _ }; _ } | Wrapped w -> w.group.name :: path | _ -> Module.path m ;; + +type enabled = + | Enabled + | Disabled + +let enabled_of_bool b = + match b with + | true -> Enabled + | false -> Disabled +;; diff --git a/src/dune_rules/modules.mli b/src/dune_rules/modules.mli index be2ef790dcb..5ed6d61ebc8 100644 --- a/src/dune_rules/modules.mli +++ b/src/dune_rules/modules.mli @@ -4,6 +4,11 @@ open Import type t +type enabled = + | Enabled + | Disabled + +val enabled_of_bool : bool -> enabled val to_dyn : t -> Dyn.t val lib diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t b/test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t index a00dede320b..58dbd31b0f5 100644 --- a/test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t +++ b/test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t @@ -27,9 +27,12 @@ in the same dune file > EOF $ dune build - File "dune", line 4, characters 0-72: - 4 | (executable - 5 | (name foo) - 6 | (enabled_if (= %{context_name} "alt-context"))) - Error: Executable "foo" appears for the second time in this directory + File "dune", line 1, characters 0-0: + Error: Module "Foo" is used in several stanzas: + - dune:1 + - dune:4 + To fix this error, you must specify an explicit "modules" field in every + library, executable, and executables stanzas in this dune file. Note that + each module cannot appear in more than one "modules" field - it must belong + to a single library or executable. [1] diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t b/test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t index d680b59de29..913c7e3ef83 100644 --- a/test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t +++ b/test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t @@ -28,9 +28,12 @@ in the same dune file > EOF $ dune build - File "dune", line 4, characters 0-76: - 4 | (melange.emit - 5 | (target foo) - 6 | (enabled_if (= %{context_name} "alt-context"))) - Error: Target "foo" appears for the second time in this directory + File "dune", line 1, characters 0-0: + Error: Module "Foo" is used in several stanzas: + - dune:1 + - dune:4 + To fix this error, you must specify an explicit "modules" field in every + library, executable, and executables stanzas in this dune file. Note that + each module cannot appear in more than one "modules" field - it must belong + to a single library or executable. [1] From 9216dad791780dded27e43748eb60c36e84f94a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Tue, 5 Mar 2024 12:34:09 +0000 Subject: [PATCH 02/17] eif: not add modules in disabled libs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/ml_sources.ml | 63 ++++++++++--------- .../eif-exe-name-collision-same-folder.t | 9 --- ...-melange-emit-name-collision-same-folder.t | 9 --- 3 files changed, 34 insertions(+), 47 deletions(-) diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index 99e16639dda..df411f71b6c 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -56,12 +56,12 @@ module Modules = struct let enabled_of_bool = Modules.enabled_of_bool let make { libraries = libs; executables = exes; melange_emits = emits } = - let libraries = + let libraries, libs = match Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> Library.best_name lib, (m, obj_dir)) with - | Ok x -> x + | Ok x -> x, libs | Error (name, (_, _, _, _, Enabled), (lib2, _, _, _, Enabled)) -> User_error.raise ~loc:lib2.buildable.loc @@ -73,16 +73,17 @@ module Modules = struct (* In this case, there are two libraries with the same name in the same folder, but at least one of them is disabled under the current context, so we filter all duplicates out *) + let libs = + List.filter libs ~f:(fun (_, _, _, _, enabled) -> + match enabled with + | Modules.Enabled -> true + | Disabled -> false) + in (match - libs - |> List.filter ~f:(fun (_, _, _, _, enabled) -> - match enabled with - | Modules.Enabled -> true - | Disabled -> false) - |> Lib_name.Map.of_list_map ~f:(fun (lib, _, m, obj_dir, _) -> + Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> Library.best_name lib, (m, obj_dir)) with - | Ok x -> x + | Ok x -> x, libs | Error (name, _, (lib2, _, _, _, _)) -> User_error.raise ~loc:lib2.buildable.loc @@ -91,12 +92,12 @@ module Modules = struct (Lib_name.to_string name) ]) in - let executables = + let executables, exes = match String.Map.of_list_map exes ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> snd (List.hd exes.names), (m, obj_dir)) with - | Ok x -> x + | Ok x -> x, exes | Error (name, (_, _, _, _, Enabled), (exes2, _, _, _, Enabled)) -> User_error.raise ~loc:exes2.buildable.loc @@ -105,46 +106,50 @@ module Modules = struct (* In this case, there are two exes with the same name in the same folder, but at least one of them is disabled under the current context, so we filter all duplicates out *) + let exes = + List.filter exes ~f:(fun (_, _, _, _, enabled) -> + match enabled with + | Modules.Enabled -> true + | Disabled -> false) + in (match - exes - |> List.filter ~f:(fun (_, _, _, _, enabled) -> - match enabled with - | Modules.Enabled -> true - | Disabled -> false) - |> String.Map.of_list_map ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> - snd (List.hd exes.names), (m, obj_dir)) + String.Map.of_list_map + exes + ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> + snd (List.hd exes.names), (m, obj_dir)) with - | Ok x -> x + | Ok x -> x, exes | Error (name, _, (exes2, _, _, _, _)) -> User_error.raise ~loc:exes2.buildable.loc [ Pp.textf "Executable %S appears for the second time in this directory" name ]) in - let melange_emits = + let melange_emits, emits = match String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> mel.target, (m, obj_dir)) with - | Ok x -> x + | Ok x -> x, emits | Error (name, (_, _, _, _, Enabled), (mel, _, _, _, Enabled)) -> User_error.raise ~loc:mel.loc [ Pp.textf "Target %S appears for the second time in this directory" name ] | Error _ -> - (* In this case, there are two exes with the same name in the same + (* In this case, there are two melange.emits with the same name in the same folder, but at least one of them is disabled under the current context, so we filter all duplicates out *) + let emits = + List.filter emits ~f:(fun (_, _, _, _, enabled) -> + match enabled with + | Modules.Enabled -> true + | Disabled -> false) + in (match - emits - |> List.filter ~f:(fun (_, _, _, _, enabled) -> - match enabled with - | Modules.Enabled -> true - | Disabled -> false) - |> String.Map.of_list_map ~f:(fun (mel, _, m, obj_dir, _) -> + String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> mel.Melange_stanzas.Emit.target, (m, obj_dir)) with - | Ok x -> x + | Ok x -> x, emits | Error (name, _, (mel, _, _, _, _)) -> User_error.raise ~loc:mel.loc diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t b/test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t index 58dbd31b0f5..aeb12d69f1e 100644 --- a/test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t +++ b/test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t @@ -27,12 +27,3 @@ in the same dune file > EOF $ dune build - File "dune", line 1, characters 0-0: - Error: Module "Foo" is used in several stanzas: - - dune:1 - - dune:4 - To fix this error, you must specify an explicit "modules" field in every - library, executable, and executables stanzas in this dune file. Note that - each module cannot appear in more than one "modules" field - it must belong - to a single library or executable. - [1] diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t b/test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t index 913c7e3ef83..21a8f936375 100644 --- a/test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t +++ b/test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t @@ -28,12 +28,3 @@ in the same dune file > EOF $ dune build - File "dune", line 1, characters 0-0: - Error: Module "Foo" is used in several stanzas: - - dune:1 - - dune:4 - To fix this error, you must specify an explicit "modules" field in every - library, executable, and executables stanzas in this dune file. Note that - each module cannot appear in more than one "modules" field - it must belong - to a single library or executable. - [1] From e02387a433875daf76ddb2ed16da996fe9d40cb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Wed, 6 Mar 2024 08:48:57 +0000 Subject: [PATCH 03/17] eif: use Toggle.t instead of a custom type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/artifacts_obj.mli | 4 ++-- src/dune_rules/ml_sources.ml | 31 ++++++++++--------------------- src/dune_rules/modules.ml | 10 ---------- src/dune_rules/modules.mli | 5 ----- 4 files changed, 12 insertions(+), 38 deletions(-) diff --git a/src/dune_rules/artifacts_obj.mli b/src/dune_rules/artifacts_obj.mli index 000b43b8229..f60596cd4b6 100644 --- a/src/dune_rules/artifacts_obj.mli +++ b/src/dune_rules/artifacts_obj.mli @@ -7,8 +7,8 @@ val empty : t val make : dir:Path.Build.t -> lib_config:Lib_config.t Memo.t - -> libs:(Library.t * _ * Modules.t * Path.Build.t Obj_dir.t * Modules.enabled) list - -> exes:(_ * _ * Modules.t * Path.Build.t Obj_dir.t * Modules.enabled) list + -> libs:(Library.t * _ * Modules.t * Path.Build.t Obj_dir.t * Toggle.t) list + -> exes:(_ * _ * Modules.t * Path.Build.t Obj_dir.t * Toggle.t) list -> t Memo.t val lookup_module : t -> Module_name.t -> (Path.Build.t Obj_dir.t * Module.t) option diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index df411f71b6c..05d8a8c574f 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -45,7 +45,7 @@ module Modules = struct * (Loc.t * Module.Source.t) Module_trie.t * Modules_group.t * Path.Build.t Obj_dir.t - * Modules.enabled + * Toggle.t type groups = { libraries : Library.t group_part list @@ -53,8 +53,6 @@ module Modules = struct ; melange_emits : Melange_stanzas.Emit.t group_part list } - let enabled_of_bool = Modules.enabled_of_bool - let make { libraries = libs; executables = exes; melange_emits = emits } = let libraries, libs = match @@ -62,7 +60,7 @@ module Modules = struct Library.best_name lib, (m, obj_dir)) with | Ok x -> x, libs - | Error (name, (_, _, _, _, Enabled), (lib2, _, _, _, Enabled)) -> + | Error (name, (_, _, _, _, `Enabled), (lib2, _, _, _, `Enabled)) -> User_error.raise ~loc:lib2.buildable.loc [ Pp.textf @@ -74,10 +72,7 @@ module Modules = struct folder, but at least one of them is disabled under the current context, so we filter all duplicates out *) let libs = - List.filter libs ~f:(fun (_, _, _, _, enabled) -> - match enabled with - | Modules.Enabled -> true - | Disabled -> false) + List.filter libs ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) in (match Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> @@ -98,7 +93,7 @@ module Modules = struct snd (List.hd exes.names), (m, obj_dir)) with | Ok x -> x, exes - | Error (name, (_, _, _, _, Enabled), (exes2, _, _, _, Enabled)) -> + | Error (name, (_, _, _, _, `Enabled), (exes2, _, _, _, `Enabled)) -> User_error.raise ~loc:exes2.buildable.loc [ Pp.textf "Executable %S appears for the second time in this directory" name ] @@ -107,10 +102,7 @@ module Modules = struct folder, but at least one of them is disabled under the current context, so we filter all duplicates out *) let exes = - List.filter exes ~f:(fun (_, _, _, _, enabled) -> - match enabled with - | Modules.Enabled -> true - | Disabled -> false) + List.filter exes ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) in (match String.Map.of_list_map @@ -131,7 +123,7 @@ module Modules = struct mel.target, (m, obj_dir)) with | Ok x -> x, emits - | Error (name, (_, _, _, _, Enabled), (mel, _, _, _, Enabled)) -> + | Error (name, (_, _, _, _, `Enabled), (mel, _, _, _, `Enabled)) -> User_error.raise ~loc:mel.loc [ Pp.textf "Target %S appears for the second time in this directory" name ] @@ -140,10 +132,7 @@ module Modules = struct folder, but at least one of them is disabled under the current context, so we filter all duplicates out *) let emits = - List.filter emits ~f:(fun (_, _, _, _, enabled) -> - match enabled with - | Modules.Enabled -> true - | Disabled -> false) + List.filter emits ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) in (match String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> @@ -460,7 +449,7 @@ let modules_of_stanzas = match Stanza.repr stanza with | Library.T lib -> let* enabled = Expander.eval_blang expander lib.enabled_if in - let enabled = Modules.enabled_of_bool enabled in + let enabled = Toggle.of_bool enabled in (* jeremiedimino: this [Resolve.get] means that if the user writes an invalid [implements] field, we will get an error immediately even if the library is not built. We should change this to carry the @@ -483,7 +472,7 @@ let modules_of_stanzas = | Executables.T exes | Tests.T { exes; _ } -> let obj_dir = Executables.obj_dir ~dir exes in let* enabled = Expander.eval_blang expander exes.enabled_if in - let enabled = Modules.enabled_of_bool enabled in + let enabled = Toggle.of_bool enabled in let+ sources, modules = let { Buildable.loc = stanza_loc; modules = modules_settings; _ } = exes.buildable @@ -508,7 +497,7 @@ let modules_of_stanzas = | Melange_stanzas.Emit.T mel -> let obj_dir = Obj_dir.make_melange_emit ~dir ~name:mel.target in let* enabled = Expander.eval_blang expander mel.enabled_if in - let enabled = Modules.enabled_of_bool enabled in + let enabled = Toggle.of_bool enabled in let+ sources, modules = Modules_field_evaluator.eval ~expander diff --git a/src/dune_rules/modules.ml b/src/dune_rules/modules.ml index 1029f98f8ef..d67466740af 100644 --- a/src/dune_rules/modules.ml +++ b/src/dune_rules/modules.ml @@ -1264,13 +1264,3 @@ let canonical_path t (group : Group.t) m = | Impl { impl = { modules = Wrapped w; _ }; _ } | Wrapped w -> w.group.name :: path | _ -> Module.path m ;; - -type enabled = - | Enabled - | Disabled - -let enabled_of_bool b = - match b with - | true -> Enabled - | false -> Disabled -;; diff --git a/src/dune_rules/modules.mli b/src/dune_rules/modules.mli index 5ed6d61ebc8..be2ef790dcb 100644 --- a/src/dune_rules/modules.mli +++ b/src/dune_rules/modules.mli @@ -4,11 +4,6 @@ open Import type t -type enabled = - | Enabled - | Disabled - -val enabled_of_bool : bool -> enabled val to_dyn : t -> Dyn.t val lib From bdbdefffc0e18130963429e03d9adf9591ae17ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Thu, 7 Mar 2024 17:54:41 +0000 Subject: [PATCH 04/17] simplify implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/melange/melange_rules.ml | 18 ++++-- src/dune_rules/ml_sources.ml | 78 +++++-------------------- 2 files changed, 29 insertions(+), 67 deletions(-) diff --git a/src/dune_rules/melange/melange_rules.ml b/src/dune_rules/melange/melange_rules.ml index 4f51081bfb4..50d45498bf3 100644 --- a/src/dune_rules/melange/melange_rules.ml +++ b/src/dune_rules/melange/melange_rules.ml @@ -643,10 +643,20 @@ let setup_emit_js_rules sctx ~dir = under_melange_emit_target ~dir >>= function | Some melange -> - gen_emit_rules sctx ~dir melange - >>| (function - | None -> Gen_rules.redirect_to_parent Gen_rules.Rules.empty - | Some melange -> Gen_rules.make melange) + let* enabled = + let* expander = + let* sctx = sctx in + Super_context.expander sctx ~dir + in + Expander.eval_blang expander melange.stanza.enabled_if + in + (match enabled with + | true -> + gen_emit_rules sctx ~dir melange + >>| (function + | None -> Gen_rules.redirect_to_parent Gen_rules.Rules.empty + | Some melange -> Gen_rules.make melange) + | false -> Memo.return (Gen_rules.redirect_to_parent Gen_rules.Rules.empty)) | None -> (* this should probably be handled by [Dir_status] *) Dune_load.stanzas_in_dir dir diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index 05d8a8c574f..93343437289 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -54,95 +54,47 @@ module Modules = struct } let make { libraries = libs; executables = exes; melange_emits = emits } = - let libraries, libs = + let keep_enabled t = + List.filter t ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) + in + let libs = keep_enabled libs in + let exes = keep_enabled exes in + let emits = keep_enabled emits in + let libraries = match Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> Library.best_name lib, (m, obj_dir)) with - | Ok x -> x, libs - | Error (name, (_, _, _, _, `Enabled), (lib2, _, _, _, `Enabled)) -> + | Ok x -> x + | Error (name, _, (lib2, _, _, _, _)) -> User_error.raise ~loc:lib2.buildable.loc [ Pp.textf "Library %S appears for the second time in this directory" (Lib_name.to_string name) ] - | Error _ -> - (* In this case, there are two libraries with the same name in the same - folder, but at least one of them is disabled under the current context, - so we filter all duplicates out *) - let libs = - List.filter libs ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) - in - (match - Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> - Library.best_name lib, (m, obj_dir)) - with - | Ok x -> x, libs - | Error (name, _, (lib2, _, _, _, _)) -> - User_error.raise - ~loc:lib2.buildable.loc - [ Pp.textf - "Library %S appears for the second time in this directory" - (Lib_name.to_string name) - ]) in - let executables, exes = + let executables = match String.Map.of_list_map exes ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> snd (List.hd exes.names), (m, obj_dir)) with - | Ok x -> x, exes - | Error (name, (_, _, _, _, `Enabled), (exes2, _, _, _, `Enabled)) -> + | Ok x -> x + | Error (name, _, (exes2, _, _, _, _)) -> User_error.raise ~loc:exes2.buildable.loc [ Pp.textf "Executable %S appears for the second time in this directory" name ] - | Error _ -> - (* In this case, there are two exes with the same name in the same - folder, but at least one of them is disabled under the current context, - so we filter all duplicates out *) - let exes = - List.filter exes ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) - in - (match - String.Map.of_list_map - exes - ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> - snd (List.hd exes.names), (m, obj_dir)) - with - | Ok x -> x, exes - | Error (name, _, (exes2, _, _, _, _)) -> - User_error.raise - ~loc:exes2.buildable.loc - [ Pp.textf "Executable %S appears for the second time in this directory" name - ]) in - let melange_emits, emits = + let melange_emits = match String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> mel.target, (m, obj_dir)) with - | Ok x -> x, emits - | Error (name, (_, _, _, _, `Enabled), (mel, _, _, _, `Enabled)) -> + | Ok x -> x + | Error (name, _, (mel, _, _, _, _)) -> User_error.raise ~loc:mel.loc [ Pp.textf "Target %S appears for the second time in this directory" name ] - | Error _ -> - (* In this case, there are two melange.emits with the same name in the same - folder, but at least one of them is disabled under the current context, - so we filter all duplicates out *) - let emits = - List.filter emits ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) - in - (match - String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> - mel.Melange_stanzas.Emit.target, (m, obj_dir)) - with - | Ok x -> x, emits - | Error (name, _, (mel, _, _, _, _)) -> - User_error.raise - ~loc:mel.loc - [ Pp.textf "Target %S appears for the second time in this directory" name ]) in let rev_map = let modules = From 50acad1ddae60eaa176c7e8919cb7fa0726394d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Fri, 8 Mar 2024 09:25:31 +0000 Subject: [PATCH 05/17] fix melange emit case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/melange/melange_rules.ml | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/dune_rules/melange/melange_rules.ml b/src/dune_rules/melange/melange_rules.ml index 50d45498bf3..4e6176d2045 100644 --- a/src/dune_rules/melange/melange_rules.ml +++ b/src/dune_rules/melange/melange_rules.ml @@ -581,20 +581,27 @@ let emit_rules sctx { stanza_dir; stanza } = ;; (* Detect if [dir] is under the target directory of a melange.emit stanza. *) -let rec under_melange_emit_target ~dir = +let rec under_melange_emit_target ~sctx ~dir = match Path.Build.parent dir with | None -> Memo.return None | Some parent -> Dune_load.stanzas_in_dir parent >>= (function - | None -> under_melange_emit_target ~dir:parent + | None -> under_melange_emit_target ~sctx ~dir:parent | Some stanzas -> Dune_file.find_stanzas stanzas Melange_stanzas.Emit.key - >>| List.find_map ~f:(fun mel -> + >>= Memo.List.find_map ~f:(fun (mel : Melange_stanzas.Emit.t) -> + let+ enabled = + let* expander = + let* sctx = sctx in + Super_context.expander sctx ~dir + in + Expander.eval_blang expander mel.enabled_if + in let target_dir = Melange_stanzas.Emit.target_dir ~dir:parent mel in - Option.some_if (Path.Build.equal target_dir dir) mel) + Option.some_if (enabled && Path.Build.equal target_dir dir) mel) >>= (function - | None -> under_melange_emit_target ~dir:parent + | None -> under_melange_emit_target ~sctx ~dir:parent | Some stanza -> Memo.return @@ Some { stanza_dir = parent; stanza })) ;; @@ -602,7 +609,7 @@ let gen_emit_rules sctx ~dir ({ stanza_dir; stanza } as for_melange) = match Path.Build.equal dir (Melange_stanzas.Emit.target_dir ~dir:stanza_dir stanza) with | false -> Memo.return None | true -> - under_melange_emit_target ~dir:stanza_dir + under_melange_emit_target ~sctx ~dir:stanza_dir >>| (function | None -> Some (emit_rules sctx for_melange) | Some { stanza_dir = parent_melange_emit_dir; stanza = parent_stanza } -> @@ -640,7 +647,7 @@ let gen_emit_rules sctx ~dir ({ stanza_dir; stanza } as for_melange) = module Gen_rules = Import.Build_config.Gen_rules let setup_emit_js_rules sctx ~dir = - under_melange_emit_target ~dir + under_melange_emit_target ~sctx ~dir >>= function | Some melange -> let* enabled = From 5189cc4412f628c48ca865b7ff98030ba68429d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Fri, 8 Mar 2024 09:30:31 +0000 Subject: [PATCH 06/17] add explanatory comment for usage of enabled in melange_rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/melange/melange_rules.ml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/dune_rules/melange/melange_rules.ml b/src/dune_rules/melange/melange_rules.ml index 4e6176d2045..157443c81d5 100644 --- a/src/dune_rules/melange/melange_rules.ml +++ b/src/dune_rules/melange/melange_rules.ml @@ -591,6 +591,9 @@ let rec under_melange_emit_target ~sctx ~dir = | Some stanzas -> Dune_file.find_stanzas stanzas Melange_stanzas.Emit.key >>= Memo.List.find_map ~f:(fun (mel : Melange_stanzas.Emit.t) -> + (* In the case where we have two melange.emit stanzas in the same folder, + with one enabled in the current context and one disabled, we want to + make sure that we pick the enabled one *) let+ enabled = let* expander = let* sctx = sctx in From 673ed9a5a62ddac20e211e4d1c5eec57793905db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Fri, 8 Mar 2024 09:50:16 +0000 Subject: [PATCH 07/17] update tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- test/blackbox-tests/test-cases/melange/enabled_if.t | 6 ++++++ test/blackbox-tests/test-cases/test-build-if/feature.t | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/test/blackbox-tests/test-cases/melange/enabled_if.t b/test/blackbox-tests/test-cases/melange/enabled_if.t index cbe7fc97769..2d67790fe90 100644 --- a/test/blackbox-tests/test-cases/melange/enabled_if.t +++ b/test/blackbox-tests/test-cases/melange/enabled_if.t @@ -34,7 +34,13 @@ > EOF $ dune build @melange --display short + Error: Alias "melange" specified on the command line is empty. + It is not defined in . or any of its descendants. + [1] No rules attached to the alias $ dune rules @melange + Error: Alias "melange" specified on the command line is empty. + It is not defined in . or any of its descendants. + [1] diff --git a/test/blackbox-tests/test-cases/test-build-if/feature.t b/test/blackbox-tests/test-cases/test-build-if/feature.t index b8dc8745e27..07fa0747de9 100644 --- a/test/blackbox-tests/test-cases/test-build-if/feature.t +++ b/test/blackbox-tests/test-cases/test-build-if/feature.t @@ -1,4 +1,4 @@ -enabled_if has a limitation: it attempts building even if enabled_if evaluates to false. +enabled_if and build_if have similar behavior $ cat > dune-project << EOF > (lang dune 3.9) @@ -39,7 +39,7 @@ We test the various combinations: $ test_all When building @all with ENABLED=unset: - build was done: YES + build was done: NO test did run: NO When building @runtest with ENABLED=unset: build was done: NO From ca2909cecbe9807a27919c83038bde21aa47d180 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Fri, 8 Mar 2024 09:56:14 +0000 Subject: [PATCH 08/17] remove unnecessary changes in setup_emit_js_rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/melange/melange_rules.ml | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/dune_rules/melange/melange_rules.ml b/src/dune_rules/melange/melange_rules.ml index 157443c81d5..46d41eb6c69 100644 --- a/src/dune_rules/melange/melange_rules.ml +++ b/src/dune_rules/melange/melange_rules.ml @@ -653,20 +653,10 @@ let setup_emit_js_rules sctx ~dir = under_melange_emit_target ~sctx ~dir >>= function | Some melange -> - let* enabled = - let* expander = - let* sctx = sctx in - Super_context.expander sctx ~dir - in - Expander.eval_blang expander melange.stanza.enabled_if - in - (match enabled with - | true -> - gen_emit_rules sctx ~dir melange - >>| (function - | None -> Gen_rules.redirect_to_parent Gen_rules.Rules.empty - | Some melange -> Gen_rules.make melange) - | false -> Memo.return (Gen_rules.redirect_to_parent Gen_rules.Rules.empty)) + gen_emit_rules sctx ~dir melange + >>| (function + | None -> Gen_rules.redirect_to_parent Gen_rules.Rules.empty + | Some melange -> Gen_rules.make melange) | None -> (* this should probably be handled by [Dir_status] *) Dune_load.stanzas_in_dir dir From 2c98f8f30f424603116186994e2467af01be1eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Sun, 10 Mar 2024 13:10:04 +0000 Subject: [PATCH 09/17] code review suggested changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/ml_sources.ml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index 93343437289..e8a62e49534 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -400,8 +400,7 @@ let modules_of_stanzas = Memo.parallel_map stanzas ~f:(fun stanza -> match Stanza.repr stanza with | Library.T lib -> - let* enabled = Expander.eval_blang expander lib.enabled_if in - let enabled = Toggle.of_bool enabled in + let* enabled = Expander.eval_blang expander lib.enabled_if >>| Toggle.of_bool in (* jeremiedimino: this [Resolve.get] means that if the user writes an invalid [implements] field, we will get an error immediately even if the library is not built. We should change this to carry the @@ -423,8 +422,7 @@ let modules_of_stanzas = `Library (lib, sources, modules, obj_dir, enabled) | Executables.T exes | Tests.T { exes; _ } -> let obj_dir = Executables.obj_dir ~dir exes in - let* enabled = Expander.eval_blang expander exes.enabled_if in - let enabled = Toggle.of_bool enabled in + let* enabled = Expander.eval_blang expander exes.enabled_if >>| Toggle.of_bool in let+ sources, modules = let { Buildable.loc = stanza_loc; modules = modules_settings; _ } = exes.buildable @@ -448,8 +446,7 @@ let modules_of_stanzas = `Executables (exes, sources, modules, obj_dir, enabled) | Melange_stanzas.Emit.T mel -> let obj_dir = Obj_dir.make_melange_emit ~dir ~name:mel.target in - let* enabled = Expander.eval_blang expander mel.enabled_if in - let enabled = Toggle.of_bool enabled in + let* enabled = Expander.eval_blang expander mel.enabled_if >>| Toggle.of_bool in let+ sources, modules = Modules_field_evaluator.eval ~expander From ecbad4f9a1725881e6a95dae0b45f7c2daa77339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Sun, 10 Mar 2024 14:11:44 +0100 Subject: [PATCH 10/17] update src/dune_rules/ml_sources.ml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/ml_sources.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index e8a62e49534..ed6dc51df55 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -53,10 +53,12 @@ module Modules = struct ; melange_emits : Melange_stanzas.Emit.t group_part list } - let make { libraries = libs; executables = exes; melange_emits = emits } = + let make = let keep_enabled t = List.filter t ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) in + fun { libraries = libs; executables = exes; melange_emits = emits } -> + let libs = keep_enabled libs in let exes = keep_enabled exes in let emits = keep_enabled emits in From ef6c1463a1f96b43e38c569bcd1e9eb4110f73a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Sun, 10 Mar 2024 13:12:05 +0000 Subject: [PATCH 11/17] fmt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/ml_sources.ml | 192 ++++++++++++++++++----------------- 1 file changed, 97 insertions(+), 95 deletions(-) diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index ed6dc51df55..8d78d4fd35f 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -53,108 +53,110 @@ module Modules = struct ; melange_emits : Melange_stanzas.Emit.t group_part list } - let make = + let make = let keep_enabled t = List.filter t ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) in fun { libraries = libs; executables = exes; melange_emits = emits } -> - - let libs = keep_enabled libs in - let exes = keep_enabled exes in - let emits = keep_enabled emits in - let libraries = - match - Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> - Library.best_name lib, (m, obj_dir)) - with - | Ok x -> x - | Error (name, _, (lib2, _, _, _, _)) -> - User_error.raise - ~loc:lib2.buildable.loc - [ Pp.textf - "Library %S appears for the second time in this directory" - (Lib_name.to_string name) - ] - in - let executables = - match - String.Map.of_list_map exes ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> - snd (List.hd exes.names), (m, obj_dir)) - with - | Ok x -> x - | Error (name, _, (exes2, _, _, _, _)) -> - User_error.raise - ~loc:exes2.buildable.loc - [ Pp.textf "Executable %S appears for the second time in this directory" name ] - in - let melange_emits = - match - String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> - mel.target, (m, obj_dir)) - with - | Ok x -> x - | Error (name, _, (mel, _, _, _, _)) -> - User_error.raise - ~loc:mel.loc - [ Pp.textf "Target %S appears for the second time in this directory" name ] - in - let rev_map = - let modules = - let by_path (origin : Origin.t) trie = - Module_trie.fold trie ~init:[] ~f:(fun (_loc, m) acc -> - (Module.Source.path m, origin) :: acc) - in - List.concat - [ List.concat_map libs ~f:(fun (l, m, _, _, _) -> by_path (Library l) m) - ; List.concat_map exes ~f:(fun (e, m, _, _, _) -> by_path (Executables e) m) - ; List.concat_map emits ~f:(fun (l, m, _, _, _) -> by_path (Melange l) m) - ] + let libs = keep_enabled libs in + let exes = keep_enabled exes in + let emits = keep_enabled emits in + let libraries = + match + Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> + Library.best_name lib, (m, obj_dir)) + with + | Ok x -> x + | Error (name, _, (lib2, _, _, _, _)) -> + User_error.raise + ~loc:lib2.buildable.loc + [ Pp.textf + "Library %S appears for the second time in this directory" + (Lib_name.to_string name) + ] in - match Module_name.Path.Map.of_list modules with - | Ok x -> x - | Error (path, _, _) -> - let locs = - List.filter_map modules ~f:(fun (n, origin) -> - Option.some_if - (Ordering.is_eq (Module_name.Path.compare n path)) - (Origin.loc origin)) - |> List.sort ~compare:Loc.compare - in - let main_message = - Pp.textf - "Module %S is used in several stanzas:" - (Module_name.Path.to_string path) - in - let loc, related_locs = - match locs with - | [] -> - (* duplicates imply at least at one module with this location *) - assert false - | loc :: related_locs -> loc, related_locs - in - let annots = - let main = User_message.make ~loc [ main_message ] in - let related = - List.map related_locs ~f:(fun loc -> - User_message.make ~loc [ Pp.text "Used in this stanza" ]) + let executables = + match + String.Map.of_list_map + exes + ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> + snd (List.hd exes.names), (m, obj_dir)) + with + | Ok x -> x + | Error (name, _, (exes2, _, _, _, _)) -> + User_error.raise + ~loc:exes2.buildable.loc + [ Pp.textf "Executable %S appears for the second time in this directory" name + ] + in + let melange_emits = + match + String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> + mel.target, (m, obj_dir)) + with + | Ok x -> x + | Error (name, _, (mel, _, _, _, _)) -> + User_error.raise + ~loc:mel.loc + [ Pp.textf "Target %S appears for the second time in this directory" name ] + in + let rev_map = + let modules = + let by_path (origin : Origin.t) trie = + Module_trie.fold trie ~init:[] ~f:(fun (_loc, m) acc -> + (Module.Source.path m, origin) :: acc) in - User_message.Annots.singleton - Compound_user_error.annot - [ Compound_user_error.make ~main ~related ] + List.concat + [ List.concat_map libs ~f:(fun (l, m, _, _, _) -> by_path (Library l) m) + ; List.concat_map exes ~f:(fun (e, m, _, _, _) -> by_path (Executables e) m) + ; List.concat_map emits ~f:(fun (l, m, _, _, _) -> by_path (Melange l) m) + ] in - User_error.raise - ~annots - ~loc:(Loc.drop_position loc) - [ main_message - ; Pp.enumerate locs ~f:(fun loc -> Pp.verbatim (Loc.to_file_colon_line loc)) - ; Pp.text - "To fix this error, you must specify an explicit \"modules\" field in \ - every library, executable, and executables stanzas in this dune file. \ - Note that each module cannot appear in more than one \"modules\" field - \ - it must belong to a single library or executable." - ] - in - { libraries; executables; melange_emits; rev_map } + match Module_name.Path.Map.of_list modules with + | Ok x -> x + | Error (path, _, _) -> + let locs = + List.filter_map modules ~f:(fun (n, origin) -> + Option.some_if + (Ordering.is_eq (Module_name.Path.compare n path)) + (Origin.loc origin)) + |> List.sort ~compare:Loc.compare + in + let main_message = + Pp.textf + "Module %S is used in several stanzas:" + (Module_name.Path.to_string path) + in + let loc, related_locs = + match locs with + | [] -> + (* duplicates imply at least at one module with this location *) + assert false + | loc :: related_locs -> loc, related_locs + in + let annots = + let main = User_message.make ~loc [ main_message ] in + let related = + List.map related_locs ~f:(fun loc -> + User_message.make ~loc [ Pp.text "Used in this stanza" ]) + in + User_message.Annots.singleton + Compound_user_error.annot + [ Compound_user_error.make ~main ~related ] + in + User_error.raise + ~annots + ~loc:(Loc.drop_position loc) + [ main_message + ; Pp.enumerate locs ~f:(fun loc -> Pp.verbatim (Loc.to_file_colon_line loc)) + ; Pp.text + "To fix this error, you must specify an explicit \"modules\" field in \ + every library, executable, and executables stanzas in this dune file. \ + Note that each module cannot appear in more than one \"modules\" field \ + - it must belong to a single library or executable." + ] + in + { libraries; executables; melange_emits; rev_map } ;; end From 050aed85f6e09c4432f2c79469f8dbff1872d2d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Sun, 10 Mar 2024 20:33:24 +0000 Subject: [PATCH 12/17] fix change in enabled_if behavior for test stanzas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- src/dune_rules/ml_sources.ml | 51 ++++++++++--------- .../test-cases/test-build-if/feature.t | 2 +- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index 8d78d4fd35f..b4283761674 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -400,6 +400,30 @@ let modules_of_stanzas = ; melange_emits = List.rev melange_emits } in + let make_executables ~dir ~enabled ~expander ~modules ~project exes = + let obj_dir = Executables.obj_dir ~dir exes in + let+ sources, modules = + let { Buildable.loc = stanza_loc; modules = modules_settings; _ } = + exes.buildable + in + Modules_field_evaluator.eval + ~expander + ~modules + ~stanza_loc + ~src_dir:dir + ~kind:Modules_field_evaluator.Exe_or_normal_lib + ~private_modules:Ordered_set_lang.Unexpanded.standard + ~version:exes.dune_version + modules_settings + in + let modules = + let obj_dir = Obj_dir.obj_dir obj_dir in + if Dune_project.wrapped_executables project + then Modules_group.make_wrapped ~obj_dir ~modules `Exe + else Modules_group.exe_unwrapped modules ~obj_dir + in + `Executables (exes, sources, modules, obj_dir, enabled) + in fun stanzas ~expander ~project ~dir ~libs ~lookup_vlib ~modules ~include_subdirs -> Memo.parallel_map stanzas ~f:(fun stanza -> match Stanza.repr stanza with @@ -424,30 +448,11 @@ let modules_of_stanzas = in let obj_dir = Library.obj_dir lib ~dir in `Library (lib, sources, modules, obj_dir, enabled) - | Executables.T exes | Tests.T { exes; _ } -> - let obj_dir = Executables.obj_dir ~dir exes in + | Executables.T exes -> let* enabled = Expander.eval_blang expander exes.enabled_if >>| Toggle.of_bool in - let+ sources, modules = - let { Buildable.loc = stanza_loc; modules = modules_settings; _ } = - exes.buildable - in - Modules_field_evaluator.eval - ~expander - ~modules - ~stanza_loc - ~src_dir:dir - ~kind:Modules_field_evaluator.Exe_or_normal_lib - ~private_modules:Ordered_set_lang.Unexpanded.standard - ~version:exes.dune_version - modules_settings - in - let modules = - let obj_dir = Obj_dir.obj_dir obj_dir in - if Dune_project.wrapped_executables project - then Modules_group.make_wrapped ~obj_dir ~modules `Exe - else Modules_group.exe_unwrapped modules ~obj_dir - in - `Executables (exes, sources, modules, obj_dir, enabled) + make_executables ~dir ~enabled ~expander ~modules ~project exes + | Tests.T { exes; _ } -> + make_executables ~dir ~enabled:`Enabled ~expander ~modules ~project exes | Melange_stanzas.Emit.T mel -> let obj_dir = Obj_dir.make_melange_emit ~dir ~name:mel.target in let* enabled = Expander.eval_blang expander mel.enabled_if >>| Toggle.of_bool in diff --git a/test/blackbox-tests/test-cases/test-build-if/feature.t b/test/blackbox-tests/test-cases/test-build-if/feature.t index 07fa0747de9..4e9f105e1e2 100644 --- a/test/blackbox-tests/test-cases/test-build-if/feature.t +++ b/test/blackbox-tests/test-cases/test-build-if/feature.t @@ -39,7 +39,7 @@ We test the various combinations: $ test_all When building @all with ENABLED=unset: - build was done: NO + build was done: YES test did run: NO When building @runtest with ENABLED=unset: build was done: NO From 20e02e3cc04278ce7a3d6d3074e7d56b301e3725 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg <me@rgrinberg.com> Date: Tue, 12 Mar 2024 21:46:59 +0000 Subject: [PATCH 13/17] _ Signed-off-by: Rudi Grinberg <me@rgrinberg.com> --- test/blackbox-tests/test-cases/test-build-if/feature.t | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/blackbox-tests/test-cases/test-build-if/feature.t b/test/blackbox-tests/test-cases/test-build-if/feature.t index 4e9f105e1e2..45fe8f74ad8 100644 --- a/test/blackbox-tests/test-cases/test-build-if/feature.t +++ b/test/blackbox-tests/test-cases/test-build-if/feature.t @@ -1,4 +1,7 @@ -enabled_if and build_if have similar behavior +enabled_if and build_if have similar behavior on the test(s) stanza. +build_if controls whether the tests builds, while enabled_if controls whether +the test runs as part of @runtest + $ cat > dune-project << EOF > (lang dune 3.9) From 1f17f12a336a102a73936efeae3de95e389e1d6e Mon Sep 17 00:00:00 2001 From: Rudi Grinberg <me@rgrinberg.com> Date: Tue, 12 Mar 2024 21:51:51 +0000 Subject: [PATCH 14/17] _ Signed-off-by: Rudi Grinberg <me@rgrinberg.com> --- src/dune_rules/melange/melange_rules.ml | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/dune_rules/melange/melange_rules.ml b/src/dune_rules/melange/melange_rules.ml index 46d41eb6c69..7687e518bed 100644 --- a/src/dune_rules/melange/melange_rules.ml +++ b/src/dune_rules/melange/melange_rules.ml @@ -591,18 +591,21 @@ let rec under_melange_emit_target ~sctx ~dir = | Some stanzas -> Dune_file.find_stanzas stanzas Melange_stanzas.Emit.key >>= Memo.List.find_map ~f:(fun (mel : Melange_stanzas.Emit.t) -> - (* In the case where we have two melange.emit stanzas in the same folder, - with one enabled in the current context and one disabled, we want to - make sure that we pick the enabled one *) - let+ enabled = - let* expander = - let* sctx = sctx in - Super_context.expander sctx ~dir - in - Expander.eval_blang expander mel.enabled_if - in let target_dir = Melange_stanzas.Emit.target_dir ~dir:parent mel in - Option.some_if (enabled && Path.Build.equal target_dir dir) mel) + match Path.Build.equal target_dir dir with + | false -> Memo.return None + | true -> + (* In the case where we have two melange.emit stanzas in the same folder, + with one enabled in the current context and one disabled, we want to + make sure that we pick the enabled one *) + let+ enabled = + let* expander = + let* sctx = sctx in + Super_context.expander sctx ~dir + in + Expander.eval_blang expander mel.enabled_if + in + Option.some_if enabled mel) >>= (function | None -> under_melange_emit_target ~sctx ~dir:parent | Some stanza -> Memo.return @@ Some { stanza_dir = parent; stanza })) From e2b665f7853417333b720a8bbdd0120443fcaa7d Mon Sep 17 00:00:00 2001 From: Rudi Grinberg <me@rgrinberg.com> Date: Tue, 12 Mar 2024 22:16:51 +0000 Subject: [PATCH 15/17] _ Signed-off-by: Rudi Grinberg <me@rgrinberg.com> --- src/dune_rules/artifacts_obj.ml | 10 +- src/dune_rules/artifacts_obj.mli | 4 +- src/dune_rules/ml_sources.ml | 323 ++++++++++++++++--------------- 3 files changed, 171 insertions(+), 166 deletions(-) diff --git a/src/dune_rules/artifacts_obj.ml b/src/dune_rules/artifacts_obj.ml index 1a4b8b4778b..a3b87e58691 100644 --- a/src/dune_rules/artifacts_obj.ml +++ b/src/dune_rules/artifacts_obj.ml @@ -12,7 +12,7 @@ let lookup_library { libraries; modules = _ } = Lib_name.Map.find libraries let make ~dir ~lib_config ~libs ~exes = let+ libraries = - Memo.List.map libs ~f:(fun ((lib : Library.t), _, _, _, _) -> + 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 @@ -25,12 +25,10 @@ let make ~dir ~lib_config ~libs ~exes = Module_name.Map.add_exn modules (Module.name m) (obj_dir, m)) in let init = - List.fold_left - exes - ~init:Module_name.Map.empty - ~f:(fun modules (_, _, m, obj_dir, _) -> by_name modules obj_dir m) + List.fold_left exes ~init:Module_name.Map.empty ~f:(fun modules (m, obj_dir) -> + by_name modules obj_dir m) in - List.fold_left libs ~init ~f:(fun modules (_, _, m, obj_dir, _) -> + List.fold_left libs ~init ~f:(fun modules (_, m, obj_dir) -> by_name modules obj_dir m) in { libraries; modules } diff --git a/src/dune_rules/artifacts_obj.mli b/src/dune_rules/artifacts_obj.mli index f60596cd4b6..16c530f556d 100644 --- a/src/dune_rules/artifacts_obj.mli +++ b/src/dune_rules/artifacts_obj.mli @@ -7,8 +7,8 @@ val empty : t val make : dir:Path.Build.t -> lib_config:Lib_config.t Memo.t - -> libs:(Library.t * _ * Modules.t * Path.Build.t Obj_dir.t * Toggle.t) list - -> exes:(_ * _ * Modules.t * Path.Build.t Obj_dir.t * Toggle.t) list + -> libs:(Library.t * Modules.t * Path.Build.t Obj_dir.t) list + -> exes:(Modules.t * Path.Build.t Obj_dir.t) list -> t Memo.t val lookup_module : t -> Module_name.t -> (Path.Build.t Obj_dir.t * Module.t) option diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml index b4283761674..7b6fd9c573d 100644 --- a/src/dune_rules/ml_sources.ml +++ b/src/dune_rules/ml_sources.ml @@ -41,11 +41,11 @@ module Modules = struct ;; type 'stanza group_part = - 'stanza - * (Loc.t * Module.Source.t) Module_trie.t - * Modules_group.t - * Path.Build.t Obj_dir.t - * Toggle.t + { stanza : 'stanza + ; sources : (Loc.t * Module.Source.t) Module_trie.t + ; modules : Modules_group.t + ; obj_dir : Path.Build.t Obj_dir.t + } type groups = { libraries : Library.t group_part list @@ -53,110 +53,103 @@ module Modules = struct ; melange_emits : Melange_stanzas.Emit.t group_part list } - let make = - let keep_enabled t = - List.filter t ~f:(fun (_, _, _, _, enabled) -> Toggle.enabled enabled) + let make { libraries = libs; executables = exes; melange_emits = emits } = + let libraries = + match + Lib_name.Map.of_list_map libs ~f:(fun part -> + Library.best_name part.stanza, (part.modules, part.obj_dir)) + with + | Ok x -> x + | Error (name, _, part) -> + User_error.raise + ~loc:part.stanza.buildable.loc + [ Pp.textf + "Library %S appears for the second time in this directory" + (Lib_name.to_string name) + ] in - fun { libraries = libs; executables = exes; melange_emits = emits } -> - let libs = keep_enabled libs in - let exes = keep_enabled exes in - let emits = keep_enabled emits in - let libraries = - match - Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir, _) -> - Library.best_name lib, (m, obj_dir)) - with - | Ok x -> x - | Error (name, _, (lib2, _, _, _, _)) -> - User_error.raise - ~loc:lib2.buildable.loc - [ Pp.textf - "Library %S appears for the second time in this directory" - (Lib_name.to_string name) - ] - in - let executables = - match - String.Map.of_list_map - exes - ~f:(fun ((exes : Executables.t), _, m, obj_dir, _) -> - snd (List.hd exes.names), (m, obj_dir)) - with - | Ok x -> x - | Error (name, _, (exes2, _, _, _, _)) -> - User_error.raise - ~loc:exes2.buildable.loc - [ Pp.textf "Executable %S appears for the second time in this directory" name - ] - in - let melange_emits = - match - String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir, _) -> - mel.target, (m, obj_dir)) - with - | Ok x -> x - | Error (name, _, (mel, _, _, _, _)) -> - User_error.raise - ~loc:mel.loc - [ Pp.textf "Target %S appears for the second time in this directory" name ] + let executables = + match + String.Map.of_list_map exes ~f:(fun (part : Executables.t group_part) -> + snd (List.hd part.stanza.names), (part.modules, part.obj_dir)) + with + | Ok x -> x + | Error (name, _, part) -> + User_error.raise + ~loc:part.stanza.buildable.loc + [ Pp.textf "Executable %S appears for the second time in this directory" name ] + in + let melange_emits = + match + String.Map.of_list_map emits ~f:(fun part -> + part.stanza.target, (part.modules, part.obj_dir)) + with + | Ok x -> x + | Error (name, _, part) -> + User_error.raise + ~loc:part.stanza.loc + [ Pp.textf "Target %S appears for the second time in this directory" name ] + in + let rev_map = + let modules = + let by_path (origin : Origin.t) trie = + Module_trie.fold trie ~init:[] ~f:(fun (_loc, m) acc -> + (Module.Source.path m, origin) :: acc) + in + List.concat + [ List.concat_map libs ~f:(fun part -> + by_path (Library part.stanza) part.sources) + ; List.concat_map exes ~f:(fun part -> + by_path (Executables part.stanza) part.sources) + ; List.concat_map emits ~f:(fun part -> + by_path (Melange part.stanza) part.sources) + ] in - let rev_map = - let modules = - let by_path (origin : Origin.t) trie = - Module_trie.fold trie ~init:[] ~f:(fun (_loc, m) acc -> - (Module.Source.path m, origin) :: acc) - in - List.concat - [ List.concat_map libs ~f:(fun (l, m, _, _, _) -> by_path (Library l) m) - ; List.concat_map exes ~f:(fun (e, m, _, _, _) -> by_path (Executables e) m) - ; List.concat_map emits ~f:(fun (l, m, _, _, _) -> by_path (Melange l) m) - ] + match Module_name.Path.Map.of_list modules with + | Ok x -> x + | Error (path, _, _) -> + let locs = + List.filter_map modules ~f:(fun (n, origin) -> + Option.some_if + (Ordering.is_eq (Module_name.Path.compare n path)) + (Origin.loc origin)) + |> List.sort ~compare:Loc.compare in - match Module_name.Path.Map.of_list modules with - | Ok x -> x - | Error (path, _, _) -> - let locs = - List.filter_map modules ~f:(fun (n, origin) -> - Option.some_if - (Ordering.is_eq (Module_name.Path.compare n path)) - (Origin.loc origin)) - |> List.sort ~compare:Loc.compare - in - let main_message = - Pp.textf - "Module %S is used in several stanzas:" - (Module_name.Path.to_string path) - in - let loc, related_locs = - match locs with - | [] -> - (* duplicates imply at least at one module with this location *) - assert false - | loc :: related_locs -> loc, related_locs - in - let annots = - let main = User_message.make ~loc [ main_message ] in - let related = - List.map related_locs ~f:(fun loc -> - User_message.make ~loc [ Pp.text "Used in this stanza" ]) - in - User_message.Annots.singleton - Compound_user_error.annot - [ Compound_user_error.make ~main ~related ] + let main_message = + Pp.textf + "Module %S is used in several stanzas:" + (Module_name.Path.to_string path) + in + let loc, related_locs = + match locs with + | [] -> + (* duplicates imply at least at one module with this location *) + assert false + | loc :: related_locs -> loc, related_locs + in + let annots = + let main = User_message.make ~loc [ main_message ] in + let related = + List.map related_locs ~f:(fun loc -> + User_message.make ~loc [ Pp.text "Used in this stanza" ]) in - User_error.raise - ~annots - ~loc:(Loc.drop_position loc) - [ main_message - ; Pp.enumerate locs ~f:(fun loc -> Pp.verbatim (Loc.to_file_colon_line loc)) - ; Pp.text - "To fix this error, you must specify an explicit \"modules\" field in \ - every library, executable, and executables stanzas in this dune file. \ - Note that each module cannot appear in more than one \"modules\" field \ - - it must belong to a single library or executable." - ] - in - { libraries; executables; melange_emits; rev_map } + User_message.Annots.singleton + Compound_user_error.annot + [ Compound_user_error.make ~main ~related ] + in + User_error.raise + ~annots + ~loc:(Loc.drop_position loc) + [ main_message + ; Pp.enumerate locs ~f:(fun loc -> Pp.verbatim (Loc.to_file_colon_line loc)) + ; Pp.text + "To fix this error, you must specify an explicit \"modules\" field in \ + every library, executable, and executables stanzas in this dune file. \ + Note that each module cannot appear in more than one \"modules\" field - \ + it must belong to a single library or executable." + ] + in + { libraries; executables; melange_emits; rev_map } ;; end @@ -400,7 +393,7 @@ let modules_of_stanzas = ; melange_emits = List.rev melange_emits } in - let make_executables ~dir ~enabled ~expander ~modules ~project exes = + let make_executables ~dir ~expander ~modules ~project exes = let obj_dir = Executables.obj_dir ~dir exes in let+ sources, modules = let { Buildable.loc = stanza_loc; modules = modules_settings; _ } = @@ -422,56 +415,66 @@ let modules_of_stanzas = then Modules_group.make_wrapped ~obj_dir ~modules `Exe else Modules_group.exe_unwrapped modules ~obj_dir in - `Executables (exes, sources, modules, obj_dir, enabled) + `Executables { Modules.stanza = exes; sources; modules; obj_dir } in fun stanzas ~expander ~project ~dir ~libs ~lookup_vlib ~modules ~include_subdirs -> Memo.parallel_map stanzas ~f:(fun stanza -> - match Stanza.repr stanza with - | Library.T lib -> - let* enabled = Expander.eval_blang expander lib.enabled_if >>| Toggle.of_bool in - (* jeremiedimino: this [Resolve.get] means that if the user writes an - invalid [implements] field, we will get an error immediately even if - the library is not built. We should change this to carry the - [Or_exn.t] a bit longer. *) - let+ sources, modules = - let lookup_vlib = lookup_vlib ~loc:lib.buildable.loc in - make_lib_modules - ~expander - ~dir - ~libs - ~lookup_vlib - ~modules - ~lib - ~include_subdirs - ~version:lib.dune_version - >>= Resolve.read_memo - in - let obj_dir = Library.obj_dir lib ~dir in - `Library (lib, sources, modules, obj_dir, enabled) - | Executables.T exes -> - let* enabled = Expander.eval_blang expander exes.enabled_if >>| Toggle.of_bool in - make_executables ~dir ~enabled ~expander ~modules ~project exes - | Tests.T { exes; _ } -> - make_executables ~dir ~enabled:`Enabled ~expander ~modules ~project exes - | Melange_stanzas.Emit.T mel -> - let obj_dir = Obj_dir.make_melange_emit ~dir ~name:mel.target in - let* enabled = Expander.eval_blang expander mel.enabled_if >>| Toggle.of_bool in - let+ sources, modules = - Modules_field_evaluator.eval - ~expander - ~modules - ~stanza_loc:mel.loc - ~kind:Modules_field_evaluator.Exe_or_normal_lib - ~version:mel.dune_version - ~private_modules:Ordered_set_lang.Unexpanded.standard - ~src_dir:dir - mel.modules - in - let modules = - Modules_group.make_wrapped ~obj_dir:(Obj_dir.obj_dir obj_dir) ~modules `Melange - in - `Melange_emit (mel, sources, modules, obj_dir, enabled) - | _ -> Memo.return `Skip) + let enabled_if = + match Stanza.repr stanza with + | Library.T lib -> lib.enabled_if + | Tests.T exes -> exes.build_if + | Executables.T exes -> exes.enabled_if + | Melange_stanzas.Emit.T mel -> mel.enabled_if + | _ -> Blang.true_ + in + Expander.eval_blang expander enabled_if + >>= function + | false -> Memo.return `Skip + | true -> + (match Stanza.repr stanza with + | Library.T lib -> + (* jeremiedimino: this [Resolve.get] means that if the user writes an + invalid [implements] field, we will get an error immediately even if + the library is not built. We should change this to carry the + [Or_exn.t] a bit longer. *) + let+ sources, modules = + let lookup_vlib = lookup_vlib ~loc:lib.buildable.loc in + make_lib_modules + ~expander + ~dir + ~libs + ~lookup_vlib + ~modules + ~lib + ~include_subdirs + ~version:lib.dune_version + >>= Resolve.read_memo + in + let obj_dir = Library.obj_dir lib ~dir in + `Library { Modules.stanza = lib; sources; modules; obj_dir } + | Executables.T exes -> make_executables ~dir ~expander ~modules ~project exes + | Tests.T { exes; _ } -> make_executables ~dir ~expander ~modules ~project exes + | Melange_stanzas.Emit.T mel -> + let obj_dir = Obj_dir.make_melange_emit ~dir ~name:mel.target in + let+ sources, modules = + Modules_field_evaluator.eval + ~expander + ~modules + ~stanza_loc:mel.loc + ~kind:Modules_field_evaluator.Exe_or_normal_lib + ~version:mel.dune_version + ~private_modules:Ordered_set_lang.Unexpanded.standard + ~src_dir:dir + mel.modules + in + let modules = + Modules_group.make_wrapped + ~obj_dir:(Obj_dir.obj_dir obj_dir) + ~modules + `Melange + in + `Melange_emit { Modules.stanza = mel; sources; modules; obj_dir } + | _ -> Memo.return `Skip)) >>| filter_partition_map ;; @@ -564,11 +567,15 @@ let make let modules = Modules.make modules_of_stanzas in let artifacts = Memo.lazy_ (fun () -> - Artifacts_obj.make - ~dir - ~lib_config - ~libs:modules_of_stanzas.libraries - ~exes:modules_of_stanzas.executables) + let libs = + List.map modules_of_stanzas.libraries ~f:(fun (part : _ Modules.group_part) -> + part.stanza, part.modules, part.obj_dir) + in + let exes = + 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) in { modules; artifacts; include_subdirs } ;; From b22ae6f4712fc4d1cc9c9b53022048bc7018bf49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Wed, 13 Mar 2024 08:01:51 +0000 Subject: [PATCH 16/17] update tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- test/blackbox-tests/test-cases/enabled_if/cma-pform.t | 4 ++-- .../test-cases/enabled_if/eif-library-disabled-cmo-error.t | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/blackbox-tests/test-cases/enabled_if/cma-pform.t b/test/blackbox-tests/test-cases/enabled_if/cma-pform.t index b27d9451943..fbba673bf25 100644 --- a/test/blackbox-tests/test-cases/enabled_if/cma-pform.t +++ b/test/blackbox-tests/test-cases/enabled_if/cma-pform.t @@ -11,7 +11,7 @@ We try to build a disabled library using the %{cma:..} pfrom > EOF $ dune build %{cma:./foo} - Error: No rule found for foo.cma - -> required by %{cma:./foo} at command line:1 + File "command line", line 1, characters 0-12: + Error: Library foo does not exist. [1] $ dune build --profile with-foo %{cma:./foo} diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-library-disabled-cmo-error.t b/test/blackbox-tests/test-cases/enabled_if/eif-library-disabled-cmo-error.t index 98ff9ffc8a1..6e6aafbd637 100644 --- a/test/blackbox-tests/test-cases/enabled_if/eif-library-disabled-cmo-error.t +++ b/test/blackbox-tests/test-cases/enabled_if/eif-library-disabled-cmo-error.t @@ -15,7 +15,7 @@ Test behavior when targeting artifacts of a disabled library > EOF $ dune build %{cmo:foo} - Error: No rule found for .foo.objs/byte/foo.cmo - -> required by %{cmo:foo} at command line:1 + File "command line", line 1, characters 0-10: + Error: Module Foo does not exist. [1] From ebfe399a073fa40ce2ed32274dbc207a2276400a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com> Date: Wed, 13 Mar 2024 08:17:54 +0000 Subject: [PATCH 17/17] + changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com> --- doc/changes/10220.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 doc/changes/10220.md diff --git a/doc/changes/10220.md b/doc/changes/10220.md new file mode 100644 index 00000000000..bfa1dc50823 --- /dev/null +++ b/doc/changes/10220.md @@ -0,0 +1,2 @@ +- Allow defining executables or melange emit stanzas with the same name in the + same folder under different contexts. (#10220, @rgrinberg, @jchavarri)