Report an error for invalid dependency between modules in wrapped libraries

Report an error when in a wrapped library, a module that is not the
toplevel module depends on the toplevel module. This doesn't make as
such a module would in theory be inaccessible from the outside

If this causes compilation failures of released packages, we'll need
to turn this into a warning.

CHANGES.md

@@ -4,6 +4,10 @@
 - Fix the error message when there are more than one `<package>.opam`
   file for a given pacakge
 
+- Report an error when in a wrapped library, a module that is not the
+  toplevel module depends on the toplevel module. This doesn't make as
+  such a module would in theory be inaccessible from the outside
+
 1.0+beta10 (08/06/2017)
 -----------------------
 

src/gen_rules.ml

@@ -233,7 +233,13 @@ module Gen(P : Params) = struct
       | Some m -> String_map.add modules ~key:m.name ~data:m
     in
 
-    let dep_graph = Ocamldep.rules sctx ~dir ~item:lib.name ~modules ~alias_module in
+    let dep_graph =
+      Ocamldep.rules sctx ~dir ~item:lib.name ~modules ~alias_module
+        ~lib_interface_module:(if lib.wrapped then
+                                 String_map.find main_module_name modules
+                               else
+                                 None)
+    in
 
     Option.iter alias_module ~f:(fun m ->
       SC.add_rule sctx
@@ -465,7 +471,10 @@ module Gen(P : Params) = struct
         ~scope
     in
     let item = List.hd exes.names in
-    let dep_graph = Ocamldep.rules sctx ~dir ~item ~modules ~alias_module:None in
+    let dep_graph =
+      Ocamldep.rules sctx ~dir ~item ~modules ~alias_module:None
+        ~lib_interface_module:None
+    in
 
     let requires, real_requires =
       SC.Libs.requires sctx ~dir ~dep_kind ~item

src/ocamldep.ml

@@ -5,7 +5,7 @@ module SC = Super_context
 
 type dep_graph = (unit, string list String_map.t) Build.t Ml_kind.Dict.t
 
-let parse_deps ~dir lines ~modules ~alias_module =
+let parse_deps ~dir lines ~modules ~alias_module ~lib_interface_module =
   List.map lines ~f:(fun line ->
     match String.index line ':' with
     | None -> die "`ocamldep` in %s returned invalid line: %S" (Path.to_string dir) line
@@ -27,6 +27,17 @@ let parse_deps ~dir lines ~modules ~alias_module =
                                                 ~len:(String.length line - (i + 1)))
         |> List.filter ~f:(fun m -> m <> unit && String_map.mem m modules)
       in
+      (match lib_interface_module with
+       | None -> ()
+       | Some (m : Module.t) ->
+         if unit <> m.name && List.mem m.name ~set:deps then
+           die "Module %s in directory %s depends on %s.\n\
+                This doesn't make sense to me.\n\
+                \n\
+                %s is the main module of the library and is the only module exposed \n\
+                outside of the library. Consequently it should be the one dependending \n\
+                on all the other modules in the library."
+             unit (Path.to_string dir) m.name m.name);
       let deps =
         match alias_module with
         | None -> deps
@@ -44,7 +55,7 @@ let parse_deps ~dir lines ~modules ~alias_module =
     die
       "`ocamldep` in %s returned %s several times" (Path.to_string dir) unit
 
-let rules sctx ~ml_kind ~dir ~item ~modules ~alias_module =
+let rules sctx ~ml_kind ~dir ~item ~modules ~alias_module ~lib_interface_module =
   let suffix = Ml_kind.suffix ml_kind in
   let files =
     List.filter_map (String_map.values modules) ~f:(fun m -> Module.file ~dir m ml_kind)
@@ -64,7 +75,7 @@ let rules sctx ~ml_kind ~dir ~item ~modules ~alias_module =
        ~stdout_to:ocamldep_output);
   Build.memoize (Path.to_string ocamldep_output)
     (Build.lines_of ocamldep_output
-     >>^ parse_deps ~dir ~modules ~alias_module)
+     >>^ parse_deps ~dir ~modules ~alias_module ~lib_interface_module)
 
 module Dep_closure =
   Top_closure.Make(String)(struct
@@ -87,5 +98,5 @@ let names_to_top_closed_cm_files ~dir ~dep_graph ~modules ~mode names =
     let m = Utils.find_module ~dir modules name in
     Module.cm_file m ~dir cm_kind)
 
-let rules sctx ~dir ~item ~modules ~alias_module =
-  Ml_kind.Dict.of_func (rules sctx ~dir ~item ~modules ~alias_module)
+let rules sctx ~dir ~item ~modules ~alias_module ~lib_interface_module =
+  Ml_kind.Dict.of_func (rules sctx ~dir ~item ~modules ~alias_module ~lib_interface_module)

src/ocamldep.mli

@@ -7,6 +7,8 @@ type dep_graph = (unit, string list String_map.t) Build.t Ml_kind.Dict.t
 (** Generate ocamldep rules for the given modules. [item] is either the internal name of a
     library of the first name of a list of executables.
 
+    For wrapped libraries, [lib_interface_module] is the main module of the library.
+
     Return arrows that evaluate to the dependency graphs.
 *)
 val rules
@@ -15,6 +17,7 @@ val rules
   -> item:string
   -> modules:Module.t String_map.t
   -> alias_module:Module.t option
+  -> lib_interface_module:Module.t option
   -> dep_graph
 
 (** Close and convert a list of module names to a list of .cm file names *)