Skip to content

Conversation

@gpetiot
Copy link

@gpetiot gpetiot commented May 15, 2023

I noticed the duplications when adding new templates for the playground.

dream_eml accepts many files and produces an .ml file for each .eml file. Maybe the former rules were an optimization to not rebuild everything but honestly I didn't notice any overhead with my patch, we don't have too many files. But the rules are much more maintainable now.

@cuihtlauac
Copy link
Collaborator

cuihtlauac commented May 16, 2023

Beautiful! Thanks a lot @gpetiot this is great.

BTW, I discovered Dune's dialect stanza recently. Would it make sense to use too?

@gpetiot
Copy link
Author

gpetiot commented May 16, 2023

BTW, I discovered Dune's dialect stanza recently. Would it make sense to use too?

From what I understand, it's only used to specify formatting rules, but no build rules.

The doc says:

When not using a custom syntax or formatting action, a dialect is nothing but a way to specify custom file extensions for OCaml code.

@cuihtlauac
Copy link
Collaborator

cuihtlauac commented May 16, 2023

That's my understanding of the docs too. I'm not sure if this discussion suggests otherwise or likewise: https://discuss.ocaml.org/t/htmx-hc-web-development-approach/9993/44 Maybe we should ask @emillon?

@emillon
Copy link
Contributor

emillon commented May 16, 2023

You can use a dialect for this.

A dialect is a way for dune to support an alternative syntax for ocaml files. That means that in addition to the usual .ml / .mli extensions, dune knows about .eml files. In every place that dune looks for ocaml source files (for example to define a library), it will also look for those .eml files and set up a preprocessing rule for each one.

You can optionally define a formatter for .eml files; if you do, dune fmt will work automatically.

When not using a custom syntax or formatting action, a dialect is nothing but a way to specify custom file extensions for OCaml code.

This is a confusing sentence to say that by default, the "preprocessor" action is a no-op, and there is no default "formatting" action. If you define a dialect like this for the mlx extension, dune will treat .mlx files like normal .ml files for compilation.

@emillon
Copy link
Contributor

emillon commented May 16, 2023

Sidenote, I had a look at the diff in this PR. If I understand correctly you're replacing N rules with each 1 dependency and 1 target, by a single rule that has N dependencies and N targets. I don't have the whole context but usually this is not the right move because it has the following drawbacks:

  • the compilation actions will not run in parallel
  • for incremental builds, a change in any of the N dependencies is going to trigger the action and run the rule that will rebuild the N targets.

@cuihtlauac
Copy link
Collaborator

Thanks, @emillon, for this great insight.

@gpetiot gpetiot marked this pull request as draft May 16, 2023 14:53
@gpetiot
Copy link
Author

gpetiot commented May 16, 2023

I tried a few things without success.

The preprocessing of *.eml files generates empty *.eml.ml files, although the pp command looks alright (it was the same on the previous commit and no error is reported).

@gpetiot
Copy link
Author

gpetiot commented May 17, 2023

I tried to define a wrapper around dream_eml to output the generated file on stdout so it can be used by dune's dialect preprocessing. But it doesn't pick up the binary yet.

@sabine
Copy link
Collaborator

sabine commented Aug 23, 2023

What is the status on this? Waiting for a dune release?

@gpetiot
Copy link
Author

gpetiot commented Aug 23, 2023

Yes camlworks/dream#228 has been merged but dream has not been released yet

@sabine sabine added upstream Tasks blocked or to be addressed upstream stalled Blocked waiting. Will not progress w/o external progress labels Aug 23, 2023
@emillon
Copy link
Contributor

emillon commented Sep 4, 2023

The dune part has been released in 3.9.0.

@tmattio
Copy link
Collaborator

tmattio commented Sep 27, 2023

This is stalled, so closing, but to anyone who'd be interested in picking this up: a PR to factorize the rule using the new dialect features in Dune 3.9.0 would be very welcome!

I was also wondering if we could simplify the whole build configuration by leveraging (include_subdirs qualified). Food for thoughts for anyone interested in working on the build config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Everything related to building stalled Blocked waiting. Will not progress w/o external progress upstream Tasks blocked or to be addressed upstream

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants