Remove some coupling between the client and daemon #29167

adammurdoch · 2024-05-15T20:01:19Z

Context

This PR removes some usages of "build process" (eg the daemon) types from the client implementation.

In particular, split BuildRequestContext to instead use separate client-side and daemon-side context. This allows the types to carry different information represented using different types.

It also applies some clean-up of the "daemon-protocol", "daemon-services", "client-services" and "build-state" projects:

Allow projects to declare cross build session services, so that "build-state" does not need to know about them all.
Move service registration around so that "build-state" does need to know about services implemented in "core", and "launcher" does not need to know about services implemented in "build-state".
Moved classes to new packages to avoid split packages.
Some renames to fix (my) spelling error.

Contributor Checklist

Review Contribution Guidelines.
Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
Provide unit tests (under <subproject>/src/test) to verify logic.
Update User Guide, DSL Reference, and Javadoc for public-facing changes.
Ensure that tests pass sanity check: ./gradlew sanityCheck.
Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

❌ ❓must be fixed
🤔 💅 should be fixed
💭 may be fixed
🎉 celebrate happy things

…` to `build-state`.

…es, in the same way they can contribute global, session, etc. scoped services. Use this to move some knowledge of cross build session scoped services provided by `core` back from `build-state` to `core`.

… from `build-state` to `core`.

…daemon-services`.

…nt-side and daemon-side types.

adammurdoch · 2024-05-15T20:04:04Z

platforms/core-runtime/build-state/build.gradle.kts


 dependencies {
    api(project(":core"))
    api(project(":base-services"))
    api(project(":java-language-extensions"))
-    api(project(":core-api"))


Moved some service registration back to core, where the services are implemented.

adammurdoch · 2024-05-15T20:04:47Z

...me/build-state/src/main/java/org/gradle/internal/buildprocess/BuildProcessScopeServices.java

+
+public class BuildProcessScopeServices {
+    void configure(ServiceRegistration registration, ClassLoaderRegistry classLoaderRegistry) {
+        final List<PluginServiceRegistry> pluginServiceFactories = new DefaultServiceLocator(classLoaderRegistry.getRuntimeClassLoader(), classLoaderRegistry.getPluginsClassLoader()).getAll(PluginServiceRegistry.class);


"build-state" is now responsible for locating all the plugin service registries, moved out of "core".

adammurdoch · 2024-05-15T20:06:19Z

...me/build-state/src/main/java/org/gradle/internal/buildprocess/BuildProcessScopeServices.java

+        GradleUserHomeScopeServiceRegistry userHomeServiceRegistry,
+        ServiceRegistry globalServices
+    ) {
+        return new SetupLoggingActionExecutor(


"build-state" is responsible for creating the outer most "build pipeline", which is the main entry point used by all of the things that need to run a build in the current process. Moved out of "launcher".

adammurdoch · 2024-05-15T20:07:31Z

...re-runtime/build-state/src/main/java/org/gradle/internal/buildprocess/BuildProcessState.java

@@ -42,7 +42,8 @@ public BuildProcessState(
        ServiceRegistryBuilder builder = ServiceRegistryBuilder.builder()
            .scopeStrictly(Scope.Global.class)
            .displayName("Global services")
-            .provider(new GlobalScopeServices(longLiving, agentStatus, additionalModuleClassPath));
+            .provider(new GlobalScopeServices(longLiving, agentStatus, additionalModuleClassPath))


GlobalScopeServices should be busted up, but this is a bit complicated, so in this PR leave it mostly unchanged.

adammurdoch · 2024-05-15T20:08:21Z

...runtime/build-state/src/main/java/org/gradle/internal/session/BuildSessionScopeServices.java

@@ -110,118 +62,9 @@ void configure(ServiceRegistration registration, List<PluginServiceRegistry> plu
        registration.add(BuildRequestMetaData.class, buildRequestMetaData);
        registration.add(BuildClientMetaData.class, buildClientMetaData);
        registration.add(BuildEventConsumer.class, buildEventConsumer);
-        registration.add(CalculatedValueContainerFactory.class);


Moved some service registration back to core, where the services are implemented.

adammurdoch · 2024-05-15T20:08:34Z

...re-runtime/build-state/src/main/java/org/gradle/internal/session/CrossBuildSessionState.java

            this.startParameter = startParameter;
        }

-        void configure(ServiceRegistration registration) {


Moved some service registration back to core, where the services are implemented.

adammurdoch · 2024-05-15T20:09:08Z

...ces/src/main/java/org/gradle/internal/daemon/client/execution/ClientBuildRequestContext.java

+/**
+ * A client-side host for {@link org.gradle.internal.invocation.BuildAction} execution.
+ */
+public class ClientBuildRequestContext {


This is the new client-side context type, split out of BuildRequestContext. It mostly mirrors the daemon-side context, but can later gain some additional state, such as the InputStream to consume "stdin" from.

adammurdoch · 2024-05-15T20:11:08Z

...e-runtime/daemon-protocol/src/main/java/org/gradle/configuration/GradleLauncherMetaData.java

 import org.gradle.internal.UncheckedException;

 import java.io.IOException;

-public class GradleLauncherMetaData implements BuildClientMetaData {
+public class GradleLauncherMetaData {


Moved to "daemon-protocol" and detangled from the "build process" type.

adammurdoch · 2024-05-15T20:11:42Z

...ms/core-runtime/daemon-protocol/src/main/java/org/gradle/launcher/daemon/protocol/Build.java

    private final long startTime;
    private final boolean interactive;
    private final BuildActionParameters parameters;

-    public Build(UUID identifier, byte[] token, BuildAction action, BuildClientMetaData buildClientMetaData, long startTime, boolean interactive, BuildActionParameters parameters) {
+    public Build(UUID identifier, byte[] token, BuildAction action, GradleLauncherMetaData buildClientMetaData, long startTime, boolean interactive, BuildActionParameters parameters) {


Use the shared type to carry client metadata from client to daemon.

adammurdoch · 2024-05-15T20:12:26Z

...untime/daemon-services/src/main/java/org/gradle/internal/daemon/services/DaemonServices.java

+import org.gradle.tooling.internal.provider.serialization.PayloadSerializer;
+import org.gradle.tooling.internal.provider.serialization.WellKnownClassLoaderRegistry;
+
+public class DaemonServices extends AbstractPluginServiceRegistry {


New service registry for the services implemented in "daemon-services", moved out of "launcher".

adammurdoch · 2024-05-15T20:14:52Z

platforms/core-runtime/launcher/src/main/java/org/gradle/launcher/cli/BuildActionsFactory.java

-            System.in,
-            globalServices.get(BuildExecutor.class)
-        );
+        BuildActionExecutor<BuildActionParameters, ClientBuildRequestContext> executor = new RunInProcess(


Use the client-side context instead of the daemon-side context in the gradle client, but when running in-process, use some adapters to convert client-side to daemon-side.

adammurdoch · 2024-05-15T20:15:28Z

platforms/core-runtime/launcher/src/main/java/org/gradle/launcher/cli/RunBuildAction.java

@@ -32,18 +30,18 @@
 import org.gradle.tooling.internal.provider.action.ExecuteBuildAction;

 public class RunBuildAction implements Runnable {
-    private final BuildActionExecutor<BuildActionParameters, BuildRequestContext> executer;
+    private final BuildActionExecutor<BuildActionParameters, ClientBuildRequestContext> executor;


Use client-side context in the gradle client.

adammurdoch · 2024-05-15T20:16:42Z

...core-runtime/launcher/src/main/java/org/gradle/launcher/daemon/server/exec/ExecuteBuild.java

@@ -53,7 +56,9 @@ protected void doBuild(final DaemonCommandExecution execution, Build build) {
        DaemonConnectionBackedEventConsumer buildEventConsumer = new DaemonConnectionBackedEventConsumer(execution);
        try {
            BuildCancellationToken cancellationToken = execution.getDaemonStateControl().getCancellationToken();
-            BuildRequestContext buildRequestContext = new DefaultBuildRequestContext(build.getBuildRequestMetaData(), cancellationToken, buildEventConsumer);
+            DefaultBuildClientMetaData clientMetaData = new DefaultBuildClientMetaData(build.getBuildClientMetaData());


Adapt from shared protocol type to daemon-side context in the daemon.

adammurdoch · 2024-05-15T20:17:43Z

...re-runtime/launcher/src/main/java/org/gradle/tooling/internal/provider/LauncherServices.java

        registration.addProvider(new ToolingGlobalScopeServices());
    }

-    @Override


Moved some service registration to "daemon-services" and "build-state".

adammurdoch · 2024-05-15T20:18:33Z

...e/launcher/src/main/java/org/gradle/tooling/internal/provider/DaemonBuildActionExecuter.java

@@ -27,20 +27,20 @@
 import org.gradle.launcher.exec.DefaultBuildActionParameters;
 import org.gradle.tooling.internal.provider.connection.ProviderOperationParameters;

-public class DaemonBuildActionExecuter implements BuildActionExecutor<ConnectionOperationParameters, BuildRequestContext> {


Use the client-side context in the tooling API client.

adammurdoch · 2024-05-15T20:18:54Z

...r/src/main/java/org/gradle/tooling/internal/provider/LoggingBridgingBuildActionExecuter.java

@@ -34,17 +34,17 @@
 /**
 * A {@link BuildActionExecutor} which routes Gradle logging to those listeners specified in the {@link ProviderOperationParameters} provided with a tooling api build request.
 */
-public class LoggingBridgingBuildActionExecuter implements BuildActionExecutor<ConnectionOperationParameters, BuildRequestContext> {
+public class LoggingBridgingBuildActionExecuter implements BuildActionExecutor<ConnectionOperationParameters, ClientBuildRequestContext> {


Use the client-side context in the tooling API client.

adammurdoch · 2024-05-15T20:19:26Z

...-runtime/launcher/src/main/java/org/gradle/tooling/internal/provider/ProviderConnection.java

@@ -258,10 +258,10 @@ private Object run(
        Parameters parameters
    ) {
        try {
-            BuildActionExecutor<ConnectionOperationParameters, BuildRequestContext> executer = createExecuter(providerParameters, parameters);


Use the client-side context in the tooling API client.

adammurdoch · 2024-05-15T20:20:33Z

subprojects/core/src/main/java/org/gradle/internal/service/scopes/CoreBuildSessionServices.java

+import java.io.Closeable;
+import java.io.File;
+
+public class CoreBuildSessionServices {


Moved some service registrations back to "core".

adammurdoch · 2024-05-15T20:20:41Z

...cts/core/src/main/java/org/gradle/internal/service/scopes/CoreCrossBuildSessionServices.java

+import org.gradle.internal.work.DefaultWorkerLeaseService;
+import org.gradle.internal.work.WorkerLeaseService;
+
+public class CoreCrossBuildSessionServices {


Moved some service registrations back to "core".

adammurdoch added 14 commits May 16, 2024 05:51

Move responsibility for locating plugin service registries from `core…

88740f1

…` to `build-state`.

Allow (core) projects to contribute cross build session scoped servic…

60e899d

…es, in the same way they can contribute global, session, etc. scoped services. Use this to move some knowledge of cross build session scoped services provided by `core` back from `build-state` to `core`.

Move some knowledge of session scope services provided by core back…

af0d90b

… from `build-state` to `core`.

Move some service registration from launcher to build-state and `…

640fa05

…daemon-services`.

Move some types to a new package to remove a split package.

a438f30

Mark some packages @NonNullApi

e8f7a5c

Rename some classes.

b6a2ccc

Move some types to new packages.

aaa4bd6

Mark some packages @NonNullApi

32c9051

Move some types to new packages.

ed89537

Mark some packages @NonNullApi

2fb9212

Remove a dependency from daemon-protocol to core.

b24f487

Split BuildClientRequestContext and BuildClientMetaData into clie…

106c48d

…nt-side and daemon-side types.

Move some types from core to daemon-protocol and daemon-services.

a9d4a99

adammurdoch requested review from a team as code owners May 15, 2024 20:01

adammurdoch self-assigned this May 15, 2024

adammurdoch added the in:daemon label May 15, 2024

adammurdoch commented May 15, 2024

View reviewed changes

gitstream-cm bot added 30 min review labels May 15, 2024

adammurdoch commented May 15, 2024

View reviewed changes

adammurdoch added this pull request to the merge queue May 16, 2024

bot-gradle added this to the 8.9 RC1 milestone May 16, 2024

Merged via the queue into master with commit 8e4699b May 16, 2024
26 of 27 checks passed

adammurdoch deleted the am/client-detangle branch May 16, 2024 21:45

alllex added the in:internal-services Internal services dependency injection label May 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove some coupling between the client and daemon #29167

Remove some coupling between the client and daemon #29167

adammurdoch commented May 15, 2024 •

edited

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024 •

edited

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024 •

edited

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

adammurdoch May 15, 2024

Remove some coupling between the client and daemon #29167

Remove some coupling between the client and daemon #29167

Conversation

adammurdoch commented May 15, 2024 • edited

Context

Contributor Checklist

Reviewing cheatsheet

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adammurdoch May 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adammurdoch May 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adammurdoch commented May 15, 2024 •

edited

adammurdoch May 15, 2024 •

edited

adammurdoch May 15, 2024 •

edited