From 211dc0440261ab0158e15218e2c9f0f00d58ddde Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Wed, 30 Sep 2020 01:08:59 -0700 Subject: [PATCH] Dedicated public cmi dir for package private libs Signed-off-by: Rudi Grinberg --- src/dune_rules/dune_file.ml | 9 ++- src/dune_rules/install_rules.ml | 29 +++++--- src/dune_rules/obj_dir.ml | 69 ++++++++++++------- src/dune_rules/obj_dir.mli | 1 + .../test-cases/private-package-lib.t/run.t | 21 +++++- 5 files changed, 91 insertions(+), 38 deletions(-) diff --git a/src/dune_rules/dune_file.ml b/src/dune_rules/dune_file.ml index b5e7128264df..746fe77d01be 100644 --- a/src/dune_rules/dune_file.ml +++ b/src/dune_rules/dune_file.ml @@ -746,9 +746,16 @@ module Library = struct let is_impl t = Option.is_some t.implements let obj_dir ~dir t = + let private_lib = + match t.visibility with + | Private (Some _) -> true + | Private None + | Public _ -> + false + in Obj_dir.make_lib ~dir ~has_private_modules:(t.private_modules <> None) - (snd t.name) + ~private_lib (snd t.name) let main_module_name t : Lib_info.Main_module_name.t = match (t.implements, t.wrapped) with diff --git a/src/dune_rules/install_rules.ml b/src/dune_rules/install_rules.ml index 9d7a6b8a60e0..ca9c96d3d6b1 100644 --- a/src/dune_rules/install_rules.ml +++ b/src/dune_rules/install_rules.ml @@ -134,12 +134,29 @@ end = struct let modes = Dune_file.Mode_conf.Set.eval lib.modes ~has_native in let { Mode.Dict.byte; native } = modes in let module_files = + let inside_subdir f = + match lib_subdir with + | None -> f + | Some d -> Filename.concat d f + in + let cmi_dir = + (* TODO we should get this from the external obj dir *) + let public_cmi_dir = + match Lib_info.status info with + | Private (_, Some _) -> Some (inside_subdir ".public_cmi") + | _ -> lib_subdir + in + let private_cmi_dir = inside_subdir ".private" in + function + | Visibility.Public -> public_cmi_dir + | Private -> Some private_cmi_dir + in let virtual_library = Library.is_virtual lib in List.concat_map installable_modules ~f:(fun m -> let cm_file_unsafe kind = Obj_dir.Module.cm_file_unsafe obj_dir m ~kind in - let cmi_file = (Module.visibility m, cm_file_unsafe Cmi) in + let cmi_file = (cmi_dir (Module.visibility m), cm_file_unsafe Cmi) in let other_cm_files = let has_impl = Module.has ~ml_kind:Impl m in [ if_ (native && has_impl) [ cm_file_unsafe Cmx ] @@ -151,7 +168,7 @@ end = struct Obj_dir.Module.cmt_file obj_dir m ~ml_kind) ] |> List.concat - |> List.map ~f:(fun f -> (Visibility.Public, f)) + |> List.map ~f:(fun f -> (lib_subdir, f)) in cmi_file :: other_cm_files) in @@ -169,13 +186,7 @@ end = struct in List.concat [ sources - ; List.map module_files ~f:(fun (visibility, file) -> - let sub_dir = - match ((visibility : Visibility.t), lib_subdir) with - | Public, _ -> lib_subdir - | Private, None -> Some ".private" - | Private, Some dir -> Some (Filename.concat dir ".private") - in + ; List.map module_files ~f:(fun (sub_dir, file) -> make_entry ?sub_dir Lib file) ; List.map lib_files ~f:(make_entry Lib) ; List.map execs ~f:(make_entry Libexec) diff --git a/src/dune_rules/obj_dir.ml b/src/dune_rules/obj_dir.ml index fffb58de72c4..9427c2d16b95 100644 --- a/src/dune_rules/obj_dir.ml +++ b/src/dune_rules/obj_dir.ml @@ -22,50 +22,65 @@ module External = struct type t = { public_dir : Path.t ; private_dir : Path.t option + ; public_cmi_dir : Path.t option } - let make ~dir ~has_private_modules = + let make ~dir ~has_private_modules ~private_lib = let private_dir = if has_private_modules then Some (Path.relative dir ".private") else None in - { public_dir = dir; private_dir } + let public_cmi_dir = + if private_lib then + Some (Path.relative dir ".public_cmi") + else + None + in + { public_dir = dir; private_dir; public_cmi_dir } let public_cmi_dir t = t.public_dir - let to_dyn { public_dir; private_dir } = + let to_dyn { public_dir; private_dir; public_cmi_dir } = let open Dyn.Encoder in record [ ("public_dir", Path.to_dyn public_dir) ; ("private_dir", option Path.to_dyn private_dir) + ; ("public_cmi_dir", option Path.to_dyn public_cmi_dir) ] let cm_dir t (cm_kind : Cm_kind.t) (visibility : Visibility.t) = - match (cm_kind, visibility, t.private_dir) with - | Cmi, Private, Some p -> p - | Cmi, Private, None -> + match (cm_kind, visibility, t.private_dir, t.public_cmi_dir) with + | Cmi, Private, Some p, _ -> p + | Cmi, Private, None, _ -> Code_error.raise "External.cm_dir" [ ("t", to_dyn t) ] - | Cmi, Public, _ - | (Cmo | Cmx), _, _ -> + | Cmi, Public, _, Some public_cmi_dir -> public_cmi_dir + | Cmi, Public, _, None + | (Cmo | Cmx), _, _, _ -> t.public_dir - let encode { public_dir; private_dir } = + let encode { public_dir; private_dir; public_cmi_dir } = let open Dune_lang.Encoder in let extract d = Path.descendant ~of_:public_dir d |> Option.value_exn |> Path.to_string in let private_dir = Option.map ~f:extract private_dir in - record_fields [ field_o "private_dir" string private_dir ] + let public_cmi_dir = Option.map ~f:extract public_cmi_dir in + record_fields + [ field_o "private_dir" string private_dir + ; field_o "public_cmi_dir" string public_cmi_dir + ] let decode ~dir = + let public_dir = dir in let open Dune_lang.Decoder in fields - (let+ private_dir = field_o "private_dir" string in - let public_dir = dir in + (let+ private_dir = field_o "private_dir" string + and+ public_cmi_dir = field_o "public_cmi_dir" string in let private_dir = Option.map ~f:(Path.relative dir) private_dir in - { public_dir; private_dir }) + let public_cmi_dir = Option.map ~f:(Path.relative dir) public_cmi_dir in + { public_dir; private_dir; public_cmi_dir }) let byte_dir t = t.public_dir @@ -79,8 +94,8 @@ module External = struct let all_obj_dirs t ~mode:_ = [ t.public_dir ] - let all_cmis { public_dir; private_dir } = - List.filter_opt [ Some public_dir; private_dir ] + let all_cmis { public_dir; private_dir; public_cmi_dir } = + List.filter_opt [ Some public_dir; private_dir; public_cmi_dir ] let cm_public_dir t (cm_kind : Cm_kind.t) = match cm_kind with @@ -96,9 +111,11 @@ module Local = struct ; native_dir : Path.Build.t ; byte_dir : Path.Build.t ; public_cmi_dir : Path.Build.t option + ; private_lib : bool } - let to_dyn { dir; obj_dir; native_dir; byte_dir; public_cmi_dir } = + let to_dyn { dir; obj_dir; native_dir; byte_dir; public_cmi_dir; private_lib } + = let open Dyn.Encoder in record [ ("dir", Path.Build.to_dyn dir) @@ -106,10 +123,11 @@ module Local = struct ; ("native_dir", Path.Build.to_dyn native_dir) ; ("byte_dir", Path.Build.to_dyn byte_dir) ; ("public_cmi_dir", option Path.Build.to_dyn public_cmi_dir) + ; ("private_lib", bool private_lib) ] - let make ~dir ~obj_dir ~native_dir ~byte_dir ~public_cmi_dir = - { dir; obj_dir; native_dir; byte_dir; public_cmi_dir } + let make ~dir ~obj_dir ~native_dir ~byte_dir ~public_cmi_dir ~private_lib = + { dir; obj_dir; native_dir; byte_dir; public_cmi_dir; private_lib } let need_dedicated_public_dir t = Option.is_some t.public_cmi_dir @@ -134,7 +152,7 @@ module Local = struct in Path.Build.Set.of_list dirs |> Path.Build.Set.to_list - let make_lib ~dir ~has_private_modules lib_name = + let make_lib ~dir ~has_private_modules ~private_lib lib_name = let obj_dir = Paths.library_object_directory ~dir lib_name in let public_cmi_dir = Option.some_if has_private_modules (Paths.library_public_cmi_dir ~obj_dir) @@ -142,14 +160,14 @@ module Local = struct make ~dir ~obj_dir ~native_dir:(Paths.library_native_dir ~obj_dir) ~byte_dir:(Paths.library_byte_dir ~obj_dir) - ~public_cmi_dir + ~public_cmi_dir ~private_lib let make_exe ~dir ~name = let obj_dir = Paths.executable_object_directory ~dir name in make ~dir ~obj_dir ~native_dir:(Paths.library_native_dir ~obj_dir) ~byte_dir:(Paths.library_byte_dir ~obj_dir) - ~public_cmi_dir:None + ~public_cmi_dir:None ~private_lib:false let cm_dir t cm_kind _ = match cm_kind with @@ -190,11 +208,11 @@ let decode ~dir = let+ external_ = External.decode ~dir in External external_ -let make_lib ~dir ~has_private_modules lib_name = - Local (Local.make_lib ~dir ~has_private_modules lib_name) +let make_lib ~dir ~has_private_modules ~private_lib lib_name = + Local (Local.make_lib ~dir ~has_private_modules ~private_lib lib_name) let make_external_no_private ~dir = - External (External.make ~dir ~has_private_modules:false) + External (External.make ~dir ~has_private_modules:false ~private_lib:false) let get_path : type a. a t -> l:(Local.t -> Path.Build.t) -> e:(External.t -> Path.t) -> a @@ -226,7 +244,8 @@ let convert_to_external (t : Path.Build.t t) ~dir = match t with | Local e -> let has_private_modules = Local.need_dedicated_public_dir e in - External (External.make ~dir ~has_private_modules) + External + (External.make ~dir ~has_private_modules ~private_lib:e.private_lib) | _ -> assert false let all_cmis (type path) (t : path t) : path list = diff --git a/src/dune_rules/obj_dir.mli b/src/dune_rules/obj_dir.mli index 9b92a9fcbab0..5785bb543169 100644 --- a/src/dune_rules/obj_dir.mli +++ b/src/dune_rules/obj_dir.mli @@ -57,6 +57,7 @@ val all_obj_dirs : 'path t -> mode:Mode.t -> 'path list val make_lib : dir:Path.Build.t -> has_private_modules:bool + -> private_lib:bool -> Lib_name.Local.t -> Path.Build.t t diff --git a/test/blackbox-tests/test-cases/private-package-lib.t/run.t b/test/blackbox-tests/test-cases/private-package-lib.t/run.t index f71dd1239e77..c66f2ea321a2 100644 --- a/test/blackbox-tests/test-cases/private-package-lib.t/run.t +++ b/test/blackbox-tests/test-cases/private-package-lib.t/run.t @@ -40,7 +40,6 @@ The naming convention puts the artifacts of private libs under __private__: $ ls _build/install/default/lib/foo/__private__/secret | grep -i \.cm secret.cma - secret.cmi secret.cmt secret.cmx secret.cmxa @@ -86,9 +85,9 @@ Cmi for secret library must not be visible for normal users. Hence they must be hidden. $ grep __private__ _build/default/foo.install + "_build/install/default/lib/foo/__private__/secret/.public_cmi/secret.cmi" {"__private__/secret/.public_cmi/secret.cmi"} "_build/install/default/lib/foo/__private__/secret/secret.a" {"__private__/secret/secret.a"} "_build/install/default/lib/foo/__private__/secret/secret.cma" {"__private__/secret/secret.cma"} - "_build/install/default/lib/foo/__private__/secret/secret.cmi" {"__private__/secret/secret.cmi"} "_build/install/default/lib/foo/__private__/secret/secret.cmt" {"__private__/secret/secret.cmt"} "_build/install/default/lib/foo/__private__/secret/secret.cmx" {"__private__/secret/secret.cmx"} "_build/install/default/lib/foo/__private__/secret/secret.cmxa" {"__private__/secret/secret.cmxa"} @@ -124,6 +123,18 @@ Now we try to use the library in a subproject: $ dune exec ./subproj/subproj.exe from library foo secret string +But we shouldn't be able to access it directly: + + $ cat >subproj/subproj.ml < print_endline Secret.secret + > EOF + $ dune exec ./subproj/subproj.exe + File "subproj/subproj.ml", line 1, characters 14-27: + 1 | print_endline Secret.secret + ^^^^^^^^^^^^^ + Error: Unbound module Secret + [1] + Now we make sure such libraries are transitively usable when installed: $ mkdir use @@ -150,4 +161,8 @@ But we cannot use such libraries directly: $ dune exec --root use -- ./run.exe Entering directory 'use' Entering directory 'use' - direct access attempt: secret string + File "run.ml", line 1, characters 43-56: + 1 | print_endline ("direct access attempt: " ^ Secret.secret) + ^^^^^^^^^^^^^ + Error: Unbound module Secret + [1]