Revert public_interfaces for now

The code to support it is starting to become increasingly complicated
and the number of problem found is a bit alarming.

We'll reinclude it later after a bit more testing and hopefully some
simplifications.

CHANGES.md

@@ -1,11 +1,6 @@
 1.0.0 (coming soon)
 -------------------
 
-- Add a field `public_interfaces` to libraries. It lists the modules
-  that are accessible outside of the scope where the libraries is
-  defined, for instance when the library is installed (Francois Bobot,
-  #106)
-
 - Fix the error message when there are more than one `<package>.opam`
   file for a given pacakge
 

doc/jbuild.rst

@@ -81,12 +81,6 @@ modules you want.
   letter. For instance to exclude module ``Foo``: ``(modules (:standard \
   foo))``
 
-- ``(public_interfaces <modules>)`` specifies the modules that are
-  visible to users of the library. Modules that are not part of this
-  list will only be visible inside the library itself.
-  ``<public_interfaces>`` uses the same `Ordered set language` than
-  ``<modules>`` and is currently only allowed for public libraries.
-
 - ``(libraries (<library-dependencies>))`` is used to specify the dependencies
   of the library. See the section about `Library dependencies`_ for more details
 

src/gen_rules.ml

@@ -66,7 +66,7 @@ module Gen(P : Params) = struct
     in
     List.map cclibs ~f
 
-  let build_lib (lib : Library.t) ~flags ~dir ~mode ~modules ~dep_graph =
+  let build_lib (lib : Library.t) ~scope ~flags ~dir ~mode ~modules ~dep_graph =
     Option.iter (Context.compiler ctx mode) ~f:(fun compiler ->
       let target = lib_archive lib ~dir ~ext:(Mode.compiled_lib_ext mode) in
       let dep_graph = Ml_kind.Dict.get dep_graph Impl in
@@ -107,7 +107,7 @@ module Gen(P : Params) = struct
            ; A "-a"; A "-o"; Target target
            ; As stubs_flags
            ; Dyn (fun (_, cclibs) -> Arg_spec.quote_args "-cclib" (map_cclibs cclibs))
-           ; As (List.map lib.library_flags ~f:(SC.expand_vars sctx ~scope:lib.scope ~dir))
+           ; As (List.map lib.library_flags ~f:(SC.expand_vars sctx ~scope ~dir))
            ; As (match lib.kind with
                | Normal -> []
                | Ppx_deriver | Ppx_rewriter -> ["-linkall"])
@@ -131,7 +131,7 @@ module Gen(P : Params) = struct
          (Dep ctx.ocamlc)
          [ As (Utils.g ())
          ; Dyn (fun (c_flags, libs) ->
-             S [ Lib.c_include_flags ~context:ctx.name ~source_dir:Internal libs
+             S [ Lib.c_include_flags libs
                ; Arg_spec.quote_args "-ccopt" c_flags
                ])
          ; A "-o"; Target dst
@@ -159,7 +159,7 @@ module Gen(P : Params) = struct
          [ S [A "-I"; Path ctx.stdlib_dir]
          ; As (SC.cxx_flags sctx)
          ; Dyn (fun (cxx_flags, libs) ->
-             S [ Lib.c_include_flags ~context:ctx.name ~source_dir:Internal libs
+             S [ Lib.c_include_flags libs
                ; As cxx_flags
                ])
          ; A "-o"; Target dst
@@ -176,7 +176,7 @@ module Gen(P : Params) = struct
   let alias_module_build_sandbox = Scanf.sscanf ctx.version "%u.%u"
      (fun a b -> a, b) <= (4, 02)
 
-  let library_rules (lib : Library.t) ~dir ~all_modules ~files =
+  let library_rules (lib : Library.t) ~dir ~all_modules ~files ~scope =
     let dep_kind = if lib.optional then Build.Optional else Required in
     let flags = Ocaml_flags.make lib.buildable in
     let modules =
@@ -225,7 +225,7 @@ module Gen(P : Params) = struct
       SC.PP.pped_modules sctx ~dir ~dep_kind ~modules ~preprocess:lib.buildable.preprocess
         ~preprocessor_deps:lib.buildable.preprocessor_deps
         ~lib_name:(Some lib.name)
-        ~scope:lib.scope
+        ~scope
     in
     let modules =
       match alias_module with
@@ -272,7 +272,7 @@ module Gen(P : Params) = struct
     Option.iter alias_module ~f:(fun m ->
       let flags = Ocaml_flags.default () in
       Module_compilation.build_module sctx m
-        ~js_of_ocaml
+         ~js_of_ocaml
         ~dynlink
         ~sandbox:alias_module_build_sandbox
         ~flags:{ flags with common = flags.common @ ["-w"; "-49"] }
@@ -349,49 +349,20 @@ module Gen(P : Params) = struct
         end
     end;
 
-    (* Setup artifact aliases for users of the library *)
-    begin
-      (* If the library is public, users of the library read the files from
-         "_build/install/..." *)
-      let artifact_dir, modules =
-        match lib.public with
-        | None -> dir, modules
-        | Some { package; sub_dir; _ } ->
-          let dir =
-            let install_dir = Config.local_install_dir ~context:ctx.name in
-            let dir = Path.append install_dir (Install.lib_install_path ~package) in
-            match sub_dir with
-            | None -> dir
-            | Some s -> Path.relative dir s
-          in
-          let modules =
-            if Ordered_set_lang.is_standard lib.public_interfaces then
-              modules
-            else
-              let public_interfaces =
-                Ordered_set_lang.eval_with_standard lib.public_interfaces
-                  ~standard:(String_map.keys modules)
-                |> String_set.of_list
-              in
-              String_map.filter modules ~f:(fun m _ -> String_set.mem m public_interfaces)
-          in
-          (dir, modules)
+    List.iter Cm_kind.all ~f:(fun cm_kind ->
+      let files =
+        String_map.fold modules ~init:[] ~f:(fun ~key:_ ~data:m acc ->
+          Module.cm_file m ~dir cm_kind :: acc)
       in
-      List.iter Cm_kind.all ~f:(fun cm_kind ->
-        let files =
-          String_map.fold modules ~init:[] ~f:(fun ~key:_ ~data:m acc ->
-            Module.cm_file m ~dir:artifact_dir cm_kind :: acc)
-        in
-        SC.Libs.setup_file_deps_alias sctx (dir, lib) ~ext:(Cm_kind.ext cm_kind)
-          files);
-      SC.Libs.setup_file_deps_group_alias sctx (dir, lib) ~exts:[".cmi"; ".cmx"];
-      SC.Libs.setup_file_deps_alias sctx (dir, lib) ~ext:".h"
-        (List.map lib.install_c_headers ~f:(fun header ->
-           Path.relative artifact_dir (header ^ ".h")));
-    end;
+      SC.Libs.setup_file_deps_alias sctx (dir, lib) ~ext:(Cm_kind.ext cm_kind)
+        files);
+    SC.Libs.setup_file_deps_group_alias sctx (dir, lib) ~exts:[".cmi"; ".cmx"];
+    SC.Libs.setup_file_deps_alias sctx (dir, lib) ~ext:".h"
+      (List.map lib.install_c_headers ~f:(fun header ->
+         Path.relative dir (header ^ ".h")));
 
     List.iter Mode.all ~f:(fun mode ->
-      build_lib lib ~flags ~dir ~mode ~modules ~dep_graph);
+      build_lib lib ~scope ~flags ~dir ~mode ~modules ~dep_graph);
     (* Build *.cma.js *)
     SC.add_rules sctx (
       let src = lib_archive lib ~dir ~ext:(Mode.compiled_lib_ext Mode.Byte) in
@@ -471,8 +442,7 @@ module Gen(P : Params) = struct
          [ Ocaml_flags.get flags mode
          ; A "-o"; Target exe
          ; As link_flags
-         ; Dyn (fun (libs, _) -> Lib.link_flags libs ~context:ctx.name
-                                   ~source_dir:Internal ~mode)
+         ; Dyn (fun (libs, _) -> Lib.link_flags libs ~mode)
          ; Dyn (fun (_, cm_files) -> Deps cm_files)
          ]);
     if mode = Mode.Byte then
@@ -724,7 +694,8 @@ Add it to your jbuild file to remove this warning.
       match (stanza : Stanza.t) with
       | Library lib  ->
         Some (library_rules lib ~dir
-                ~all_modules:(Lazy.force all_modules) ~files:(Lazy.force files))
+                ~all_modules:(Lazy.force all_modules) ~files:(Lazy.force files)
+                ~scope)
       | Executables  exes ->
         Some (executables_rules exes ~dir ~all_modules:(Lazy.force all_modules)
                 ~scope)
@@ -874,20 +845,10 @@ Add it to your jbuild file to remove this warning.
             sprintf "<module table for context %s>"
               (Path.to_string ctx.build_dir))
       in
-      let public_interfaces =
-        Ordered_set_lang.eval_with_standard lib.public_interfaces
-          ~standard:(List.map modules ~f:(fun s -> s.Module.name))
-        |> String_set.of_list
-      in
       List.concat
         [ List.concat_map modules ~f:(fun m ->
-            let intf =
-              if String_set.mem m.Module.name public_interfaces
-              then [ Module.cm_file m ~dir Cmi ]
-              else []
-            in
             List.concat
-              [ intf
+              [ [ Module.cm_file m ~dir Cmi ]
               ; if_ native [ Module.cm_file m ~dir Cmx ]
               ; List.filter_map Ml_kind.all ~f:(Module.cmt_file m ~dir)
               ; [ match Module.file m ~dir Intf with

src/install.ml

@@ -128,8 +128,6 @@ module Entry = struct
     Path.relative main_dir dst
 end
 
-let lib_install_path ~package = Path.relative Entry.Paths.lib package.Package.name
-
 module SMap = Map.Make(Section)
 
 let files entries =

src/install.mli

@@ -33,5 +33,3 @@ end
 
 val files : Entry.t list -> Path.Set.t
 val gen_install_file : Entry.t list -> string
-
-val lib_install_path: package:Package.t -> Path.t

src/jbuild.ml

@@ -70,9 +70,6 @@ module Scope = struct
     ; root     : Path.t
     }
 
-  let compare t1 t2 = Path.compare t1.root t2.root
-  let name t = Option.value ~default:"[root]" t.name
-
   let empty =
     { name     = None
     ; packages = String_map.empty
@@ -526,7 +523,6 @@ module Library = struct
 
   type t =
     { name                     : string
-    ; scope                    : Scope.t
     ; public                   : Public_lib.t option
     ; synopsis                 : string option
     ; install_c_headers        : string list
@@ -545,7 +541,6 @@ module Library = struct
     ; optional                 : bool
     ; buildable                : Buildable.t
     ; dynlink                  : bool
-    ; public_interfaces        : Ordered_set_lang.t
     }
 
   let v1 pkgs =
@@ -569,15 +564,8 @@ module Library = struct
        field_b    "optional"                                                 >>= fun optional                 ->
        field      "self_build_stubs_archive" (option string) ~default:None   >>= fun self_build_stubs_archive ->
        field_b    "no_dynlink"                                               >>= fun no_dynlink               ->
-       map_validate (field_o "public_interfaces" Ordered_set_lang.t) ~f:(fun public_interfaces ->
-         match public_interfaces, public with
-         | Some _, None ->
-           Error "Field 'public_interfaces' is not allowed for private libraries."
-         | _ -> Ok public_interfaces)
-       >>= fun public_interfaces ->
        return
          { name
-         ; scope = pkgs
          ; public
          ; synopsis
          ; install_c_headers
@@ -596,8 +584,6 @@ module Library = struct
          ; optional
          ; buildable
          ; dynlink = not no_dynlink
-         ; public_interfaces =
-             Option.value public_interfaces ~default:Ordered_set_lang.standard
          })
 
   let has_stubs t =

src/jbuild.mli

@@ -17,10 +17,6 @@ module Scope : sig
     ; root     : Path.t
     }
 
-  val compare : t -> t -> int
-  val name: t -> string
-  (** Pretty name *)
-
   val make : Package.t list -> t
 
   val empty : t
@@ -132,7 +128,6 @@ module Library : sig
 
   type t =
     { name                     : string
-    ; scope                    : Scope.t
     ; public                   : Public_lib.t option
     ; synopsis                 : string option
     ; install_c_headers        : string list
@@ -151,7 +146,6 @@ module Library : sig
     ; optional                 : bool
     ; buildable                : Buildable.t
     ; dynlink                  : bool
-    ; public_interfaces        : Ordered_set_lang.t
     }
 
   val has_stubs : t -> bool

src/lib.ml

@@ -22,39 +22,23 @@ end
 include T
 module Set = Set.Make(T)
 
-module Source_dir = struct
-  type t =
-    | Internal
-    | Install
-end
-
-let dir ~context ~source_dir = function
-  | Internal (dir, lib) -> begin
-      match lib.public, (source_dir : Source_dir.t) with
-      | None, _ | _, Internal ->
-        dir
-      | Some { package; sub_dir; _ }, Install ->
-        let install_dir = Config.local_install_dir ~context in
-        let dir =  Path.append install_dir (Install.lib_install_path ~package) in
-        match sub_dir with
-        | None -> dir
-        | Some x -> Path.relative dir x
-    end
+let dir = function
+  | Internal (dir, _) -> dir
   | External pkg -> pkg.dir
 
-let include_paths ~context ~source_dir ts =
+let include_paths ts =
   List.fold_left ts ~init:Path.Set.empty ~f:(fun acc t ->
-    Path.Set.add (dir ~context ~source_dir t) acc)
+    Path.Set.add (dir t) acc)
 
-let include_flags ~context ~source_dir ts =
-  let dirs = include_paths ~context ~source_dir ts in
+let include_flags ts =
+  let dirs = include_paths ts in
   Arg_spec.S (List.concat_map (Path.Set.elements dirs) ~f:(fun dir ->
     [Arg_spec.A "-I"; Path dir]))
 
-let c_include_flags ~context ~source_dir ts =
+let c_include_flags ts =
   let dirs =
     List.fold_left ts ~init:Path.Set.empty ~f:(fun acc t ->
-      Path.Set.add (dir ~context ~source_dir t) acc)
+      Path.Set.add (dir t) acc)
   in
   Arg_spec.S (List.concat_map (Path.Set.elements dirs) ~f:(fun dir ->
     [Arg_spec.A "-I"; Path dir]))
@@ -68,9 +52,9 @@ let describe = function
   | External pkg ->
     sprintf "%s (external)" pkg.name
 
-let link_flags ~context ~source_dir ts ~mode =
+let link_flags ts ~mode =
   Arg_spec.S
-    (include_flags ~context ~source_dir ts ::
+    (include_flags ts ::
      List.map ts ~f:(fun t ->
        match t with
        | External pkg ->

src/lib.mli

@@ -12,38 +12,13 @@ module Set : Set.S with type elt := t
 
 (*val deps : t -> string list*)
 
-module Source_dir : sig
-  (** Which include directory to use for internal public libraries *)
-  type t =
-    | Internal (** use the directory from "_build/<context>" *)
-    | Install  (** use the directory from "_build/install/<context>" *)
-end
+val include_paths : t list -> Path.Set.t
 
-(** The scope given is the current one *)
-val include_paths
-  :  context:string
-  -> source_dir:Source_dir.t
-  -> t list
-  -> Path.Set.t
+val include_flags : t list -> _ Arg_spec.t
 
-val include_flags
-  :  context:string
-  -> source_dir:Source_dir.t
-  -> t list
-  -> _ Arg_spec.t
+val c_include_flags : t list -> _ Arg_spec.t
 
-val c_include_flags
-  :  context:string
-  -> source_dir:Source_dir.t
-  -> t list
-  -> _ Arg_spec.t
-
-val link_flags
-  :  context:string
-  -> source_dir:Source_dir.t
-  -> t list
-  -> mode:Mode.t
-  -> _ Arg_spec.t
+val link_flags : t list -> mode:Mode.t -> _ Arg_spec.t
 
 val archive_files : t list -> mode:Mode.t -> ext_lib:string -> Path.t list
 

src/merlin.mli

@@ -9,3 +9,4 @@ type t =
 
 (** Add rules for generating the .merlin in a directory *)
 val add_rules : Super_context.t -> dir:Path.t -> t list -> unit
+

src/module_compilation.ml

@@ -66,7 +66,7 @@ let build_cm sctx ?sandbox ~dynlink ~flags ~cm_kind ~(dep_graph:Ocamldep.dep_gra
            ~extra_targets
            [ Ocaml_flags.get_for_cm flags ~cm_kind
            ; cmt_args
-           ; Dyn (Lib.include_flags ~context:ctx.name ~source_dir:Install)
+           ; Dyn Lib.include_flags
            ; As extra_args
            ; if dynlink || cm_kind <> Cmx then As [] else A "-nodynlink"
            ; A "-no-alias-deps"

src/odoc.ml

@@ -134,7 +134,7 @@ let setup_library_rules sctx (lib : Library.t) ~dir ~modules ~requires
         (requires
          >>>
          SC.Libs.file_deps sctx ~ext:odoc_ext
-         >>^ Lib.include_flags ~context:context.name ~source_dir:Internal)
+         >>^ Lib.include_flags)
     in
     let modules_and_odoc_files =
       List.map (String_map.values modules)

src/super_context.ml

@@ -786,7 +786,7 @@ module PP = struct
        >>>
        Build.run ~context:ctx (Dep compiler)
          [ A "-o" ; Target target
-         ; Dyn (Lib.link_flags ~context:ctx.name ~source_dir:Internal ~mode)
+         ; Dyn (Lib.link_flags ~mode)
          ])
 
   let get_ppx_driver sctx pps ~dir ~dep_kind =

test/jbuild

@@ -104,15 +104,3 @@
   (deps ((alias sleep5)
          (alias sleep4-and-fail)
          (alias sleep1-and-fail)))))
-
-(alias
- ((name runtest)
-  (deps ((files_recursively_in workspaces/public_interfaces)))
-  (action
-   (chdir workspaces/public_interfaces
-     (progn
-      (run ${exe:run.exe} -log log1 --
-       ${bin:jbuilder} build --verbose -j1 @install --root . --only good,mylib,mylib2)
-      (run ${exe:run.exe} -log log2 -inverse --
-       ${bin:jbuilder} build -j1 @install --root . --only bad,mylib,mylib2
-       ))))))

test/workspaces/public_interfaces/bad/jbuild

@@ -1,5 +0,0 @@
-(library (
-   (name bad)
-   (public_name bad)
-   (libraries (mylib))
-))

test/workspaces/public_interfaces/bad/main.ml

@@ -1 +0,0 @@
-let () = Printf.printf "%s" Mylib.Bad.v

test/workspaces/public_interfaces/good/jbuild

@@ -1,5 +0,0 @@
-(library (
-   (name good)
-   (public_name good)
-   (libraries (mylib mylib2.see_private))
-))

test/workspaces/public_interfaces/good/main.ml

@@ -1 +0,0 @@
-let () = Printf.printf "%s (and bad size is %i)" Mylib.Good.v Mylib2_see_private.See_private.nb_bad

test/workspaces/public_interfaces/mylib/bad.ml

@@ -1 +0,0 @@
-let v = "Bad"

test/workspaces/public_interfaces/mylib/good.ml

@@ -1 +0,0 @@
-let v = "Good"

test/workspaces/public_interfaces/mylib/jbuild

@@ -1,13 +0,0 @@
-(library (
-   (name mylib)
-   (public_name mylib)
-   (modules (good bad))
-   (public_interfaces (:standard \ Bad))
-))
-
-(library (
-   (name mylib2_see_private)
-   (public_name mylib2.see_private)
-   (modules (see_private))
-   (libraries (mylib))
-))

test/workspaces/public_interfaces/mylib/see_private.ml

@@ -1 +0,0 @@
-let nb_bad = String.length Mylib.Bad.v