From 13b8be149418cebeff30025f98f6d2948969ffd6 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 12 Sep 2025 10:25:41 -0700 Subject: [PATCH] Add --resolve-conflict flag to transport version generate task (#134421) 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 a --resolve-conflict 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 --resolve-conflict --- .../TransportVersionGenerationFuncTest.groovy | 71 +++++++++++++++++++ .../GenerateInitialTransportVersionTask.java | 4 +- ...enerateTransportVersionDefinitionTask.java | 54 ++++++++++++-- .../TransportVersionResourcesService.java | 12 +++- 4 files changed, 133 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 a6a421e545557..0762607e15b9b 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("--resolve-conflict").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("--resolve-conflict").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("--resolve-conflict", "--backport-branches=9.1").buildAndFail() + + then: + assertGenerateFailure(result, "Cannot use --resolve-conflict 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 931c207e69111..21af9e10557e3 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 @@ -72,6 +72,14 @@ 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 = "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 * this equates to VersionProperties.getElasticsearchVersion(). @@ -95,7 +103,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()) { @@ -121,6 +129,7 @@ private List updateUpperBounds( throw new IllegalArgumentException("Invalid increment " + increment + ", must be a positive integer"); } List ids = new ArrayList<>(); + boolean stageInGit = getResolveConflict().getOrElse(false); TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromUpstream(definitionName); for (TransportVersionUpperBound existingUpperBound : existingUpperBounds) { @@ -134,12 +143,12 @@ private List updateUpperBounds( int targetIncrement = upperBoundName.equals(currentUpperBoundName) ? increment : 1; targetId = createTargetId(existingUpperBound, 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); } } @@ -180,7 +189,19 @@ private String getTargetDefinitionName( } } - private Set getTargetUpperBoundNames(List upstreamUpperBounds) { + private Set getTargetUpperBoundNames( + TransportVersionResourcesService resources, + List upstreamUpperBounds, + String targetDefinitionName + ) throws IOException { + if (getResolveConflict().getOrElse(false)) { + if (getBackportBranches().isPresent()) { + throw new IllegalArgumentException("Cannot use --resolve-conflict with --backport-branches"); + } + + return getUpperBoundNamesFromDefinition(resources, upstreamUpperBounds, targetDefinitionName); + } + Set targetUpperBoundNames = new HashSet<>(); targetUpperBoundNames.add(getCurrentUpperBoundName().get()); if (getBackportBranches().isPresent()) { @@ -204,9 +225,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 */