From b970f3c8a2505cac50197171266f4bc70aa42990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= Date: Fri, 15 Mar 2024 08:34:57 +0000 Subject: [PATCH] show proper errors on public libs collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Javier Chávarri --- src/dune_rules/gen_rules.ml | 45 +++++++++++-------- src/dune_rules/lib.ml | 8 ++++ src/dune_rules/lib.mli | 1 + src/dune_rules/stanzas/library.ml | 1 + src/dune_rules/stanzas/library.mli | 1 + .../lib-collision-private-same-folder.t | 13 ++++++ .../lib-collision/lib-collision-private.t | 15 +++++++ .../lib-collision-public-same-folder.t | 15 +++++-- .../lib-collision/lib-collision-public.t | 21 +++++++++ 9 files changed, 98 insertions(+), 22 deletions(-) diff --git a/src/dune_rules/gen_rules.ml b/src/dune_rules/gen_rules.ml index 61eec829192..21bb3ec438f 100644 --- a/src/dune_rules/gen_rules.ml +++ b/src/dune_rules/gen_rules.ml @@ -111,25 +111,32 @@ end = struct let+ () = Toplevel.Stanza.setup ~sctx ~dir ~toplevel in empty_none | Library.T lib -> - let* lib_info = - let* ocaml = - let ctx = Super_context.context sctx in - Context.ocaml ctx - in - let lib_config = ocaml.lib_config in - Memo.return (Library.to_lib_info lib ~dir ~lib_config) - in - let* enabled_in_context = - let* enabled = Lib_info.enabled lib_info in - match enabled with - | Disabled_because_of_enabled_if -> Memo.return false - | Normal | Optional -> Memo.return true - in - let* available = Lib.DB.available (Scope.libs scope) (Library.best_name lib) in - if_available_buildable - ~loc:lib.buildable.loc - (fun () -> Lib_rules.rules lib ~sctx ~dir ~scope ~dir_contents ~expander) - (enabled_in_context && available) + let db = Scope.libs scope in + (* This check reveals conflicts between the private names of public libraries, + otherwise the user will see duplicated rules for their cmxs *) + let* res = Lib.DB.find_invalid db (Library.private_name lib) in + (match res with + | Some err -> User_error.raise [ User_message.pp err ] + | None -> + let* lib_info = + let* ocaml = + let ctx = Super_context.context sctx in + Context.ocaml ctx + in + let lib_config = ocaml.lib_config in + Memo.return (Library.to_lib_info lib ~dir ~lib_config) + in + let* enabled_in_context = + let* enabled = Lib_info.enabled lib_info in + match enabled with + | Disabled_because_of_enabled_if -> Memo.return false + | Normal | Optional -> Memo.return true + in + let* available = Lib.DB.available (Scope.libs scope) (Library.best_name lib) in + if_available_buildable + ~loc:lib.buildable.loc + (fun () -> Lib_rules.rules lib ~sctx ~dir ~scope ~dir_contents ~expander) + (enabled_in_context && available)) | Foreign.Library.T lib -> Expander.eval_blang expander lib.enabled_if >>= if_available (fun () -> diff --git a/src/dune_rules/lib.ml b/src/dune_rules/lib.ml index 292356b9a06..b9ec225bc6f 100644 --- a/src/dune_rules/lib.ml +++ b/src/dune_rules/lib.ml @@ -1941,6 +1941,14 @@ module DB = struct | Ignore | Not_found | Invalid _ | Hidden _ -> None ;; + let find_invalid t name = + let open Memo.O in + Resolve_names.find_internal t name + >>| function + | Invalid err -> Some err + | Found _ | Ignore | Not_found | Hidden _ -> None + ;; + let find_even_when_hidden t name = let open Memo.O in Resolve_names.find_internal t name diff --git a/src/dune_rules/lib.mli b/src/dune_rules/lib.mli index 816b92a0ff4..5320652a21a 100644 --- a/src/dune_rules/lib.mli +++ b/src/dune_rules/lib.mli @@ -122,6 +122,7 @@ module DB : sig -> t val find : t -> Lib_name.t -> lib option Memo.t + val find_invalid : t -> Lib_name.t -> User_message.t option Memo.t val find_even_when_hidden : t -> Lib_name.t -> lib option Memo.t val available : t -> Lib_name.t -> bool Memo.t diff --git a/src/dune_rules/stanzas/library.ml b/src/dune_rules/stanzas/library.ml index 7410ce896a2..d9cd485f054 100644 --- a/src/dune_rules/stanzas/library.ml +++ b/src/dune_rules/stanzas/library.ml @@ -366,6 +366,7 @@ let best_name t = | Public p -> snd p.name ;; +let private_name t = Lib_name.of_local t.name let is_virtual t = Option.is_some t.virtual_modules let is_impl t = Option.is_some t.implements diff --git a/src/dune_rules/stanzas/library.mli b/src/dune_rules/stanzas/library.mli index b5f2ba5a63d..6e39702c8d9 100644 --- a/src/dune_rules/stanzas/library.mli +++ b/src/dune_rules/stanzas/library.mli @@ -72,6 +72,7 @@ val foreign_lib_files val archive : t -> dir:Path.Build.t -> ext:string -> Path.Build.t val best_name : t -> Lib_name.t +val private_name : t -> Lib_name.t val is_virtual : t -> bool val is_impl : t -> bool val obj_dir : dir:Path.Build.t -> t -> Path.Build.t Obj_dir.t diff --git a/test/blackbox-tests/test-cases/lib-collision/lib-collision-private-same-folder.t b/test/blackbox-tests/test-cases/lib-collision/lib-collision-private-same-folder.t index 791a3630949..ebd61016af6 100644 --- a/test/blackbox-tests/test-cases/lib-collision/lib-collision-private-same-folder.t +++ b/test/blackbox-tests/test-cases/lib-collision/lib-collision-private-same-folder.t @@ -15,6 +15,13 @@ the same folder. Without any consumers of the libraries $ dune build + Error: + File "dune", line 1, characters 0-21: + Error: A library with name "foo" is defined in two folders: _build/default + and _build/default. Either change one of the names, or enable them + conditionally using the 'enabled_if' field. + + [1] With some consumer of the library @@ -33,6 +40,12 @@ With some consumer of the library > EOF $ dune build + Error: + File "dune", line 1, characters 0-21: + Error: A library with name "foo" is defined in two folders: _build/default + and _build/default. Either change one of the names, or enable them + conditionally using the 'enabled_if' field. + File "dune", line 3, characters 0-21: 3 | (library 4 | (name foo)) diff --git a/test/blackbox-tests/test-cases/lib-collision/lib-collision-private.t b/test/blackbox-tests/test-cases/lib-collision/lib-collision-private.t index 14211aad3d9..418d5587d34 100644 --- a/test/blackbox-tests/test-cases/lib-collision/lib-collision-private.t +++ b/test/blackbox-tests/test-cases/lib-collision/lib-collision-private.t @@ -20,6 +20,14 @@ different folders. Without any consumers of the libraries $ dune build + Error: + File "b/dune", line 1, characters 0-21: + Error: A library with name "foo" is defined in two folders: _build/default/a + and _build/default/b. Either change one of the names, or enable them + conditionally using the 'enabled_if' field. + + -> required by alias default + [1] With some consumer of the library @@ -34,6 +42,13 @@ With some consumer of the library > EOF $ dune build + Error: + File "b/dune", line 1, characters 0-21: + Error: A library with name "foo" is defined in two folders: _build/default/a + and _build/default/b. Either change one of the names, or enable them + conditionally using the 'enabled_if' field. + + -> required by alias default File "b/dune", line 1, characters 0-21: 1 | (library 2 | (name foo)) diff --git a/test/blackbox-tests/test-cases/lib-collision/lib-collision-public-same-folder.t b/test/blackbox-tests/test-cases/lib-collision/lib-collision-public-same-folder.t index 3e8af8c0fb3..ef1b9226e5d 100644 --- a/test/blackbox-tests/test-cases/lib-collision/lib-collision-public-same-folder.t +++ b/test/blackbox-tests/test-cases/lib-collision/lib-collision-public-same-folder.t @@ -19,9 +19,12 @@ the same folder. Without any consumers of the libraries $ dune build - Error: Multiple rules generated for _build/default/foo.cmxs: - - dune:4 - - dune:1 + Error: + File "dune", line 1, characters 0-44: + Error: A library with name "foo" is defined in two folders: _build/default + and _build/default. Either change one of the names, or enable them + conditionally using the 'enabled_if' field. + [1] With some consumer @@ -43,6 +46,12 @@ With some consumer > EOF $ dune build + Error: + File "dune", line 1, characters 0-44: + Error: A library with name "foo" is defined in two folders: _build/default + and _build/default. Either change one of the names, or enable them + conditionally using the 'enabled_if' field. + File "dune", line 1, characters 0-0: Error: Module "Main" is used in several stanzas: - dune:1 diff --git a/test/blackbox-tests/test-cases/lib-collision/lib-collision-public.t b/test/blackbox-tests/test-cases/lib-collision/lib-collision-public.t index 9821639da72..55cdfff23d6 100644 --- a/test/blackbox-tests/test-cases/lib-collision/lib-collision-public.t +++ b/test/blackbox-tests/test-cases/lib-collision/lib-collision-public.t @@ -24,6 +24,17 @@ different folders. Without any consumers of the libraries $ dune build + Error: + File "b/dune", line 1, characters 0-44: + Error: A library with name "foo" is defined in two folders: _build/default/a + and _build/default/b. Either change one of the names, or enable them + conditionally using the 'enabled_if' field. + + -> required by _build/install/default/lib/bar/foo/foo.a + -> required by _build/default/bar.install + -> required by alias all + -> required by alias default + [1] With some consumer @@ -38,6 +49,16 @@ With some consumer > EOF $ dune build + Error: + File "b/dune", line 1, characters 0-44: + Error: A library with name "foo" is defined in two folders: _build/default/a + and _build/default/b. Either change one of the names, or enable them + conditionally using the 'enabled_if' field. + + -> required by _build/install/default/lib/bar/foo/foo.a + -> required by _build/default/bar.install + -> required by alias all + -> required by alias default File "b/dune", line 1, characters 0-44: 1 | (library 2 | (name foo)