Add a "plugin" linking mode for executables #3141

nojb · 2020-02-15T00:23:42Z

This PR implements #1544 (comment). Concretely, it adds a plugin linking mode for executable and executables stanzas, which produce a .cmxs (native) and .cma (bytecode) targets. By default the libraries in (libraries ...) are used for compilation, but are not linked into the resulting plugin. However, a new field is also added (statically_linked_libraries ...) (provisional name) which can specify an explicit list of libraries to be linked in.

Should solve #3136, #1544 and https://discuss.ocaml.org/t/dune-problems-using-dynlink-plugins/2874

src/dune/binary_kind.ml

rgrinberg · 2020-02-15T13:59:12Z

src/dune/exe.ml

@@ -36,6 +36,10 @@ module Linkage = struct

  let js = { mode = Byte; ext = ".bc.js"; flags = [] }

+  let is_plugin = function
+    | { ext = ".cma" | ".cmxs"; _ } -> true


Use Mode to avoid the hard coded extensions please.

I would need to extend Link_mode.t, roughly as follows:

type t = | Byte | Native | Byte_with_stubs_statically_linked_in (* This two are new *) | Plugin_native | Plugin_byte

it it worth it? By the way we also match on the extension in the case of JS compilation.

Seems like it's worth it. Although I would do Plugin of Mode.t instead.

But actually, you could also just do:

let is_plugin t = List.mem t.ext [Mode.plugin_ext Byte; Mode.plugin_ext Native]

Not very beautiful, but it's best to avoid having all these extensions everywhere.

The issue with the .js extension is indeed something that we need to fix.

I took the second suggestion, it was a less invasive change.

rgrinberg · 2020-02-15T14:01:03Z

src/dune/dune_file.ml

@@ -1565,6 +1571,10 @@ module Executables = struct
                     "See https://github.com/ocaml/dune/issues/745 for more \
                      details."
                 ]))
+    and+ statically_linked_libraries =
+      field_o "statically_linked_libraries"


Why do we need this field? Why can't we just statically link all the libraries in libraries.

The precise reason is that the plugin may require a library which is also linked into the main program. If you link the library into the plugin, then you won't be able to dynlink the plugin into the program because the modules of the library are already loaded. This is fairly common: 1) the common library may contains functionality that is used both by the main program and the plugins, or 2) it could be small utility libraries (think uutf) that happen to be used by both the main application and the plugins, etc.

Even more concretely, the typical way plugins work is that they "register" themselves with the main program via state living on some shared module. This shared module is linked to the main program and should not/could not be linked into the plugins.

talex5 · 2020-02-15T17:22:11Z

Thanks - this looks excellent! With this, the 0install dune file can be reduced to just:

[...]

let plugin_ext =
  match List.assoc "natdynlink_supported" Jbuild_plugin.V1.ocamlc_config with
  | "true" -> "cmxs"
  | _ -> "cma"
  | exception Not_found -> "cma"

let dune = {|
  (executable
    (name gui_gtk)
    (modes plugin)
    (libraries lablgtk2 lwt_glib zeroinstall support lwt lwt.unix lwt_react)
    (statically_linked_libraries lablgtk2 lwt_glib))

  (install
    (section lib)
|}
 ^ Printf.sprintf "(files gui_gtk.%s))" plugin_ext

let () =
  Jbuild_plugin.V1.send (if have_gtk then dune else "")

Is there an easy way to get the extension of the plugin, so that it can be installed? I tried public_name, but that wasn't allowed (fair enough).

nojb · 2020-02-15T19:56:07Z

Is there an easy way to get the extension of the plugin, so that it can be installed? I tried public_name, but that wasn't allowed (fair enough).

Good point. Maybe it makes sense to make plugins (with a public_name) installable to the lib directory by default? What do you think?

bobot · 2020-02-15T22:14:14Z

The MR in #3104 makes trivial to add a plugin system to an executable or library, and make it also easy to dynlink plugins (the documentation is not written yet). I'm interested in understanding how the features of this two MR intersects in the use cases.

nojb · 2020-02-16T08:11:40Z

The MR in #3104 makes trivial to add a plugin system to an executable or library, and make it also easy to dynlink plugins (the documentation is not written yet). I'm interested in understanding how the features of this two MR intersects in the use cases.

Hi @bobot, thanks for pointing me to your PR, I was not aware about it.

Just from a brief glance at the PR description, I would say that maybe there is some overlap on the "plugin" stanza that you propose. But if I understand correctly, your PR assumes plugins are split into libraries and use findlib.dynload to handle dependencies. This may be useful for some (or many) use-cases and "large" plugin systems, but sometimes one "just wants to build a plugin" without depending on findlib.dynload, not having to split the code into libraries (not so feasible when your plugins consist of arbitrary subsets of a universe of hundreds of modules) and in general making full use of the cmxs format.

This (tiny, less than 30 lines of meaningful code) PR exposes the flexibility of the the cmxs format in a "minimal" way. The approach used (extending the executable stanza) matches the underlying semantics rather well. In contrast, the scope of your PR seems to be significantly larger and geared towards specifying conventions for programs with "large" plugin systems. So I think that both PRs are orthogonal in that way. Personally, I would not be able to make use of the approach in your PR and I think both #3136 and #3141 (comment) point to a want for the kind of flexibility afforded by this PR.

nojb · 2020-02-16T08:41:24Z

Giving it some more thought, I don't see why the plugins produced by the executable stanza as in this PR could not be used by your #3104, so in that sense both PRs are complementary. Maybe even there is no need for your "plugin" stanza if it can be subsumed by the executable stanza as in this PR?

nojb · 2020-02-17T13:40:32Z

@diml do you have any comments on the proposed (statically_linked_libraries ...) ?

bobot · 2020-02-17T14:37:40Z

The plugin stanza is needed for creating the META in the site directory of the package.

I don't see why the plugins produced by the executable stanza as in this PR could not be used by your #3104, so in that sense both PRs are complementary.

It is possible, but @diml prefered that I don't add any specific plugin related fields to executable, he prefered to add a new stanza with a selected subset of the library stanza. Currently there is only the libraries field. So the new fields of this PR could go into the new plugin stanza of the other.

My worry are more at the big picture:

We can't link twice the same module, so when linking an external libraries in a cmxs are we not creating problems later? e.g the main executable start to link with the external library or another plugin link with the same external library
If Sites, locations, relocation #3104 is good enough for large plugin system, is it really too complicated for simple one?

bobot · 2020-02-17T14:38:19Z

I updated #3104 with an example so that it is easier to understand.

nojb · 2020-02-17T15:02:12Z

The plugin stanza is needed for creating the META in the site directory of the package.

I don't see why the plugins produced by the executable stanza as in this PR could not be used by your #3104, so in that sense both PRs are complementary.

It is possible, but @diml prefered that I don't add any specific plugin related fields to executable, he prefered to add a new stanza with a selected subset of the library stanza. Currently there is only the libraries field. So the new fields of this PR could go into the new plugin stanza of the other.

For me the following two requirements are absolutely crucial:

I need to be able to share modules between plugins. This is trivially doable with the executables stanza, but otherwise impossible in dune (and splitting the modules into libraries is not an option).
I need be able to link certain libraries statically into the plugins.

My worry are more at the big picture:

We can't link twice the same module, so when linking an external libraries in a cmxs are we not creating problems later? e.g the main executable start to link with the external library or another plugin link with the same external library

Indeed this can happen, and the linking mode in this PR is not meant to be a general solution to the question of plugins. If your plugins needs anything beyond what is offered in this PR then it should use a system such as the one proposed in your PR. For example, if you need to deal with non-trivial dependencies between plugins.

The point of this PR is that it is a trivial extension of the executables stanza (both syntactically and semantically), that allows access to features of the compiler toolchain that are not accessible otherwise and are useful. It is not meant to respond to the needs of full-featured plugin systems, which have other specific needs and constraints.

If Sites, locations, relocation #3104 is good enough for large plugin system, is it really too complicated for simple one?

See my two points above; in principle I am happy with any system that can provide them... but I am very partial to the approach in this PR due to its simplicity.

bobot · 2020-02-17T17:00:03Z

Indeed this can happen, and the linking mode in this PR is not meant to be a general solution to the question of plugins. If your plugins needs anything beyond what is offered in this PR then it should use a system such as the one proposed in your PR. For example, if you need to deal with non-trivial dependencies between plugins.

To be clear it is not just for non-trivial dependencies between plugins, it is just for behind able to reliably dynlink two independently created plugins.

From #1544 what I understood of your use case and so what I think is offered by this PR is to be able on windows to create a relocatable directory which contains an executable and different plugins. My goal in #3104 is also to support this use case (which is an important one). Everything is in place, for example findlib is abstracted away and so can be made relocatable, and also the choice of localisation is deferred at installation time so we can add a dune install --relocatable option.

The PR #1544 is going to be an experimental feature at first, so this PR could also be one and we could see how the use cases are supported. However I would be happy to discuss your use case in #1544 so that it could be fully supported.

ghost · 2020-02-19T13:32:39Z

Hey, I'll let you discuss whether we need both PRs or whether one is enough, bust just a comment on linking mode vs stanza. For me, the main difference is the public name. If we attach a name to a plugin and it lives in its own namespace, then it doesn't seem right to me to use the executable stanza and it seems better to me to have a dedicated plugin stanza. If we just want to produce a file and we let the user decide what they want to do with the file, a linking mode seems just fine.

@diml do you have any comments on the proposed (statically_linked_libraries ...) ?

That seems fine, but it could be mis-interpreted for plain executables. i.e. intuitively, I would interpret it as: these libraries are statically linked and the other ones are dynamically linked, which is not the case. What about libraries_embed_in_plugin? BTW, what happens for libraries in between the direct dependencies and the statically linked ones? For instance, if a depends on b, b depends on c and we have:

(executable
 ...
 (libraries a)
 (statically_linked_libries c))

is b statically linked or not? I can't think of a scenario where not statically linking b would work. i.e. either b is linked in the main program and in this case c as well, or b isn't and it needs to be linked in the plugin.

ghost · 2020-02-20T17:54:18Z

We discussed this in the meeting today, and everyone agreed that having both this PR and #3104 is fine as they serve different purposes. #3104 is a full key in hand system, which will allow users to very quickly get started and add a plugin system to their applications, which is a great feature to add. However, we are adventuring in the runtime territory, i.e. we are not only controlling aspects of how the software is built, but also how it will behave at runtime and it's likely that it won't work for everybody. Indeed, some developers might want to retain control over how their plugins are packaged and distributed and some might simply not have a choice. And if you cannot use #3104 and you use Dune, you are a bit on your own. This PR will bridge the gap and provide a more manual but still manageable solution.

nojb · 2020-02-20T20:28:29Z

Is there an easy way to get the extension of the plugin, so that it can be installed? I tried public_name, but that wasn't allowed (fair enough).

I added %{ext_plugin} for this. It is equal to .cmxs if natdynlink is supported, otherwise .cma.

nojb · 2020-02-20T23:34:01Z

src/dune/exe.ml

+                Result.map link_time_code_gen
+                  ~f:(fun { Link_time_code_gen.to_link; force_linkall } ->
+                    let to_link =
+                      let rec depends_on lib0 lib =
+                        Lib_name.equal lib0 (Lib.name lib)
+                        || List.exists (Lib.requires_exn lib)
+                             ~f:(depends_on lib0)
+                      in
+                      List.filter_map to_link ~f:(function
+                        | Module _ as x -> Some x
+                        | Lib lib as x ->
+                          if
+                            List.exists embed_in_plugin_libraries
+                              ~f:(fun (_, lib0) -> depends_on lib0 lib)
+                          then
+                            Some x
+                          else
+                            None)
+                    in
+                    { Link_time_code_gen.to_link; force_linkall })


This code filters out the library dependencies which are not inverse dependencies of the libraries listed in the (embed_in_plugin_libraries ...) field. The remaining libraries are linked into the plugin. I would appreciate a second pair of eyes on this code.

One thing that isn't clear to me is:

if a library is mentioned in (embed_in_plugin_libraries ..), do we guarantee in will be linked in the plugin?

This code seems to suggest we also require that the library be a (compile or link?) dependency of the executable. But the name of the field suggests that the libraries should be unconditionally linked.

I updated the PR along the lines of your suggestion -- now the libraries listed on that field are unconditionally linked, no questions asked. It does not try to be clever, just simple to understand and predictable.

rgrinberg · 2020-02-21T09:58:42Z

src/dune/pform.ml

@@ -241,6 +241,13 @@ module Map = struct
      let model = Ocaml_config.model context.ocaml_config in
      let system = Ocaml_config.system context.ocaml_config in
      let version_string = Ocaml_config.version_string context.ocaml_config in
+      let ext_plugin =
+        Mode.plugin_ext
+          ( if Ocaml_config.natdynlink_supported context.ocaml_config then


We have this Dynlink_supported module that's supposed to be used for making these checks. I don't quite remember what its purpose is but it's worth looking at to make sure we aren't missing anything.

I just looked, I think it is OK like this. The Dynlink_supported module seems to be used to handle (no_dynlink). I could use Dynlink_supported.By_the_os.get instead, but it amounts to the same thing, and I think it ends up obfuscating things more than anything else.

rgrinberg · 2020-02-21T10:57:55Z

Agreed. I never liked that thing. So what happens when your plugin executable depends on a library with `no_dynlink`? Should this be an issue?

…

On Feb 21, 2020, 10:28 AM +0000, Nicolás Ojeda Bär ***@***.***>, wrote: @nojb commented on this pull request. In src/dune/pform.ml: > @@ -241,6 +241,13 @@ module Map = struct let model = Ocaml_config.model context.ocaml_config in let system = Ocaml_config.system context.ocaml_config in let version_string = Ocaml_config.version_string context.ocaml_config in + let ext_plugin = + Mode.plugin_ext + ( if Ocaml_config.natdynlink_supported context.ocaml_config then I just looked, I think it is OK like this. The Dynlink_supported module seems to be used to handle (no_dynlink). I could use Dynlink_supported.By_the_os.get instead, but it amounts to the same thing, and I think it ends up obfuscating things more than anything else. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

rgrinberg · 2020-02-21T16:23:44Z

doc/dune-files.rst

+  to link statically when using ``plugin`` linking mode. By default, no
+  libraries are linked in.  Any library which is a dependency of the executable
+  that is an inverse dependency of one the libraries in ``<library-list>`` will
+  also be statically linked.


This wording is a bit confusing to me.

Any library which is a dependency of the executable

Dependency in the compilation or linking sense?

that is an inverse dependency of one the libraries in ```

So if we have:

(libraries foo baz) (embed_in_plugin_libraries bar)

and foo depends on bar, we should link in foo and bar?

What about the transitive dependencies of bar?

I updated the doc according to the new, simpler implementation.

rgrinberg · 2020-03-05T17:25:20Z

src/dune/compilation_context.ml

+    lazy
+      (Result.List.map
+         ~f:(fun (loc, name) ->
+           match Lib.DB.find libs name with


What about using Lib.DB.resolve?

src/dune/lib.mli

rgrinberg · 2020-03-05T17:27:17Z

src/dune/pform.ml

@@ -338,6 +345,7 @@ module Map = struct
      ; ("ext_lib", string context.lib_config.ext_lib)
      ; ("ext_dll", string context.lib_config.ext_dll)
      ; ("ext_exe", string ext_exe)
+      ; ("ext_plugin", since ~version:(2, 4) (Var.Values [ String ext_plugin ]))


We should reflect this variable in the change log as well.

ghost · 2020-03-05T22:44:48Z

doc/dune-files.rst

@@ -668,6 +668,12 @@ Executables can also be linked as object or shared object files. See
  pulled in. This field is available since the 2.0 version of the dune
  language.

+- ``(embed_in_plugin_libraries <library-list>)`` especifies a list of libraries


Suggested change

- ``(embed_in_plugin_libraries <library-list>)`` especifies a list of libraries

- ``(embed_in_plugin_libraries <library-list>)`` specifies a list of libraries

Also adds a %{ext_plugin} variable Signed-off-by: Nicolás Ojeda Bär <[email protected]>

rgrinberg · 2020-03-06T13:02:04Z

The issues pointed out in the review were all minor so I just fixed them myself. Thanks @nojb!

@NathanReb

…lugin, dune-private-libs and dune-glob (2.4.0) CHANGES: - Add `mdx` extension and stanza version 0.1 (ocaml/dune#3094, @NathanReb) - Allow to make Odoc warnings fatal. This is configured from the `(env ...)` stanza. (ocaml/dune#3029, @Julow) - Fix separate compilation of JS when findlib is not installed. (ocaml/dune#3177, @nojb) - Add a `dune describe` command to obtain the topology of a dune workspace, for projects such as ROTOR. (ocaml/dune#3128, @diml) - Add `plugin` linking mode for executables and the `(embed_in_plugin_libraries ...)` field. (ocaml/dune#3141, @nojb) - Add an `%{ext_plugin}` variable (ocaml/dune#3141, @nojb) - Dune will no longer build shared objects for stubs if `supports_shared_libraries` is false (ocaml/dune#3225, fixes ocaml/dune#3222, @rgrinberg) - Fix a memory leak in the file-watching mode (`dune build -w`) (ocaml/dune#3220, @snowleopard and @aalekseyev)

nojb · 2020-03-07T05:40:37Z

The issues pointed out in the review were all minor so I just fixed them myself. Thanks @nojb!

Thanks a lot @rgrinberg, much appreciated! (Sorry, I was on holidays this week and only saw the comments now.)

nojb mentioned this pull request Feb 15, 2020

Unable to specify a dynlink plugin build #3136

Closed

nojb force-pushed the exe_plugins branch 2 times, most recently from dffb8e2 to be8bea2 Compare February 15, 2020 11:10

rgrinberg reviewed Feb 15, 2020

View reviewed changes

src/dune/binary_kind.ml Outdated Show resolved Hide resolved

rgrinberg requested changes Feb 15, 2020

View reviewed changes

nojb force-pushed the exe_plugins branch from 0533aff to 8f09230 Compare February 15, 2020 19:53

nojb force-pushed the exe_plugins branch from 8f09230 to c44437d Compare February 16, 2020 08:17

nojb requested a review from emillon as a code owner February 16, 2020 08:17

nojb force-pushed the exe_plugins branch 2 times, most recently from ed891be to 695f4b6 Compare February 16, 2020 08:20

nojb force-pushed the exe_plugins branch 2 times, most recently from a5ad9ab to d5e858a Compare February 17, 2020 13:38

nojb force-pushed the exe_plugins branch 2 times, most recently from f3ece0b to f4f5aa9 Compare February 20, 2020 17:16

nojb commented Feb 20, 2020

View reviewed changes

rgrinberg reviewed Feb 21, 2020

View reviewed changes

nojb force-pushed the exe_plugins branch from 8d2b759 to af628bf Compare March 2, 2020 01:07

rgrinberg reviewed Mar 5, 2020

View reviewed changes

src/dune/lib.mli Outdated Show resolved Hide resolved

rgrinberg reviewed Mar 5, 2020

View reviewed changes

ghost reviewed Mar 5, 2020

View reviewed changes

Add 'plugin' linking mode for executables

26d097f

Also adds a %{ext_plugin} variable Signed-off-by: Nicolás Ojeda Bär <[email protected]>

rgrinberg force-pushed the exe_plugins branch from d258110 to 26d097f Compare March 6, 2020 13:01

rgrinberg merged commit 055e267 into ocaml:master Mar 6, 2020

rgrinberg mentioned this pull request Mar 6, 2020

[new release] dune-build-info, dune, dune-configurator, dune-action-plugin, dune-private-libs and dune-glob (2.4.0) ocaml/opam-repository#15956

Merged

yawaramin mentioned this pull request Apr 21, 2020

Investigate server hot-reload on recompile yawaramin/re-web#25

Open

Et7f3 mentioned this pull request Jun 7, 2020

build: Upgrade dune file. revery-ui/revery#903

Open

gasche mentioned this pull request Mar 27, 2021

confused by Dune foreign_libraries doc: how to define a C library used by C stubs? #4409

Open

ivg mentioned this pull request Aug 18, 2022

The embed_in_plugin_libraries stanza fails with libraries requiring link-time code #6086

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a "plugin" linking mode for executables #3141

Add a "plugin" linking mode for executables #3141

nojb commented Feb 15, 2020

rgrinberg Feb 15, 2020

nojb Feb 15, 2020

rgrinberg Feb 15, 2020

nojb Feb 17, 2020

rgrinberg Feb 15, 2020

nojb Feb 15, 2020

nojb Feb 15, 2020

talex5 commented Feb 15, 2020

nojb commented Feb 15, 2020

bobot commented Feb 15, 2020 •

edited

Loading

nojb commented Feb 16, 2020

nojb commented Feb 16, 2020 •

edited

Loading

nojb commented Feb 17, 2020

bobot commented Feb 17, 2020

bobot commented Feb 17, 2020

nojb commented Feb 17, 2020

bobot commented Feb 17, 2020

ghost commented Feb 19, 2020

ghost commented Feb 20, 2020

nojb commented Feb 20, 2020

nojb Feb 20, 2020

rgrinberg Feb 21, 2020

nojb Mar 2, 2020

rgrinberg Feb 21, 2020

nojb Feb 21, 2020

rgrinberg commented Feb 21, 2020 via email

rgrinberg Feb 21, 2020

nojb Mar 2, 2020

rgrinberg Mar 5, 2020

rgrinberg Mar 5, 2020

ghost Mar 5, 2020

rgrinberg commented Mar 6, 2020

nojb commented Mar 7, 2020

	- ``(embed_in_plugin_libraries <library-list>)`` especifies a list of libraries
	- ``(embed_in_plugin_libraries <library-list>)`` specifies a list of libraries

Add a "plugin" linking mode for executables #3141

Add a "plugin" linking mode for executables #3141

Conversation

nojb commented Feb 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

talex5 commented Feb 15, 2020

nojb commented Feb 15, 2020

bobot commented Feb 15, 2020 • edited Loading

nojb commented Feb 16, 2020

nojb commented Feb 16, 2020 • edited Loading

nojb commented Feb 17, 2020

bobot commented Feb 17, 2020

bobot commented Feb 17, 2020

nojb commented Feb 17, 2020

bobot commented Feb 17, 2020

ghost commented Feb 19, 2020

ghost commented Feb 20, 2020

nojb commented Feb 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgrinberg commented Feb 21, 2020 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgrinberg commented Mar 6, 2020

nojb commented Mar 7, 2020

bobot commented Feb 15, 2020 •

edited

Loading

nojb commented Feb 16, 2020 •

edited

Loading