Passive watch mode for tests

[aalekseyev]

This PR makes the following changes:

1. Add a new dune flag:

       --passive-watch-mode
           Do not automatically start or re-start a build. Only do it when a
           build RPC is received.

This is much better for dune automation than the vanilla watching mode itself because it lets the automation harness decide when to proceed with the build. We'll use it for tests now, but it can also be useful for the CI.

2. Implement dune rpc build command which sends the build rpc to dune, to interact with the passive watch mode. I re-purposed a stub of an existing build rpc which already existed, but was not usable and behaved strangely.

3. To implement (dune rpc build) in a race-free way, added an inotify Sync mechanism to the filesystem watcher. By doing an inotify sync you're making sure that you've consumed all inotify events generated so far before proceeding with the build.

4. Add the beginning of watching mode test suite that makes use of this functionality.

[ghost]

Could we make poll and poll_passive have a more similar API? I find it odd that one takes a "one step" function, while the other takes a get_build_request that returns a "one step" function. The fact that get_build_request is blocking on the RPC is an implementation detail that is not very interesting from the point of view the scheduler.

[aalekseyev]

I'm thinking of the RPC as not being the part of the build because including RPCs into build seems weird (breaks some intuitions about the build being deterministic).
But there are also practical reasons related to scheduler:

• The build can be cancelled by a filesystem event, while the wait for the RPC can not.
• The inotify sync needs to happen after the rpc, but before the build.

I'm not sure how to abstract that away.
Overall, I feel like polling loop is inherently going to interact with these concerns, but the scheduler does not have to. Having polling loop integrated with the scheduler is what seems weird to me.

I took a bite at one step in what seems the right direction to me in:
aalekseyev@dbf0c25

This moves the filesystem event notifications into a separate variable (Invalidation_accumulator_var.t), but it's not really achieving enough to claim any separation.

[ghost]

Yeah, I'm not arguing for having a single function, but at least similar APIs to learn to work with. The two run_... functions in build_cmd.ml end up having pretty different structure, which makes the code harder to review. If the two poll function had a similar API, then the two functions in build_cmd.ml should be more similar as well.

[aalekseyev]

Oh, got it. I'd argue build_cmd.ml is messy for a different reason: because I wrote it badly.
I just pushed a commit that simplifies code in build_cmd.ml.

As for the scheduler.mli, my initial desire was to move the entire implementation of poll_passive into build_cmd.ml, but I couldn't do it without exposing details of the state machine. Any ideas for any specific improvements are very welcome!

[aalekseyev]

@jeremiedimino, OK, I merged and the types in scheduler.mli look nicer now.

The error handling unification also let me unify the Hooks.End_of_build.run here in bin/build_cmd.ml, so it's done both in polling and in non-polling mode.

[ghost]

Looks good. I didn't check the changes to the RPC, but the other changes look good to me.

[aalekseyev]

@rgrinberg, I made some changes to rpc code in this PR (mainly: factor pieces out to be able to support the --wait flag in dune rpc build).
Would you like to look at them before I merge, or should I just go ahead?

[aalekseyev]

By the way, @rgrinberg, I never tested if Sync events actually work with fswatch. Our CI does not have fswatch installed. If we can install it, we could try to enable the tests on a mac (preferably in a separate PR).