ortac-dune plugin

[n-osborne]

Plugin to generate dune rules for other plugins.

[n-osborne]

Still in draft as the plugin does not load...

[n-osborne]

I've put a no-changelog-needed label as this is a new not-yet-released-plugin.

[shym]

Thank you for that!
A few remarks:

• I think I would find it nicer for ortac dune qcheck-stm to expect the same command-line arguments as ortac qcheck-stm plus the necessary addition for the generation of rules; in particular, that would mean expect --include=... instead of a positional argument. Would it be possible to share the code creating those arguments, maybe?
• I’d prefer not to see commented rules in plugins/dune-rules/src/dune.
• The plugin generates a rule calling ortac_dune: I imagine it should be ortac dune?

[n-osborne]

Thanks for these remarks.
1 and 2 are just relics from the past, I'll remove them.
I also agree with 3. I was wondering about a better user interface (there are a lot of arguments to give).
The --include=... is easy to share and use in this plugin, and it makes a lot of sense.
I am wondering about the output files, because we have two of them: the output_file option to give ortac qcheck-stm and the one to write the dune rules in.
On the one hand, it doesn't make a lot of sense (I think) to have dune rules without an output file for the generated OCaml code: we want to be able to run the generated tests.
On the other hand, it is easy to have to optional argument for output files: the --output=... from ortac-core for the generated OCaml code and something like --dune-output=... for the generated rules.

[n-osborne]

I'm also thinking about making the package argument optional.

[shym]

Good point, I was thinking about ortac qcheck-stm optional arguments which should stay optional, but not about the ones that should be required 😅

[shym]

Do you know whether it would be possible to make an interface such as:

ortac dune -o dune.inc qcheck-stm -o ...

with cmdliner?

[n-osborne]

I'm guessing it is not.
Here qcheck-stm is a subcommand of (the ortac's subcommand) dune. So it is build with Cmd.group which takes a Cmd.t list, but no argument terms.

[n-osborne]

I've renamed the --output option to --with-stdout-to to make it clearer and avoid confusion with the --output option of ortac qcheck-stm.

[n-osborne]

I should get rid of the ugly empty line generated where an optional argument is absent (see line 25 of the cram test).

[shym]

The code that generates the dune rules is a pleasure to read, congratulations!

I have 2 main remarks:

• The rule that will run ortac qcheck-stm on some .mli file must depend on that file (this dependency is currently missing in the generated rule). On the other hand, it doesn’t need to depend on the included file, I think: the generated test will only expect the module to be available, doesn’t it?
• varray_circular_incl.ml and varray_incl.ml are so similar (I resorted to diff-ing them, to be honest) that I would have used a functor, even if they are so short.

and a couple of small ones attached to their positions.

[n-osborne]

Thanks for the review!

I think I've cover all of your remarks but one: the use of a functor.
I totally agree that having to file that just differ by a module that is opened in them is not ideal.
That being said, using a functor will bring some complexity to the dune rules:

• declaring the functor as a library
• the included file will depend on the new library containing the functor, this implies some change in the dependencies of the generated tests (trickier as these are generated and not all included files are a functor instantiation)

I'd propose to postpone this feature (easy support of included files for testing functor instantiation) in order to have a clean way of dealing with them.

Also, putting the body of the current included file in a functor, abstracting over the opened file does not work out of the box: I still have an error on the type extension that I don't manage to fix for now:

File "examples/make_varray_incl.ml", line 24, characters 2-39:
24 |   type _ ty += Elt : 'a ty -> 'a elt ty
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: In this definition, a type variable cannot be deduced
       from the type parameters.

[shym]

I’m sorry I sent you on a wild goose chase :-/
Anyway, it looks good to me now, thanks!

[shym]

Thank you for the fix. CI is happy so I merge it.