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

Proposal for automated generation of dune diff rules in examples/ #231

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

mbarbin
Copy link
Contributor

@mbarbin mbarbin commented Jan 14, 2024

Hello team,

I'd like to propose a change related to the diff rules used under the examples/ directory, which were recently added in #230.

The idea is to automate the generation of these diff rules using a helper program. The generated rules would be written to the dune.inc file, which is then included in the main dune file. If there are any changes, running dune promote would update the dune.inc file accordingly.

I understand that this introduces a bit more complexity into the dune files, and it might be a matter of personal preference. Therefore, I thought it would be best to propose this change as a separate PR, so you can decide whether the benefits outweigh the added complexity.

In the future, this setup could potentially be replaced by generic dune rules, once they are supported. Until then, using gen-dune type helper programs is not an uncommon workaround that I've found to be quite effective.

I look forward to hearing your thoughts on this proposal.

- Generating the diff rules using a helper program should make it
  easier to add new examples.
@mbarbin mbarbin changed the title Proposal for Automated Generation of Dune Diff Rules in examples/ Proposal for automated generation of dune diff rules in examples/ Jan 14, 2024
Copy link
Collaborator

@c-cube c-cube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this but I don't think we need to check in dune.inc, do we?

@mbarbin
Copy link
Contributor Author

mbarbin commented Jan 14, 2024

From my understanding, the (include {file}) construct in dune requires the
file to be present in the tree. It doesn't seem to work with files dynamically
generated by a rule. I don't think a file can both be generated by a rule, and
be checked in at the same time.

This forces to check in the file dune.inc. To keep this file in sync, we
generate another file and use the standard dune runtest + dune promote
workflow to synchronize the file in the tree.

This process can be likened to a fixpoint operation. If a new rule is added to
the file, a runtest+promote step should be executed first to update the dune
file, followed by another runtest+promote step to validate the new test.
Running with the -w option makes this process seamless.

While I acknowledge my familiarity with this pattern might influence my
perspective, I also believe there's value in having the generated rules visible
in the tree. It provides quick access for understanding the rules, serving as an
'expect test' for the code generating the rules. Even if we could include a
dynamic file, there's a chance I'd still lean towards this approach for its
clarity and accessibility. YMMV.

@mbarbin
Copy link
Contributor Author

mbarbin commented Jan 14, 2024

Just one more thing to mention:

serving as an 'expect test' for the code generating the rules

I put this to good use while I was developing the code that generates the rule. I could visualize the incremental changes to the dune file until I was satisfied with the result (dune promote).

A word of caution though: once you experience the efficiency of the expect-test workflow, it's hard to go back. Consider yourself warned! 😃

@c-cube
Copy link
Collaborator

c-cube commented Jan 15, 2024

Oh that's great, thank you for the explanation. Then it's perfect, I'm merging this. That makes me think that maybe ocaml-protoc should also ship with a small binary (ocaml-protoc-genrules?) that takes a list of .proto files as arguments as well as arguments for ocaml-protoc itself, and spits out a dune.inc file that contains the rules for these particular modules? Like ocaml-protoc-genrules foo.proto bar.proto --pp --binary -I dir produces a file with:

; dune.inc

(rule
  (targets foo.ml foo.mli)
  (deps (:file foo.proto))
  (action ocaml-protoc %{file} --pp --binary --ml_dir=.))

; … same for bar

What do you think?

I like expect tests, I just don't use them enough :-).

@c-cube c-cube merged commit 3827425 into mransan:master Jan 15, 2024
2 checks passed
@mbarbin mbarbin deleted the generate-examples-dune-rules branch January 15, 2024 07:41
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

Successfully merging this pull request may close these issues.

2 participants