Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

odoc compile-deps does not work on .cmt files #162

Closed
dbuenzli opened this issue Jul 27, 2018 · 7 comments
Closed

odoc compile-deps does not work on .cmt files #162

dbuenzli opened this issue Jul 27, 2018 · 7 comments
Labels
bug Something isn't working

Comments

@dbuenzli
Copy link
Contributor

> odoc --version
v1.1.1-710-gdd47a98
> odoc compile-deps int64.cmt
odoc: internal error, uncaught exception:
      Cmi_format.Error(_)
      Raised at file "typing/cmi_format.ml", line 74, characters 6-21
      Called from file "src/odoc/depends.ml", line 31, characters 18-43
      Called from file "src/odoc/bin/main.ml", line 218, characters 17-72
      Called from file "src/cmdliner_term.ml", line 27, characters 19-24
      Called from file "src/cmdliner.ml", line 106, characters 32-39
@dbuenzli
Copy link
Contributor Author

Basically this bit needs to be conditionalized for .cmt files. See e.g. here on how to read that info.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Jul 27, 2018

Also it might be nice to do a little bit of data cleaning:

  1. Do not report self-references.
  2. Do not report duplicates self-reference duplicates (which may exist on ocaml versions before this PR was merged):

This function in omod does that.

Here's an example of unclean data on 4.03:

> odoc compile-deps odig_odoc.cmti
Buffer 3bd1af04573ce2da7fc3dc04403e852e
CamlinternalFormatBasics 9642e3ed163e46770985ca668738ed5f
Digest 23fdbfc720a71002434f407c37d040a3
Format 60c2e7663dd57d13b5920931742e1c10
Lazy b2e565a5cdbd351dc15bc9061d30c458
Map f23f0e2510f18d4b11ad6f7771618294
Odig fd0e5fd19fad6558e8c10b09070120b6
Odig_odoc 059818369bd7b50c5a94fb604d892d22
Odig_support 2c79861dbb19e31112fc6447b622ca6a
Pervasives 999b28e3b7638771c87eebf5a8325e42
Set a16cc25d9afe91eb22559bef41e9ee28
String c28a3ca42a30c6ffa0bbab4a05329226
Odig_odoc 059818369bd7b50c5a94fb604d892d22

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Jul 27, 2018

And a question, is it ok to simply drop module aliases at that point ?

@trefis
Copy link
Contributor

trefis commented Jul 30, 2018

And a question, is it ok to simply drop module aliases at that point ?

Yes, and it's actually necessary if you don't want to report cyclic dependencies which aren't there.

@trefis trefis added bug Something isn't working good first issue and removed command line labels Jul 31, 2018
@sharno
Copy link
Contributor

sharno commented Aug 27, 2018

I tried to pick this up and wrote a fix but I couldn't reproduce the issue, made a simple project and compiled it with dune to get a .cmt file but it produces regular output and doesn't give an error even after checking out to the commit you have in your example.

example I tried:

> odoc compile-deps _build/default/.test.eobjs/test.cmt
CamlinternalFormatBasics 79ae8c0eb753af6b441fe05456c7970b
Pervasives 9b04ecdc97e5102c1d342892ef7ad9a2
Test 68dbfaae03db05fb385024df98126f19

I'm a bit new to the OCaml ecosystem so am I missing something?

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Aug 27, 2018

My guess is that your test.ml file might be empty (and self dependencies are nevertheless still reported). Just test the tool on one of the cmt file you can find in $(opam var ocaml:lib). The current code is obviously wrong since it's not special cased for .cmt files, see the various links I posted above.

dbuenzli added a commit to dbuenzli/odoc that referenced this issue Dec 18, 2018
Make it work on cmt files and do not report the unit itself as
as a dependency. Closes ocaml#162.
dbuenzli added a commit to dbuenzli/odoc that referenced this issue Jan 10, 2019
Make it work on cmt files. Closes ocaml#162.
@dbuenzli
Copy link
Contributor Author

  1. Do not report self-references.

In the end this is still useful, see discussion in #271; so it kept but documented as such.

dbuenzli added a commit that referenced this issue Jan 21, 2019
Make it work on cmt files. Closes #162.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants