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

Dune Site Plugins do not work in toplevel #6081

Closed
ivg opened this issue Aug 17, 2022 · 9 comments · Fixed by #6082 or ocaml/opam-repository#23814
Closed

Dune Site Plugins do not work in toplevel #6081

ivg opened this issue Aug 17, 2022 · 9 comments · Fixed by #6082 or ocaml/opam-repository#23814
Milestone

Comments

@ivg
Copy link
Member

ivg commented Aug 17, 2022

Expected Behavior

It should be possible to load plugins in the toplevel.

Actual Behavior

There are actually two independent problems.

Problem 1

First, the dune-site.plugins library is incomplete as it misses the Dune_site_data module.

Reference to undefined global `Dune_site__Dune_site_data'

Workaround/Solution

This module is generated at link time when an executable is linked, so to make it work, we can link our own toplevel, and we just need to add dune-site.plugins to the libraries stanza that will force dune to generate this file. Once, the toplevel is linked (I am just using let () = Utop_main () but one could run the vanilla toplevel using Topmain.main ().

While this problem has a workaround it would be nice to have the ability to generate self-contained libraries that are loadable in runtime. I didn't test it, but it might be that using the shared_object mode of an executable will build such a library. I will update the report if it will work.

Problem 2

Now to the main problem. When we try loading plugins from our toplevel, every plugin fails with,

(Invalid_argument
 "The dynlink.cma library cannot be used inside the OCaml toplevel")

Indeed, dynlink is not supposed to work inside the toplevel, which provides its own dynamic loading facilities. Therefore, the plugin system of dune has to check if we are in the interactive mode (in the toplevel) and use Topdirs.dir_load instead of Dynlink.loadfile.

Solution

I have a solution that I will PR in a second (the link should appear below). It has some things to discuss. Especially from the usability point of view. But at least it allows me to load all BAP plugins in a toplevel without any issues.

@ivg
Copy link
Member Author

ivg commented Aug 18, 2022

Update on Problem 1

I was trying to build a self-contained .cma plugin that could be loaded into OCaml top-level as a solution to a more specific problem - trying to load a library that depends on link-time generated code. After hot-fixing #6086, it turned out that linking the link-time generated code into the library, which depends on a library that requires this link-time generated code, doesn't help. Obviously, since the dependency is loaded before our plugin so we fail long before we load the plugin with the missing code. I tried to build a fully self-contained plugin, but the embed_in_plugin_libraries stanza neither sorts the specified libraries in the topological order (which looks like a bug)1 nor it walks through the transitive closure of dependencies. The latter might be the desired behavior, but in general it makes this feature hardly usable. E.g., I ended up manually discovering the depdencies, so my link list looks like this,

 (embed_in_plugin_libraries
   dynlink
   ordering
   dyn.pp
   dyn
   stdune.filesystem_stubs stdune.csexp
   stdune
   dune-private-libs.dune-section
   dune-private-libs.dune_re
   dune-private-libs.meta_parser
   dune-private-libs.ocaml-config
   dune-site.private
   dune-site dune-site.toplevel dune-site.plugins findlib.dynload
   monads
   bap-plugins
   bap-bundle bap-main bap)

Which is obviously counter-productive and should never be done in real life, so I forfeited this idea. And this brings us back to the main issue - is that libraries that require link-time code do not work in OCaml toplevel. Maybe a simpler solution would be to provide for such libraries a sublibrary <package>.top that will have the generated code already linked? And the instruct ocamlfind via meta to load this library, when toplevel is used? This is quite a killer feature, to be able to use OCaml toplevel and denying it to libraries that use dune-sites is a big problem. Or maybe I am missing some obvious solution?


1) So the order in which libraries are enumerated matters, which is counter-intuitive, people would expect that it is the same as in the libraries stanza, i.e., that they can specify it in the arbitrary order. If you think that this is a bug, I can open a new issue.

ivg added a commit to ivg/bap that referenced this issue Aug 18, 2022
Both require ocaml/dune#6082
And bap.top only works inside `baptop` but not vanila
`utop` or `ocaml`, see ocaml/dune#6081
@nojb
Copy link
Collaborator

nojb commented Aug 18, 2022

cc @bobot

@ejgallego
Copy link
Collaborator

@ivg not sure if that's your problem but after OCaml 4.08, the linker will reject to link twice a module, so indeed the loader itself (as in Fl_dynload) must keep track of the deps and loading order as to ensure diamonds in the dep chain are properly shared among all possible loading setups.

@ejgallego
Copy link
Collaborator

The classical example is plugins A B both depending on plugin C. C must be loaded once, thus you can't link C on neither of them.

@ejgallego
Copy link
Collaborator

Before 4.08 OCaml allowed this so a lot of people got away with doubly-linking.

@ivg
Copy link
Member Author

ivg commented Aug 18, 2022

@ejgallego, not it is not the problem. We're maintaining our own plugin system for nearly a decade, back in the times when dynlink was unsound, so am pretty aware of it). But this issue has nothing to do with Dynlink or Fl_dynload, but with a different dynamic loading facility that is provided by the top-level. Dynlink and findlib.dynload work only in native or bytecode, but not in the top-level mode.

@nojb
Copy link
Collaborator

nojb commented Aug 19, 2022

Dynlink and findlib.dynload work only in native or bytecode, but not in the top-level mode.

Sorry for the naïve question, but couldn't #require be used to load a plugin into the toplevel?

@ivg
Copy link
Member Author

ivg commented Aug 19, 2022

Sorry for the naïve question, but couldn't #require be used to load a plugin into the toplevel?

Well, when a dune plugin is installed into a path that is known by ocamlfind and if the plugin name is known, then it could be manually loaded with the #require directive (which is provided by ocamlfind). Of course, it won't work from other sites (like when the plugin is not installed). And it will load only one plugin, not a hundred plugins. And this is if we will put aside problem 1. With problem 1 you can't load in top-level (using #require or #load, doesn't matter) any library that depends on link-time generated code because those libraries have unresolved symbols (that are supposed to be generated during an executable linking). This is another big problem and unlike problem 2, which is solvable (see the #6082) I don't have a solution for this problem.

ivg added a commit to ivg/bap that referenced this issue Aug 19, 2022
After we switched to OCaml 4.08 and above, we trashed custom unit
tracking from our plugin system and started to rely on the Dynlink
facility, provided by OCaml, that offers precise unit
tracking. However, Dynlink is not allowed inside OCaml Toplevel, which
wasn't noticed, because it wasn't properly initialized (though
apparently it worked fine for our needs). But after 4.12
(see ocaml/ocaml#9790) it was properly
initialized so we started to get "The dynlink.cma library cannot be used
inside the OCaml toplevel" messages when trying to use `baptop` or
`bap.top`. A quick-fix (BinaryAnalysisPlatform#1541) did only worse as it disabled any unit
tracking and enabled double-loading of plugins and libraries (which is
perfectly allowed in OCaml toplevel and is not a bug, as it was in
regular OCaml runtime). Of course, double-loading could crash bap as
many of bap libraries have their own state, which got reset after
reloading.

This PR restores the old findlib-based unit tracker. This tracker is
not precise and may still miss loaded units. For example, if you load
bap manually and the do `#require "bap.top"` it will reload bap (which
should be a problem, but who knows). With that said, we had used this
tracker for many years, up until BAP 2.4.0, so I do have some trust in
it. Also, I don't want to invest more time in this issue as we are in
the process of switching to dune and thhe dune plugin system (that has
its own unit tracking system, which is precise). To be honest, dune
plugins also do not work in toplevel right now, but this is a
developing story (ocaml/dune#6081) and I
hope that eventually we will have it working.
ivg added a commit to BinaryAnalysisPlatform/bap that referenced this issue Aug 19, 2022
After we switched to OCaml 4.08 and above, we trashed custom unit
tracking from our plugin system and started to rely on the Dynlink
facility, provided by OCaml, that offers precise unit
tracking. However, Dynlink is not allowed inside OCaml Toplevel, which
wasn't noticed, because it wasn't properly initialized (though
apparently it worked fine for our needs). But after 4.12
(see ocaml/ocaml#9790) it was properly
initialized so we started to get "The dynlink.cma library cannot be used
inside the OCaml toplevel" messages when trying to use `baptop` or
`bap.top`. A quick-fix (#1541) did only worse as it disabled any unit
tracking and enabled double-loading of plugins and libraries (which is
perfectly allowed in OCaml toplevel and is not a bug, as it was in
regular OCaml runtime). Of course, double-loading could crash bap as
many of bap libraries have their own state, which got reset after
reloading.

This PR restores the old findlib-based unit tracker. This tracker is
not precise and may still miss loaded units. For example, if you load
bap manually and the do `#require "bap.top"` it will reload bap (which
should be a problem, but who knows). With that said, we had used this
tracker for many years, up until BAP 2.4.0, so I do have some trust in
it. Also, I don't want to invest more time in this issue as we are in
the process of switching to dune and thhe dune plugin system (that has
its own unit tracking system, which is precise). To be honest, dune
plugins also do not work in toplevel right now, but this is a
developing story (ocaml/dune#6081) and I
hope that eventually we will have it working.
ivg added a commit to ivg/bap that referenced this issue Aug 31, 2022
Both require ocaml/dune#6082
And bap.top only works inside `baptop` but not vanila
`utop` or `ocaml`, see ocaml/dune#6081
@ivg
Copy link
Member Author

ivg commented Aug 31, 2022

Also, using dune ocaml top as well reveals problem 1.

$ ocaml
OCaml version 4.14.0
Enter #help;; for help.

# #use_output "dune ocaml top";;
File "(command-output)", line 1:          
Error: Reference to undefined global `Dune_site__Dune_site_data'

ivg added a commit to ivg/bap that referenced this issue Aug 31, 2022
Both require ocaml/dune#6082
And bap.top only works inside `baptop` but not vanila
`utop` or `ocaml`, see ocaml/dune#6081
ivg added a commit to ivg/bap that referenced this issue Aug 31, 2022
Both require ocaml/dune#6082
And bap.top only works inside `baptop` but not vanila
`utop` or `ocaml`, see ocaml/dune#6081
@rgrinberg rgrinberg added this to the 3.8.0 milestone Mar 7, 2023
emillon added a commit to emillon/opam-repository that referenced this issue Apr 18, 2023
CHANGES:

- When a rule's action is interrupted, delete any leftover directory targets.
  This is consistent with how we treat file targets. (@rgrinberg, 7564)

- Fix plugin loading with findlib. The functionality was broken in 3.7.0.
  (ocaml/dune#7556, @anmonteiro)

- Introduce a `public_headers` field on libraries. This field is like
  `install_c_headers`, but it allows to choose the extension and choose the
  paths for the installed headers. (ocaml/dune#7512, @rgrinberg)

- Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes
  ocaml/dune#1701, @rgrinberg, @anmonteiro)

- Resolve `ppx_runtime_libraries` in the target context when cross compiling
  (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro)

- Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary  (ocaml/dune#7469, fixes
  ocaml/dune#2572, @anmonteiro)

- Preliminary support for Coq compiled intefaces (`.vos` files) enabled via
  `(mode vos)` in `coq.theory` stanzas. This can be used in combination with
  `dune coq top` to obtain fast re-building of dependencies (with no checking
  of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre)

- Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD`
  (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg)

- Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro,
  @rizo)

- Find `pps` dependencies in the host context when cross-compiling,  (ocaml/dune#7410,
  fixes ocaml/dune#4156, @anmonteiro)

- Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395
  @rgrinberg, @jchavarri)

- `dune coq top` now correctly respects the project root when called from a
  subdirectory. However, absolute filenames passed to `dune coq top` are no
  longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and
  @Alizter)

- Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380,
  fixes ocaml/dune#7355, @Alizter)

- RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes
  ocaml/dune#6879, @rgrinberg)

- Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg)

- Bootstrap: remove reliance on shell. Previously, we'd use the shell to get
  the number of processors. (ocaml/dune#7274, @rgrinberg)

- Bootstrap: correctly detect the number of processors by allowing `nproc` to be
  looked up in `$PATH` (ocaml/dune#7272, @Alizter)

- Speed up file copying on macos by using `clonefile` when available
  (@rgrinberg, ocaml/dune#7210)

- Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081,
  @ivg, @richardlford)

- Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188,
  @Alizter)

- Speed up rule generation for libraries and executables with many modules
  (ocaml/dune#7187, @jchavarri)

- Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford)

- Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix
  ocaml/dune#7184, @rgrinberg)

- Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133)

- Non-user proccesses such as version control or config checking are now run
  silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter)

- Add the `--display-separate-messages` flag to separate the error messages
  produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope)

- Accept the Ordered Set Language for the `modes` field in `library` stanzas
  (ocaml/dune#6611, @anmonteiro).

- dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes
  ocaml/dune#7106, @Alizter)

- Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not
  be used as dependencies of libraries in the workspace (eg when compiling to
  bytecode and/or Javascript).  This is now fixed. (ocaml/dune#7151, @nojb)

- Allow the main module of a library with `(stdlib ...)` to depend on other
  libraries (ocaml/dune#7154, @anmonteiro).

- Bytecode executables built for JSOO are linked with `-noautolink` and no
  longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb)

- Added a new user action `(concurrent )` which is like `(progn )` but runs the
  actions concurrently. (ocaml/dune#6933, @Alizter)

- Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas
  (ocaml/dune#7139, @anmonteiro).

- Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo)

- Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416,
  @nojb)

- Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445,
  @Leonidas-from-XIV)

- Always include `opam` files in the generated `.install` file. Previously, it
  would not be included whenever `(generate_opam_files true)` was set and the
  `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg)
emillon added a commit to emillon/opam-repository that referenced this issue May 23, 2023
CHANGES:

- Fix string quoting in the json file written by `--trace-file` (ocaml/dune#7773,
  @rleshchinskiy)

- Read `pkg-config` arguments from the `PKG_CONFIG_ARGN` environment variable
  (ocaml/dune#1492, ocaml/dune#7734, @anmonteiro)

- Correctly set `MANPATH` in `dune exec`. Previously, we would use the `bin/`
  directory of the context. (ocaml/dune#7655, @rgrinberg)

- Allow overriding the `ocaml` binary with findlib configuration (ocaml/dune#7648,
  @rgrinberg)

- merlin: ignore instrumentation settings for preprocessing. (ocaml/dune#7606, fixes
  ocaml/dune#7465, @Alizter)

- When a rule's action is interrupted, delete any leftover directory targets.
  This is consistent with how we treat file targets. (ocaml/dune#7564, @rgrinberg)

- Fix plugin loading with findlib. The functionality was broken in 3.7.0.
  (ocaml/dune#7556, @anmonteiro)

- Introduce a `public_headers` field on libraries. This field is like
  `install_c_headers`, but it allows to choose the extension and choose the
  paths for the installed headers. (ocaml/dune#7512, @rgrinberg)

- Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes
  ocaml/dune#1701, @rgrinberg, @anmonteiro)

- Add a `coqdoc_flags` field to the `coq.theory` stanza allowing the user to
  pass extra arguments to `coqdoc`. (ocaml/dune#7676, fixes ocaml/dune#7954 @Alizter)

- Resolve `ppx_runtime_libraries` in the target context when cross compiling
  (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro)

- Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary  (ocaml/dune#7469, fixes
  ocaml/dune#2572, @anmonteiro)

- Modules that were declared in `(modules_without_implementation)`,
  `(private_modules)` or `(virtual_modules)` but not declared in `(modules)`
  will cause Dune to emit a warning which will become an error in 3.9. (ocaml/dune#7608,
  fixes ocaml/dune#7026, @Alizter)

- Preliminary support for Coq compiled intefaces (`.vos` files) enabled via
  `(mode vos)` in `coq.theory` stanzas. This can be used in combination with
  `dune coq top` to obtain fast re-building of dependencies (with no checking
  of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre)

- Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD`
  (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg)

- Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro,
  @rizo)

- Find `pps` dependencies in the host context when cross-compiling,  (ocaml/dune#7410,
  fixes ocaml/dune#4156, @anmonteiro)

- Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395
  @rgrinberg, @jchavarri)

- Dune can now detect Coq theories from outside the workspace. This allows for
  composition with installed theories (not necessarily installed with Dune).
  (ocaml/dune#7047, @Alizter, @ejgallego)

- `dune coq top` now correctly respects the project root when called from a
  subdirectory. However, absolute filenames passed to `dune coq top` are no
  longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and
  @Alizter)

- Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380,
  fixes ocaml/dune#7355, @Alizter)

- RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes
  ocaml/dune#6879, @rgrinberg)

- Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg)

- Bootstrap: remove reliance on shell. Previously, we'd use the shell to get
  the number of processors. (ocaml/dune#7274, @rgrinberg)

- Bootstrap: correctly detect the number of processors by allowing `nproc` to be
  looked up in `$PATH` (ocaml/dune#7272, @Alizter)

- Speed up file copying on macos by using `clonefile` when available
  (@rgrinberg, ocaml/dune#7210)

- Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081,
  @ivg, @richardlford)

- Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188,
  @Alizter)

- Speed up rule generation for libraries and executables with many modules
  (ocaml/dune#7187, @jchavarri)

- Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford)

- Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix
  ocaml/dune#7184, @rgrinberg)

- Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133)

- Non-user proccesses such as version control or config checking are now run
  silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter)

- Add the `--display-separate-messages` flag to separate the error messages
  produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope)

- Accept the Ordered Set Language for the `modes` field in `library` stanzas
  (ocaml/dune#6611, @anmonteiro).

- dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes
  ocaml/dune#7106, @Alizter)

- Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not
  be used as dependencies of libraries in the workspace (eg when compiling to
  bytecode and/or Javascript).  This is now fixed. (ocaml/dune#7151, @nojb)

- Allow the main module of a library with `(stdlib ...)` to depend on other
  libraries (ocaml/dune#7154, @anmonteiro).

- Bytecode executables built for JSOO are linked with `-noautolink` and no
  longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb)

- Added a new user action `(concurrent )` which is like `(progn )` but runs the
  actions concurrently. (ocaml/dune#6933, @Alizter)

- Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas
  (ocaml/dune#7139, @anmonteiro).

- Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo)

- Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416,
  @nojb)

- Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445,
  @Leonidas-from-XIV)

- Always include `opam` files in the generated `.install` file. Previously, it
  would not be included whenever `(generate_opam_files true)` was set and the
  `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg)

- Fix regression where Merlin was unable to handle filenames with uppercase
  letters under Windows. (ocaml/dune#7577, @nojb)

- On nix+macos, pass `-f` to the codesign hook to avoid errors when the binary
  is already signed (ocaml/dune#7183, fixes ocaml/dune#6265, @greedy)

- Fix bug where RPC clients built with dune-rpc-lwt would crash when closing
  their connection to the server (ocaml/dune#7581, @gridbugs)

- Introduce mdx stanza 0.4 requiring mdx >= 2.3.0 which updates the default
  list of files to include `*.mld` files (ocaml/dune#7582, @Leonidas-from-XIV)

- Fix RPC server on Windows (used for OCaml-LSP). (ocaml/dune#7666, @nojb)

- Coq language versions less 0.8 are deprecated, and will be removed
  in an upcoming Dune version. All users are required to migrate to
  `(coq lang 0.8)` which provides the right semantics for theories
  that have been globally installed, such as those coming from opam
  (@ejgallego, @Alizter)

- Bump minimum version of the dune language for the melange syntax extension
  from 3.7 to 3.8 (ocaml/dune#7665, @jchavarri)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants