Add support for expression evaluation in Frontend Server + Build Daemon environment

[Markzipan]

Notable Changes:

• Introduce builder_pkgs/build_frontend_server, which handles aspects of the build system related to the Frontend Server. fes_manager.dart can be started by webdev, after which its state will be shared with the build system. If not supplied, the build system with init FES itself (as usual).
• A single frontend server instance is maintained between webdev and the build system. Builds, rebuilds, and expression evals are all sent to this instance via a socket (whose port is written to fesWorkerPortPath).
• Added some synchronization logic to prevent request/response desyncs

To support tests:

• Add ScratchSpace.existing to create a scratch space from a pre-existing directory.
• Add scratch-space-dir to specify the directory for scratch_space in build_daemon.

Pardon the large PR, but most of this is tests and copying expression eval APIs. The meat is the comms/API between the persistent Frontend Server and Build Daemon / Builder.

[github-actions]

PR Health

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

[davidmorgan]

At first glance I'm struggling to see how this is a good direction to take: let's discuss?

The web builds already have to do lots of workarounds to use build_runner, they are not at all using it like all the other builders do. So when adding more support for web, I think we want one of:

• keep it fully within web-specific packages and out of build_daemon/build_runner
• or, make it something that's an overall improvement for everyone, either a simplification or a feature non-web builders will want too

adding expression evaluation details to build_daemon and build_runner doesn't seem to fit that.

I think maybe the core feature is to be able to find and use the frontend server that's already started by the web build?

If I correctly understood what the web-hot-reload flag does, I think there is already a problem: the web build does not start the frontend server early enough. That makes sense, build_runner is not for managing servers.

So it looks to me like something else should manage the frontend server and its reuse. Maybe new code in frontend server client? Then the frontend server will start either on the first expression evaluation or the first build, and be reused after ways. If build_runner needs new general support for startup/shutdown to make this work well then that's the kind of feature I'd be up for adding.

Of course, I can be persuaded of anything that makes sense ;) happy to VC, I could do Thu morning for you / evening for me.

[Markzipan]

Thanks! I'm very open to taking this in a new direction; this is just a working initial implementation to illustrate DDC's reqs. The major requirements here are just:

1. Keep alive a persistent FES instance across all rebuilds/builds for an app
2. Clients can send messages to that FES and receive responses

Re: the the web build not starting frontend server early enough, this is only an issue since the frontend server chooses a port dynamically. If we start it late (in a builder), we don't have an easy way to connect with it (especially since we can't read/write one-off files).

Moving this to frontend_server_client makes logical sense, but we'd still need the host process to be able to receive messages to satisfy 2). One option could be to introduce a "Frontend Server Manager" process that we spin up early (perhaps in webdev) whose presence enables expression eval. We can continue spinning up an isolated FES in the builder if that earlier instance wasn't detected/passed in. That way the build system remains mostly unchanged (bar somehow passing a FES port and allowing a 'magic' API for socket comms).

I can throw an early meeting on your calendar tomorrow 🙏🙏

[davidmorgan]

Yup yup, let's chat. Thanks!

[davidmorgan]

This is looking promising--please let me know when it's ready for review :)

[github-actions]

Package publishing

Package	Version	Status	Publish tag (post-merge)
package:build	4.0.6	already published at pub.dev
package:build_config	1.3.0	already published at pub.dev
package:build_daemon	4.1.2-wip	WIP (no publish necessary)
package:build_frontend_server	1.0.0	ready to publish	build_frontend_server-v1.0.0
package:build_modules	5.2.0	ready to publish	build_modules-v5.2.0
package:build_runner	2.15.1-wip	WIP (no publish necessary)
package:build_test	3.5.16-wip	WIP (no publish necessary)
package:build_web_compilers	4.5.0	ready to publish	build_web_compilers-v4.5.0
package:mockito	5.6.5	already published at pub.dev
package:scratch_space	1.2.1	ready to publish	scratch_space-v1.2.1

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

[Markzipan]

I've updated the description and approach. The TL;DR from earlier is that I've moved a lot of the webdev/build_daemon comms to a separate pkg build_frontend_server. The protocol to communicate between them is effectively the same. fes_manager.dart runs as a separate process (started by webdev) and accepts FES queries from both webdev and the builders while sharing state. PTAL @davidmorgan @nshahan!

[davidmorgan]

Thanks! A couple of high level questions.

There is still --web-hot-reload in the description but I don't see it in the code, is it no longer needed? If it's not needed that fully decouples from core build_runner/build_daemon so that's great :)

About the new package build_frontend_server, is it intended for general/public use? If so would it make more sense to add to https://pub.dev/packages/frontend_server_client ? ... alternatively if it's really only for build_web_compilers+webdev, I'd be tempted to put it somewhere more private.

Actually looking at users of these packages, build_modules and scratch_space are already pretty much private to webdev https://pub.dev/packages?q=dependency%3Abuild_modules ... I would consider merging them into build_web_compilers, and saying that as a library it's only intended for use by webdev + jaspr. And then build_frontend_server as well. I found it useful to merge build_runner_core and build_resolver into build_runner, it makes releasing much easier!

[Markzipan]

There is still --web-hot-reload in the description but I don't see it in the code, is it no longer needed?

Woops - I forgot to remove that from the desc!

About the new package build_frontend_server, is it intended for general/public use? If so would it make more sense to add to https://pub.dev/packages/frontend_server_client ? ... alternatively if it's really only for build_web_compilers+webdev, I'd be tempted to put it somewhere more private.

Actually looking at users of these packages, build_modules and scratch_space are already pretty much private to webdev https://pub.dev/packages?q=dependency%3Abuild_modules ... I would consider merging them into build_web_compilers, and saying that as a library it's only intended for use by webdev + jaspr.

It's not intended for general use, but anyone who wants to support a Dart web client that supports expression eval (or other frontend server-related functionality) would need. So far that's webdev, build_web_compilers, and jaspr. scratch_space seems more 'generic' and used by other packages, but merging both build_modules and build_frontend_server into build_web_compilers seems more than reasonable to me. I'm curious why we kept build_modules separate - as it's effectively a web-only thing?

Lemme try to merge these together.

Part 1: #4918
Part 2: Markzipan#2 #4922

[davidmorgan]

I guess build_modules was planned for wider use, but we ended up here :)

I reviewed the build_modules merge, the second one is against your depot, do you want me to review it there?

[Markzipan]

🙃 🙃

@davidmorgan Thanks for review! That one was for easier diff-ing. I opened a new one on the main branch: #4922