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

cant get (special_builtin_support) to work with findlib.dynload #2595

Closed
avsm opened this issue Aug 30, 2019 · 5 comments
Closed

cant get (special_builtin_support) to work with findlib.dynload #2595

avsm opened this issue Aug 30, 2019 · 5 comments

Comments

@avsm
Copy link
Member

avsm commented Aug 30, 2019

With the duniverse port of findlib, I can't get ppxfind to compile due to some interaction with the special handling of dynlink.

To repro:

$ opam source ppxfind
$ git clone git://github.com/dune-universe/lib-findlib -b duniverse-1.8.1 ppxfind.1.3/findlib
$ cd ppxfind.1.3
$ dune build --verbose
<snip>
$ (cd _build/default && /home/avsm2/.opam/407/bin/ocamlc.opt -bin-annot -I src/.ppxfind.eobjs/byte -I findlib/src/findlib/.findlib_public.objs/byte -I findlib/src/findlib/.findlib_public.objs/native -no-alias-deps -opaque -o src/.ppxfind.eobjs/byte/findlib_initl.cmo -c -impl src/.ppxfind.eobjs/findlib_initl.ml-gen)
File "src/.ppxfind.eobjs/findlib_initl.ml-gen", line 1, characters 0-22:
Error: Unbound module Findlib
Hint: Did you mean Stdlib?

The dune port of findlib has:

(library
  (name findlib_dynload)
  (public_name findlib.dynload)
  (wrapped false)
  (libraries findlib.internal dynlink)
  (special_builtin_support findlib_dynload)
  (modules fl_dynload))

in ppxfind, removing findlib.dynload from the dune file and commenting out the relevant line allows it to compile fine. But as soon as the findlib.dynload package is referenced, it fails despite the use of special_builtin_support

@ghost
Copy link

ghost commented Sep 2, 2019

This is the reason of the failure:

  • when compiling the generated findlib_init.ml-gen, dune only passes -I <dir of findlib> and doesn't pass the -I for the transitive dependencies of findlib
  • in the duniverse port, findlib is just a proxy library for findlib.internal

We could fix this particular problem by passing the -I for the transitive dependencies of findlib when compiling the init file, however this indirection will also break any project using (implicit_transitive_deps false), which in particular is the default in Dune 2.0.

Another idea would be somehow get dune to expand findlib to findlib.internal, either by marking findlib explicitly as a proxy library, or by automatically recognising it as such. @rgrinberg what do you think?

@rgrinberg
Copy link
Member

Another idea would be somehow get dune to expand findlib to findlib.internal, either by marking findlib explicitly as a proxy library, or by automatically recognising it as such. @rgrinberg what do you think?

This problem is not unique to findlib, so I favor a solution that isn't findlib specific. One idea that I thought about was a (exports ..) field. This would be a predicate field that would select a subset of libraries to be exported. So in this case, findlib would need to add findlib.internal.

@ghost
Copy link

ghost commented Sep 2, 2019

An exports field seems. I suggest to call it re_export, to make it clear we are re-exporting existing stuff rather than exporting the library's own stuff.

avsm added a commit to dune-universe/lib-findlib that referenced this issue Sep 2, 2019
@avsm
Copy link
Member Author

avsm commented Sep 3, 2019

Many thanks for identifying the issue. The workaround in dune-universe/lib-findlib@33d2057 does the trick to unblock ppxfind, by simply reversing the order of the findlib and findlib.internal dependencies. findlib.internal is now the empty library.

@rgrinberg
Copy link
Member

I believe this issue can now be closed. @avsm note the re_exports field for making findlib_public work well with (implicit_transitive_deps false).

patricoferris pushed a commit to patricoferris/lib-findlib that referenced this issue Feb 10, 2022
jonludlam pushed a commit to jonludlam/lib-findlib that referenced this issue Sep 29, 2022
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

No branches or pull requests

2 participants