diff --git a/CHANGES.md b/CHANGES.md index 5a693673..5de1da6e 100644 --- a/CHANGES.md +++ b/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) ----------------------- diff --git a/Makefile b/Makefile index 79a7073f..8e10e071 100644 --- a/Makefile +++ b/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 diff --git a/src/gen_rules.ml b/src/gen_rules.ml index a83e547e..a8524a2c 100644 --- a/src/gen_rules.ml +++ b/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 diff --git a/src/jbuild.ml b/src/jbuild.ml index 9c186cc2..9b77817e 100644 --- a/src/jbuild.ml +++ b/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 diff --git a/src/jbuild.mli b/src/jbuild.mli index 171a1850..b905652d 100644 --- a/src/jbuild.mli +++ b/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 diff --git a/src/jbuild_load.ml b/src/jbuild_load.ml index c0622e7a..6950f1b7 100644 --- a/src/jbuild_load.ml +++ b/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 } diff --git a/src/jbuild_load.mli b/src/jbuild_load.mli index 67998263..d88915ff 100644 --- a/src/jbuild_load.mli +++ b/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 diff --git a/src/lib.ml b/src/lib.ml index a546ce11..6c768eed 100644 --- a/src/lib.ml +++ b/src/lib.ml @@ -11,10 +11,7 @@ module T = struct let best_name = function | External pkg -> pkg.name - | Internal (_, lib) -> - match lib.public with - | Some p -> p.name - | None -> lib.name + | Internal (_, lib) -> Jbuild.Library.best_name lib let compare a b = String.compare (best_name a) (best_name b) end diff --git a/src/lib_db.ml b/src/lib_db.ml index 2c09397c..cbf49993 100644 --- a/src/lib_db.ml +++ b/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 .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 -> - Hashtbl.add t.by_internal_name ~key:dir - ~data:(ref String_map.empty)); + List.iter scopes ~f:(fun (scope : Scope.t) -> + Hashtbl.add t.by_internal_name ~key:scope.root + ~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 diff --git a/src/lib_db.mli b/src/lib_db.mli index 4b430bfa..eefdeb99 100644 --- a/src/lib_db.mli +++ b/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 diff --git a/src/odoc.ml b/src/odoc.ml index abff2e3e..42756eb0 100644 --- a/src/odoc.ml +++ b/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) diff --git a/src/super_context.ml b/src/super_context.ml index 678b8aa2..dace6b45 100644 --- a/src/super_context.ml +++ b/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 diff --git a/src/super_context.mli b/src/super_context.mli index 33747edd..8256d4eb 100644 --- a/src/super_context.mli +++ b/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 diff --git a/test/blackbox-tests/test-cases/multiple-private-libs/run.t b/test/blackbox-tests/test-cases/multiple-private-libs/run.t index 2432defd..8391b310 100644 --- a/test/blackbox-tests/test-cases/multiple-private-libs/run.t +++ b/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 - [1] + odoc _doc/odoc.css + 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