New odoc rules

[trefis]

Recreating after the accidental close of the previous PR.

The odoc rules, as implemented in this PR, work with the git version of odoc and follow the guidelines listed here.
Currently, for a given library, .mld files are found in the same directory as normal source files (i.e. in the same directory as the jbuild).

Things that need to happen before merging this

• odoc should be released on opam
• we need a way to put .mld files in an arbitrary directory and tell jbuilder where to find them. The way I currently have in mind is an addition to the jbuild type of libraries to allow writing:

  (library
    ((name foo)
      ...
      (odoc <something>)
      ))

  where something should probably follow this syntax
• we need to install all the .mld files in $DOCDIR/$PKG/odoc-pages (at least that's what I understand from this comment, but @dbuenzli will correct me if I'm wrong).

[dbuenzli]

@dbuenzli will correct me if I'm wrong

According to b0-system/odig#31 and b0-system/odig#18 it's rather $DOCDIR/$PKG/odoc-pages.

[dbuenzli]

Currently, for a given library, .mld files are found in the same directory as normal source files (i.e. in the same directory as the jbuild).

Do you then namespace the .mld files ?

[trefis]

Do you then namespace the .mld files ?

Yes, in the sense that every .odoc file contain information about which package it belongs too (which you specify when calling odoc compile).
Unfortunately there is currently no way to tell odoc (when it resolves references) "I want the page Foo of package P", but we will definitely add that (rather soon I hope).
In any case, this is purely an odoc issue and doesn't impact jbuilder (or odig) or the work being discussed here.

[dbuenzli]

In any case, this is purely an odoc issue and doesn't impact jbuilder (or odig) or the work being discussed here.

I think it does. If two libraries define p.mld those are going to overwrite when you copy them to odoc-pages. An easy way to get out would be to support only this:

we need a way to put .mld files in an arbitrary directory and tell jbuilder where to find them.

and only once per package.

[rgrinberg]

Curious as to why we need to allow jbuilder to specify .mld files in different dirs? The way we solved a similar problem for source files is using copy_files. Why can't we rely on copying to work for this as well? Not suggesting that this is the way to go, but would like to understand the use case first.

[trefis]

I think it does. If two libraries define p.mld those are going to overwrite when you copy them to odoc-pages

Right, because library and package are two distinct things.
From what I've seen, currently jbuilder will use the "public library name" as package name when invoking odoc; I assumed it would also use that as package name when installing files but I'm probably wrong there.

Curious as to why we need to allow jbuilder to specify .mld files in different dirs? The way we solved a similar problem for source files is using copy_files. Why can't we rely on copying to work for this as well? Not suggesting that this is the way to go, but would like to understand the use case first.

I've personally always found that distasteful.
I understand that you want to discourage users spreading their sources in several directories if this isn't reflected later in the language by proper namespacing. Here however I don't think it is bad if users put their .mld files in a doc/ or manual/ subdirectory.
It is not too important but I think it'd be nicer.

[trefis]

Ping

[rgrinberg]

Is this a ping to me? I'm fine with you allowing mld files in other directories. Though I'd like to hear a second opinion from another maintainer (like @bobot for example).

Seems like this PR is still WIP though? Or is there something you wanted us to work on?

Btw, this needs a rebase on top of master.

[trefis]

For both of you actually.
I don't feel like I can properly answer @dbuenzli's concerns, I think a discussion needs to happen. And that point needs to get resolved before progress can be made on this feature. So let's also ping @lpw25 and @aantron .

As for the second question yes, the ping was for "jbuilder's dev" in the general sense yes.
We could decide that the second item in the todo list (i.e. putting .mld in arbitrary directories) is not that important and delay the decision. But it's probably better to just make a decision now, or at least to make sure we're not going to force people to change their codebase in a few months because we've changed our mind (i.e. make sure that in the future we're still going to support the choice we make in this PR).

[bobot]

But it's probably better to just make a decision now, or at least to make sure we're not going to force people to change their codebase in a few months because we've changed our mind.

In that case same directory is the safest. We could still use the unsatisfying (copy-files ). When we will have a better grasp of all the files that jbuilder support we will be able to make a global decision and perhaps choose the exceptions.

[dbuenzli]

In that case same directory is the safest.

So I gather you will then namespace the mld files the jbuilder does with ml files, right ?

My impression is that manuals are going to be per package rather than per library this is why I would rather advocate a doc directory (which would also eschew the namespacing problem).

[trefis]

In that case same directory is the safest.

Sounds good to me!

My impression is that manuals are going to be per package rather than per library this is why I would rather advocate a doc directory (which would also eschew the namespacing problem).

The can be added after the fact then, with a (documentation ...) stanza (i.e. it'd be its own thing, rather than part of the (library ...) stanza).
It would also probably interact nicely with ocaml/odoc#94

I do think that pages inside a library, i.e. not part of a "standalone" manual, still make sense though. Don't they?
And I still don't know where these should be installed :/

[bobot]

My impression is that manuals are going to be per package rather than per library this is why I would rather advocate a doc directory (which would also eschew the namespacing problem).

I agree that the manuals installed will be by package, since only packages are installed. Libraries that are not part of a package should still be able to have a documentation, locally installed.

I think it does. If two libraries define p.mld those are going to overwrite when you copy them to odoc-pages.

Currently two libraries can't be part of the same exact package, but in the future we could accept it. If they are installed in the same directory they are part of the same package and so jbuilder could detect the conflict. Or if the basename can't be arbitrary because it must corresponds to a module name, I don't see how the doc directory solves the problem.

I do think that pages inside a library, i.e. not part of a "standalone" manual, still make sense though. Don't they? And I still don't know where these should be installed :/

I'm also a little lost on where and what odoc wants to install.

[dbuenzli]

I think the discussion is getting a little bit confusing here.

I don't see how ocaml/odoc#94 would interact nicely with my proposal this is a global namespace. I'm not sure exactly what @bobot says with "currently two libraries can't be part of the same exact package". As far as documentation is generated by odoc as driven by odig this doesn't make sense: a package corresponds to an opam package and treated as such with the --pkg option of odoc.

Here are a few points:

1. AFAIK for now .mld processing in odoc is per package. Each .mld name has to be unique in the package like toplevel modules of libraries need to be.
2. Whether you want to have the .mld in the directory of libraries or in one or more separate directories, the problem of name clashes subsists both at generation time and at install time regardless of where the files are installed (since usually libraries are installed in a flat directory).
3. Given 1. and 2. if you centralize the .mlds for a given package to a single directory you solve the problem of name clashes.
4. Regarding where the .mld files need to be installed a proposal was made here.

[lpw25]

I haven't been following this too closely, but I basically agree with @dbuenzli on this. SInce odoc manages documentation for opam packages rather than libraries it is packages that jbuilder should associate the .mld files with. I don't know the details, but it seems like libraries are associated with packages by just associating any (library ...) stanzas with the surrounding package. So I would assume that having a (documentation ...) stanza which associates the listed documentation files with the surrounding package would be most reasonable approach. Just as (I assume) jbuilder checks libraries in a package have unique names it should check that documentation in a package has a unique name.

[trefis]

it seems like libraries are associated with packages by just associating any (library ...) stanzas with the surrounding package.

I don't think that's quite right. I think the public name of the library has to be of the form package-name[.whatever].

Anyway, thanks for the precisions / clarifications.
It seems like have the .mld files live among the .ml(i) files is just a source of confusion, leading people to think that that the doc granularity is the library, not the package. (Note that this confusion is already present in the way jbuilder generate documentation before this PR, by pretending that libraries are packages)
I'll try to update this PR sometime in the next couple of weeks to clean up all of that.

[lpw25]

It seems like have the .mld files live among the .ml(i) files is just a source of confusion

Just to clarify, I don't think it is the mixing of .mld files with .ml files that is the problem, so much as having the odoc construct appear within the library construct. I think that is what makes it seem like the documentation is associated with the library rather than the package.

[rgrinberg]

There's a test failure with one of my recently introduced tests:

        cram alias test/blackbox-tests/runtest (exit 1)
(cd _build/default/test/blackbox-tests/test-cases/meta-gen && ../../cram.exe run.t)
------ /Users/rgrinberg/reps/jsc/jbuilder/test/blackbox-tests/test-cases/meta-gen/run.t
++++++ /Users/rgrinberg/reps/jsc/jbuilder/test/blackbox-tests/test-cases/meta-gen/run.t.corrected
File "/Users/rgrinberg/reps/jsc/jbuilder/test/blackbox-tests/test-cases/meta-gen/run.t", line 2, characters 0-1:
 |  $ $JBUILDER runtest --force -j1 --root .
-|  description = "contains \"quotes\""
-|  requires = "bytes"
-|  archive(byte) = "foobar.cma"
-|  archive(native) = "foobar.cmxa"
-|  plugin(byte) = "foobar.cma"
-|  plugin(native) = "foobar.cmxs"
-|  package "baz" (
-|    directory = "baz"
-|    description = "sub library with modes set to byte"
-|    requires = "bytes"
-|    archive(byte) = "foobar_baz.cma"
-|    archive(native) = "foobar_baz.cmxa"
-|    plugin(byte) = "foobar_baz.cma"
-|    plugin(native) = "foobar_baz.cmxs"
-|  )
-|  package "rewriter" (
-|    directory = "rewriter"
-|    description = "ppx rewriter"
-|    requires(ppx_driver) = "bytes foobar"
-|    archive(ppx_driver,byte) = "foobar_rewriter.cma"
-|    archive(ppx_driver,native) = "foobar_rewriter.cmxa"
-|    plugin(ppx_driver,byte) = "foobar_rewriter.cma"
-|    plugin(ppx_driver,native) = "foobar_rewriter.cmxs"
-|    # This is what jbuilder uses to find out the runtime dependencies of
-|    # a preprocessor
-|    ppx_runtime_deps = "bytes foobar.baz"
-|    # This line makes things transparent for people mixing preprocessors
-|    # and normal dependencies
-|    requires(-ppx_driver) = "bytes foobar.baz"
-|    ppx(-ppx_driver,-custom_ppx) = "./ppx.exe --as-ppx"
-|  )
+|  Multiple rules generated for _build/default/page-index.odoc
+|  [1]
make: *** [test] Error 1

I've seen an error like this pop up before as well in ppx_tools_versioned or omp - can't remember. You should be able to reproduce by rebasing and running $ make test.

[rgrinberg]

I've added some real odoc tests now as well. So this PR should be rebased on top, and those be updated if necessary.

[rgrinberg]

I've resolved the conflicts in this branch with master. @trefis please have a look.

The odoc test suite is still failing because it doesn't yet support generating an index.html for multiple libraries defined in the same directory. In this situation we have to be a bit more clever and make sure that the index.html is generated for all libraries These libraries may not be a part of the same package so scratch that. We have to find a way to solve this if we want to generate docs for existing packages without making them change their structure.

OK so the (documentation ..) stanza should take care of these ambiguity problems for us. Something like:

(documentation
  ((packge ...)
    (mlds (...))))

Where mlds is optional and work about the same way as modules does.

[avsm]

cc @aantron as well on this issue

[rgrinberg]

Another thing to consider is the generation of documentation for libraries that don't belong to any opam package because they aren't being installed. It seems like @MiloDavis uses that feature so it might make sense to consider attaching .mld files to libraries as well.

[trefis]

That sounds right.
Also, I'm in favor of the glob when the field is missing, and I see no reason not to have an ordered set lang :)

[rgrinberg]

We could probably generate the .odoc file in a directory named as the package. I haven't thought much about it but off-hand I don't see why that wouldn't work.

I tried doing this but there are a lot of complications with this. Like fixing all the globs. I still think this is a good idea but I also don't think it's such a good idea to have special code for this just for documentation. What I'd like to see instead is dune creating separate dirs in _build for building libraries, executables, documentation in isolation.

What ended being easier was just renaming the .odoc files and then doing a little preprocessing on the generated .html because odoc doesn't let me control the .html names generated.

[rgrinberg]

Disregard my last comment as I gave it another and it turned out not to be so bad to generate the odoc stuff in its own dir.

[avsm]

What ended being easier was just renaming the .odoc files and then doing a little preprocessing on the generated .html because odoc doesn't let me control the .html names generated.

Just highlighting this bit for @aantron -- it sounds like something that can be fixed in the odoc frontend to make build system integration easier?

[aantron]

It's possible. I'm thinking through several related issues. I think one of the main reasons odoc doesn't give control over the filenames generated is that it one .mli file's worth of AST can generate multiple HTML pages, due to nested modules and classes getting their own pages. I personally would like to remove that, and replace it by one .mli generates one HTML page (and potentially markup that is included into another HTML page that corresponds to another .mli file, with the inline tag).

[lpw25]

Even if some support for inlining some modules is added to odoc it is not going to be for all submodules as that won't work properly. It would put every library on a single page for a start. I don't know if it helps in this case, but you are supposed to be able to ask odoc what files it will create using something like odoc html-targets ....

[lpw25]

I think odoc doesn't give control over the generated HTML locations because to have at least some idea of where they are to generate correct links.

[MiloDavis]

@rgrinberg We restructured the project in question (Tezos)[https://github.com/tezos/tezos/], to avoid this issue.

[rgrinberg]

Implementation continues at #570