From 572774490af168a05fa602920e2d4abb5a282211 Mon Sep 17 00:00:00 2001 From: Jeremie Dimino Date: Fri, 9 Jun 2017 12:24:34 +0100 Subject: [PATCH] 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 ++++ src/gen_rules.ml | 13 +++++++++++-- src/ocamldep.ml | 21 ++++++++++++++++----- src/ocamldep.mli | 3 +++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a5170db4..adf96cb6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,10 @@ - Fix the error message when there are more than one `.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) ----------------------- diff --git a/src/gen_rules.ml b/src/gen_rules.ml index b34aeee2..e91a1808 100644 --- a/src/gen_rules.ml +++ b/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 diff --git a/src/ocamldep.ml b/src/ocamldep.ml index ef9b0156..153ab493 100644 --- a/src/ocamldep.ml +++ b/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) diff --git a/src/ocamldep.mli b/src/ocamldep.mli index d44a738e..9975253d 100644 --- a/src/ocamldep.mli +++ b/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 *)