search

ocaml / odoc

Documentation compiler for OCaml and Reason
Other
319 stars 88 forks source link

odoc compile misleading and incorrect warning #300

Open dbuenzli opened 5 years ago

dbuenzli commented 5 years ago

In presence of library variants odoc compile entices to wrap libraries which is obviously not the solution. Also when it reports the ambiguity it sometimes reports the same path twice. Here's an example output:

[spawn:21151] ['/Users/dbuenzli/.opam/4.03.0/bin/odoc' 'compile' '--pkg' 'ocaml' '-o' '/tmp/odig-cache/odoc/ocaml/page-index.odoc' '/tmp/odig-cache/odoc/ocaml/index.mld' '-I' '/tmp/odig-cache/odoc/ocaml/' '-I' '/tmp/odig-cache/odoc/ocaml/camlp4/' '-I' '/tmp/odig-cache/odoc/ocaml/camlp4/Camlp4Filters/' '-I' '/tmp/odig-cache/odoc/ocaml/camlp4/Camlp4Parsers/' '-I' '/tmp/odig-cache/odoc/ocaml/camlp4/Camlp4Printers/' '-I' '/tmp/odig-cache/odoc/ocaml/camlp4/Camlp4Top/' '-I' '/tmp/odig-cache/odoc/ocaml/compiler-libs/' '-I' '/tmp/odig-cache/odoc/ocaml/ocamldoc/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-compat/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-config/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-debug/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-file/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-lang/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-stdlib/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-subcmd/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-system/' '-I' '/tmp/odig-cache/odoc/ocaml/ocplib-unix/' '-I' '/tmp/odig-cache/odoc/ocaml/threads/' '-I' '/tmp/odig-cache/odoc/ocaml/vmthreads/']:
 Warning, ambiguous lookup. Please wrap your libraries. Possible files:
  /tmp/odig-cache/odoc/ocaml/topdirs.odoc
  /tmp/odig-cache/odoc/ocaml/topdirs.odoc
Warning, ambiguous lookup. Please wrap your libraries. Possible files:
  /tmp/odig-cache/odoc/ocaml/threads/condition.odoc
  /tmp/odig-cache/odoc/ocaml/threads/condition.odoc
Warning, ambiguous lookup. Please wrap your libraries. Possible files:
  /tmp/odig-cache/odoc/ocaml/threads/event.odoc
  /tmp/odig-cache/odoc/ocaml/threads/event.odoc
Warning, ambiguous lookup. Please wrap your libraries. Possible files:
  /tmp/odig-cache/odoc/ocaml/threads/mutex.odoc
  /tmp/odig-cache/odoc/ocaml/threads/mutex.odoc
Warning, ambiguous lookup. Please wrap your libraries. Possible files:
  /tmp/odig-cache/odoc/ocaml/vmthreads/thread.odoc
  /tmp/odig-cache/odoc/ocaml/threads/thread.odoc
Warning, ambiguous lookup. Please wrap your libraries. Possible files:
  /tmp/odig-cache/odoc/ocaml/vmthreads/threadUnix.odoc
  /tmp/odig-cache/odoc/ocaml/threads/threadUnix.odoc
Warning, ambiguous lookup. Please wrap your libraries. Possible files:
  /tmp/odig-cache/odoc/ocaml/topdirs.odoc
  /tmp/odig-cache/odoc/ocaml/topdirs.odoc

More generally it's a bit unclear what to do in presence of library variants.

lpw25 commented 5 years ago

Do these library variants provide different implementations of a single interface or do the interfaces change as well? In the first case the documentation should be of the shared interface, although how to achieve that depends on the details of the tools driving odoc. In the second case it is much less clear what to do. Perhaps the variants should be treated as separate packages/libraries and one of them selected to be used for links from other packages.

dbuenzli commented 5 years ago

I'm a bit unsure. I would have said we were in the first case. But: 

> ocamlobjinfo vmthreads/thread.cmi
File vmthreads/thread.cmi
Unit name: Thread
Interfaces imported:
    d9a58381d35229eeac58d4550953f6de    Thread
    331e41ad213579d006efd84f19005bd5    Unix
    999b28e3b7638771c87eebf5a8325e42    Pervasives
    9642e3ed163e46770985ca668738ed5f    CamlinternalFormatBasics
> ocamlobjinfo threads/thread.cmi
File threads/thread.cmi
Unit name: Thread
Interfaces imported:
    bcedf119ce5ed84c14661894f3c8111e    Thread
    331e41ad213579d006efd84f19005bd5    Unix
    999b28e3b7638771c87eebf5a8325e42    Pervasives
    9642e3ed163e46770985ca668738ed5f    CamlinternalFormatBasics

and this is in 4.03 that is before --keep-locs became the default.

jonludlam commented 5 years ago

AIUI, variants (as implemented in dune) have to implement precisely the same interface - you define a virtual library by having only mli files, and the concrete implementations fill in the ml files. For these it seems appropriate to only produce documentation by looking at the mlis and ignore the mls entirely since the aim is for the user of the virtual library to be agnostic to the actual implementation.

The threads vs vmthreads thing is more interesting - under what circumstances do we end up linking against both?

jonludlam commented 5 years ago

In the limitations section of the dune variants documentation, it mentions that implementations can't even introduce new public modules, though it's not clear from there whether this is a restriction that's going to be lifted. If not though, it seems reasonable to completely ignore all virtual library implementations for the purposes of documentation.

dbuenzli commented 5 years ago

Note that the problem here is not about compiling compilation unit odoc files; there I arbitrarily choose one if there's an ambiguity.

The problem arises here when I automatically generate the index page of a package (if the package did not provide its own index.ml file) like here.

Maybe I should simply choose one if I have two module names that are the same but again because of the inherent limitations and unpreciseness of -I (https://github.com/ocaml/odoc/issues/81 is really something I would like to see being fixed, in almost all my invocations of odoc I have the precise list of files and I trim them down to -I...) I may still end up including conflicting directories even though I trimmed it. Suppose for example I'd like to automatically generate an index for a package that has:

pkg/v1/{a,c}.cmti
pkg/v2/{a,b}.cmti

I can certainly chose only one for a, but since I need to list c and b aswell my includes with have both dirs...

Note that in the second case @lwp25 mentions we are stuck anyways, we'd need a notation to disambiguate the references.

jonludlam commented 5 years ago

So you're hitting the issue the comment mentions here: https://github.com/ocaml/odoc/blob/master/src/odoc/env.ml#L20-L35 - that when compiling the index mld you're effectively trying to link things that an OCaml program couldn't link.

github-actions[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.