From cf458cb66575c10603ec74992d08b9854b2a5605 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 9 Sep 2025 21:24:10 -0700 Subject: [PATCH 1/5] Add --update flag to transport version generate task When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds an `--update` flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: `./gradlew generateTransportVersion --update` --- .../TransportVersionGenerationFuncTest.groovy | 71 +++++++++++++++++++ .../GenerateInitialTransportVersionTask.java | 4 +- ...enerateTransportVersionDefinitionTask.java | 51 +++++++++++-- .../TransportVersionResourcesService.java | 12 +++- 4 files changed, 130 insertions(+), 8 deletions(-) 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 21a8b6a95e105..3da5476ca30c6 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 @@ -277,6 +277,77 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes assertUpperBound("9.2", "second_tv,8124000") } + 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 = runGenerateAndValidateTask("--update").build() + + then: + assertGenerateAndValidateSuccess(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 = runGenerateAndValidateTask("--update").build() + + then: + assertGenerateAndValidateSuccess(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 "update flag cannot be used with backport branches"() { + when: + def result = runGenerateTask("--update", "--backport-branches=9.1").buildAndFail() + + then: + assertGenerateFailure(result, "Cannot use --update with --backport-branches") + } + def "branches param order does not matter"() { given: referencedTransportVersion("test_tv") diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateInitialTransportVersionTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateInitialTransportVersionTask.java index a10c3fabb7ba9..31909819c2ad5 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateInitialTransportVersionTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateInitialTransportVersionTask.java @@ -55,13 +55,13 @@ public void run() throws IOException { var definition = new TransportVersionDefinition(initialDefinitionName, List.of(id)); resources.writeUnreferableDefinition(definition); var newUpperBound = new TransportVersionUpperBound(upperBoundName, initialDefinitionName, id); - resources.writeUpperBound(newUpperBound); + resources.writeUpperBound(newUpperBound, false); if (releaseVersion.getRevision() == 0) { Version currentVersion = getCurrentVersion().get(); String currentUpperBoundName = getUpperBoundName(currentVersion); var currentUpperBound = new TransportVersionUpperBound(currentUpperBoundName, initialDefinitionName, id); - resources.writeUpperBound(currentUpperBound); + resources.writeUpperBound(currentUpperBound, false); } } 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 8c45823ea6de0..5412e5213b464 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 @@ -67,6 +67,11 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask @Option(option = "increment", description = "The amount to increment the id from the current upper bounds file by") public abstract Property getIncrement(); + @Input + @Optional + @Option(option = "update", description = "Update the transport version currently being added to upstream") + public abstract Property getIsUpdate(); + /** * The name of the upper bounds file which will be used at runtime on the current branch. Normally * this equates to VersionProperties.getElasticsearchVersion(). @@ -82,7 +87,7 @@ public void run() throws IOException { String targetDefinitionName = getTargetDefinitionName(resources, referencedNames, changedDefinitionNames); List upstreamUpperBounds = resources.getUpperBoundsFromUpstream(); - Set targetUpperBoundNames = getTargetUpperBoundNames(upstreamUpperBounds); + Set targetUpperBoundNames = getTargetUpperBoundNames(resources, upstreamUpperBounds, targetDefinitionName); getLogger().lifecycle("Generating transport version name: " + targetDefinitionName); if (targetDefinitionName.isEmpty()) { @@ -108,6 +113,7 @@ private List updateUpperBounds( throw new IllegalArgumentException("Invalid increment " + increment + ", must be a positive integer"); } List ids = new ArrayList<>(); + boolean stageInGit = getIsUpdate().getOrElse(false); TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromUpstream(definitionName); for (TransportVersionUpperBound existingUpperBound : existingUpperBounds) { @@ -121,12 +127,12 @@ private List updateUpperBounds( int targetIncrement = upperBoundName.equals(currentUpperBoundName) ? increment : 1; targetId = TransportVersionId.fromInt(existingUpperBound.definitionId().complete() + targetIncrement); var newUpperBound = new TransportVersionUpperBound(upperBoundName, definitionName, targetId); - resources.writeUpperBound(newUpperBound); + resources.writeUpperBound(newUpperBound, stageInGit); } ids.add(targetId); } else { // Default case: we're not targeting this branch so reset it - resources.writeUpperBound(existingUpperBound); + resources.writeUpperBound(existingUpperBound, false); } } @@ -167,7 +173,19 @@ private String getTargetDefinitionName( } } - private Set getTargetUpperBoundNames(List upstreamUpperBounds) { + private Set getTargetUpperBoundNames( + TransportVersionResourcesService resources, + List upstreamUpperBounds, + String targetDefinitionName + ) throws IOException { + if (getIsUpdate().getOrElse(false)) { + if (getBackportBranches().isPresent()) { + throw new IllegalArgumentException("Cannot use --update with --backport-branches"); + } + + return getUpperBoundNamesFromDefinition(resources, upstreamUpperBounds, targetDefinitionName); + } + Set targetUpperBoundNames = new HashSet<>(); targetUpperBoundNames.add(getCurrentUpperBoundName().get()); if (getBackportBranches().isPresent()) { @@ -191,9 +209,32 @@ private Set getTargetUpperBoundNames(List up return targetUpperBoundNames; } + private Set getUpperBoundNamesFromDefinition( + TransportVersionResourcesService resources, + List upstreamUpperBounds, + String targetDefinitionName + ) throws IOException { + TransportVersionDefinition definition = resources.getReferableDefinition(targetDefinitionName); + Set upperBoundNames = new HashSet<>(); + upperBoundNames.add(getCurrentUpperBoundName().get()); + + // skip the primary id as that is current, which we always add + for (int i = 1; i < definition.ids().size(); ++i) { + TransportVersionId id = definition.ids().get(i); + // we have a small number of upper bound files, so just scan for the ones we want + for (TransportVersionUpperBound upperBound : upstreamUpperBounds) { + if (upperBound.definitionId().base() == id.base()) { + upperBoundNames.add(upperBound.name()); + } + } + } + + return upperBoundNames; + } + private void resetAllUpperBounds(TransportVersionResourcesService resources) throws IOException { for (TransportVersionUpperBound upperBound : resources.getUpperBoundsFromUpstream()) { - resources.writeUpperBound(upperBound); + resources.writeUpperBound(upperBound, false); } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java index 7df2931c93717..c115a2a7b1712 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java @@ -107,6 +107,12 @@ Map getReferableDefinitions() throws IOExcep return readDefinitions(transportResourcesDir.resolve(REFERABLE_DIR)); } + /** Return a single referable definition by name */ + TransportVersionDefinition getReferableDefinition(String name) throws IOException { + Path resourcePath = transportResourcesDir.resolve(getReferableDefinitionRelativePath(name)); + return TransportVersionDefinition.fromString(resourcePath, Files.readString(resourcePath, StandardCharsets.UTF_8)); + } + /** Get a referable definition from upstream if it exists there, or null otherwise */ TransportVersionDefinition getReferableDefinitionFromUpstream(String name) { Path resourcePath = getReferableDefinitionRelativePath(name); @@ -218,10 +224,14 @@ List getUpperBoundsFromUpstream() throws IOException } /** Write the given upper bound to a file in the transport resources */ - void writeUpperBound(TransportVersionUpperBound upperBound) throws IOException { + void writeUpperBound(TransportVersionUpperBound upperBound, boolean stageInGit) throws IOException { Path path = transportResourcesDir.resolve(getUpperBoundRelativePath(upperBound.name())); logger.debug("Writing upper bound [" + upperBound + "] to [" + path + "]"); Files.writeString(path, upperBound.definitionName() + "," + upperBound.definitionId().complete() + "\n", StandardCharsets.UTF_8); + + if (stageInGit) { + gitCommand("add", path.toString()); + } } /** Return the path within the repository of the given latest */ From 1f80cee7249b6f3535555329883cbcdaecdd7d83 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 10 Sep 2025 11:28:13 -0700 Subject: [PATCH 2/5] update flag name --- .../transport/GenerateTransportVersionDefinitionTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5412e5213b464..2efc64a8391aa 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 @@ -69,7 +69,7 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask @Input @Optional - @Option(option = "update", description = "Update the transport version currently being added to upstream") + @Option(option = "resolveConflict", description = "Regenerate the transport version currently being added to upstream to resolve a merge conflict") public abstract Property getIsUpdate(); /** From e005705b82718340e635e51d404eada43db82e3b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 10 Sep 2025 11:29:21 -0700 Subject: [PATCH 3/5] iter --- .../GenerateTransportVersionDefinitionTask.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 2efc64a8391aa..27a7ce8e4d10b 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 @@ -69,8 +69,8 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask @Input @Optional - @Option(option = "resolveConflict", description = "Regenerate the transport version currently being added to upstream to resolve a merge conflict") - public abstract Property getIsUpdate(); + @Option(option = "resolve-conflict", description = "Regenerate the transport version currently being added to upstream to resolve a merge conflict") + public abstract Property getResolveConflict(); /** * The name of the upper bounds file which will be used at runtime on the current branch. Normally @@ -113,7 +113,7 @@ private List updateUpperBounds( throw new IllegalArgumentException("Invalid increment " + increment + ", must be a positive integer"); } List ids = new ArrayList<>(); - boolean stageInGit = getIsUpdate().getOrElse(false); + boolean stageInGit = getResolveConflict().getOrElse(false); TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromUpstream(definitionName); for (TransportVersionUpperBound existingUpperBound : existingUpperBounds) { @@ -178,9 +178,9 @@ private Set getTargetUpperBoundNames( List upstreamUpperBounds, String targetDefinitionName ) throws IOException { - if (getIsUpdate().getOrElse(false)) { + if (getResolveConflict().getOrElse(false)) { if (getBackportBranches().isPresent()) { - throw new IllegalArgumentException("Cannot use --update with --backport-branches"); + throw new IllegalArgumentException("Cannot use --resolve-conflict with --backport-branches"); } return getUpperBoundNamesFromDefinition(resources, upstreamUpperBounds, targetDefinitionName); From 2dcec3d808d4a8c27679e9560e4c0022a657e6cd Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 10 Sep 2025 18:37:16 +0000 Subject: [PATCH 4/5] [CI] Auto commit changes from spotless --- .../transport/GenerateTransportVersionDefinitionTask.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 27a7ce8e4d10b..3c78385de74d2 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 @@ -69,7 +69,10 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask @Input @Optional - @Option(option = "resolve-conflict", description = "Regenerate the transport version currently being added to upstream to resolve a merge conflict") + @Option( + option = "resolve-conflict", + description = "Regenerate the transport version currently being added to upstream to resolve a merge conflict" + ) public abstract Property getResolveConflict(); /** From 1d264ff5dfaf4fce55c543ee61c89e55de128d46 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 11 Sep 2025 09:01:36 -0700 Subject: [PATCH 5/5] fix flag in tests --- .../transport/TransportVersionGenerationFuncTest.groovy | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 3da5476ca30c6..e59d56b1c9af0 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 @@ -290,7 +290,7 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes """.strip() when: - def result = runGenerateAndValidateTask("--update").build() + def result = runGenerateAndValidateTask("--resolve-conflict").build() then: assertGenerateAndValidateSuccess(result) @@ -328,7 +328,7 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes """.strip() when: - def result = runGenerateAndValidateTask("--update").build() + def result = runGenerateAndValidateTask("--resolve-conflict").build() then: assertGenerateAndValidateSuccess(result) @@ -342,10 +342,10 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes def "update flag cannot be used with backport branches"() { when: - def result = runGenerateTask("--update", "--backport-branches=9.1").buildAndFail() + def result = runGenerateTask("--resolve-conflict", "--backport-branches=9.1").buildAndFail() then: - assertGenerateFailure(result, "Cannot use --update with --backport-branches") + assertGenerateFailure(result, "Cannot use --resolve-conflict with --backport-branches") } def "branches param order does not matter"() {