[Gradle] Some tweaks to transport related build logic #133413

breskeby · 2025-08-22T20:41:22Z

We want to avoid project.allprojects as sooner or later this will start breaking
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

elasticsearchmachine · 2025-08-22T20:41:47Z

Pinging @elastic/es-delivery (Team:Delivery)

rjernst

A couple questions about the subscriber setup

rjernst · 2025-08-26T12:47:47Z

...s-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeBuildService.java

+
+public abstract class ProjectSubscribeBuildService implements BuildService<BuildServiceParameters.None> {
+
+    public static final String TRANSPORT_REFERENCES_TOPIC = "transportReferences";


nit: since this class should be generic, could this topic string be moved into the transport version classes, perhaps in TransportVersionResourcesPlugin?

rjernst · 2025-08-26T12:49:03Z

...s-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeBuildService.java

+    private Map<String, Collection<String>> versionsByTopic = new HashMap<>();
+
+    public Collection<String> getProjectsByTopic(String topic) {
+        return versionsByTopic.get(topic);


Can you add a javadoc explaining the expectation of the returned collection? It appears it's supposed to be live, right?

Separately, I think this also needs to be a computeIfAbsent. Otherwise if getProjectsByTopic is called before any register, it will return null rather than an empty list that will be updated.

Yeah, "live" isn't really the write term here. We are just returning a mutable collection, which seems wrong. To me I think it would make sense to return a Provider which wraps and immutable collection representing that registered projects at the time whenever you call get().

I also think we should standardize on using Provider-based APIs pretty much anytime we have "lazy" stuff like this, or when we want to make it obvious that eagerly resolving this might (a) have performance impacts or (b) might give you incomplete results.

rjernst · 2025-08-26T12:52:58Z

...c/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java

+            c.setCanBeConsumed(false);
+            c.setCanBeResolved(true);
+            c.attributes(TransportVersionReference::addArtifactAttribute);
+            c.defaultDependencies(t -> {


When will this be called relative to other projects? Can there not be projects added later?

this will be called when the configuration is resolved. usually thats part of the taskgraph calculation when using configuration cache or even later when evaluating inputs of the task required.
At this point all projects should be evaluated.

At one point we want to support only partial gradle configuration so we don't need to reevaluate the world when just triggering subprojects. then we probably need an approach to handle those scenario. but we and gradle is not there yet

As I mentioned above, I think if we make ProjectSubscribeBuildService a Provider-based API this becomes a lot less confusing. You can then just use DependencyHandler.addProvider and use .map() to have it be providers all the way down. IMO calling get() anywhere in configuration logic is a smell. It means we're relying on some kind of implicit deferral to happen or we're forced to awkwardly integrate with another non-Provider API.

In theory thats nice and the way to go. unfortunately (again) the provider based interfaces in Gradle often break that by themself. In this case we have a Provider<Collection> but DependencyHandler.addProvider only takes a Provider<String> representing a dependencyNotation. To have that working nicely we would need DependencyHandler.addProvider(String, Collection<Object>). Alternatively we return a Collection<Provider<String>> :/

We still can be good citizens but we're limited by the reality that the provider api in Gradle is lacking proper coverage in certain places.

I found a different api by now that is closer to idomatic provider usage

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()) ); });

mark-vieira · 2025-08-26T22:14:03Z

.../java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java

 */
 @CacheableTask
-public abstract class ValidateTransportVersionResourcesTask extends DefaultTask {
+public abstract class ValidateTransportVersionResourcesTask extends DefaultTask implements VerificationTask {


Are we throwing error messages that align with this expecation? My understanding was that Develocity also requires the failure message to match a certain pattern. See https://docs.gradle.com/develocity/failure-classification/

good catch! I was too naive here thinking using the task interface is enough. I'll have a quick look here

I fixed the throwing exceptions here

- 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

rjernst

LGTM

rjernst · 2025-08-27T15:55:01Z

...s-internal/src/main/java/org/elasticsearch/gradle/internal/ProjectSubscribeBuildService.java

+     * The values are LinkedHashSet to preserve the order of registration mostly to provide a predicatable order
+     * when running consecutive builds.
+     * */
+    private Map<String, Collection<String>> versionsByTopic = new HashMap<>();


nit: this can be final

* [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

elasticsearchmachine · 2025-08-28T06:02:15Z

💚 Backport successful

Status	Branch	Result
✅	9.1
✅	9.0
✅	8.18
✅	8.19

) * [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

elastic#133695) * [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

breskeby requested a review from a team as a code owner August 22, 2025 20:41

breskeby added >non-issue :Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team auto-backport Automatically create backport pull requests when merged v9.2.0 v8.17.11 v8.18.6 v8.19.3 v9.1.4 v9.0.7 labels Aug 22, 2025

breskeby self-assigned this Aug 22, 2025

breskeby requested a review from a team August 26, 2025 06:47

elasticsearchmachine added v8.19.4 v8.18.7 and removed v8.19.3 v8.18.6 labels Aug 26, 2025

rjernst reviewed Aug 26, 2025

View reviewed changes

mark-vieira reviewed Aug 26, 2025

View reviewed changes

breskeby added 2 commits August 27, 2025 08:33

Address some review feedback

86fc83b

breskeby force-pushed the gradle-transport-polishing branch from 54b1a8c to 86fc83b Compare August 27, 2025 08:21

breskeby added 2 commits August 27, 2025 15:54

Rework mapping project path list to dependencies

9861447

Some more review feedback

066f879

breskeby requested review from mark-vieira and rjernst August 27, 2025 14:19

rjernst approved these changes Aug 27, 2025

View reviewed changes

breskeby and others added 2 commits August 27, 2025 18:32

Minor tweak after review feedback

8c06add

Merge branch 'main' into gradle-transport-polishing

cd833dc

mark-vieira approved these changes Aug 27, 2025

View reviewed changes

breskeby removed the v8.17.11 label Aug 28, 2025

breskeby merged commit fa531c9 into elastic:main Aug 28, 2025
33 of 34 checks passed

breskeby mentioned this pull request Aug 28, 2025

[9.1] [Gradle] Some tweaks to transport related build logic (#133413) #133692

Merged

breskeby mentioned this pull request Aug 28, 2025

[9.0] [Gradle] Some tweaks to transport related build logic (#133413) #133693

Merged

breskeby mentioned this pull request Aug 28, 2025

[8.18] [Gradle] Some tweaks to transport related build logic (#133413) #133694

Merged

breskeby mentioned this pull request Aug 28, 2025

[8.19] [Gradle] Some tweaks to transport related build logic (#133413) #133695

Merged


		public abstract class ProjectSubscribeBuildService implements BuildService<BuildServiceParameters.None> {

		public static final String TRANSPORT_REFERENCES_TOPIC = "transportReferences";

[Gradle] Some tweaks to transport related build logic #133413

[Gradle] Some tweaks to transport related build logic #133413

Uh oh!

Conversation

breskeby commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Aug 22, 2025

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-vieira Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Aug 28, 2025

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

breskeby commented Aug 22, 2025 •

edited

Loading

mark-vieira Aug 26, 2025 •

edited

Loading