Fix the test added in the previous commit

CHANGES.md

@@ -24,6 +24,8 @@ next
 
 - Build documentation for non public libraries (#306)
 
+- Fix doc generation when several private libraries have the same name
+
 1.0+beta16 (05/11/2017)
 -----------------------
 

Makefile

@@ -54,7 +54,7 @@ update-jbuilds: $(BIN)
 
 accept-corrections:
 	for i in `find . -name \*.corrected`; do \
-	  cp $$i $${i/.corrected}; \
+	  cp $$i $${i%.corrected}; \
 	done
 
 .DEFAULT_GOAL := default

src/gen_rules.ml

@@ -1106,13 +1106,8 @@ end
 let gen ~contexts ?(filter_out_optional_stanzas_with_missing_deps=true)
       ?only_packages ?(unlink_aliases=[]) conf =
   let open Future in
-  let { Jbuild_load. file_tree; jbuilds; packages } = conf in
+  let { Jbuild_load. file_tree; jbuilds; packages; scopes } = conf in
   let aliases = Alias.Store.create () in
-  let dirs_with_dot_opam_files =
-    String_map.fold packages ~init:Path.Set.empty
-      ~f:(fun ~key:_ ~data:{ Package. path; _ } acc ->
-        Path.Set.add path acc)
-  in
   let packages =
     match only_packages with
     | None -> packages
@@ -1141,7 +1136,7 @@ let gen ~contexts ?(filter_out_optional_stanzas_with_missing_deps=true)
       Super_context.create
         ~context
         ~aliases
-        ~dirs_with_dot_opam_files
+        ~scopes
         ~file_tree
         ~packages
         ~filter_out_optional_stanzas_with_missing_deps

src/jbuild.ml

@@ -599,6 +599,11 @@ module Library = struct
 
   let all_lib_deps t =
     List.map t.virtual_deps ~f:(fun s -> Lib_dep.Direct s) @ t.buildable.libraries
+
+  let best_name t =
+    match t.public with
+    | None -> t.name
+    | Some p -> p.name
 end
 
 module Install_conf = struct

src/jbuild.mli

@@ -162,6 +162,7 @@ module Library : sig
   val has_stubs : t -> bool
   val stubs_archive : t -> dir:Path.t -> ext_lib:string -> Path.t
   val all_lib_deps : t -> Lib_deps.t
+  val best_name : t -> string
 end
 
 module Install_conf : sig

src/jbuild_load.ml

@@ -153,6 +153,7 @@ type conf =
   { file_tree : File_tree.t
   ; jbuilds   : Jbuilds.t
   ; packages  : Package.t String_map.t
+  ; scopes    : Scope.t list
   }
 
 let load ~dir ~scope =
@@ -203,6 +204,12 @@ let load ?extra_ignored_subtrees () =
     |> Path.Map.of_alist_multi
     |> Path.Map.map ~f:Scope.make
   in
+  let scopes =
+    if Path.Map.mem Path.root scopes then
+      scopes
+    else
+      Path.Map.add scopes ~key:Path.root ~data:Scope.empty
+  in
   let rec walk dir jbuilds scope =
     if File_tree.Dir.ignored dir then
       jbuilds
@@ -227,4 +234,5 @@ let load ?extra_ignored_subtrees () =
   { file_tree = ftree
   ; jbuilds
   ; packages
+  ; scopes = Path.Map.values scopes
   }

src/jbuild_load.mli

@@ -11,6 +11,7 @@ type conf =
   { file_tree : File_tree.t
   ; jbuilds   : Jbuilds.t
   ; packages  : Package.t String_map.t
+  ; scopes    : Scope.t list
   }
 
 val load : ?extra_ignored_subtrees:Path.Set.t -> unit -> conf

src/lib.ml

@@ -11,10 +11,7 @@ module T = struct
 
   let best_name = function
     | External pkg -> pkg.name
-    | Internal (_, lib) ->
+    | Internal (_, lib) -> Jbuild.Library.best_name lib
-      match lib.public with
-      | Some p -> p.name
-      | None -> lib.name
 
   let compare a b = String.compare (best_name a) (best_name b)
 end

src/lib_db.ml

@@ -1,12 +1,17 @@
 open Import
 open Jbuild
 
+type scope =
+  { mutable libs : Lib.Internal.t String_map.t
+  ; scope        : Scope.t
+  }
+
 type t =
   { findlib                  : Findlib.t
   ; (* This include both libraries from the current workspace and external ones *)
     by_public_name           : (string, Lib.t) Hashtbl.t
   ; (* This is to implement the scoping described in the manual *)
-    by_internal_name         : (Path.t, Lib.Internal.t String_map.t ref) Hashtbl.t
+    by_internal_name         : (Path.t, scope) Hashtbl.t
   ; (* This is to filter out libraries that are not installable because of missing
        dependencies *)
     instalable_internal_libs : Lib.Internal.t String_map.t
@@ -27,7 +32,7 @@ let rec internal_name_scope t ~dir =
 
 let find_by_internal_name t ~from name =
   let scope = internal_name_scope t ~dir:from in
-  String_map.find name !scope
+  String_map.find name scope.libs
 
 let find_exn t ~from name =
   match find_by_internal_name t ~from name with
@@ -99,7 +104,7 @@ let compute_instalable_internal_libs t ~internal_libraries =
       else
         t)
 
-let create findlib ~dirs_with_dot_opam_files internal_libraries =
+let create findlib ~scopes 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
@@ -116,12 +121,14 @@ let create findlib ~dirs_with_dot_opam_files internal_libraries =
   in
   (* Initializes the scopes, including [Path.root] so that when there are no <pkg>.opam
      files in parent directories, the scope is the whole workspace. *)
-  Path.Set.iter (Path.Set.add Path.root dirs_with_dot_opam_files) ~f:(fun dir ->
+  List.iter scopes ~f:(fun (scope : Scope.t) ->
-    Hashtbl.add t.by_internal_name ~key:dir
+    Hashtbl.add t.by_internal_name ~key:scope.root
-      ~data:(ref String_map.empty));
+      ~data:{ libs = String_map.empty
+            ; scope
+            });
   List.iter internal_libraries ~f:(fun ((dir, lib) as internal) ->
     let scope = internal_name_scope t ~dir in
-    scope := String_map.add !scope ~key:lib.Library.name ~data:internal;
+    scope.libs <- String_map.add scope.libs ~key:lib.Library.name ~data:internal;
     Option.iter lib.public ~f:(fun { name; _ } ->
       match Hashtbl.find t.by_public_name name with
       | None
@@ -190,3 +197,15 @@ let resolve_selects t ~from lib_deps =
         | None -> "no solution found"
       in
       Some { dst_fn = result_fn; src_fn })
+
+let unique_library_name t (lib : Lib.t) =
+  match lib with
+  | External pkg -> pkg.name
+  | Internal (dir, lib) ->
+    match lib.public with
+    | Some x -> x.name
+    | None ->
+      let scope = internal_name_scope t ~dir in
+      match scope.scope.name with
+      | None -> lib.name ^ "@"
+      | Some s -> lib.name ^ "@" ^ s

src/lib_db.mli

@@ -9,7 +9,7 @@ type t
 
 val create
   :  Findlib.t
-  -> dirs_with_dot_opam_files:Path.Set.t
+  -> scopes:Jbuild.Scope.t list
   -> (Path.t * Jbuild.Library.t) list
   -> t
 
@@ -39,3 +39,6 @@ val lib_is_available : t -> from:Path.t -> string -> bool
 
 (** For [Findlib.closure] *)
 val local_public_libs : t -> Path.t String_map.t
+
+(** Unique name, even for internal libraries *)
+val unique_library_name : t -> Lib.t -> string

src/odoc.ml

@@ -19,7 +19,7 @@ let module_deps (m : Module.t) ~dir ~dep_graph ~modules =
          Module.odoc_file m ~dir))
 
 let compile_module sctx (m : Module.t) ~odoc ~dir ~includes ~dep_graph ~modules
-      ~lib_public_name =
+      ~lib_unique_name =
   let context = SC.context sctx in
   let odoc_file = Module.odoc_file m ~dir in
   SC.add_rule sctx
@@ -31,15 +31,15 @@ let compile_module sctx (m : Module.t) ~odoc ~dir ~includes ~dep_graph ~modules
        [ A "compile"
        ; Dyn (fun x -> x)
        ; A "-I"; Path dir
-       ; As ["--pkg"; lib_public_name]
+       ; As ["--pkg"; lib_unique_name]
        ; Dep (Module.cmti_file m ~dir)
        ]);
   (m, odoc_file)
 
 let to_html sctx (m : Module.t) odoc_file ~doc_dir ~odoc ~dir ~includes
-      ~lib_public_name ~(lib : Library.t) =
+      ~lib_unique_name ~(lib : Library.t) =
   let context = SC.context sctx in
-  let html_dir = doc_dir ++ lib_public_name ++ String.capitalize_ascii m.obj_name in
+  let html_dir = doc_dir ++ lib_unique_name ++ String.capitalize_ascii m.obj_name in
   let html_file = html_dir ++ "index.html" in
   SC.add_rule sctx
     (SC.Libs.static_file_deps (dir, lib) ~ext:odoc_ext
@@ -61,7 +61,7 @@ let to_html sctx (m : Module.t) odoc_file ~doc_dir ~odoc ~dir ~includes
     );
   html_file
 
-let lib_index sctx ~odoc ~dir ~(lib : Library.t) ~lib_public_name ~doc_dir ~modules
+let lib_index sctx ~odoc ~dir ~(lib : Library.t) ~lib_name ~lib_unique_name ~doc_dir ~modules
       ~includes =
   let context = SC.context sctx in
   let generated_index_mld = dir ++ sprintf "%s-generated.mld" lib.name in
@@ -78,7 +78,7 @@ let lib_index sctx ~odoc ~dir ~(lib : Library.t) ~lib_public_name ~doc_dir ~modu
                {2 Library %s}\n\
                The entry point for this library is module {!module:%s}."
               header
-              lib_public_name
+              lib_name
               (String.capitalize_ascii lib.name)
           else
             sprintf
@@ -86,12 +86,12 @@ let lib_index sctx ~odoc ~dir ~(lib : Library.t) ~lib_public_name ~doc_dir ~modu
                {2 Library %s}\n\
                This library exposes the following toplevel modules:\n{!modules:%s}"
               header
-              lib_public_name
+              lib_name
               (String_map.keys modules |> String.concat ~sep:" "))))
      >>>
      Build.write_file_dyn generated_index_mld);
   let html_file =
-    doc_dir ++ lib_public_name ++ "index.html"
+    doc_dir ++ lib_unique_name ++ "index.html"
   in
   SC.add_rule sctx
     (SC.Libs.static_file_deps (dir, lib) ~ext:odoc_ext
@@ -103,7 +103,7 @@ let lib_index sctx ~odoc ~dir ~(lib : Library.t) ~lib_public_name ~doc_dir ~modu
        ; Dyn (fun x -> x)
        ; A "-I"; Path dir
        ; A "-o"; Path doc_dir
-       ; A "--index-for"; A lib_public_name
+       ; A "--index-for"; A lib_unique_name
        ; Dep generated_index_mld
        ]);
   html_file
@@ -116,7 +116,8 @@ let toplevel_index ~doc_dir = doc_dir ++ "index.html"
 
 let setup_library_rules sctx (lib : Library.t) ~dir ~modules ~requires
       ~(dep_graph:Ocamldep.dep_graph) =
-  let name = Option.value (Option.map ~f:(fun x -> x.name) lib.public) ~default:lib.name in
+  let lib_unique_name = SC.unique_library_name sctx (Internal (dir, lib)) in
+  let lib_name = Library.best_name lib in
   let context = SC.context sctx in
   let dep_graph =
     Build.memoize "odoc deps"
@@ -141,7 +142,7 @@ let setup_library_rules sctx (lib : Library.t) ~dir ~modules ~requires
   let modules_and_odoc_files =
     List.map (String_map.values modules)
       ~f:(compile_module sctx ~odoc ~dir ~includes ~dep_graph ~modules
-            ~lib_public_name:name)
+            ~lib_unique_name)
   in
   SC.Libs.setup_file_deps_alias sctx ~ext:odoc_ext (dir, lib)
     (List.map modules_and_odoc_files ~f:snd);
@@ -158,10 +159,10 @@ let setup_library_rules sctx (lib : Library.t) ~dir ~modules ~requires
   let html_files =
     List.map modules_and_odoc_files ~f:(fun (m, odoc_file) ->
       to_html sctx m odoc_file ~doc_dir ~odoc ~dir ~includes ~lib
-        ~lib_public_name:name)
+        ~lib_unique_name)
   in
   let lib_index_html =
-    lib_index sctx ~dir ~lib ~lib_public_name:name ~doc_dir
+    lib_index sctx ~dir ~lib ~lib_unique_name ~lib_name ~doc_dir
       ~modules ~includes ~odoc
   in
   Alias.add_deps (SC.aliases sctx) (Alias.doc ~dir)

src/super_context.ml

@@ -88,7 +88,7 @@ let resolve_program t ?hint bin =
 let create
       ~(context:Context.t)
       ~aliases
-      ~dirs_with_dot_opam_files
+      ~scopes
       ~file_tree
       ~packages
       ~stanzas
@@ -111,14 +111,13 @@ let create
         | Library lib -> Some (ctx_dir, lib)
         | _ -> None))
   in
-  let dirs_with_dot_opam_files =
-    Pset.elements dirs_with_dot_opam_files
-    |> List.map ~f:(Path.append context.build_dir)
-    |> Pset.of_list
-  in
   let libs =
+    let scopes =
+      List.map scopes ~f:(fun scope ->
+        { scope with Scope.root = Path.append context.build_dir scope.Scope.root })
+    in
     Lib_db.create context.findlib internal_libraries
-      ~dirs_with_dot_opam_files
+      ~scopes
   in
   let stanzas_to_consider_for_install =
     if filter_out_optional_stanzas_with_missing_deps then
@@ -243,6 +242,8 @@ let sources_and_targets_known_so_far t ~src_path =
   | None -> sources
   | Some set -> String_set.union sources set
 
+let unique_library_name t lib =
+  Lib_db.unique_library_name t.libs lib
 
 module Libs = struct
   open Build.O

src/super_context.mli

@@ -23,7 +23,7 @@ type t
 val create
   :  context:Context.t
   -> aliases:Alias.Store.t
-  -> dirs_with_dot_opam_files:Path.Set.t
+  -> scopes:Scope.t list
   -> file_tree:File_tree.t
   -> packages:Package.t String_map.t
   -> stanzas:(Path.t * Scope.t * Stanzas.t) list
@@ -71,6 +71,9 @@ val resolve_program
   -> string
   -> Action.Prog.t
 
+(** Unique name, even for internal libraries *)
+val unique_library_name : t -> Lib.t -> string
+
 module Libs : sig
   val find : t -> from:Path.t -> string -> Lib.t option
 

test/blackbox-tests/test-cases/multiple-private-libs/run.t

@@ -1,5 +1,16 @@
 This test checks that there is no clash when two private libraries have the same name
 
   $ $JBUILDER build -j1 --root . @doc
-  Multiple rules generated for _build/default/_doc/test/index.html
+          odoc _doc/odoc.css
-  [1]
+      ocamldep a/test.depends.ocamldep-output
+      ocamldep a/test.dependsi.ocamldep-output
+      ocamldep b/test.depends.ocamldep-output
+      ocamldep b/test.dependsi.ocamldep-output
+        ocamlc a/test.{cmi,cmo,cmt}
+        ocamlc b/test.{cmi,cmo,cmt}
+          odoc a/test.odoc
+          odoc b/test.odoc
+          odoc _doc/test@a/index.html
+          odoc _doc/test@a/Test/.jbuilder-keep,_doc/test@a/Test/index.html
+          odoc _doc/test@b/index.html
+          odoc _doc/test@b/Test/.jbuilder-keep,_doc/test@b/Test/index.html