Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable running two builds in separate terminals (a.k.a. dune monitor) #7308

Closed
jchavarri opened this issue Mar 13, 2023 · 10 comments · Fixed by #8152
Closed

Enable running two builds in separate terminals (a.k.a. dune monitor) #7308

jchavarri opened this issue Mar 13, 2023 · 10 comments · Fixed by #8152
Assignees

Comments

@jchavarri
Copy link
Collaborator

#6360 disabled multiple build commands running concurrently.

However, as dune expands its scope and supports more tools, the case for running dune build with two or more apps in separate terminals gains strength.

For the particular use case I can speak of, at Ahrefs we have a web server written in native OCaml, and a frontend built with Melange. It is desirable to track the build status of both apps on separate terminals to not mix errors, and also because each environment might have different binaries that are executed on a successful build (e.g. the web server binary is relaunched after every build succeeds, same for webpack or other JS bundlers on the Melange side).

@rgrinberg suggested to have a new subcommand monitor that would help mirror the output of a running dune process. Then, add to dune build the ability to delegate a job, and then attach dune monitor until the job is finished.

I wonder, would it be possible to solve this problem without adding a new subcommand? I.e. when dune build sees that another dune process is already running, it can request it to build some target, and get the output back. Essentially making the lock system transparent for users.

Another idea to sidestep the new subcommand would be to make the lock system more granular, and instead of locking the whole tree, lock the specific folders that the current build is targeting. Not sure if this is possible though 🤔

@Alizter
Copy link
Collaborator

Alizter commented Mar 13, 2023

Currently we have no way of sending (error) messages via RPC. So if there is a Dune in watch mode, having another Dune connect by a user will mean they won't see the messages. Everything else should work however.

@rgrinberg
Copy link
Member

I wonder, would it be possible to solve this problem without adding a new subcommand? I.e. when dune build sees that another dune process is already running, it can request it to build some target, and get the output back. Essentially making the lock system transparent for users.

Getting the output back is exactly what implementing dune monitor is about. Currently, we have no way of getting any useful output from a dune instance running in watch mode.

Another idea to sidestep the new subcommand would be to make the lock system more granular, and instead of locking the whole tree, lock the specific folders that the current build is targeting. Not sure if this is possible though

It's not possible to do it per directory because there's some global database files that aren't per directory. It would be possible for the lock to be released by watch mode when it's idle though.

@jchavarri
Copy link
Collaborator Author

Getting the output back is exactly what implementing dune monitor is about. Currently, we have no way of getting any useful output from a dune instance running in watch mode.

What I mean is that I'm not sure if exposing the complexity of a new subcommand to users is needed / worth it. Under the idea I'm suggesting, dune build would operate in two modes:

  • If no lock is there, runs the builds and operates in normal mode (rpc active), I understand this is essentially how dune build works today
  • If lock is there, instead of erroring out, it runs in "monitor" mode connecting to the rpc

For users, it would be transparent. For dune maintenance, it avoids extra subcommand (docs and maintenance costs).

@Alizter
Copy link
Collaborator

Alizter commented Mar 13, 2023

What I mean is that I'm not sure if exposing the complexity of a new subcommand to users is needed / worth it. Under the idea I'm suggesting, dune build would operate in two modes:

This only works if the first dune is run in watch mode. What happens when dune build is already running (not in watch mode). If we connect to this dune, then it will surely lose connection as soon as it finishes. By the way, I think we don't start RPC anyway for regular builds so there would be nothing to connect to, just a lock.

@rgrinberg
Copy link
Member

@jchavarri Personally, I would find the command to be useful on its own. It's often the case that I run dune in watch mode in the background, but would like to inspect the output from my editor. I would prefer not to run the build from the editor, because I want dune to stay running even if the editor is killed. For this reason, I'd like to see dune monitor.

In any case, if you prefer not to add a dune monitor command, that's OK with me. It could always be added later.

@jchavarri
Copy link
Collaborator Author

My main concern or hesitation about dune monitor is how it would work for the use case I mentioned in the description. Would users have to run 3 terminals instead of 2?

  • one for dune build
  • another to monitor the api server build
  • another to monitor the melange build
    ?

Then, add to dune build the ability to delegate a job

@rgrinberg could you expand on this a bit please? I think I am not getting how this part would work.

@rgrinberg
Copy link
Member

rgrinberg commented Mar 13, 2023

Okay, I think we might be talking about different things. I think that running dune build while dune is already running in watch mode is for one off tasks. For example running the tests, formatting, executing some binary once, etc. What you're describing doesn't require anything more than doing:

$ dune build @server @melange -w

And being smart about separating the commands that want to stream their outputs. Doing this is a much smaller task than dune monitor and we should probably tackle it first.

could you expand on this a bit please? I think I am not getting how this part would work.

It would take these steps:

  1. connect to the rpc server if it's running
  2. start streaming the UX state from the server
  3. ask the server to build a target
  4. disconnect from the server once the target is built

@jchavarri
Copy link
Collaborator Author

It seems something similar to this idea was already implemented in #4715? I can run dune diagnostics successfully:

  • Shows Error: rpc server not running is server not running
  • Empty output if build is successful or ongoing
  • Shows errors in case there are

Some of the things missing are:

  • ask the server to build a target
  • disconnect from the server once the target is built
  • colorized output (is this possible?)

But maybe dune monitor can be built in a way that diagnostics and monitor share some of the code, as the functionality is very similar.

@jchavarri
Copy link
Collaborator Author

jchavarri commented Mar 23, 2023

Another command that seems related to what this issue is about is dune rpc. It does the following:

  • Can ask server to build a target
  • Show Success or Failure after build finishes
  • Disconnect from the server once target is built

The missing parts are:

  • Show details in case of error
  • Show build progress
  • Allow to build aliases
  • Should work with --watch, besides --passive-watch-mode

So it seems a combination of diagnostics and rpc subcommands would take it already a large part of the way to monitor.

@rgrinberg
Copy link
Member

That's right. dune rpc build is quite close to what we want - minus the correct mirroring of output.

dune rpc diagnostics is not very useful though. It's specialized for errors, and that's simply not enough to replicate the UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants