ocaml-protoc icon indicating copy to clipboard operation
ocaml-protoc copied to clipboard

Discussing the inclusion of a helper binary in ocaml-protoc for automated rule generation

Open mbarbin opened this issue 1 year ago • 2 comments

I'm quoting an exchange we had with Simon in #231:

@c-cube:

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?

Here is my response:

I support this idea. In fact, I've implemented a similar concept in a project of mine, calling it "protoc-helper". At the time, I didn't consider proposing it for upstream inclusion, but having something like that in the distribution makes sense to me.

My implementation cannot be directly reusable due to its dependency footprint, but I'm including it here as a data point.

let main =
  Command.basic
    ~summary:"generate dune stanza for all *.proto files present in the directory"
    (let%map_open.Command exclude =
       flag
         "--exclude"
         (optional_with_default [] (Arg_type.comma_separated string))
         ~doc:"FILE[,..]* file to exclude"
     in
     fun () ->
       Eio_main.run
       @@ fun env ->
       let files =
         Auto_format.find_files_in_cwd_by_extensions
           ~cwd:(Eio.Stdenv.fs env)
           ~extensions:[ ".proto" ]
         |> List.filter ~f:(fun file -> not (List.mem exclude file ~equal:String.equal))
       in
       let generate_rule ~file =
         let open Sexp in
         let list s = List s
         and atom s = Atom s in
         let basename = Fpath.rem_ext (Fpath.v file) in
         let include_services =
           String.is_suffix (Fpath.to_string basename) ~suffix:"_service"
         in
         list
           [ atom "rule"
           ; list
               [ atom "targets"
               ; atom (Fpath.add_ext "ml" basename |> Fpath.to_string)
               ; atom (Fpath.add_ext "mli" basename |> Fpath.to_string)
               ]
           ; list [ atom "deps"; list [ atom "glob_files"; atom "*.proto" ] ]
           ; list
               [ atom "action"
               ; list
                   [ atom "run"
                   ; atom "ocaml-protoc"
                   ; atom file
                   ; atom "--binary"
                   ; (if include_services then atom "--services" else atom "--yojson")
                   ; atom "--ml_out"
                   ; atom "."
                   ]
               ]
           ]
       in
       Eio_writer.with_flow (Eio.Stdenv.stdout env)
       @@ fun stdout ->
       Eio_writer.printf
         stdout
         "; dune file generated by '%s' -- do not edit.\n"
         (Sys.get_argv ()
          |> Array.to_list
          |> List.mapi ~f:(fun i s -> if i = 0 then Fpath.v s |> Fpath.basename else s)
          |> String.concat ~sep:" ");
       List.iter files ~f:(fun file -> Eio_writer.print_s stdout (generate_rule ~file)))
;;

Here's how it can be used in a dune file:

(rule
 (deps
  (glob_files *.proto))
 (action
  (with-stdout-to
   dune.proto.inc.gen
   (pipe-stdout
    (bash "%{bin:protoc-helper} gen-dune")
    (run dune format-dune-file)))))

Note: I used dune format-dune-file to avoid worrying about formatting during generation. This requires the pipe-stdout construct, introduced in dune.2.7, so may need to update your dune-project config to use it.

I suspect that the dependency computation in my implementation might be flawed. I was concerned about whether the compiler needed to revisit a file, say dep.proto, while compiling another file, a.proto, that depends on dep.proto. As a result, I made all invocations depend on all proto files systematically. This might be an overkill and potentially unnecessary.

mbarbin avatar Jan 15 '24 08:01 mbarbin