ocaml / dune

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

[RFC] Richer callback for generating rules #4105

Open rgrinberg opened 3 years ago

rgrinberg commented 3 years ago

This is an idea that came up in a meeting with @jeremiedimino.

It's useful to know the targets produced by a rule without evaluating the rules. For example, it would be nice to know the targets produced by Simple_rules.user_rule without evaluating the rule.

So if we consider

val user_rule :
     Super_context.t
  -> ?extra_bindings:Pform.Map.t
  -> dir:Path.Build.t
  -> expander:Expander.t
  -> Rule.t
  -> Path.Build.Set.t

Conceptually, we'd like this to be split into:

val user_rule_targets : ?extra_bindings:Pform.Map.t -> dir:Path.Build.t ->expander:Expander.t -> Path.Build.Set.t

val user_rule :
     Super_context.t
  -> ?extra_bindings:Pform.Map.t
  -> dir:Path.Build.t
  -> expander:Expander.t
  -> Rule.t
  -> unit

Notably, the Super_context.t parameter is absent from user_rule_targets which allows us to drop the Super_context dependency in Dir_contents. The question that remains is how to share the computation the work between the two computations as much as possible.

ghost commented 3 years ago

To complement what Rudi said, the general idea is to add a new operation get_target_list : Path.Build.t -> String.Set.t in addition to the gen_rules one. I feel like it would clarify the control flow. For instance, get_target_list is exactly what we need to evaluate the list of modules attached to a library or executable stanza.

rgrinberg commented 1 year ago

@snowleopard do you think this is still relevant given the proposal #5251 ?

snowleopard commented 1 year ago

I think it would be nice to write up an RFC with an up-to-date plan for making rule generation more flexible, merging ideas from various places (possibly discarding some which are no longer relevant). I don't think I'll have time to do this in the next few weeks, but I'm happy to give feedback on the RFC if you get to it earlier.

rgrinberg commented 1 year ago

I would say this RFC is generalized by #5251. The main distinction is:

So it should be easy enough to extend #5251 to cover this use as well.