New odoc rules #8803

jonludlam · 2023-09-30T13:31:59Z

This supercedes PR #7840.

This is a new set of rules that lives alongside the original odoc rules. At present it builds docs for your packages, their libraries, your private libraries and all of their dependencies. I've tested this on dune itself, ocaml.org, odoc and tezos-shell, though because this is a new target, we can and should view this as a bit experimental for a bit.

It's invoked with dune build @doc-new and the results are index by the main index page at _build/default/_doc_new/html/docs/index.html

jonludlam · 2023-09-30T13:33:01Z

Almost 1000 PRs/issues in between this and the previous incarnation!

jonludlam · 2023-09-30T14:25:37Z

The test failures are interesting:

@@ -114,7 +114,7 @@ true
 let%expect_test _ =
   is_descendant (e "/foo/bar") ~of_:(e "/foo");
   [%expect {|
-false
+true
 |}]
 ;;
 
@@ -135,14 +135,14 @@ false
 let%expect_test _ =
   is_descendant (e "/foo/bar/") ~of_:(e "/foo/bar");
   [%expect {|
-false
+true
 |}]
 ;;
 
 let%expect_test _ =
   is_descendant (e "/foo/bar") ~of_:(e "/");
   [%expect {|
-false
+true
 |}]
 ;;

On the surface it seems the new results are an improvement - but I assume there's a good reason that these tests are there?

rgrinberg · 2023-10-01T13:32:02Z

otherlibs/stdune/src/path.ml

@@ -862,6 +862,12 @@ let external_of_in_source_tree x = external_of_local x ~root:(Lazy.force abs_roo

 let reach t ~from =
  match t, from with
+  | External t, External from ->
+    (match
+       String.drop_prefix ~prefix:(External.to_string from ^ "/") (External.to_string t)


Why is this needed? Can't you pass external paths as absolute paths?

It's for Odoc_new.local_path_of_findlib_path - there's a comment explaining what it does. Is there a better way to do that?

We have Path.drop_prefix which might be more sensible. It also gives a Local.Path.t which I believe is what you actually want. I'm still a bit confused about the Odoc_new.local_path_of_findlib_path function however. It seems everywhere you use it, you split on / and then take the head, so I think there is some other property you are trying to extract here.

Ah that's probably better! This evolved over time, so I'm not at all surprised to find I'm finding a local rather than global minimum :-)

and I think with that I can drop the reach changes. Thanks!

I don't - you're spot on, I really want the path prefix, and getting back a Path.Local.t would be great. Sorry, I wasn't clear above. The above is what Path.drop_prefix is giving me, not what I actually want - ie, I think Path.drop_prefix is broken. For the above, what I'd like is to return the local path representing ocaml-compiler-libs/shadow.

OK, let me try to repro in a unit test.

Or actually maybe intended. I guess what we really need is a Path.drop_path_prefix function which doesn't exist.

I've created a test #8870 so that we can discuss this behaviour there.

rgrinberg · 2023-10-02T03:24:50Z

src/dune_rules/odoc_new.ml

+  ;;
+end
+
+let contains_double_underscore s =


Which packages require this hack to work?

Dream is one - the sub libraries are named with double underscores.

Oh, and eio too: ocaml/odoc#1013

Alizter · 2023-10-06T15:43:19Z

src/dune_rules/odoc_new.ml

+      let* findlib_paths = Context.findlib_paths ctx in
+      let+ dwms = Valid.filter_dune_with_modules ctx ~all dwms in


You can combine this into a single let+ and+.

Alizter · 2023-10-06T16:08:54Z

src/dune_rules/odoc_new.ml

+      in
+      String.split ~on:'/' local_full
+    in
+    let init = [ Package (ty, List.hd local) ] in


This won't work in general. Path.explode will do what you want here. For that you will need to modify what local_path_of_findlib returns. Windows for example does not have the same separators.

But see my other comments as I am dubious to the utility of the "local_path_of_findlib_path" function.

jonludlam · 2023-10-18T22:37:21Z

OK, the dodgy Odoc_new.local_path_of_findlib_path has gone, replaced by a new type External_location in Dune_package that is effectively the split I wanted. This was good because it highlighted something I hadn't considered - that with a system switch the stdlib and associated libs are not necessarily in a findlib dir.

Secondly, @rgrinberg pointed out that there might potentially be a directory name clash for some nasty cases, so added a layer of hierarchy to the output to ensure this can never happen.

Thirdly, this prompted me to push through with some more logical simplifications -- essentially to structure the code more around libraries than packages, meaning mostly that I'm more treating things on a per-directory basis than a per-package-tree-of-directories basis.

Sorry it took some time, it ended up being a bit of a big change.

Alizter · 2023-10-19T17:20:05Z

FTR Path.drop_prefix was buggy and a fix can be found here:

fix(stdune): make sure Path.drop_prefix drops path prefixes only #8965

Sorry about closing the PR I misclicked.

jonludlam · 2023-10-21T09:17:24Z

I think this is now ready for review again.

Signed-off-by: Jon Ludlam <jon@recoil.org>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

It fell off during a refactor. Signed-off-by: Jon Ludlam <jon@recoil.org>

Signed-off-by: Jon Ludlam <jon@recoil.org>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

Signed-off-by: Jon Ludlam <jon@recoil.org>

- Switch to a new layout, with subdirs per findlib dir, one for stdlib and another for local packages. This is to avoid any potential clashes in names of directories. - Switch to a per-library (or per-directory) treatment rather than basing the behaviour on the whole package tree. E.g. treating something as fallback is now based only on the contents of the directory itself rather than any subdir in a package causing the whole package tree to be treated in the fallback way. Signed-off-by: Jon Ludlam <jon@recoil.org>

Signed-off-by: Jon Ludlam <jon@recoil.org>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg · 2023-11-16T02:16:00Z

Great Work. Sorry for all the delays.

@emillon

CHANGES: - Introduce new experimental odoc rules (ocaml/dune#8803, @jonjudlam) - Cherry-pick ocaml/dune#9177 and ocaml/dune#9201 (@emillon)

@EmileTrotignon

CHANGES: - Introduce `$ dune ocaml doc` to open and browse documentation. (ocaml/dune#7262, fixes ocaml/dune#6831, @EmileTrotignon) - `dune cache trim` now accepts binary byte units: `KiB`, `MiB`, etc. (ocaml/dune#8618, @Alizter) - No longer force colors for OCaml 4.03 and 4.04 (ocaml/dune#8778, @rgrinberg) - Introduce new experimental odoc rules (ocaml/dune#8803, @jonjudlam) - Introduce the `runtest_alias` field to the `cram` stanza. This allows removing default `runtest` alias from tests. (@rgrinberg, ocaml/dune#8887) - Do not ignore libraries named `bigarray` when they are defined in conjunction with OCaml 5.0 (ocaml/dune#8902, fixes ocaml/dune#8901, @rgrinberg) - Dependencies in the copying sandbox are now writeable (ocaml/dune#8920, @rgrinberg) - Absent packages shouldn't prevent all rules from being loaded (ocaml/dune#8948, fixes ocaml/dune#8630, @rgrinberg) - Correctly determine the stanza of menhir modules when `(include_subdirs qualified)` is enabled (@rgrinberg, ocaml/dune#8949, fixes ocaml/dune#7610) - Display cache location in Dune log (ocaml/dune#8974, @nojb) - Re-run actions whenever `(expand_aliases_in_sandbox)` changes (ocaml/dune#8990, @rgrinberg) - Rules that only use internal dune actions (`write-file`, `echo`, etc.) can now be sandboxed. (ocaml/dune#9041, fixes ocaml/dune#8854, @rgrinberg) - Do not re-run rules when their location changes (ocaml/dune#9052, @rgrinberg) - Correctly ignore `bigarray` on recent version of OCaml (ocaml/dune#9076, @rgrinberg) - Add `test_` prefix to default test name in `dune init project` (ocaml/dune#9257, fixes ocaml/dune#9131, @9sako6) - Add `coqdoc_flags` field to `coq` field of `env` stanza allowing the setting of workspace-wide defaults for `coqdoc_flags`. (ocaml/dune#9280, fixes ocaml/dune#9139, @Alizter) - [coq rules] Be more tolerant when coqc --print-version / --config don't work properly, and fallback to a reasonable default. This fixes problems when building Coq projects with `(stdlib no)` and likely other cases. (ocaml/dune#8966, fix ocaml/dune#8958, @Alizter, reported by Lasse Blaauwbroek) - Dune will now run at a lower framerate of 15 fps rather than 60 when `INSIDE_EMACS`. (ocaml/dune#8812, @Alizter) - dune-build-info: when `version=""` is found in a `META` file, we now return `None` as a version string (ocaml/dune#9177, @emillon) - Dune can now be built and installed on Haiku (ocaml/dune#8795, fix ocaml/dune#8551, @Alizter) - Mark installed directories in `dune-package` files. This fixes `(package)` dependencies against packages that contain such directories. (ocaml/dune#8953, fixes ocaml/dune#8915, @emillon)

Alizter self-requested a review September 30, 2023 14:48

rgrinberg reviewed Oct 1, 2023

View reviewed changes

rgrinberg added the odoc label Oct 1, 2023

rgrinberg reviewed Oct 2, 2023

View reviewed changes

jonludlam force-pushed the new-odoc-rules branch from aadebed to cb6a066 Compare October 6, 2023 15:33

Alizter reviewed Oct 6, 2023

View reviewed changes

jonludlam force-pushed the new-odoc-rules branch from 2f6634a to 26320fb Compare October 19, 2023 15:41

Alizter closed this Oct 19, 2023

Alizter reopened this Oct 19, 2023

jonludlam force-pushed the new-odoc-rules branch 2 times, most recently from e01812f to bc1f86d Compare October 19, 2023 22:20

jonludlam force-pushed the new-odoc-rules branch from 1fc882f to 56fdaf0 Compare November 14, 2023 17:03

jonludlam and others added 12 commits November 15, 2023 20:10

New odoc rules

1cc6149

Signed-off-by: Jon Ludlam <jon@recoil.org>

_

125ddfc

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

00c8fda

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

4587213

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

86733b9

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

9a92737

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

416f16d

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

5bbf19d

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

Reinstate call to Valid.filter_dune_with_modules

535503e

It fell off during a refactor. Signed-off-by: Jon Ludlam <jon@recoil.org>

Better: use and+

ebed7ea

Signed-off-by: Jon Ludlam <jon@recoil.org>

_

343b8dc

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

82b3f0e

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

jonludlam and others added 8 commits November 15, 2023 20:10

Add External_location to Dune_package library

ce3fac4

Signed-off-by: Jon Ludlam <jon@recoil.org>

Update tests cases following change in doc-new layout

2632205

Signed-off-by: Jon Ludlam <jon@recoil.org>

Revert unnecessary change to Path.reach

51d4ba4

Signed-off-by: Jon Ludlam <jon@recoil.org>

Formatting

1fc14b5

Signed-off-by: Jon Ludlam <jon@recoil.org>

Robustify tests for @doc-new

b2f02c6

Signed-off-by: Jon Ludlam <jon@recoil.org>

Fix odoc rules following change in Dune_package files representation

bbe2f8e

Signed-off-by: Jon Ludlam <jon@recoil.org>

_

8c16d88

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg force-pushed the new-odoc-rules branch from 56fdaf0 to 8c16d88 Compare November 16, 2023 02:15

rgrinberg added this to the 3.12.0 milestone Nov 16, 2023

rgrinberg merged commit 0a48200 into ocaml:main Nov 16, 2023
25 of 27 checks passed

emillon mentioned this pull request Nov 16, 2023

[new release] dune (15 packages) (3.12.0~alpha2) ocaml/opam-repository#24805

Closed

emillon added a commit to emillon/opam-repository that referenced this pull request Nov 16, 2023

[new release] dune (15 packages) (3.12.0~alpha2)

726d5cd

CHANGES: - Introduce new experimental odoc rules (ocaml/dune#8803, @jonjudlam) - Cherry-pick ocaml/dune#9177 and ocaml/dune#9201 (@emillon)

emillon mentioned this pull request Nov 28, 2023

[new release] dune (15 packages) (3.12.0) ocaml/opam-repository#24862

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New odoc rules #8803

New odoc rules #8803

jonludlam commented Sep 30, 2023

jonludlam commented Sep 30, 2023 •

edited

jonludlam commented Sep 30, 2023

rgrinberg Oct 1, 2023

jonludlam Oct 1, 2023

Alizter Oct 6, 2023

jonludlam Oct 6, 2023

jonludlam Oct 6, 2023

jonludlam Oct 6, 2023

Alizter Oct 6, 2023

This comment was marked as outdated.

Alizter Oct 6, 2023

Alizter Oct 6, 2023

rgrinberg Oct 2, 2023

jonludlam Oct 2, 2023

jonludlam Oct 6, 2023

Alizter Oct 6, 2023

Alizter Oct 6, 2023 •

edited

Alizter Oct 6, 2023

jonludlam commented Oct 18, 2023

Alizter commented Oct 19, 2023

jonludlam commented Oct 21, 2023

rgrinberg commented Nov 16, 2023

		let* findlib_paths = Context.findlib_paths ctx in
		let+ dwms = Valid.filter_dune_with_modules ctx ~all dwms in

New odoc rules #8803

New odoc rules #8803

Conversation

jonludlam commented Sep 30, 2023

jonludlam commented Sep 30, 2023 • edited

jonludlam commented Sep 30, 2023

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

This comment was marked as outdated.

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

Alizter Oct 6, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonludlam commented Oct 18, 2023

Alizter commented Oct 19, 2023

jonludlam commented Oct 21, 2023

rgrinberg commented Nov 16, 2023

jonludlam commented Sep 30, 2023 •

edited

Alizter Oct 6, 2023 •

edited