Add a "`doc_depends`" field to the `package` stanza
This is the first of a series of PRs to support odoc 3 features.
Odoc 3 has added the ability to link to other package docs when built together, eg in ocaml.org. (This may include docs in the reverse dependency cone, so it's a new kind of dependency to avoid circularity).
To do so, one need to
- Fill a
x-extra-doc-depsfield in the opam file, - Install a
odoc-config.sexpfile.
See this part of odoc's doc for more information.
This PR adds a doc_depends field to the package stanza, to be able to generate those two files:
(package
(name foo)
(doc_depends
(libraries <lib spec>)
(packages <pkg spec>)))
where packages takes the same as the depends stanza (a list of packages with potential version constraints), while libraries takes the same as (library (libraries ...)) stanza (a string list).
This PR also includes a test and the update of the documentation.
Thanks for the PR. Before merging we'll need a changelog entry, so I'll start with that to not forget :-)
This is quite some amount of bikeshedding but I think the new stanza is a bit weird. It is called doc_depends but it doesn't have the syntax of depends;:packages has the syntax of depends. And libraries suddenly exists in dune-project whereas before it only ever existed in dune files.
To be constructive, I have a few alternate suggestions:
- Rename
doc_dependsintodocumentationandpackagesintodepeds. This way it makesdoc_dependsthe place to configure odoc in the project and other things can be set up there as well (like the hierarchies). Alldependsfields have the same syntax so it is easier for people to understand what format goes where. - Move
packagesintodependswith a special variable like:odoc-depends. It's a bit weird and ad-hoc. Maybedependswith a(depends x y z (:doc-depends foo bar baz))? That said,librariesneeds to live somewhere. I wonder, maybe it should actually go into thedunefiles? That would be the cleanest, but also rather tedious to add them everywhere. Or maybe they should live on the top-level ofdune-project?
Generally, do we expect people to use different doc_depends per package? Or maybe we should just go through every depends and detect every findlib library and automatically add it? Having to specify the kind of same things (the opam package yojson has a findlib package called yojson which supplies a module called Yojson) is one of the things that make setup in OCaml tedious and while we can't do much about what exists already, maybe we can avoid the same kind of mistakes in the future?
Thanks a lot @Leonidas-from-XIV for your answer and helpful suggestions!
I'll introduce a specific example scenario, and try to extract from your alternative suggestions the concrete files that we would need. And after that, I discuss which alternative I would prefer, and why!
So, the common scenario is a file whose generated opam file has to look like:
depends: [
"lwt"
"conf-jq" {with-test}
]
x-extra-doc-deps: [
"fmt" {= version}
"odig"
]
and odoc-config.sexp is:
(libraries lwt fmt.tty)
(packages odig sherlodoc)
Let's look at the alternatives of dune configurations that would generate those files.
Option 1: Everything in the documentation stanza of the package (with referenced_libraries not to confuse with libraries):
(package
(name ...)
(depends
lwt
(conf-jq :with-test))
(documentation
(depends
(fmt (= :version))
odig)
(referenced_libraries
lwt
fmt.tty)))
Option A: :odoc-doc variable in depends, referenced_libraries in package stanza of dune-project file
(package
(name ...)
(depends
lwt
(conf-jq :with-test)
(fmt (and (= :version) :odoc-doc))
(odig :odoc-doc))
(documentation
(referenced_libraries
lwt
fmt.tty)))
Option α: :odoc-doc variable in depends, referenced_libraries in documentation stanza of dune file
(package
(name ...)
(depends
lwt
(conf-jq :with-test)
(fmt (and (= :version) :odoc-doc))
(odig :odoc-doc)))
(documentation
(referenced_libraries
lwt
fmt.tty))
On having referenced_libraries in the documentation stanza of dune files
I find it a bit missleading. For example:
(documentation
(mld_files file1)
(referenced_libraries lib1))
(documentation
(mld_files file2)
(referenced_libraries lib2))
seems to suggest that file1 can reference lib1's doc while file2 can reference lib2's doc but actually there is only a global, per package, odoc-config.sexp, so both mld files can reference both lib1 and lib2's documentation.
That plays against option α.
On putting documentation dependencies in the same place as regular dependencies
Opam has a few package variables/flags, which are [available](also used in) in the depends field of dune, but whose interpretation is left to opam.
If we add some new variable which has a meaning in dune but none in opam, how would we generate the opam depends and x-extra-doc-deps of eg:
(package
(depends
(cmdliner (or (<= 2.0) :odoc-doc))))
There would be several options
- Add support for
:odoc-docin opam first, which would replace the currentx-extra-doc-depsfield, - Inconditionally move, from "
depends" to "x-extra-doc-deps", any element in dune'sdependsthat has:odoc-docin the filter AST.
Conclusion
I think we should go with option 1:
- Rename
doc_dependsbydocumentation - Rename
packagesin olddoc_depends/newdocumentationtodepends - Rename
librariesin olddoc_depends/newdocumentationtoreferenced_libraries
Generally, do we expect people to use different doc_depends per package? Or maybe we should just go through every depends and detect every findlib library and automatically add it? Having to specify the kind of same things (the opam package yojson has a findlib package called yojson which supplies a module called Yojson) is one of the things that make setup in OCaml tedious and while we can't do much about what exists already, maybe we can avoid the same kind of mistakes in the future?
I agree that the current situation is confusing, with packages containing libraries containing modules, most of the time with the same name but sometimes with different ones. In our case, it surfaces with the following:
- If we want to link to the manuals of a package, we need to add the package name in the
dependsfield - If we want to reference a library of a package, we also need to specify that, by adding the library name in the
referenced_librariesfield.
When the library and package names are the same that means that we seem to have a redundant information:
(package
(documentation
(depends fmt)
(referenced_libraries fmt)))
However, I'm not sure how to avoid avoid that. Can we, by default, add all libraries from all packages we (doc-)depend on? Do we statically know all libraries a package installs?
I'll try to answer more convincingly whether this is forced by the past and we can't do anything about it, or if it's a mistake we should avoid :)
Can we, by default, add all libraries from all packages we (doc-)depend on? Do we statically know all libraries a package installs?
Statically not. Unless we use dune pkg to download and build the packages we don't know what findlib names a given opam package will expose, unfortunately. However, if OPAM is used instead, then we don't even know that at runtime, because they all just exist in the switch, with no record which opam package they came from.
So unfortunately, if we want to have this information available statically or avoid depending on dune pkg, the libraries will have to be declared by hand.
Thanks for the precision.
I'm currently discussing with @jonludlam to see how we could simplify some things (eg using :post and :with-doc instead of x-extra-doc-deps; and maybe also avoiding the use of odoc-config.sexp if possible).
So, I'll mark this PR as draft and undraft it as soon as the design has stabilized!
Finally, this PR can be undrafted!
Since this PR has been merged, there is no need to specify the libraries we want to reference in our docs, we only need to do that for packages.
With this in mind, I've simplified this to:
(package
(documentation
(depends ...)))
where this depends field allows the user to define which packages are referenced in the docs. It shares the same syntax as the depends field for normal dependencies.
There are a few details to mention:
-
The
documentationfield already exists inpackagestanza. It is used to give theurlto the documentation:(package (documentation "http://url.to/doc")).For retro-compatibility, this is still supported. And, in order to specify both the URL and the package dependencies, we can do
(package (documentation (url "http://url.to/doc") (depends ...))) -
The
documentationfield also exist in the toplevel ofdune-projectfiles. In those case, it was extended for consistency with the new syntax, except that defining dependencies outside of a package raise a warning. -
I haven't handled the dune-lang version checks...
@punchagan (with assist form me) will be helping land this, now that PE is away. We are ready to respond to reviews at this point, and would appreciate getting feedback!
As @panglesd states, the main missing bit is the dune-lang check and the test for it.
The rest are fairly minor things that I came across when reading through it, but overall the code looks really solid to me.
Thanks for the quick review @Leonidas-from-XIV! I'll try and address your comments as I can, starting with the dune-lang check.
I haven't followed the entire PR, but is there an explanation on how this new field differs from with-doc?