Detect conflicts between internal and external libraries

src/findlib.ml

@@ -431,14 +431,41 @@ let available t ~required_by name =
   | (_ : package) -> true
   | exception (Package_not_found _) -> false
 
-let closure pkgs =
+module External_dep_conflicts_with_local_lib = struct
+  type t =
+    { package             : string
+    ; required_by         : string
+    ; required_locally_in : Path.t
+    ; defined_locally_in  : Path.t
+    }
+end
+
+exception External_dep_conflicts_with_local_lib of External_dep_conflicts_with_local_lib.t
+
+let check_deps_consistency ~required_by ~local_public_libs pkg requires =
+  List.iter requires ~f:(fun pkg' ->
+    match String_map.find pkg'.name local_public_libs with
+    | None -> ()
+    | Some path ->
+      raise (External_dep_conflicts_with_local_lib
+               { package             = pkg'.name
+               ; required_by         = pkg.name
+               ; required_locally_in = required_by
+               ; defined_locally_in  = path
+               }))
+
+let closure ~required_by ~local_public_libs pkgs =
   remove_dups_preserve_order
-    (List.concat_map pkgs ~f:(fun pkg -> pkg.requires)
+    (List.concat_map pkgs ~f:(fun pkg ->
+       check_deps_consistency ~required_by ~local_public_libs pkg pkg.requires;
+       pkg.requires)
      @ pkgs)
 
-let closed_ppx_runtime_deps_of pkgs =
+let closed_ppx_runtime_deps_of ~required_by ~local_public_libs pkgs =
   remove_dups_preserve_order
-    (List.concat_map pkgs ~f:(fun pkg -> pkg.ppx_runtime_deps))
+    (List.concat_map pkgs ~f:(fun pkg ->
+       check_deps_consistency ~required_by ~local_public_libs pkg pkg.ppx_runtime_deps;
+       pkg.ppx_runtime_deps))
 
 let root_packages t =
   let pkgs =

src/findlib.mli

@@ -1,5 +1,7 @@
 (** Findlib database *)
 
+open Import
+
 module Package_not_found : sig
   type t =
     { package     : string
@@ -9,6 +11,17 @@ end
 
 exception Package_not_found of Package_not_found.t
 
+module External_dep_conflicts_with_local_lib : sig
+  type t =
+    { package             : string
+    ; required_by         : string
+    ; required_locally_in : Path.t
+    ; defined_locally_in  : Path.t
+    }
+end
+
+exception External_dep_conflicts_with_local_lib of External_dep_conflicts_with_local_lib.t
+
 (** Findlib database *)
 type t
 
@@ -38,8 +51,18 @@ val available : t -> required_by:string list -> string -> bool
 
 val root_package_name : string -> string
 
-val closure : package list -> package list
-val closed_ppx_runtime_deps_of : package list -> package list
+(** [local_public_libs] is a map from public library names to where they are defined in
+    the workspace. These must not appear as dependency of a findlib package *)
+val closure
+  :  required_by:Path.t
+  -> local_public_libs:Path.t String_map.t
+  -> package list
+  -> package list
+val closed_ppx_runtime_deps_of
+  :  required_by:Path.t
+  -> local_public_libs:Path.t String_map.t
+  -> package list
+  -> package list
 
 val root_packages : t -> string list
 val all_packages  : t -> package list

src/gen_rules.ml

@@ -204,6 +204,8 @@ module Gen(P : Params) = struct
       | None -> build
       | Some f -> Build.fail f >>> build
 
+    let local_public_libs = Lib_db.local_public_libs t
+
     let closure ~dir ~dep_kind lib_deps =
       let internals, externals, fail = Lib_db.interpret_lib_deps t ~dir lib_deps in
       with_fail ~fail
@@ -214,8 +216,10 @@ module Gen(P : Params) = struct
               load_requires ~dir ~item:lib.name))
          >>^ (fun internal_deps ->
            let externals =
-             List.map (Findlib.closure externals) ~f:(fun pkg ->
-               Lib.External pkg)
+             Findlib.closure externals
+               ~required_by:dir
+               ~local_public_libs
+             |> List.map ~f:(fun pkg -> Lib.External pkg)
            in
            Lib.remove_dups_preserve_order
              (List.concat (externals :: internal_deps) @
@@ -231,8 +235,10 @@ module Gen(P : Params) = struct
               load_runtime_deps ~dir ~item:lib.name))
          >>^ (fun libs ->
            let externals =
-             List.map (Findlib.closed_ppx_runtime_deps_of externals)
-               ~f:(fun pkg -> Lib.External pkg)
+             Findlib.closed_ppx_runtime_deps_of externals
+               ~required_by:dir
+               ~local_public_libs
+             |> List.map ~f:(fun pkg -> Lib.External pkg)
            in
            Lib.remove_dups_preserve_order (List.concat (externals :: libs))))
 

src/jbuild_load.ml

@@ -85,7 +85,7 @@ end
         let pkgs =
           List.map requires ~f:(Findlib.find_exn context.findlib
                                   ~required_by:[Utils.jbuild_name_in ~dir:dir])
-          |> Findlib.closure
+          |> Findlib.closure ~required_by:dir ~local_public_libs:String_map.empty
         in
         let includes =
           List.fold_left pkgs ~init:Path.Set.empty ~f:(fun acc pkg ->

src/lib_db.ml

@@ -10,8 +10,11 @@ type t =
   ; (* This is to filter out libraries that are not installable because of missing
        dependencies *)
     instalable_internal_libs : Lib.Internal.t String_map.t
+  ; local_public_libs        : Path.t String_map.t
   }
 
+let local_public_libs t = t.local_public_libs
+
 let rec internal_name_scope t ~dir =
   match Hashtbl.find t.by_internal_name dir with
   | Some scope -> scope
@@ -97,11 +100,18 @@ let compute_instalable_internal_libs t ~internal_libraries =
         t)
 
 let create findlib ~dirs_with_dot_opam_files internal_libraries =
+  let local_public_libs =
+    List.fold_left internal_libraries ~init:String_map.empty ~f:(fun acc (dir, lib) ->
+      match lib.Library.public with
+      | None -> acc
+      | Some { name; _ } -> String_map.add acc ~key:name ~data:dir)
+  in
   let t =
     { findlib
     ; by_public_name   = Hashtbl.create 1024
     ; by_internal_name = Hashtbl.create 1024
     ; instalable_internal_libs = String_map.empty
+    ; local_public_libs
     }
   in
   (* Initializes the scopes, including [Path.root] so that when there are no <pkg>.opam

src/lib_db.mli

@@ -33,3 +33,6 @@ val resolve_selects
   -> resolved_select list
 
 val lib_is_available : t -> from:Path.t -> string -> bool
+
+(** For [Findlib.closure] *)
+val local_public_libs : t -> Path.t String_map.t

src/main.ml

@@ -89,7 +89,7 @@ let report_error ?(map_fname=fun x->x) ppf exn ~backtrace =
     Format.fprintf ppf "%s\n" (String.capitalize_ascii msg)
   | Findlib.Package_not_found { package; required_by } ->
     Format.fprintf ppf
-      "@{<error>Error@}: Findlib package %S not found.\n" package;
+      "@{<error>Error@}: External library %S not found.\n" package;
     List.iter required_by ~f:(Format.fprintf ppf "-> required by %S\n");
     let cmdline_suggestion =
       (* CR-someday jdimino: this is ugly *)
@@ -102,6 +102,19 @@ let report_error ?(map_fname=fun x->x) ppf exn ~backtrace =
     Format.fprintf ppf
       "Hint: try: %s\n"
       (List.map cmdline_suggestion ~f:quote_for_shell |> String.concat ~sep:" ")
+  | Findlib.External_dep_conflicts_with_local_lib
+      { package; required_by; required_locally_in; defined_locally_in } ->
+    Format.fprintf ppf
+      "@{<error>Error@}: Conflict between internal and external version of library %S:\n\
+       - it is defined locally in %s\n\
+       - it is required by external library %S\n\
+       - external library %S is required in %s\n\
+       This cannot work.\n"
+      package
+      (Path.to_string defined_locally_in)
+      required_by
+      required_by
+      (Utils.jbuild_name_in ~dir:required_locally_in)
   | Code_error msg ->
     let bt = Printexc.raw_backtrace_to_string backtrace in
     Format.fprintf ppf "@{<error>Internal error, please report upstream \