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

Fix unwrapped dune libraries #126

Closed
wants to merge 1 commit into from

Conversation

jonludlam
Copy link
Collaborator

Untested beyond the 'do' phase, but before this produced:

{0 camlp-streams 5.0.1}
{1 Libraries}
This package provides the following libraries (via dune):
{2 camlp-streams}
Documentation: {!modules:}

and after it produces:

{0 camlp-streams 5.0.1}
{1 Libraries}
This package provides the following libraries (via dune):
{2 camlp-streams}
Documentation: {!modules:Genlex Stream}

@jonludlam
Copy link
Collaborator Author

Fixes #125

@jonludlam
Copy link
Collaborator Author

Looks like something changed in dune - in a switch I have with dune 3.6.1, the relevant part of dune-package contains:

  (unwrapped
   ((name Tyxml) (obj_name tyxml) (visibility public) (impl))
   ((name Tyxml_html) (obj_name tyxml_html) (visibility public) (impl) (intf))
   ((name Tyxml_svg) (obj_name tyxml_svg) (visibility public) (impl) (intf))
   ((name Tyxml_xml) (obj_name tyxml_xml) (visibility public) (impl) (intf)))))

whereas another switch with dune 3.10.0 I have:

 (modules
  (unwrapped
   (module
    (obj_name tyxml)
    (visibility public)
    (source (path Tyxml) (impl (path tyxml.ml))))
   (module
    (obj_name tyxml_html)
    (visibility public)
    (source
     (path Tyxml_html)
     (intf (path tyxml_html.mli))
     (impl (path tyxml_html.ml))))
   (module
    (obj_name tyxml_svg)
    (visibility public)
    (source
     (path Tyxml_svg)
     (intf (path tyxml_svg.mli))
     (impl (path tyxml_svg.ml))))
   (module
    (obj_name tyxml_xml)
    (visibility public)
    (source
     (path Tyxml_xml)
     (intf (path tyxml_xml.mli))
     (impl (path tyxml_xml.ml)))))))

Voodoo is being very bad here and using an internal data format of dune's - reading the dune-package sexp file directly. We should probably fix this more permanently in a better way.

@gpetiot
Copy link
Contributor

gpetiot commented Sep 29, 2023

I had a quick look and couldn't find any tool or API to parse the dune-package either. I think it's worth asking this question on the dune repo.

@sabine
Copy link
Collaborator

sabine commented Jan 14, 2024

#136 resolves this issue by removing the logic that inspects dune-package.

Since this is an internal data format, I think we are getting into more pain than we are getting benefits from reading dune-package. When upstream provides a means to read dune-package, or another way to inspect a package via some stable interface, we can consider adding that.

Proposing to close this.

@jonludlam
Copy link
Collaborator Author

Sure OK.

@jonludlam jonludlam closed this Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants