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

Library variants poll: keep or drop support? #3417

Closed
ghost opened this issue Apr 23, 2020 · 11 comments
Closed

Library variants poll: keep or drop support? #3417

ghost opened this issue Apr 23, 2020 · 11 comments

Comments

@ghost
Copy link

ghost commented Apr 23, 2020

About a year ago, Dune got a new experimental feature called "library variants". This feature is an automated selection mechanism for virtual libraries. Because this feature has a high-cost in terms of code complexity and maintenance and also getting the design right turned out to be difficult, we decided to mark this feature as experimental and guard it under a (using library_variants 0.1) stanza in the dune-project file.

We would now like to move forward and either commit to long term support by promoting this feature to the vanilla language, or discard it entirely and simplify the Dune code base, making room for other exciting features.

From our point of view, it seems that this feature is unused. However, we wanted to get feedback from the community before making a decision.

So please react to this message with:

  • 👍 if you are using library variants (please also post a link to the use case if public)
  • 👎 if you are not using library variants
  • 👀 if you wanted to try library variants but didn't because they were experimental

The default choice will be to drop support for library variants.

Note that this only concerns library variants, not virtual libraries. Virtual libraries are part of the vanilla dune language and there is no plan to get rid of them at this point.

@dinosaure
Copy link

dinosaure commented Apr 23, 2020

Hi,

Currently variants are used mainly by Mirage to be able to provide 2 implementations of libraries:

Even if variants are not virtual modules, for some others projects like decompress, ocaml-git or irmin, the ability to choose a default implementation between a C implementation or an OCaml implementation fix a well-know linking error on these projects.

Currently, our fundamental use case is to properly build a project with dune and let it to choose our default implementation - but we want to keep the ability to choose which implementation we want when some people want a JavaScript support of these libraries (and to use the OCaml implementations of them).

@avsm
Copy link
Member

avsm commented Apr 23, 2020

I think there are two separate features here:

  • support for multiple implementations across a common cmi (the so-called 'linking hack')
  • support for link time selection of a variant based on string tags for dependencies

I use library variants in the ctypes dune port to support threaded/unthreaded variants, and it's also handy in checkseum and digestif and other Mirage libraries.

However, I think we can live without the link time selection feature, as users can explicitly select the dependency set in their executables pretty easily.

@samoht
Copy link
Member

samoht commented Apr 23, 2020

However, I think we can live without the link time selection feature, as users can explicitly select the dependency set in their executables pretty easily.

It's true in general, but without a default implementation we've seen many users very confused with the resulting error (as it can happen very deep in your dependency tree).

@rgrinberg
Copy link
Member

I think we can keep the default implementations without variants. I would definitely prefer to do that

@ghost
Copy link
Author

ghost commented Apr 28, 2020

Keeping the default implementation seems good. Otherwise, turning a normal library into a virtual library cannot be done without a breaking change.

@dinosaure could you show us an example of what users of checkseum and digestif have to write in their dune file when they define an executable, both when using variants and when not using variants?

@avsm, given your message I'm considering that we can safely ignore the ctypes case in the decision process here.

Finally, just a remark: a few people replied that they wanted to try library variants but didn't because they were experimental. Next time, we should think whether we can do better. For instance, in this case we could have proceeded as follow: make the variant field on libraries part of the vanilla syntax and only guard the variants field on executables by (using library-variants 0.1). This way, we could still have easily dropped support for variants and authors of libraries might have been more keen to add variants to their libraries. THe only difference is that we would have had to keep the parser for the variant field on libraries if we decided to drop support, but that's not too bad. Anyway, just food for thoughts for next time.

@dinosaure
Copy link

@dinosaure could you show us an example of what users of checkseum and digestif have to write in their dune file when they define an executable, both when using variants and when not using variants?

Currently, due to the default mechanism provided by dune, end-user needs to do nothing to rightly link with our default implementation of these libraries. However, outside the scope of dune, the user must put at the front of any others libraries the right implementation. If the user use ocamlbuild (such as MirageOS), the user must fill a _tags file with:

true: package(digestif.c)

Otherwise, it will have a link-error (reproducible with utop -require digestif and any call of a Digestif function).

@ghost
Copy link
Author

ghost commented Apr 29, 2020

Indeed. That's something that could probably be improved.

So regarding the current topic, would you be sad if we dropped support for library variants? We would keep support for default implementation though. The only thing you would have to do on your side is re-release your package without (use library-variant 0.1) and with a bump of the (lang dune x.y)

@dinosaure
Copy link

So regarding the current topic, would you be sad if we dropped support for library variants? We would keep support for default implementation though.

From what I know about variants, the default implementation is the only feature needed by this libraries, so if you keep this behavior, I will happy to give you a chance to delete variants 👍.

@ghost
Copy link
Author

ghost commented Apr 29, 2020

Perfect, that's settled then. Let's promote default_implementation to the vanilla dune language and drop support for variants.

Thanks to all who participated in this poll/discussion!

@avsm
Copy link
Member

avsm commented Apr 29, 2020

Agreed, that would work great for all the usecases I can find. Thanks @jeremiedimino!

@ghost
Copy link
Author

ghost commented May 13, 2020

This poll is now closed, I opened #3483 to track the change

@ghost ghost closed this as completed May 13, 2020
This issue was closed.
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

4 participants