From 23c3df16f5a33dd7b478418abc20906ff8958a2c Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 25 Sep 2025 10:52:05 -0700 Subject: [PATCH] Avoid running resolve logic with no transport version name (#135379) When resolving transport version conflicts we must lookup the definition that is being added by the current branch. We do this by detecting the resource file that has been added. When no files are added, we should not do anything. This commit moves the logic that detects branches closer to where it is needed so that we don't possibly run it with an empty transport version name. --- ...lveTransportVersionConflictFuncTest.groovy | 99 +++++++++++++++++++ .../TransportVersionGenerationFuncTest.groovy | 2 +- ...enerateTransportVersionDefinitionTask.java | 6 +- 3 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/ResolveTransportVersionConflictFuncTest.groovy diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/ResolveTransportVersionConflictFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/ResolveTransportVersionConflictFuncTest.groovy new file mode 100644 index 0000000000000..974637020ea37 --- /dev/null +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/ResolveTransportVersionConflictFuncTest.groovy @@ -0,0 +1,99 @@ +/* + * 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.transport + +import org.gradle.testkit.runner.BuildResult +import org.gradle.testkit.runner.GradleRunner +import org.gradle.testkit.runner.TaskOutcome + +class ResolveTransportVersionConflictFuncTest extends AbstractTransportVersionFuncTest { + + GradleRunner runResolveAndValidateTask() { + List args = List.of(":myserver:validateTransportVersionResources", ":myserver:resolveTransportVersionConflict") + return gradleRunner(args.toArray()) + } + + void assertResolveAndValidateSuccess(BuildResult result) { + assert result.task(":myserver:resolveTransportVersionConflict").outcome == TaskOutcome.SUCCESS + assert result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS + } + + def "update flag works with current"() { + given: + referableAndReferencedTransportVersion("new_tv", "8123000") + file("myserver/src/main/resources/transport/latest/9.2.csv").text = + """ + <<<<<<< HEAD + existing_92,8123000 + ======= + new_tv,8123000 + >>>>>> name + """.strip() + + when: + def result = runResolveAndValidateTask().build() + + then: + assertResolveAndValidateSuccess(result) + assertReferableDefinition("existing_92", "8123000,8012001") + assertReferableDefinition("new_tv", "8124000") + assertUpperBound("9.2", "new_tv,8124000") + } + + def "update flag works with multiple branches"() { + given: + referableAndReferencedTransportVersion("new_tv", "8123000,8012001,7123001") + file("myserver/src/main/resources/transport/latest/9.2.csv").text = + """ + <<<<<<< HEAD + existing_92,8123000 + ======= + new_tv,8123000 + >>>>>> name + """.strip() + file("myserver/src/main/resources/transport/latest/9.1.csv").text = + """ + <<<<<<< HEAD + existing_92,8012001 + ======= + new_tv,8012001 + >>>>>> name + """.strip() + file("myserver/src/main/resources/transport/latest/8.19.csv").text = + """ + <<<<<<< HEAD + initial_8.19.7,7123001 + ======= + new_tv,7123001 + >>>>>> name + """.strip() + + when: + def result = runResolveAndValidateTask().build() + + then: + assertResolveAndValidateSuccess(result) + assertReferableDefinition("existing_92", "8123000,8012001") + assertUnreferableDefinition("initial_8.19.7", "7123001") + assertReferableDefinition("new_tv", "8124000,8012002,7123002") + assertUpperBound("9.2", "new_tv,8124000") + assertUpperBound("9.1", "new_tv,8012002") + assertUpperBound("8.19", "new_tv,7123002") + } + + def "no new transport version is idempotent"() { + when: + def result = runResolveAndValidateTask().build() + + then: + assertResolveAndValidateSuccess(result) + assertUpperBound("9.2", "existing_92,8123000") + } +} diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy index d1ca9eae48b97..dcc3bd97e0451 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy @@ -499,7 +499,7 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes def "upper bounds files must exist for backport branches"() { when: - def result = runGenerateTask("--backport-branches=9.1,8.13,7.17,6.0").buildAndFail() + def result = runGenerateTask("--name", "new_tv", "--backport-branches=9.1,8.13,7.17,6.0").buildAndFail() then: assertGenerateFailure(result, "Missing upper bounds files for branches [6.0, 7.17, 8.13], known branches are [8.19, 9.0, 9.1, 9.2]") diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java index f0d910730dce9..2587fe55ecd85 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java @@ -102,14 +102,14 @@ public void run() throws IOException { List changedDefinitionNames = resources.getChangedReferableDefinitionNames(); String targetDefinitionName = getTargetDefinitionName(resources, referencedNames, changedDefinitionNames); - List upstreamUpperBounds = resources.getUpperBoundsFromUpstream(); - Set targetUpperBoundNames = getTargetUpperBoundNames(resources, upstreamUpperBounds, targetDefinitionName); - getLogger().lifecycle("Generating transport version name: " + targetDefinitionName); if (targetDefinitionName.isEmpty()) { // TODO: resetting upper bounds needs to be done locally, otherwise it pulls in some (incomplete) changes from upstream main // resetAllUpperBounds(resources); } else { + List upstreamUpperBounds = resources.getUpperBoundsFromUpstream(); + Set targetUpperBoundNames = getTargetUpperBoundNames(resources, upstreamUpperBounds, targetDefinitionName); + List ids = updateUpperBounds(resources, upstreamUpperBounds, targetUpperBoundNames, targetDefinitionName); // (Re)write the definition file. resources.writeDefinition(new TransportVersionDefinition(targetDefinitionName, ids, true));