ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.61k stars 400 forks source link

[RFC] Allow to pass more information to ppx rewriters #2020

Closed ghost closed 4 years ago

ghost commented 5 years ago

This proposal describes a way to pass more information from dune to ppx rewriters.

Description

We extend the ppx_rewriter and ppx_deriver library kinds to take a cookies parameter. This parameters describe a list of cookies that dune should pass to the ppx rewriter. The argument of cookies is a list of cookie definition of the form (<name> <string-with-variables>), such as (profile %{profile}).

For instance:

(library
 (name ppx_inline_test)
 (kind (ppx_rewriter (cookies (profile "\"%{profile\""})))))

When invoking the final ppx driver that links all the ppx rewriter, dune will collect all the cookies requested by the various ppx rewriters and pass them via the command line as follow:

$ ppx.exe --cookie profile="release" ...

When two ppx rewriters request the same cookie, the associated value must be equal. Otherwise dune will error out.

To match the current behavior, dune will also add the following cookie to this list: (library-name \"%{library-name}\").

Khady commented 5 years ago

I feel that using the profile to encode information is a bad pattern. We have for example multiple profiles that are equivalent to release. The ppx has no way to guess that those other profiles should be treated like release.

ghost commented 5 years ago

That makes sense. I suppose we could have a new parameter drop-inline-tests. It would default to (= %{profile} release), but you could manually set it for other profiles.

In that case, we don't even need to implement the full generic proposal right now, we can only pass drop-inline-tests manually as we do for library-name and leave the general solution for later since it is a bit more work.

rgrinberg commented 4 years ago

I thought about this a bit more, and I'm wondering if this is at all necessary with link time code gen. We can very easily create a little dune library that dune can generate on every build with information such as profile, context etc. That seems like a useful feature for other tools invoked at build time as well.

ghost commented 4 years ago

That's a good point.

The feature described in this issues has been added to dune BTW, so we can safely close it.

rgrinberg commented 4 years ago

In retrospect, using link time code gen is a bad idea here. We don't want to rebuild the ppx binary just because context/profile changed. I'd much prefer if there was a simple way to pass these values at runtime.