Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Warnings to unresolved references make odoc unusable #1120

Open
dbuenzli opened this issue May 12, 2024 · 15 comments
Open

Warnings to unresolved references make odoc unusable #1120

dbuenzli opened this issue May 12, 2024 · 15 comments
Labels
bug Something isn't working cross-referencer
Milestone

Comments

@dbuenzli
Copy link
Contributor

dbuenzli commented May 12, 2024

I was pretty sure there was an issue about this. But I can't find one. So there it is.

I think this should really be acted upon and prioritized as odoc becomes quickly unusable when working on partial docsets1 and this problem has been here for ages.

Let's take a simple example:

> cat m.mli
(** My doc.

    The max [int] value is {!Sys.max_int}. *)

This will typically report this warning:

File "lib/m.mli", line 3, characters 27-41:
Warning: Failed to resolve reference unresolvedroot(Sys).max_int Couldn't find "Sys"

I really don't have the compilation pipeline of odoc in my head right now but I suspect there is a point where we can record this symbol exists despite the fact that no documentation is going to be generated for it by simply providing an appropriate include into the stdlib's library directory or the path to the Sys.cm[i,ti] file.

I'm pretty sure people working on odoc have a better idea than I have of when/where in the pipeline this should be done (I vaguely remember a discussion with @trefis a few years ago where he indicated this would be possible and not too hard).

(This will not work as is stands for external references into .mld but in my case it would, most of the time remove 100% of these warnings).

Footnotes

  1. Case in point I have a medium scale project that generates 122 warnings from links into other libraries and the Stdlib and it is of course no longer possible to sift through that to find out if a couple of doc strings I just wrote made wrong or ambiguous links.

@dbuenzli dbuenzli added bug Something isn't working cross-referencer labels May 12, 2024
@jonludlam
Copy link
Member

Thanks for bringing this up - I totally agree that it's really important to focus on issues like this, otherwise we're putting people off making good docs.

I'm very skeptical of trying to solve the issue in the way you're suggesting though - essentially I don't believe we can do anything less than generating the correct odoc files for your dependencies. I think the only reliable way to distinguish between references that will and won't resolve is to do a complete docs build for all your dependencies, and then try to resolve the references!

Now, a second issue is that of partial docsets. I'm going to assume that the main reason for having partial docsets is that someone would like to publish the docs for their libraries without having to additionally publish the docs for all of their dependencies. For this, if we start from the premise that we've built docs for all our deps locally, we've got a couple of options:

  1. We could continue to do what many people do today: publish docs where all links that would go to other packages are rendered as unresolved links. This would be pretty straightforward.
  2. We could assume that links to other packages could be rewritten as links to ocaml.org/p/.... The downside to this is that there's no guarantee the links will be exactly correct.

I'm leaning in the second direction, but it seems likely that the path to get there would mean that implementing '1' would be trivial.

Building a complete doc set is obviously not easy, but there are already several options available. Obviously there's your own odig, which is really nicely suited to producing docs for packages once they've been installed, but not quite so useful when you're still developing the docs though. There is the dune prototype -- dune build @doc-new -- which isn't quite so battle tested, and is missing some features, but very useful (when it works) for developing your docs. Then we're also developing a driver within the odoc repo itself, although this is more intended for developing features in odoc than as a dev tool for documentation authors -- though this is likely to be the most 'feature complete' driver at any moment, and is likely a good candidate for building docs for publishing. It could even end up replacing chunks of voodoo which is what we use to build the docs for ocaml.org.

I'm curious what you're using to build and test your docs?

Anyway, clearly, fixing this isn't the only thing we need to do to make the experience of writing docs better - the duplicates @dlesbre mentioned in #1127 need a separate fix - but I'm very keen to work on these sorts of issues with high priority, as you suggest.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 27, 2024

essentially I don't believe we can do anything less than generating the correct odoc files for your dependencies.

If your positive that's the way, then so be it (though I fear a bit that the time to generate simple docs can explode a bit if you depend on large stuff like core). However it would perhaps be nice to try to characterize a bit more what counts as a dependency and how to get them:

  1. Regarding the dependencies to render module signatures, I suspect it corresponds to the dependencies for the compilation phase we get via the new -H flag, see here.
  2. Then you have the links to the .mld pages of other packages and possible references to modules that are not part of code dependencies of the modules. We need odoc to be able to spit them out in some way for the driver to resolve them.

Regarding partial docsets I think we should really have them, I'm fine with 1. as far as generation is concerned rather than trying to be smart but failing too often.

I'm curious what you're using to build and test your docs?

At the moment I'm using the brzo tool. It was not made for that but it works quite well in practice since my packages are rather simple. The brzo driver, which shares some of the code with odig via the b0 project is here (high-level description). I guess I could add the dependencies that brzo found to get rid of the warnings but then I'd really like to be able to generate partial docsets.

@NicNomadic
Copy link

  1. We could continue to do what many people do today: publish docs where all links that would go to other packages are rendered as unresolved links. This would be pretty straightforward.

[I react here as the solution to this issue could also solve #1144]

In think this simple solution would be acceptable (and even good!) if we can define somewhere (options, config file, etc) a list of external libraries. Otherwise, how can odoc distinguish between an external lib and a misspelled lib among the ones we are just submitting?

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Feb 4, 2025

According to @jonludlam this should be fixed.

Could you please, @jonludlam, just mention which PR and/or doc bits which implement/indicate the way drivers have to be tweaked ?

After that I think this can be closed.

@AllanBlanchard
Copy link

I am also curious about that because basic usage of odoc_driver still generates hundreds of messages on Frama-C (+ having to specify all packages on which the driver must be run by hand is super painful ...).

@jonludlam
Copy link
Member

Could you please, @jonludlam, just mention which PR and/or doc bits which implement/indicate the way drivers have to be tweaked ?

Currently this works by passing the flag --suppress-warnings when compiling cmt{i}s for which you don't care about the warnings - ie, all the dependencies. However, I'm considering changing this, as this is going to ruin caching between projects.

@jonludlam
Copy link
Member

I am also curious about that because basic usage of odoc_driver still generates hundreds of messages on Frama-C (+ having to specify all packages on which the driver must be run by hand is super painful ...).

I had a go at running odoc_driver on frama-c - you're right that there are a lot of warnings produced, but as far as I could tell they were mostly warnings that came from source files in the frama-c source itself. The point of the changes was that you're mostly no longer seeing any warnings that come from the source of dependency packages.

(this isn't entirely true - there's at least one bug where dependency warnings are creeping in, for example, in the warnings when compiling frama-c there were:

File "src/sig.mli", line 68, characters 16-31:
Warning: While resolving the expansion of include at File "src/graphviz.mli", line 380, character 2
Failed to resolve reference unresolvedroot(ORDERED_TYPE) Couldn't find "ORDERED_TYPE"
File "src/sig.mli", line 51, characters 19-32:
Warning: While resolving the expansion of include at File "src/graphviz.mli", line 380, character 2
Failed to resolve reference unresolvedroot(COMPARABLE) Couldn't find "COMPARABLE"

which I think come from ocamlgraph)

On your second point, I don't understand what you mean about having to specify all packages on which the driver is run?

@AllanBlanchard
Copy link

AllanBlanchard commented Feb 6, 2025

I should have read the documentation of odoc_driver, it would have avoided misinterpretation of the word "package". Anyway, now that I better understand that, the result that I obtain is still quite surprising. After installing Frama-C, I only get documentation for:

  • the Frama-C kernel
  • the Eva plug-in of Frama-C
  • the WP plug-in of Frama-C

and I cannot really understand why. I would have expected either to have only the documentation for the Kernel (behavior in Dune if we do not provide all dune packages names) or for the kernel + all the plug-ins. Anyway, this is clearly not an issue related to this one, so I guess I should open another issue.

Also found a crash when trying to get documentation for Frama-C plug-ins: #1302

but as far as I could tell they were mostly warnings that came from source files in the frama-c source itself.

For example, in kernel.mli, we have repeatedly the warning:

Warning: While resolving the expansion of include at File "src/kernel_services/plugin_entry_points/kernel.mli", line 30, character 0
While resolving the expansion of include at File "src/kernel_services/plugin_entry_points/plugin.mli", line 99, character 2
While resolving the expansion of include at File "src/kernel_services/cmdline_parameters/parameter_sig.ml", line 407, character 2
Failed to resolve reference unresolvedroot(Builder).Make_user_dir_opt Couldn't find "Builder"

I guess the line that really triggers the error is the following line from parameter_sig.ml (because of a module inclusion):

      {!Builder.Make_user_dir} and {!Builder.Make_user_dir_opt}.

but it is really strange to have such a warning: Builder is a module type local to this file and there is no other module Builder exposed at this code location.

Is it:

  • the same issue as this one?
  • another issue with the same kind of message?
  • not an issue?

In the last case, I'd really like to understand what is wrong with the line we have written in the documentation.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Feb 6, 2025

Thanks @jonludlam for the answer.

However, I'm considering changing this, as this is going to ruin caching between projects.

Indeed it would be nicer if that'd be something that happen at link and/or generation time rather than during .odoc file generation. Will the change happen before 3 is released ?

@jonludlam
Copy link
Member

I should have read the documentation of odoc_driver, it would have avoided misinterpretation of the word "package". Anyway, now that I better understand that, the result that I obtain is still quite surprising. After installing Frama-C, I only get documentation for:

* the Frama-C kernel

* the Eva plug-in of Frama-C

* the WP plug-in of Frama-C
  and I cannot really understand why. I would have expected either to have only the documentation for the Kernel (behavior in Dune if we do not provide all dune packages names) or for the kernel + all the plug-ins. Anyway, this is clearly not an issue related to this one, so I guess I should open another issue.

It's entirely based around opam packages - you should get the docs for precisely the opam packages you specify and those that are dependencies. Don't forget to rm -rf the _html dir in between calls to odoc_driver (like I keep doing!)

Also found a crash when trying to get documentation for Frama-C plug-ins: #1302

Thanks!

but as far as I could tell they were mostly warnings that came from source files in the frama-c source itself.

For example, in kernel.mli, we have repeatedly the warning:

Warning: While resolving the expansion of include at File "src/kernel_services/plugin_entry_points/kernel.mli", line 30, character 0
While resolving the expansion of include at File "src/kernel_services/plugin_entry_points/plugin.mli", line 99, character 2
While resolving the expansion of include at File "src/kernel_services/cmdline_parameters/parameter_sig.ml", line 407, character 2
Failed to resolve reference unresolvedroot(Builder).Make_user_dir_opt Couldn't find "Builder"

I guess the line that really triggers the error is the following line from parameter_sig.ml (because of a module inclusion):

      {!Builder.Make_user_dir} and {!Builder.Make_user_dir_opt}.

Currently those references are resolved where the expansion is made, so if there's no module Builder there, it's not going to work. There's a PR to fix this (by pushing the reference resolving code more in the direction of how Paths work), but it's a bit experimental at the moment so I'm not too keen to merge that into the 3.0 release. The alternative that will work right now is to put in the fully qualified path.

I had a go at fixing the most annoying warnings - I've got a patch that I'll make available somehow. I've also got a fix for the ocamlgraph warnings that were creeping in here.

but it is really strange to have such a warning: Builder is a module type local to this file and there is no other module Builder exposed at this code location.

Is it:

* the same issue as this one?

* another issue with the same kind of message?

* not an issue?

In the last case, I'd really like to understand what is wrong with the line we have written in the documentation.

@jonludlam
Copy link
Member

frama-c.patch

Here's the frama-c patch - it's based on 30.0

@jonludlam jonludlam added this to the 3.0 milestone Feb 7, 2025
@jonludlam
Copy link
Member

Indeed it would be nicer if that'd be something that happen at link and/or generation time rather than during .odoc file generation. Will the change happen before 3 is released ?

I've made a PR - #1304 - with the change. The idea there is that when you're compiling an odoc file, you supply an optional key. When you subsequently do a link, you give a list of keys for which you want warnings reported.

@dbuenzli
Copy link
Contributor Author

I've made a PR - #1304 - with the change. The idea there is that when you're compiling an odoc file, you supply an optional key. When you subsequently do a link, you give a list of keys for which you want warnings reported.

Does that mean that I need to invent new names ? That looks rather unconvenient.

@jonludlam
Copy link
Member

Does that mean that I need to invent new names ? That looks rather unconvenient.

odoc_driver is using the package names as keys. I think that's probably the right level of granularity.

@AllanBlanchard
Copy link

Currently those references are resolved where the expansion is made, so if there's no module Builder there, it's not going to work. There's a PR to fix this (by pushing the reference resolving code more in the direction of how Paths work), but it's a bit experimental at the moment so I'm not too keen to merge that into the 3.0 release. The alternative that will work right now is to put in the fully qualified path.

Ok, thank you for the explanation :)

frama-c.patch

Here's the frama-c patch - it's based on 30.0

Wow! Thank a lot for this!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working cross-referencer
Projects
None yet
Development

No branches or pull requests

4 participants