Stabilizing the Action Plugin

[rgrinberg]

This ticket is about our dynamic action plugin. A fine feature that is just missing a bit of polish before our users can profit from it.

I'll split the issues into two categories. Issues blocking the release of this feature and nice to haves that would be great to have but aren't necessary for v1.

Blockers

• The current API is monadic. This imposes serious limitations on users that might be working with their own monad already (lwt, fiber, memo). The obvious solution is to introduce a transformer.

Non-blockers

• The dynamic-run constructor in the action language. IMO this extra bit information is something that breaks abstraction. Users shouldn't worry about the details of how a binary is interacting with the build system. We should allow custom rules, preprocessing rules, builtin rules to use the action API without any additional annotations in build files.

• Currently, >>= in the action plugin is needlessly slow. It could be made much faster if action plugins were rpc clients.

• There's a lot of duplication between the versioning story of dune's rpc protocol and the dap protocol. We should use the same API and versioning concepts for both.

The last 3 points can all be addressed at once by re-implementing dap with dune rpc. The only downside is that now we'll need dune to run the rpc server outside of watch mode as well. I don't see how this would cause issues though. If there's agreement on this point, I can describe the design in a little more detail.

cc @snowleopard @Alizter @tmattio

[rgrinberg]

cc @staronj who is the original author of this feature.

and @bobot that would like to promise a different api altogether to reduce the reliance on the "M word".

[rgrinberg]

There was a discussion about this feature between the team today and I'd like to summarize the conclusions. The attendants were @emillon, @staronj, @snowleopard, and one more dev whose github username I don't know.

• Everyone were in favor of re-implementing DAP on top of RPC. The downsides seem tiny, and the advantages are huge.

• Dropping the dynamic-run constructor was something people were against. However it was agreed that we can think of other ways of reducing the boilerplate of this constructor. For example, annotating binaries as "action plugins" in the executable stanzas.

I'd like to propose the following interface that gives users more flexibility in implementing actions:

module Make (IO : sig
    type 'a t

    ..
end) : sig
  module Request : sig
    type 'a t

    val both : 'a t -> 'b t -> ('a * 'b) t
    
    val map : 'a t -> f:('a -> 'b) -> 'b t
  end

  type t

  val connect : unit -> t IO.t

  val read_file : path:Path.t -> string Request.t

  val write_file : path:Path.t -> data:string -> unit Request.t

  val submit : t -> 'a Request.t -> 'a IO.t
end

The point is that those of us that prefer monadic concurrency can instantiate IO.t with Fiber, Lwt, while those that don't mind blocking can instantiate with Id.

[snowleopard]

The downsides seem tiny, and the advantages are huge.

I think it's worth being more explicit about downsides and advantages. Could you list some concrete points?

[rgrinberg]

Sure thing:

Advantages:

• No need to restart the binary when it's unnecessary.
• A single versioning mechanism used everywhere
• Lots of RPC features we can reuse such as batching.

Disadvantages:

• We need to start the RPC server. On Windows, we still rely on TCP so it's a bit intrusive. But newer versions of Windows are getting unix domain sockets, so this will be temporary

[snowleopard]

Thanks!

A single versioning mechanism used everywhere

Not sure what you mean here exactly. Perhaps, this: RPC already has some versioning, but the action plugin (in its current form) introduces a new versioning requirement -- the binary and Dune need to communicate using some protocol, which might evolve and hence requires versioning. This doesn't seem like a very strong point to me, since Dune can communicate with the binary using the RPC protocol (with negotiation and everything). It's not like we must use something different if we're not using sockets.

[rgrinberg]

@snowleopard If I understand correctly, your point is that DAP can still use the RPC protocol to communicate but it can keep its own transport mechanism. You're right about that, but I guess my original point was to adopt the protocol and our socket based transport mechanism. I should have clarified that I was in fact pushing for two different things at once.

But I can't imagine a separate transport for DAP to be a good thing. It's more code to maintain the whole file based request/response stuff, subtle bugs when something assumes socket semantics, and just downright more work to create some tools that give us visibility into what's happening.