dune icon indicating copy to clipboard operation
dune copied to clipboard

Add synopsis to dune rules/aliases. Show synopsis in `dune show aliases`

Open maxim092001 opened this issue 1 year ago • 17 comments

Desired Behavior

As a user of dune, I define a lot of different rules in my project:

  • Generate OCaml types from some schema
  • Run CLI tool to make network request to do some validation of my OCaml code
  • Download some binary executable to run before/during build
  • Run tests/executable with some pre-defined flags

Usually, I add an alias to each dune rule:

(rule
 (deps
  (glob_files schema.json))
 (alias validate_schema)
 (action
  (run
   validation_bin
   --mode
   validate
   --schema
   schema.json)))

Rules may get really complex and it is hard for me as a user to understand what kind of rules I have in project, especially when I switch projects often.

Currently, dune has support to show aliases in a top level dune file via dune show aliases. I propose to enhance this functionality.

Example

User should be able to add synopsis to a dune rule:

(rule
 (synopsis "Schema validation via validation_bin tool")
 (deps
  (glob_files schema.json))
 (alias validate_schema)
 (action
  (run
   validation_bin
   --mode
   validate
   --schema
   schema.json)))

And dune show aliases should be able to display validate_schema with synopsis in its output such as:

all
default
doc
fmt
install
validate_schema --- Schema validation via validation_bin tool

I believe synopsis for default aliases can also be added as part of this feature.

I would be happy to contribute this functionality if there is an agreement that it is useful and needed. I, though, would need some on boarding....

Thanks!

maxim092001 avatar Mar 10 '25 09:03 maxim092001

Thanks for opening this issue! 😄 Regarding your idea, it could be interesting to document the aliases as you suggest. However, wouldn't there be an issue if you attached multiple rules to the same alias? What would be the expected outcome with two rules attached to the same alias and the two of them are declaring a synopsis?

maiste avatar Mar 10 '25 13:03 maiste

@maiste thanks for coming back so quickly!

In my personal experience, same alias for different dune rules is useful for runtest or other tests. I didn't see custom aliases for different rules outside of tests, so eager to learn of such use cases.

That being said, there are multiple options we can take:

  • Single definition
    • Display first synopsis that was found during dune show aliases.
    • Enforce synopsis to be unique per alias. Fail build if two rules with same alias try to add synopsis
  • Additive aggregation: combine synopsis for dune rules into one synopsis.
    • There won't be order guarantees I assume, so it doesn't seem very useful for user.
  • Additive aggregation v2: dune show aliases has output like:
    - validate_schema:
      - dune:35:24 - Validate schema rule alias synopsis 1
      - dune:80:35 - Validate schema rule alias synopsis 2
    
    • Shows rules with different synopsis for same aliases
  • Attach synopsis to alias definition
    • Currently you can define top level alias in dune file without attaching it to rule: (alias (name validate_schema))
    • We can allow synopsis for aliases only for such definitions and show only them during dune show aliases:
      • (alias (name validate_schema) (synopsis "Schema validation"))
    • It is kinda similar to CMake add_custom_target with COMMENT approach

As a user, I would prefer either "Additive aggregation v2" or "attaching synopsis to alias definition".

Let me know what you think

maxim092001 avatar Mar 11 '25 09:03 maxim092001

I have already seen some projects where people would use aliases to generate some binary dependencies (the best example being ocaml.org itself 👍).

My intuition would be to attach a synopsis to alias, as you suggested. It would make it clearer about the purpose of such declarations. Maybe this is something that could live inside the dune-project. I don't know what the best is.

However, I would like to have other Dune people input on this to make sure it is not heading into the wrong direction and to make sure I'm not the only one to see interest in this feature 😄

maiste avatar Mar 11 '25 15:03 maiste

However, I would like to have other Dune people input on this to make sure it is not heading into the wrong direction and to make sure I'm not the only one to see interest in this feature 😄

@rgrinberg and @emillon you would probably have an opinion!

maxim092001 avatar Mar 11 '25 15:03 maxim092001

Sure, I don't mind. Seems like a straightforward enhancement

rgrinberg avatar Mar 11 '25 21:03 rgrinberg

My suggestion would be the following

(rule
 (deps
  (glob_files schema.json))
 (alias (name validate_schema) (synopsis "Validates a schema"))
 (action
  (run
   validation_bin
   --mode
   validate
   --schema
   schema.json)))

So when the synopsis is missing it would be equivalent to (alias validate_schema).

Leonidas-from-XIV avatar Mar 12 '25 09:03 Leonidas-from-XIV

Thanks for feedback!

My concern with attaching it inside the rule is when you end up to use an alias at multiple places with multiple synopsis. As mentioned by @maxim092001, it can be handled:

  • by raising an error if we already have the synopsis (if we go for the inside rule solution, this seems to be the best) or
  • by concatenating all the synopsis or
  • remember the position and display the synopsis linked with the location.

My first intuition was to put it inside the dune-project but I can't think of a good syntax to reflect this. Also, putting the information far from the alias input wouldn't be a good UX.

maiste avatar Mar 12 '25 09:03 maiste

You could reuse the alias stanza to do this.

emillon avatar Mar 12 '25 09:03 emillon

I'd like to discuss issue this during next dune-dev call on Wed 19 March if there is time!

maxim092001 avatar Mar 14 '25 09:03 maxim092001

I've added it to the agenda for the meeting.

Leonidas-from-XIV avatar Mar 14 '25 09:03 Leonidas-from-XIV

Started working on implementation for "Additive aggregation v2" in https://github.com/ocaml/dune/pull/11609

maxim092001 avatar Apr 05 '25 22:04 maxim092001

Looking more into implementation of aliases in rules from rule_conf there is a case such as this:

(rule
 (alias baz)
 (action
  (echo "From baz")))

(rule
 (aliases foo bar)
 (action
  (echo "From foo or bar")))

I believe for baz, with this change, it should be just:

(rule
 (alias (name baz) (synopsis "Baz synopsis"))
 (action
  (echo "From baz")))

For foo and bar it would make sense to have only one synopsis and from my intuition it can be either done like:

(rule
 (aliases (names foo bar) (synopsis "Foo or bar synopsis"))
 (action
  (echo "From foo or bar")))

Or by attaching it to the rule itself:

(rule
 (aliases foo bar)
 (synopsis "Foo or bar synopsis)
 (action
  (echo "From foo or bar")))

Second one makes more sense to me, so I'll just attack synopsis to rules stanza, rather than to alias/aliases. @Leonidas-from-XIV @maiste wdyt? It is possible that rule might have synopsis, but not alias. This is fine, it won't be shown in dune show aliases.

maxim092001 avatar Apr 06 '25 20:04 maxim092001

To me it seems somewhat strange to attach a synopsis to a rule rather than an alias in some cases, in such case I think it would make more sense to always attach the synopsis to a rule and never to an alias (essentially arriving back at your example from the first post). dune show aliases would then iterate over all attached rules and show these synopsi.

It does feel however a bit strange that those rules without an alias would never have a visible synopsis.

Here's another option, but it feels somewhat heavy-handed and uses lists of lists in a way that isn't really common in the rest of Dune lang.

(rule
 (aliases ((name foo) (synopsis "Synposis of foo")) ((name bar) (synopsis "Synopsis of bar"))
 (action
  (echo "From foo or bar")))

A more idiomatic way could be (alias <mapping>):

(rule
 (aliases
  (alias 
    (name foo) 
    (synopsis "Synposis of foo"))
  (alias
    (name bar)
    (synopsis "Synopsis of bar")))
 (action
  (echo "From foo or bar")))

@rgrinberg What do you think about attaching the synopsis to the rule?

Leonidas-from-XIV avatar Apr 07 '25 14:04 Leonidas-from-XIV

Thanks @maxim092001 for working on this!

Regarding the solutions, I'm torn between the two solutions:

  • On one hand, it makes sense to attach the synopsis to the rule itself as we would capture all the synopsis thanks to the aggregation, and we would only show the rules with an alias when calling to dune show aliases.
  • On the other hand, this feature is supposed to document aliases not rules. It would make sense to attach it to an alias and not a rule. It might end up with more than one documentation, but this is fine, regarding the additive syntax.

I don't have a clear opinion on this yet. My first thought would be the same as @Leonidas-from-XIV with the (aliases (alias ...)) system.

maiste avatar Apr 07 '25 15:04 maiste

I was digging deeper into dune code base and discovered dune describe and dune describe rules.

https://github.com/ocaml/dune/blob/main/bin/describe/describe.ml#L32 https://github.com/ocaml/dune/blob/main/bin/print_rules.ml

Describe what is in the current workspace in either human or machine readable form. By default, this command output a human readable description of the current workspace. This output is aimed at human and is not suitable for machine processing. In particular, it is not versioned. If you want to interpret the output of this command from a program, you must use the $(b,--format) option to specify a machine readable format as well as the $(b,--lang) option to get a stable output.

I guess if I can attach synopsis to a rule then I can add addiitonal flag to dune describe rules that would print out location of a rule, aliases associated with it and of course synopsis.

I, though, do not fully understand what is the difference between show and describe...

For now, in my draft PR, I have added synopsis to rule and test that verifies that it just can be added (build is succeeding). I guess in order to proceed we need to make decision if this is a right approach and which visualization command I should enhance

maxim092001 avatar Apr 14 '25 16:04 maxim092001

A little update about this issue. Current implementation is able to show/describe synopsises for targets as well as aliases.

Little cram test below:

  $ cat > dune-project << EOF
  > (lang dune 3.18)
  > EOF
  $ cat > dune << EOF
  > (rule
  >  (targets touched-file.ml)
  >  (alias rule-with-synopsis)
  >  (synopsis "Synopsis for rule with alias rule-with-synopsis")
  >  (action
  >   (write-file touched-file.ml "Echo from rule with alias rule-with-synopsis")))
  > EOF

  $ dune build @rule-with-synopsis
  $ cat _build/default/touched-file.ml
  Echo from rule with alias rule-with-synopsis
  $ dune show targets
  dune
  dune-project
  touched-file.ml
  - Synopsis for rule with alias rule-with-synopsis
  $ dune show aliases
  all
  default
  fmt
  ocaml-index
  pkg-install
  rule-with-synopsis
  - Synopsis for rule with alias rule-with-synopsis

I am working on more tests such as:

  • [x] Alias attached to multiple rules with different synopsis
  • [x] Multiple aliases attached to multiple rules with different synopsis

I also need to work on:

  • [x] Directory targets support
  • [x] Attach some synopsis to default aliases
  • [x] Make UX a little fancier, put space
  • [x] Add location of synopsis to output

maxim092001 avatar Apr 25 '25 12:04 maxim092001

PR is ready to review

maxim092001 avatar Apr 25 '25 21:04 maxim092001