Docs CI compatibility #1208

jonludlam · 2024-10-08T11:10:37Z

This PR contains the contents of #1192 as well as a few more bugfixes and improvements required to ensure we can use odoc_driver in place of voodoo for building the docs for ocaml.org.

Best reviewed commit-by-commit.

panglesd

First pass of review! (only very minor comments)

panglesd · 2024-10-09T07:48:36Z

src/odoc/classify.cppo.ml

+    let eq (l1 : t) (l2 : t) =
+      List.for_all
+        (fun (x, deps) ->
+          try
+            let deps' = List.assoc x l2 in
+            StringSet.equal deps deps'
+          with Not_found -> false)
+        l1
+    in


Isn't that $l_1\subseteq l_2$ instead of equality?

Yes; but the keys never actually change, only the values, so it's OK here.

panglesd · 2024-10-09T07:50:37Z

src/odoc/bin/main.ml

@@ -688,6 +688,7 @@ end = struct
          current_dir;
        }
    in
+    let directories = directories @ List.map ~f:snd lib_roots in


Maybe this require an update in the -L entry of the --help page?

Actually, the help already states:

Specifies a library called libname containing the modules in directory
DIR. Modules can be referenced both using the flat module namespace
{!Module} and the absolute reference {!/libname/Module}.

so I don't think it needs updating.

Should the -I be declared as deprecated in its doc ?

panglesd · 2024-10-09T08:42:02Z

src/driver/odoc_unit.ml

+                h lib.modules
+            in
+            let lib_dir = lib_dir pkg lib.lib_name in
+            let lds' = Util.StringMap.add lib.lib_name lib_dir lds in


Can it happen that two different packages provide a library with the same name?

Should we detect that and print an error?

I don't think so - at least not in 'normal' mode as these are the public library names, so if there's a clash you can't actually link. In 'dune-style' mode they can be internal names, but once again I think dune would complain about this before it got to us.

panglesd · 2024-10-09T08:58:00Z

src/driver/run.ml

  in
  (* Logs.debug (fun m ->
      m "Finished running cmd %a" Fmt.(list ~sep:sp string) cmd); *)
  let t_end = Unix.gettimeofday () in
  let r = String.split_on_char '\n' r in
  let time = t_end -. t_start in
-  let errors = Buffer.contents stderr in
+  let errors = "" in


We should remove the error field from the executed_command type, as it is not used anymore.
Or, return it alongside r and add it to the field!

Ah yes, need to do something there.

panglesd · 2024-10-09T13:47:14Z

src/driver/voodoo.ml

        Logs.debug (fun m ->
            m "Processing directory without META: %a" Fpath.pp libdir);


With those List.map with a very long function I find it not always easy to see what list we are manipulating.
Either putting the debug log at the beginning (or a comment), or using libdirs_without_meta |> List.map (fun libdir -> namy lines) ?

Yeah that's fair, I'll try and make it a bit more legible.

Right, done some tidying up. Is it easier to follow now?

Yes, much better, thanks!

jonludlam · 2024-10-22T15:05:29Z

src/driver/odoc_unit.ml

    in
    i :: m
  in
  let of_lib pkg (lib : Packages.libty) : [ impl | intf ] unit list =
-    List.concat_map (of_module pkg lib.lib_name) lib.modules
+    let lib_deps = Util.StringSet.add lib.lib_name lib.lib_deps in
+    List.concat_map (of_module pkg lib.lib_name lib_deps) lib.modules


Are we adding our own lib twice? See line 165 above?

panglesd

I made both a "cosmetic" review and a deeper one with @jonludlam, and that looks all good to me! The modification is well structured in commits with informative details.

(Thanks, I always learn many things with those PRs. This time, --no-alias-deps, the ocamlfind search path one included in the other (<switchroot>/lib/ocaml and <switchroot>/lib), some packaging quirks of ocaml, ...)

Julow

Looks good to me :)

Julow · 2024-10-29T10:18:17Z

src/odoc/classify.cppo.ml

+            let deps' = List.assoc x l2 in
+            StringSet.equal deps deps'
+          with Not_found -> false)
+        l1


This seems like a very bad algorithm. What about making t a map ?

Julow · 2024-10-29T10:20:35Z

src/odoc/bin/main.ml

@@ -688,6 +688,7 @@ end = struct
          current_dir;
        }
    in
+    let directories = directories @ List.map ~f:snd lib_roots in


Should the -I be declared as deprecated in its doc ?

Julow · 2024-10-29T10:31:47Z

src/driver/odoc_unit.ml

+    let odoc_file = odoc_dir / (String.uncapitalize_ascii name ^ ".odoc") in
+    (* odoc will uncapitalise the output filename *)
+    let odocl_file =
+      linked_dir // rel_dir / (String.uncapitalize_ascii name ^ ".odocl")


I'm not sure this is a reasonable things for Odoc to do, as capitalization rules became more complex in ocaml/ocaml#12664.
Though, it's reasonable to do this in the driver as long as Odoc does this.

Mmm, odoc is currently doing it, which is why the driver needed fixing here. I'm not 100% convinced it's the best way, but I'd rather fix that in a separate PR, if necessary.

src/driver/odoc_unit.ml

If there's a failure, we simply don't highlight at all. This can happen when non-preprocessed files are installed, for example.

The polymorphic equality wasn't working correctly.

In some rare circumstances this was causing an infinite loop in CI

So we don't have to have a `-I` argument for each `-L` arg.

We need to know from an external source what the linking deps are for a library. One way to get this info is from the META files when we're dealing with installed libraries.

Prior to this commit the link phase was only using the include paths coming from `compile-deps` on the unit being linked. The fix is to use the library dependencies coming from an external source - META files or dune metadata. We can't simply use the union of the compile-deps paths from all units as we might still miss some dependencies due to `--no-alias-deps`.

Log it when the command has failed.

This works by finding META files and querying them for the dependencies.

odoc uncapitalises some output filenames, we need to do the same in the driver.

For debugging.

The docs-ci pipeline requires this.

This was just broken before

The HTML output is easier for browsing, but the consumers of this will likely want the json files. Output both.

This is for where partitioning of modules into libraries isn't so straightforward. A good example of this is 'ocamlmiddleend' which contains many of the same modules as 'ocamloptcomp'. Rather than trying to assign a module to one or other of these libraries, instead we're allowing duplication, and so the docs for that module will appear in _both_ libraries.

The name of an archive is not necessarily unique. The path+name definitely is!

This is in order to find the dependencies between libraries.

Virtual libraries are implemented by effectively just separating where the mli and ml files are found. We have one mli and multiple ml files. Before this commit, the mli files would be ignored as we use archives to define libraries. Then each implemention of the virtual library would use the cmt from the ml file, meaning that we exposed all the internals of the library that aren't actually accessible. After this commit, we now document mli-only libraries, and we effectively copy the cmti files into the implementation libraries and use them rather than the cmts. This means that every implementation will have docs that look identical to the main virtual library. However, since the cmts are different, the source links will go to the correct place in the implementation.

jonludlam · 2024-10-29T15:50:42Z

Thanks all

@jonludlam

CHANGES: ### Highlight - Hierarchical documentation (@jonludlam, @panglesd, @Julow) Pages can now be organized in a directory tree structure. Relative and absolute references are added: `{!./other_page.label}`, `{!//other_page}`. - Improved sidebar and breadcrumbs navigation (@panglesd, @gpetiot) The documentation pages and the libraries of the entire package are shown on the left sidebar. - Added support for images, videos, audio and other assets The syntax is `{image!/reference/to/asset}` or `{image:URL}` for images. The syntax for `{video...}` and `{audio...}` is the same. (@panglesd, @EmileTrotignon, ocaml/odoc#1170, ocaml/odoc#1171, ocaml/odoc#1184, ocaml/odoc#1185) - Search using Sherlodoc (@panglesd, @EmileTrotignon, @Julow) A new search bar that supports full-text and type-based search. ### Added - Experimental driver (@jonludlam, @panglesd) The driver builds the documentation for a collection of Opam packages using the newer Odoc features. It supports linking external packages to ocaml.org and markdown files. This is experimental and will break in the future. - Cross-package references (@panglesd, @Julow) Pages and modules from other packages can be referenced: `{!/otherpackage/page}`, `{!/otherpackage/Module.t}`. - Option to remap links to other packages to ocaml.org or other site. See the `--remap` option of the driver or the `--remap-file` option of `odoc html-generate`. (@jonludlam, ocaml/odoc#1189, ocaml/odoc#1248) - Option to compute occurrences of use of each identifiers The commands `aggregate-occurrences` and `count-occurrences` are added. (@panglesd, ocaml/odoc#976, ocaml/odoc#1076, ocaml/odoc#1206) - Added the `odoc classify` command (@jonludlam, ocaml/odoc#1121) Helps driver detecting which modules belong to which libraries. - Added `--suppress-warnings` to the CLI to remove warnings from a unit, even if they end up being raised in another unit through expansion (@jonludlam, ocaml/odoc#1260) - Add clock emoji before `@since` tag (@yawaramin, ocaml/odoc#1089) - Navigation for the search bar : use '/' to enter search, up and down arrows to select a result, and enter to follow the selected link. (@EmileTrotignon, ocaml/odoc#1088) - Fix a big gap between the preamble and the content of a page (@EmileTrotignon, ocaml/odoc#1147) - Add a marshalled search index consumable by sherlodoc (@EmileTrotignon, @panglesd, ocaml/odoc#1084) - Allow referencing of polymorphic constructors in polymorphic variant type aliases (@panglesd, ocaml/odoc#1115) - Added a home icon in the breacrumbs (@panglesd, ocaml/odoc#1251) It can be disabled with a CLI option. - Add a frontmatter syntax for mld pages (@panglesd, ocaml/odoc#1187, ocaml/odoc#1193, ocaml/odoc#1243, ocaml/odoc#1246, ocaml/odoc#1251) Allows to specify the title of a page, the order of sub-pages and other behaviors in the sidebar. - Added `odoc-md` to process standalone Markdown pages (@jonludlam, ocaml/odoc#1234) ### Changed - The command line interface changed to support the new features. + Packages and libraries: `odoc link` must now be aware of packages and libraries with the `-L libname:path` and `-P pkgname:path` options. The module search path should still be passed with the `-I` option. The current package should be specified with `--current-package=pkgname`. + Hierarchy: `odoc compile` now outputs `.odoc` in the directory tree specified with `--output-dir=DIR` and the parent identifier must be specified with `--parent-id=PARENT`. The option `--source-parent-file` is removed. + Source code: Implementations are compiled with `compile-impl` instead of with `compile`. The options `--cmt=..` and `--source-name=..` are removed. Source code pages are generated with `html-generate-source`. + Assets: The commands `compile-asset`, `html-generate-asset` are added. The option `html-generate --asset` is removed. + Sidebar: The index is built using `compile-index`. The sidebar data is extracted from the index with `sidebar-generate` and passed to `html-generate --sidebar=..`. - The syntax for `@tag` is now delimited (@panglesd, ocaml/odoc#1239) A `@tag` can now be followed by a paragraph or other elements. - Updated colors for code fragments (@EmileTrotignon, ocaml/odoc#1023) - Fixed complexity of looking up `.odoc` files (@panglesd, ocaml/odoc#1075) - Normalize whitespaces in codespans (@gpetiot, ocaml/odoc#1085) A newline followed by any whitespaces is normalized as one space character. - Reduce size of `Odoc_html_frontend` when compiled to javascript (@EmileTrotignon, ocaml/odoc#1072) - Overhaul of module-type-of expansions and shadowing code (@jonludlam, ocaml/odoc#1081) - Output file paths and labels in the man and latex backends changed to avoid name clashes (@Julow, ocaml/odoc#1191) ### Fixed - Fix variant constructors being hidden if they contain hidden types (@jonludlam, ocaml/odoc#1105) - Fix rare assertion failure due to optional parameters (@jonludlam, ocaml/odoc#1272, issue ocaml/odoc#1001) - Fix resolution of module synopses in {!modules} lists that require --open (@jonludlam, ocaml/odoc#1104} - Fix top comment not being taken from includes often enough (@panglesd, ocaml/odoc#1117) - Fixed 404 links from search results (@panglesd, ocaml/odoc#1108) - Fixed title content not being picked up across pages when rendering references (ocaml/odoc#1116, @panglesd) - Fix wrong links to standalone comments in search results (ocaml/odoc#1118, @panglesd) - Remove duplicated or unwanted comments with inline includes (@Julow, ocaml/odoc#1133) - Fix bug where source rendering would cause odoc to fail completely if it encounters invalid syntax (@jonludlam ocaml/odoc#1208) - Add missing parentheses in 'val (let*) : ...' (@Julow, ocaml/odoc#1268) - Fix syntax highlighting not working for very large files (@jonludlam, @Julow, ocaml/odoc#1277)

@jonludlam

CHANGES: - Hierarchical documentation (@jonludlam, @panglesd, @Julow) Pages can now be organized in a directory tree structure. Relative and absolute references are added: `{!./other_page.label}`, `{!//other_page}`. - Improved sidebar and breadcrumbs navigation (@panglesd, @gpetiot) The documentation pages and the libraries of the entire package are shown on the left sidebar. - Added support for images, videos, audio and other assets The syntax is `{image!/reference/to/asset}` or `{image:URL}` for images. The syntax for `{video...}` and `{audio...}` is the same. (@panglesd, @EmileTrotignon, ocaml/odoc#1170, ocaml/odoc#1171, ocaml/odoc#1184, ocaml/odoc#1185) - Search using Sherlodoc (@panglesd, @EmileTrotignon, @Julow) A new search bar that supports full-text and type-based search. - Experimental driver (@jonludlam, @panglesd) The driver builds the documentation for a collection of Opam packages using the newer Odoc features. It supports linking external packages to ocaml.org and markdown files. This is experimental and will break in the future. - Cross-package references (@panglesd, @Julow) Pages and modules from other packages can be referenced: `{!/otherpackage/page}`, `{!/otherpackage/Module.t}`. - Option to remap links to other packages to ocaml.org or other site. See the `--remap` option of the driver or the `--remap-file` option of `odoc html-generate`. (@jonludlam, ocaml/odoc#1189, ocaml/odoc#1248) - Option to compute occurrences of use of each identifiers The commands `aggregate-occurrences` and `count-occurrences` are added. (@panglesd, ocaml/odoc#976, ocaml/odoc#1076, ocaml/odoc#1206) - Added the `odoc classify` command (@jonludlam, ocaml/odoc#1121) Helps driver detecting which modules belong to which libraries. - Added `--suppress-warnings` to the CLI to remove warnings from a unit, even if they end up being raised in another unit through expansion (@jonludlam, ocaml/odoc#1260) - Add clock emoji before `@since` tag (@yawaramin, ocaml/odoc#1089) - Navigation for the search bar : use '/' to enter search, up and down arrows to select a result, and enter to follow the selected link. (@EmileTrotignon, ocaml/odoc#1088) - Fix a big gap between the preamble and the content of a page (@EmileTrotignon, ocaml/odoc#1147) - Add a marshalled search index consumable by sherlodoc (@EmileTrotignon, @panglesd, ocaml/odoc#1084) - Allow referencing of polymorphic constructors in polymorphic variant type aliases (@panglesd, ocaml/odoc#1115) - Added a home icon in the breacrumbs (@panglesd, ocaml/odoc#1251) It can be disabled with a CLI option. - Add a frontmatter syntax for mld pages (@panglesd, ocaml/odoc#1187, ocaml/odoc#1193, ocaml/odoc#1243, ocaml/odoc#1246, ocaml/odoc#1251) Allows to specify the title of a page, the order of sub-pages and other behaviors in the sidebar. - Added `odoc-md` to process standalone Markdown pages (@jonludlam, ocaml/odoc#1234) - The command line interface changed to support the new features. + Packages and libraries: `odoc link` must now be aware of packages and libraries with the `-L libname:path` and `-P pkgname:path` options. The module search path should still be passed with the `-I` option. The current package should be specified with `--current-package=pkgname`. + Hierarchy: `odoc compile` now outputs `.odoc` in the directory tree specified with `--output-dir=DIR` and the parent identifier must be specified with `--parent-id=PARENT`. The option `--source-parent-file` is removed. + Source code: Implementations are compiled with `compile-impl` instead of with `compile`. The options `--cmt=..` and `--source-name=..` are removed. Source code pages are generated with `html-generate-source`. + Assets: The commands `compile-asset`, `html-generate-asset` are added. The option `html-generate --asset` is removed. + Sidebar: The index is built using `compile-index`. The sidebar data is extracted from the index with `sidebar-generate` and passed to `html-generate --sidebar=..`. - The syntax for `@tag` is now delimited (@panglesd, ocaml/odoc#1239) A `@tag` can now be followed by a paragraph or other elements. - Updated colors for code fragments (@EmileTrotignon, ocaml/odoc#1023) - Fixed complexity of looking up `.odoc` files (@panglesd, ocaml/odoc#1075) - Normalize whitespaces in codespans (@gpetiot, ocaml/odoc#1085) A newline followed by any whitespaces is normalized as one space character. - Reduce size of `Odoc_html_frontend` when compiled to javascript (@EmileTrotignon, ocaml/odoc#1072) - Overhaul of module-type-of expansions and shadowing code (@jonludlam, ocaml/odoc#1081) - Output file paths and labels in the man and latex backends changed to avoid name clashes (@Julow, ocaml/odoc#1191) - Fix variant constructors being hidden if they contain hidden types (@jonludlam, ocaml/odoc#1105) - Fix rare assertion failure due to optional parameters (@jonludlam, ocaml/odoc#1272, issue ocaml/odoc#1001) - Fix resolution of module synopses in {!modules} lists that require --open (@jonludlam, ocaml/odoc#1104} - Fix top comment not being taken from includes often enough (@panglesd, ocaml/odoc#1117) - Fixed 404 links from search results (@panglesd, ocaml/odoc#1108) - Fixed title content not being picked up across pages when rendering references (ocaml/odoc#1116, @panglesd) - Fix wrong links to standalone comments in search results (ocaml/odoc#1118, @panglesd) - Remove duplicated or unwanted comments with inline includes (@Julow, ocaml/odoc#1133) - Fix bug where source rendering would cause odoc to fail completely if it encounters invalid syntax (@jonludlam ocaml/odoc#1208) - Add missing parentheses in 'val (let*) : ...' (@Julow, ocaml/odoc#1268) - Fix syntax highlighting not working for very large files (@jonludlam, @Julow, ocaml/odoc#1277)

jonludlam mentioned this pull request Oct 8, 2024

Careful library deps #1192

Closed

jonludlam added no changelog This pull request does not need a changelog entry and removed no changelog This pull request does not need a changelog entry labels Oct 8, 2024

panglesd reviewed Oct 9, 2024

View reviewed changes

jonludlam commented Oct 22, 2024

View reviewed changes

panglesd approved these changes Oct 28, 2024

View reviewed changes

Julow approved these changes Oct 29, 2024

View reviewed changes

jonludlam added 21 commits October 29, 2024 12:01

Don't fail if the syntax highlighter throws an exception

e5a19f8

If there's a failure, we simply don't highlight at all. This can happen when non-preprocessed files are installed, for example.

Fix classify

1b07272

The polymorphic equality wasn't working correctly.

Normalize path in mkdir_p

3e7e47d

In some rare circumstances this was causing an infinite loop in CI

Add -L dirs to include path when linking

d8998be

So we don't have to have a `-I` argument for each `-L` arg.

Driver: Extract library dependencies from META files

d03b1a4

We need to know from an external source what the linking deps are for a library. One way to get this info is from the META files when we're dealing with installed libraries.

Driver: Capture stderr when executing commands

517b08e

Log it when the command has failed.

Driver: Compute deps in voodoo mode

5c4b736

This works by finding META files and querying them for the dependencies.

Driver: Be more careful with capitalisation

7618aaa

odoc uncapitalises some output filenames, we need to do the same in the driver.

Driver: Pretty-printers

5c61d82

For debugging.

Driver: Support linked and compiled output in different dirs

1e26c6a

The docs-ci pipeline requires this.

Driver: Fix check for non-META dirs

e6ece52

This was just broken before

Driver: Output as-json as well as normal html

3bd6d15

The HTML output is easier for browsing, but the consumers of this will likely want the json files. Output both.

Driver: Better handling of duplicate archive names

68400da

The name of an archive is not necessarily unique. The path+name definitely is!

Driver: Use dune describe in dune-style driver

9f6d824

This is in order to find the dependencies between libraries.

Add CHANGES entry

dced88b

Fixes following review

c6f805c

Formatting

1ce6755

Driver: Slightly nicer pkg_args

a25c878

jonludlam force-pushed the odoc-driver-errors4 branch from 07212e8 to a25c878 Compare October 29, 2024 14:31

jonludlam merged commit c0f4c89 into ocaml:master Oct 29, 2024
13 checks passed

jonludlam mentioned this pull request Jan 23, 2025

[new release] odoc (5 packages) (3.0.0~beta1) ocaml/opam-repository#27309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docs CI compatibility #1208

Docs CI compatibility #1208

jonludlam commented Oct 8, 2024

panglesd left a comment

panglesd Oct 9, 2024

jonludlam Oct 9, 2024

panglesd Oct 9, 2024

jonludlam Oct 9, 2024

jonludlam Oct 9, 2024

Julow Oct 29, 2024

panglesd Oct 9, 2024

jonludlam Oct 9, 2024

panglesd Oct 9, 2024

jonludlam Oct 9, 2024

panglesd Oct 9, 2024

jonludlam Oct 9, 2024

jonludlam Oct 9, 2024

panglesd Oct 10, 2024

jonludlam Oct 22, 2024

panglesd left a comment •

edited

Loading

Julow left a comment

Julow Oct 29, 2024

Julow Oct 29, 2024

Julow Oct 29, 2024

jonludlam Oct 29, 2024

jonludlam commented Oct 29, 2024

		Logs.debug (fun m ->
		m "Processing directory without META: %a" Fpath.pp libdir);

Docs CI compatibility #1208

Docs CI compatibility #1208

Conversation

jonludlam commented Oct 8, 2024

panglesd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

panglesd left a comment • edited Loading

Choose a reason for hiding this comment

Julow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonludlam commented Oct 29, 2024

panglesd left a comment •

edited

Loading