Allow rpc-triggered builds in eager watch mode (#11622)

When dune is started in eager watch mode (e.g. by running `dune build
--watch`) it starts an RPC server, but prior to this change it would
reject RPC requests to build targets. This is a step towards being able
to run dune commands that trigger build (e.g. `build`, `exec`, `test`)
while watch mode is running.

Signed-off-by: Stephen Sherratt <[email protected]>

Author: gridbugs

bin/build_cmd.ml

@@ -83,28 +83,38 @@ let run_build_system ~common ~request =
       Fiber.return ())
 ;;
 
-let run_build_command_poll_eager ~(common : Common.t) ~config ~request : unit =
-  Scheduler.go_with_rpc_server_and_console_status_reporting ~common ~config (fun () ->
-    Scheduler.Run.poll (run_build_system ~common ~request))
-;;
-
-let run_build_command_poll_passive ~(common : Common.t) ~config ~request:_ : unit =
-  (* CR-someday aalekseyev: It would've been better to complain if [request] is
-     non-empty, but we can't check that here because [request] is a function.*)
+let poll_handling_rpc_build_requests ~(common : Common.t) ~config =
   let open Fiber.O in
   let rpc =
     match Common.rpc common with
     | `Allow server -> server
     | `Forbid_builds -> Code_error.raise "rpc server must be allowed in passive mode" []
   in
+  Scheduler.Run.poll_passive
+    ~get_build_request:
+      (let+ (Build (targets, ivar)) = Dune_rpc_impl.Server.pending_build_action rpc in
+       let request setup =
+         Target.interpret_targets (Common.root common) config setup targets
+       in
+       run_build_system ~common ~request, ivar)
+;;
+
+let run_build_command_poll_eager ~(common : Common.t) ~config ~request : unit =
+  Scheduler.go_with_rpc_server_and_console_status_reporting ~common ~config (fun () ->
+    let open Fiber.O in
+    (* Run two fibers concurrently. One is responible for rebuilding targets
+       named on the command line in reaction to file system changes. The other
+       is responsible for building targets named in RPC build requests. *)
+    let+ () = Scheduler.Run.poll (run_build_system ~common ~request)
+    and+ () = poll_handling_rpc_build_requests ~common ~config in
+    ())
+;;
+
+let run_build_command_poll_passive ~common ~config ~request:_ : unit =
+  (* CR-someday aalekseyev: It would've been better to complain if [request] is
+     non-empty, but we can't check that here because [request] is a function.*)
   Scheduler.go_with_rpc_server_and_console_status_reporting ~common ~config (fun () ->
-    Scheduler.Run.poll_passive
-      ~get_build_request:
-        (let+ (Build (targets, ivar)) = Dune_rpc_impl.Server.pending_build_action rpc in
-         let request setup =
-           Target.interpret_targets (Common.root common) config setup targets
-         in
-         run_build_system ~common ~request, ivar))
+    poll_handling_rpc_build_requests ~common ~config)
 ;;
 
 let run_build_command_once ~(common : Common.t) ~config ~request =

bin/common.ml

@@ -1155,7 +1155,6 @@ let build (builder : Builder.t) =
              ~registry
              ~root:root.dir
              ~handle:Dune_rules_rpc.register
-             ~watch_mode_config:builder.watch
              ~parse_build:Dune_rules_rpc.parse_build
              stats))
     else `Forbid_builds

doc/changes/11622.md

@@ -0,0 +1,2 @@
+- Allow build RPC messages to be handled by dune's RPC server in eager watch
+  mode (#11622, @gridbugs)

doc/rpc.rst

@@ -56,10 +56,17 @@ uses the ``dune-rpc`` and ``dune-rpc-lwt`` packages.
 Connecting
 ==========
 
-To connect to Dune's RPC server, it needs to be started in watch mode. It is
-possible to use ``dune build --passive-watch-mode`` to start an RPC server which
-will listen for requests without starting a build by itself. Then ``dune rpc
-build .`` will connect to it, trigger a build, and report status.
+To connect to Dune's RPC server, it needs to be started in watch mode. There
+are two ways of doing this:
+
+- Run ``dune build --passive-watch-mode`` to start an RPC server which will
+  listen for requests without starting a build by itself...
+- Or run ``dune build --watch`` to start a build server that rebuilds the
+  project's default target when source files change and *also* starts an RPC
+  server that listens for requests.
+
+Then ``dune rpc build .`` will connect to it, trigger a build, and report
+status.
 
 .. _lwt: https://github.com/ocsigen/lwt
 .. _Dune_rpc: https://github.com/ocaml/dune/blob/main/otherlibs/dune-rpc/dune_rpc.mli

src/dune_rpc_impl/server.ml

@@ -192,7 +192,6 @@ type 'a t =
   { config : Run.t
   ; pending_build_jobs : ('a list * Build_outcome.t Fiber.Ivar.t) Job_queue.t
   ; parse_build : string -> 'a
-  ; watch_mode_config : Watch_mode_config.t
   ; mutable clients : Clients.t
   }
 
@@ -321,23 +320,10 @@ let handler (t : _ t Fdecl.t) handle : 'a Dune_rpc_server.Handler.t =
   let () =
     let build _session targets =
       let server = Fdecl.get t in
-      match server.watch_mode_config with
-      | No -> assert false
-      | Yes Eager ->
-        let error =
-          Dune_rpc.Response.Error.create
-            ~kind:Invalid_request
-            ~message:
-              "the rpc server is running with eager watch mode using --watch. to run \
-               builds through an rpc client, start the server using --passive-watch-mode"
-            ()
-        in
-        raise (Dune_rpc.Response.Error.E error)
-      | Yes Passive ->
-        let ivar = Fiber.Ivar.create () in
-        let targets = List.map targets ~f:server.parse_build in
-        let* () = Job_queue.write server.pending_build_jobs (targets, ivar) in
-        Fiber.Ivar.read ivar
+      let ivar = Fiber.Ivar.create () in
+      let targets = List.map targets ~f:server.parse_build in
+      let* () = Job_queue.write server.pending_build_jobs (targets, ivar) in
+      Fiber.Ivar.read ivar
     in
     Handler.implement_request rpc Decl.build build
   in
@@ -405,7 +391,7 @@ let handler (t : _ t Fdecl.t) handle : 'a Dune_rpc_server.Handler.t =
   rpc
 ;;
 
-let create ~lock_timeout ~registry ~root ~watch_mode_config ~handle stats ~parse_build =
+let create ~lock_timeout ~registry ~root ~handle stats ~parse_build =
   let t = Fdecl.create Dyn.opaque in
   let pending_build_jobs = Job_queue.create () in
   let handler = Dune_rpc_server.make (handler t handle) in
@@ -443,14 +429,7 @@ let create ~lock_timeout ~registry ~root ~watch_mode_config ~handle stats ~parse
     ; server_ivar = Fiber.Ivar.create ()
     }
   in
-  let res =
-    { config
-    ; pending_build_jobs
-    ; watch_mode_config
-    ; clients = Clients.empty
-    ; parse_build
-    }
-  in
+  let res = { config; pending_build_jobs; clients = Clients.empty; parse_build } in
   Fdecl.set t res;
   res
 ;;

src/dune_rpc_impl/server.mli

@@ -4,7 +4,6 @@ val create
   :  lock_timeout:float option
   -> registry:[ `Add | `Skip ]
   -> root:string
-  -> watch_mode_config:Watch_mode_config.t
   -> handle:(unit Dune_rpc_server.Handler.t -> unit)
        (** register additional requests or notifications *)
   -> Dune_stats.t option

test/blackbox-tests/test-cases/watching/watching-eager-with-rpc.t

@@ -1,4 +1,4 @@
-Show error when trying to build through rpc when server is running eager watch
+Demonstrate building through rpc when server is running in eager watch
 
   $ echo '(lang dune 3.8)' > dune-project
   $ mkdir src
@@ -9,13 +9,10 @@ Show error when trying to build through rpc when server is running eager watch
   $ dune build @all
   $ dune build --watch &
   Success, waiting for filesystem changes...
+  Success, waiting for filesystem changes...
 
   $ dune rpc build --wait .
-  Error: { payload = None
-  ; message =
-      "the rpc server is running with eager watch mode using --watch. to run builds through an rpc client, start the server using --passive-watch-mode"
-  ; kind = Invalid_request
-  }
+  Success
 
   $ dune shutdown
   $ wait