From 35090eaf5066fa3bf8e183f0457c46a8c8ee40e0 Mon Sep 17 00:00:00 2001 From: Rene Groeschke Date: Thu, 28 Aug 2025 08:00:36 +0200 Subject: [PATCH] [Gradle] Some tweaks to transport related build logic (#133413) * [Gradle] Some tweaks to transport related build logic - We want to avoid project.allprojects - Implementing Verification task in the transport validation tasks - Allows better filtering for verification vs. other build failures. e.g. in Develocity Failures Overview - Use project.layout.settingsDirectory instead of project.rootProject.projectDir - Avoiding accessing other projects properties * Address some review feedback * Rework mapping project path list to dependencies * Some more review feedback * Minor tweak after review feedback --- .../ProjectSubscribeBuildService.java | 51 +++++++++++++++++++ .../ProjectSubscribeServicePlugin.java | 44 ++++++++++++++++ .../GenerateTransportVersionManifestTask.java | 1 - .../TransportVersionReferencesPlugin.java | 14 +++-- .../TransportVersionResourcesPlugin.java | 34 ++++++------- ...alidateTransportVersionReferencesTask.java | 6 ++- ...ValidateTransportVersionResourcesTask.java | 8 +-- 7 files changed, 131 insertions(+), 27 deletions(-) create mode 100644 build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeBuildService.java create mode 100644 build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeServicePlugin.java diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeBuildService.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeBuildService.java new file mode 100644 index 0000000000000..d9daf8e9d91b0 --- /dev/null +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeBuildService.java @@ -0,0 +1,51 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.gradle.internal; + +import org.gradle.api.Project; +import org.gradle.api.provider.Provider; +import org.gradle.api.provider.ProviderFactory; +import org.gradle.api.services.BuildService; +import org.gradle.api.services.BuildServiceParameters; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; + +import javax.inject.Inject; + +public abstract class ProjectSubscribeBuildService implements BuildService { + + private final ProviderFactory providerFactory; + + /** + * The filling of this map depends on the order of #registerProjectForTopic being called. + * This is usually done during configuration phase, but we do not enforce yet the time of this method call. + * The values are LinkedHashSet to preserve the order of registration mostly to provide a predicatable order + * when running consecutive builds. + * */ + private final Map> versionsByTopic = new HashMap<>(); + + @Inject + public ProjectSubscribeBuildService(ProviderFactory providerFactory) { + this.providerFactory = providerFactory; + } + + /** + * Returning a provider so the evaluation of the map value is deferred to when the provider is queried. + * */ + public Provider> getProjectsByTopic(String topic) { + return providerFactory.provider(() -> versionsByTopic.computeIfAbsent(topic, k -> new java.util.LinkedHashSet<>())); + } + + public void registerProjectForTopic(String topic, Project project) { + versionsByTopic.computeIfAbsent(topic, k -> new java.util.LinkedHashSet<>()).add(project.getPath()); + } +} diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeServicePlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeServicePlugin.java new file mode 100644 index 0000000000000..32ed1cf875ddf --- /dev/null +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeServicePlugin.java @@ -0,0 +1,44 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.gradle.internal; + +import org.gradle.api.Plugin; +import org.gradle.api.Project; +import org.gradle.api.provider.Provider; + +/** + * This plugin registers a {@link ProjectSubscribeBuildService} to allow projects to + * communicate with each other during the configuration phase. + * + * For example, a project can register itself as a publisher of a topic, and other + * projects can resolve projects that have registered as publishers of that topic. + * + * The actual resolution of whatever data is usually done using dependency declarations. + * Be aware the state of the list depends on the order of project configuration and + * consuming on configuration phase before task graph calculation phase should be avoided. + * + * We want to avoid discouraged plugin api usage like project.allprojects or project.subprojects + * in plugins to avoid unnecessary configuration of projects and not break project isolation and break + * See https://docs.gradle.org/current/userguide/isolated_projects.html + * */ +public class ProjectSubscribeServicePlugin implements Plugin { + + private Provider publishSubscribe; + + @Override + public void apply(Project project) { + publishSubscribe = project.getGradle().getSharedServices().registerIfAbsent("publishSubscribe", ProjectSubscribeBuildService.class); + } + + public Provider getService() { + return publishSubscribe; + } + +} diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java index 153fab4723ac0..f0a0ee09cd3a2 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java @@ -44,7 +44,6 @@ public Path getDefinitionsDirectory() { @TaskAction public void generateTransportVersionManifest() throws IOException { - Path definitionsDir = getDefinitionsDirectory(); Path manifestFile = getManifestFile().get().getAsFile().toPath(); try (var writer = Files.newBufferedWriter(manifestFile)) { diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReferencesPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReferencesPlugin.java index da3f056825aeb..5ee561764b5a4 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReferencesPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReferencesPlugin.java @@ -9,19 +9,27 @@ package org.elasticsearch.gradle.internal.transport; +import org.elasticsearch.gradle.internal.ProjectSubscribeServicePlugin; import org.elasticsearch.gradle.util.GradleUtils; import org.gradle.api.Plugin; import org.gradle.api.Project; -import org.gradle.api.artifacts.Configuration; import org.gradle.api.tasks.SourceSet; import org.gradle.language.base.plugins.LifecycleBasePlugin; +import static org.elasticsearch.gradle.internal.transport.TransportVersionResourcesPlugin.TRANSPORT_REFERENCES_TOPIC; + public class TransportVersionReferencesPlugin implements Plugin { @Override public void apply(Project project) { project.getPluginManager().apply(LifecycleBasePlugin.class); + project.getPlugins() + .apply(ProjectSubscribeServicePlugin.class) + .getService() + .get() + .registerProjectForTopic(TRANSPORT_REFERENCES_TOPIC, project); + var collectTask = project.getTasks() .register("collectTransportVersionReferences", CollectTransportVersionReferencesTask.class, t -> { t.setGroup("Transport Versions"); @@ -31,9 +39,7 @@ public void apply(Project project) { t.getOutputFile().set(project.getLayout().getBuildDirectory().file("transport-version/references.txt")); }); - Configuration tvReferencesConfig = project.getConfigurations().create("transportVersionReferences", c -> { - c.setCanBeConsumed(true); - c.setCanBeResolved(false); + var tvReferencesConfig = project.getConfigurations().consumable("transportVersionReferences", c -> { c.attributes(TransportVersionReference::addArtifactAttribute); }); project.getArtifacts().add(tvReferencesConfig.getName(), collectTask); diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java index 763d50e6e1267..2b1c61ecd75d4 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java @@ -9,10 +9,9 @@ package org.elasticsearch.gradle.internal.transport; +import org.elasticsearch.gradle.internal.ProjectSubscribeServicePlugin; import org.gradle.api.Plugin; import org.gradle.api.Project; -import org.gradle.api.artifacts.Configuration; -import org.gradle.api.artifacts.dsl.DependencyHandler; import org.gradle.api.file.Directory; import org.gradle.api.plugins.JavaPlugin; import org.gradle.api.tasks.Copy; @@ -22,32 +21,33 @@ public class TransportVersionResourcesPlugin implements Plugin { + public static final String TRANSPORT_REFERENCES_TOPIC = "transportReferences"; + @Override public void apply(Project project) { project.getPluginManager().apply(LifecycleBasePlugin.class); - - String resourceRoot = getResourceRoot(project); + var psService = project.getPlugins().apply(ProjectSubscribeServicePlugin.class).getService(); + var resourceRoot = getResourceRoot(project); project.getGradle() .getSharedServices() .registerIfAbsent("transportVersionResources", TransportVersionResourcesService.class, spec -> { Directory transportResources = project.getLayout().getProjectDirectory().dir("src/main/resources/" + resourceRoot); spec.getParameters().getTransportResourcesDirectory().set(transportResources); - spec.getParameters().getRootDirectory().set(project.getRootProject().getRootDir()); + spec.getParameters().getRootDirectory().set(project.getLayout().getSettingsDirectory().getAsFile()); }); - DependencyHandler depsHandler = project.getDependencies(); - Configuration tvReferencesConfig = project.getConfigurations().create("globalTvReferences"); - tvReferencesConfig.setCanBeConsumed(false); - tvReferencesConfig.setCanBeResolved(true); - tvReferencesConfig.attributes(TransportVersionReference::addArtifactAttribute); - - // iterate through all projects, and if the management plugin is applied, add that project back as a dep to check - for (Project subProject : project.getRootProject().getSubprojects()) { - subProject.getPlugins().withType(TransportVersionReferencesPlugin.class).configureEach(plugin -> { - tvReferencesConfig.getDependencies().add(depsHandler.project(Map.of("path", subProject.getPath()))); - }); - } + var depsHandler = project.getDependencies(); + var tvReferencesConfig = project.getConfigurations().create("globalTvReferences", c -> { + c.setCanBeConsumed(false); + c.setCanBeResolved(true); + c.attributes(TransportVersionReference::addArtifactAttribute); + c.getDependencies() + .addAllLater( + psService.flatMap(t -> t.getProjectsByTopic(TRANSPORT_REFERENCES_TOPIC)) + .map(projectPaths -> projectPaths.stream().map(path -> depsHandler.project(Map.of("path", path))).toList()) + ); + }); var validateTask = project.getTasks() .register("validateTransportVersionResources", ValidateTransportVersionResourcesTask.class, t -> { diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java index d72a4f097218e..6f7319bd4bcc6 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java @@ -20,6 +20,8 @@ import org.gradle.api.tasks.PathSensitive; import org.gradle.api.tasks.PathSensitivity; import org.gradle.api.tasks.TaskAction; +import org.gradle.api.tasks.VerificationException; +import org.gradle.api.tasks.VerificationTask; import java.io.IOException; import java.nio.file.Path; @@ -28,7 +30,7 @@ * Validates that each transport version reference has a referable definition. */ @CacheableTask -public abstract class ValidateTransportVersionReferencesTask extends DefaultTask { +public abstract class ValidateTransportVersionReferencesTask extends DefaultTask implements VerificationTask { @ServiceReference("transportVersionResources") abstract Property getTransportResources(); @@ -51,7 +53,7 @@ public void validateTransportVersions() throws IOException { for (var tvReference : TransportVersionReference.listFromFile(namesFile)) { if (resources.referableDefinitionExists(tvReference.name()) == false) { - throw new RuntimeException( + throw new VerificationException( "TransportVersion.fromName(\"" + tvReference.name() + "\") was used at " diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java index fb88e2b359299..6212485e30b29 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java @@ -22,6 +22,8 @@ import org.gradle.api.tasks.PathSensitive; import org.gradle.api.tasks.PathSensitivity; import org.gradle.api.tasks.TaskAction; +import org.gradle.api.tasks.VerificationException; +import org.gradle.api.tasks.VerificationTask; import java.io.IOException; import java.nio.file.Path; @@ -38,7 +40,7 @@ * Validates that each defined transport version constant is referenced by at least one project. */ @CacheableTask -public abstract class ValidateTransportVersionResourcesTask extends DefaultTask { +public abstract class ValidateTransportVersionResourcesTask extends DefaultTask implements VerificationTask { @InputDirectory @Optional @@ -255,11 +257,11 @@ private void validateBase(int base, List ids) { private void throwDefinitionFailure(TransportVersionDefinition definition, String message) { Path relativePath = getResources().get().getReferableDefinitionRepositoryPath(definition); - throw new IllegalStateException("Transport version definition file [" + relativePath + "] " + message); + throw new VerificationException("Transport version definition file [" + relativePath + "] " + message); } private void throwUpperBoundFailure(TransportVersionUpperBound upperBound, String message) { Path relativePath = getResources().get().getUpperBoundRepositoryPath(upperBound); - throw new IllegalStateException("Transport version upper bound file [" + relativePath + "] " + message); + throw new VerificationException("Transport version upper bound file [" + relativePath + "] " + message); } }