diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy index 5c63de1701c4f..edfca7e6918b5 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy @@ -14,6 +14,7 @@ import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.TaskOutcome class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest { + def javaResource(String project, String path, String content) { file("${project}/src/main/resources/${path}").withWriter { writer -> writer << content @@ -45,10 +46,18 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest { return referableAndReferencedTransportVersion(name, ids, "Test${name.capitalize()}") } - def referableAndReferencedTransportVersion(String name, String ids, String classname) { + def referencedTransportVersion(String name) { + referencedTransportVersion(name, "Test${name.capitalize()}") + } + + def referencedTransportVersion(String name, String classname) { javaSource("myserver", "org.elasticsearch", classname, "", """ static final TransportVersion usage = TransportVersion.fromName("${name}"); """) + } + + def referableAndReferencedTransportVersion(String name, String ids, String classname) { + referencedTransportVersion(name, classname) referableTransportVersion(name, ids) } @@ -74,6 +83,20 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest { assertOutputContains(result.output, expectedOutput) } + void assertReferableDefinition(String name, String content) { + File definitionFile = file("myserver/src/main/resources/transport/definitions/referable/${name}.csv") + assert definitionFile.exists() + assert definitionFile.text.strip() == content + } + + void assertReferableDefinitionDoesNotExist(String name) { + assert file("myserver/src/main/resources/transport/definitions/referable/${name}.csv").exists() == false + } + + void assertUpperBound(String name, String content) { + assert file("myserver/src/main/resources/transport/upper_bounds/${name}.csv").text.strip() == content + } + def setup() { configurationCacheCompatible = false internalBuild() @@ -86,12 +109,17 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest { apply plugin: 'java-library' apply plugin: 'elasticsearch.transport-version-references' apply plugin: 'elasticsearch.transport-version-resources' + + tasks.named('generateTransportVersionDefinition') { + currentUpperBoundName = '9.2' + } """ referableTransportVersion("existing_91", "8012000") referableTransportVersion("existing_92", "8123000,8012001") unreferableTransportVersion("initial_9_0_0", "8000000") transportVersionUpperBound("9.2", "existing_92", "8123000") transportVersionUpperBound("9.1", "existing_92", "8012001") + transportVersionUpperBound("9.0", "initial_9_0_0", "8000000") // a mock version of TransportVersion, just here so we can compile Dummy.java et al javaSource("myserver", "org.elasticsearch", "TransportVersion", "", """ public static TransportVersion fromName(String name) { 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 new file mode 100644 index 0000000000000..49f849a9996d3 --- /dev/null +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy @@ -0,0 +1,446 @@ +/* + * 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.TaskOutcome + +class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTest { + + def runGenerateAndValidateTask(String... additionalArgs) { + List args = new ArrayList<>() + args.add(":myserver:validateTransportVersionResources") + args.add(":myserver:generateTransportVersionDefinition") + args.addAll(additionalArgs); + return gradleRunner(args.toArray()) + } + + def runGenerateTask(String... additionalArgs) { + List args = new ArrayList<>() + args.add(":myserver:generateTransportVersionDefinition") + args.addAll(additionalArgs); + return gradleRunner(args.toArray()) + } + + def runValidateTask() { + return gradleRunner(":myserver:validateTransportVersionResources") + } + + void assertGenerateSuccess(BuildResult result) { + assert result.task(":myserver:generateTransportVersionDefinition").outcome == TaskOutcome.SUCCESS + } + + void assertGenerateFailure(BuildResult result, String expectedOutput) { + assert result.task(":myserver:generateTransportVersionDefinition").outcome == TaskOutcome.FAILED + assertOutputContains(result.output, expectedOutput) + } + + void assertValidateSuccess(BuildResult result) { + assert result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS + } + + void assertGenerateAndValidateSuccess(BuildResult result) { + assertGenerateSuccess(result) + assertValidateSuccess(result) + } + + def "setup is valid"() { + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + } + + def "a definition should be generated when specified by an arg but no code reference exists yet"() { + when: + def result = runGenerateTask("--name=new_tv").build() + + then: + assertGenerateSuccess(result) + assertReferableDefinition("new_tv", "8124000") + assertUpperBound("9.2", "new_tv,8124000") + + when: + referencedTransportVersion("new_tv") + def validateResult = runValidateTask().build() + + then: + assertValidateSuccess(validateResult) + } + + def "a definition should be generated when only a code reference exists"() { + given: + referencedTransportVersion("new_tv") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("new_tv", "8124000") + assertUpperBound("9.2", "new_tv,8124000") + } + + def "invalid changes to a upper bounds should be reverted"() { + given: + transportVersionUpperBound("9.2", "modification", "9000000") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertUpperBound("9.2", "existing_92,8123000") + } + + def "invalid changes to multiple upper bounds should be reverted"() { + given: + transportVersionUpperBound("9.2", "modification", "9000000") + transportVersionUpperBound("9.1", "modification", "9000000") + + when: + def result = runGenerateAndValidateTask("--backport-branches=9.1").build() + + then: + assertGenerateAndValidateSuccess(result) + assertUpperBound("9.2", "existing_92,8123000") + assertUpperBound("9.1", "existing_92,8012001") + } + + def "unreferenced referable definition should be reverted"() { + given: + referableTransportVersion("test_tv", "8124000") + transportVersionUpperBound("9.2", "test_tv", "8124000") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinitionDoesNotExist("test_tv") + assertUpperBound("9.2", "existing_92,8123000") + } + + def "unreferenced referable definition in multiple branches should be reverted"() { + given: + referableTransportVersion("test_tv", "8124000") + transportVersionUpperBound("9.2", "test_tv", "8124000") + transportVersionUpperBound("9.1", "test_tv", "8012002") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinitionDoesNotExist("test_tv") + assertUpperBound("9.2", "existing_92,8123000") + assertUpperBound("9.1", "existing_92,8012001") + } + + def "a reference can be renamed"() { + given: + referableTransportVersion("first_tv", "8124000") + transportVersionUpperBound("9.2", "first_tv", "8124000") + referencedTransportVersion("renamed_tv") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinitionDoesNotExist("first_tv") + assertReferableDefinition("renamed_tv", "8124000") + assertUpperBound("9.2", "renamed_tv,8124000") + } + + def "a reference with a patch version can be renamed"() { + given: + referableTransportVersion("first_tv", "8124000,8012002") + transportVersionUpperBound("9.2", "first_tv", "8124000") + transportVersionUpperBound("9.1", "first_tv", "8012002") + referencedTransportVersion("renamed_tv") + + when: + def result = runGenerateAndValidateTask("--backport-branches=9.1").build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinitionDoesNotExist("first_tv") + assertReferableDefinition("renamed_tv", "8124000,8012002") + assertUpperBound("9.2", "renamed_tv,8124000") + assertUpperBound("9.1", "renamed_tv,8012002") + } + + def "a missing definition will be regenerated"() { + given: + referencedTransportVersion("test_tv") + transportVersionUpperBound("9.2", "test_tv", "8124000") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("test_tv", "8124000") + assertUpperBound("9.2", "test_tv,8124000") + } + + def "an upper bound can be regenerated"() { + given: + file("myserver/src/main/resources/transport/upper_bounds/9.2.csv").delete() + referableAndReferencedTransportVersion("test_tv", "8124000") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertUpperBound("9.2", "test_tv,8124000") + } + + def "branches for definition can be removed"() { + given: + // previously generated with 9.1 and 9.2 + referableAndReferencedTransportVersion("test_tv", "8124000,8012002") + transportVersionUpperBound("9.2", "test_tv", "8124000") + transportVersionUpperBound("9.1", "test_tv", "8012002") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("test_tv", "8124000") + assertUpperBound("9.2", "test_tv,8124000") + assertUpperBound("9.1", "existing_92,8012001") + } + + def "branches for definition can be added"() { + given: + referableAndReferencedTransportVersion("test_tv", "8124000") + transportVersionUpperBound("9.2", "test_tv", "8124000") + + when: + def result = runGenerateAndValidateTask("--backport-branches=9.1").build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("test_tv", "8124000,8012002") + assertUpperBound("9.2", "test_tv,8124000") + assertUpperBound("9.1", "test_tv,8012002") + } + + def "unreferenced definitions are removed"() { + given: + referableTransportVersion("test_tv", "8124000,8012002") + transportVersionUpperBound("9.2", "test_tv", "8124000") + transportVersionUpperBound("9.1", "test_tv", "8012002") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinitionDoesNotExist("test_tv") + assertUpperBound("9.2", "existing_92,8123000") + assertUpperBound("9.1", "existing_92,8012001") + } + + def "merge conflicts in latest files can be regenerated"() { + given: + file("myserver/src/main/resources/transport/latest/9.2.csv").text = + """ + <<<<<<< HEAD + existing_92,8123000 + ======= + second_tv,8123000 + >>>>>> name + """.strip() + referableAndReferencedTransportVersion("second_tv", "8123000") + + when: + def result = runGenerateAndValidateTask("--name=second_tv", ).build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("existing_92", "8123000,8012001") + assertReferableDefinition("second_tv", "8124000") + assertUpperBound("9.2", "second_tv,8124000") + } + + def "branches param order does not matter"() { + given: + referencedTransportVersion("test_tv") + + when: + def result = runGenerateAndValidateTask("--backport-branches=9.0,9.1").build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("test_tv", "8124000,8012002,8000001") + assertUpperBound("9.2", "test_tv,8124000") + assertUpperBound("9.1", "test_tv,8012002") + assertUpperBound("9.0", "test_tv,8000001") + } + + def "if the files for a new definition already exist, no change should occur"() { + given: + transportVersionUpperBound("9.2", "test_tv", "8124000") + referableAndReferencedTransportVersion("test_tv", "8124000") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("test_tv", "8124000") + assertUpperBound("9.2", "test_tv,8124000") + } + + def "can add backport to latest upper bound"() { + when: + def result = runGenerateAndValidateTask("--name=existing_92", "--backport-branches=9.1,9.0").build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("existing_92", "8123000,8012001,8000001") + assertUpperBound("9.2", "existing_92,8123000") + assertUpperBound("9.1", "existing_92,8012001") + assertUpperBound("9.0", "existing_92,8000001") + } + + def "can add backport to older definition"() { + given: + execute("git checkout main") + referableAndReferencedTransportVersion("latest_tv", "8124000,8012002") + transportVersionUpperBound("9.2", "latest_tv", "8124000") + transportVersionUpperBound("9.1", "latest_tv", "8012002") + execute("git commit -a -m added") + execute("git checkout -b mybranch") + + when: + def result = runGenerateAndValidateTask("--name=existing_92", "--backport-branches=9.1,9.0").build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("existing_92", "8123000,8012001,8000001") + assertUpperBound("9.2", "latest_tv,8124000") + assertUpperBound("9.1", "latest_tv,8012002") + assertUpperBound("9.0", "existing_92,8000001") + } + + def "a different increment can be specified"() { + given: + referencedTransportVersion("new_tv") + file("myserver/build.gradle") << """ + tasks.named('validateTransportVersionResources') { + shouldValidateDensity = false + shouldValidatePrimaryIdNotPatch = false + } + """ + + when: + def result = runGenerateAndValidateTask("--increment=100").build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("new_tv", "8123100") + assertUpperBound("9.2", "new_tv,8123100") + } + + def "an invalid increment should fail"() { + given: + referencedTransportVersion("new_tv") + + when: + def result = runGenerateTask("--increment=0").buildAndFail() + + then: + assertOutputContains(result.output, "Invalid increment 0, must be a positive integer") + } + + def "a new definition exists and is in the latest file, but the version id is wrong and needs to be updated"(){ + given: + referableAndReferencedTransportVersion("new_tv", "1000000") + transportVersionUpperBound("9.2", "new_tv", "1000000") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("new_tv", "8124000") + assertUpperBound("9.2", "new_tv,8124000") + } + + def "backport branches is optional"() { + given: + referencedTransportVersion("new_tv") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertReferableDefinition("new_tv", "8124000") + assertUpperBound("9.2", "new_tv,8124000") + } + + def "deleted upper bounds files are restored"() { + given: + file("myserver/src/main/resources/transport/upper_bounds/9.2.csv").delete() + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertUpperBound("9.2", "existing_92,8123000") + } + + def "upper bounds files must exist for backport branches"() { + when: + def result = runGenerateTask("--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 [9.0, 9.1, 9.2]") + } + + def "name can be found from committed definition"() { + given: + referableAndReferencedTransportVersion("new_tv", "8123000") + execute("git add .") + execute("git commit -m added") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertUpperBound("9.2", "new_tv,8124000") + assertReferableDefinition("new_tv", "8124000") + } + + def "name can be found from staged definition"() { + given: + referableAndReferencedTransportVersion("new_tv", "8123000") + execute("git add .") + + when: + def result = runGenerateAndValidateTask().build() + + then: + assertGenerateAndValidateSuccess(result) + assertUpperBound("9.2", "new_tv,8124000") + assertReferableDefinition("new_tv", "8124000") + } +} 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 new file mode 100644 index 0000000000000..0075b2398a414 --- /dev/null +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java @@ -0,0 +1,240 @@ +/* + * 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.api.DefaultTask; +import org.gradle.api.file.ConfigurableFileCollection; +import org.gradle.api.provider.Property; +import org.gradle.api.services.ServiceReference; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.InputFiles; +import org.gradle.api.tasks.Optional; +import org.gradle.api.tasks.PathSensitive; +import org.gradle.api.tasks.PathSensitivity; +import org.gradle.api.tasks.TaskAction; +import org.gradle.api.tasks.options.Option; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * This task generates transport version definition files. These files + * are runtime resources that TransportVersion loads statically. + * They contain a comma separated list of integer ids. Each file is named the same + * as the transport version name itself (with the .csv suffix). + * + * Additionally, when definition files are added or updated, the upper bounds files + * for each relevant branch's upper bound file are also updated. + */ +public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask { + + /** + * Files that contain the references to the transport version names. + */ + @InputFiles + @PathSensitive(PathSensitivity.RELATIVE) + public abstract ConfigurableFileCollection getReferencesFiles(); + + @ServiceReference("transportVersionResources") + abstract Property getResourceService(); + + @Input + @Optional + @Option(option = "name", description = "The name of the Transport Version definition, e.g. --name=my_new_tv") + public abstract Property getDefinitionName(); + + @Input + @Optional + @Option( + option = "backport-branches", + description = "The branches this definition will be backported to, e.g. --backport-branches=9.1,8.19" + ) + public abstract Property getBackportBranches(); + + @Input + @Optional + @Option(option = "increment", description = "The amount to increment the id from the current upper bounds file by") + public abstract Property getIncrement(); + + /** + * The name of the upper bounds file which will be used at runtime on the current branch. Normally + * this equates to VersionProperties.getElasticsearchVersion(). + */ + @Input + public abstract Property getCurrentUpperBoundName(); + + @TaskAction + public void run() throws IOException { + TransportVersionResourcesService resources = getResourceService().get(); + Set referencedNames = TransportVersionReference.collectNames(getReferencesFiles()); + List changedDefinitionNames = resources.getChangedReferableDefinitionNames(); + String targetDefinitionName = getTargetDefinitionName(resources, referencedNames, changedDefinitionNames); + + List upstreamUpperBounds = resources.getUpperBoundsFromUpstream(); + Set targetUpperBoundNames = getTargetUpperBoundNames(upstreamUpperBounds); + + getLogger().lifecycle("Generating transport version name: " + targetDefinitionName); + if (targetDefinitionName.isEmpty()) { + resetAllUpperBounds(resources); + } else { + List ids = updateUpperBounds(resources, upstreamUpperBounds, targetUpperBoundNames, targetDefinitionName); + // (Re)write the definition file. + resources.writeReferableDefinition(new TransportVersionDefinition(targetDefinitionName, ids)); + } + + removeUnusedNamedDefinitions(resources, referencedNames, changedDefinitionNames); + } + + private List updateUpperBounds( + TransportVersionResourcesService resources, + List existingUpperBounds, + Set targetUpperBoundNames, + String definitionName + ) throws IOException { + String currentUpperBoundName = getCurrentUpperBoundName().get(); + int increment = getIncrement().get(); + if (increment <= 0) { + throw new IllegalArgumentException("Invalid increment " + increment + ", must be a positive integer"); + } + List ids = new ArrayList<>(); + + TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromUpstream(definitionName); + for (TransportVersionUpperBound existingUpperBound : existingUpperBounds) { + String upperBoundName = existingUpperBound.name(); + + if (targetUpperBoundNames.contains(upperBoundName)) { + // Case: targeting this upper bound, find an existing id if it exists + TransportVersionId targetId = maybeGetExistingId(existingUpperBound, existingDefinition, definitionName); + if (targetId == null) { + // Case: an id doesn't yet exist for this upper bound, so create one + int targetIncrement = upperBoundName.equals(currentUpperBoundName) ? increment : 1; + targetId = TransportVersionId.fromInt(existingUpperBound.definitionId().complete() + targetIncrement); + var newUpperBound = new TransportVersionUpperBound(upperBoundName, definitionName, targetId); + resources.writeUpperBound(newUpperBound); + } + ids.add(targetId); + } else { + // Default case: we're not targeting this branch so reset it + resources.writeUpperBound(existingUpperBound); + } + } + + Collections.sort(ids); + return ids; + } + + private String getTargetDefinitionName( + TransportVersionResourcesService resources, + Set referencedNames, + List changedDefinitions + ) { + if (getDefinitionName().isPresent()) { + // an explicit name was passed in, so use it + return getDefinitionName().get(); + } + + // First check for unreferenced names. We only care about the first one. If there is more than one + // validation will fail later and the developer will have to remove one. When that happens, generation + // will re-run and we will fixup the state to use whatever new name remains. + for (String referencedName : referencedNames) { + if (resources.referableDefinitionExists(referencedName) == false) { + return referencedName; + } + } + + // Since we didn't find any missing names, we use the first changed name. If there is more than + // one changed name, validation will fail later, just as above. + if (changedDefinitions.isEmpty()) { + return ""; + } else { + String changedDefinitionName = changedDefinitions.get(0); + if (referencedNames.contains(changedDefinitionName)) { + return changedDefinitionName; + } else { + return ""; // the changed name is unreferenced, so go into "reset mode" + } + } + } + + private Set getTargetUpperBoundNames(List upstreamUpperBounds) { + Set targetUpperBoundNames = new HashSet<>(); + targetUpperBoundNames.add(getCurrentUpperBoundName().get()); + if (getBackportBranches().isPresent()) { + targetUpperBoundNames.addAll(List.of(getBackportBranches().get().split(","))); + } + + Set missingBranches = new HashSet<>(targetUpperBoundNames); + List knownUpperBoundNames = new ArrayList<>(); + for (TransportVersionUpperBound upperBound : upstreamUpperBounds) { + knownUpperBoundNames.add(upperBound.name()); + missingBranches.remove(upperBound.name()); + } + if (missingBranches.isEmpty() == false) { + List sortedMissing = missingBranches.stream().sorted().toList(); + List sortedKnown = knownUpperBoundNames.stream().sorted().toList(); + throw new IllegalArgumentException( + "Missing upper bounds files for branches " + sortedMissing + ", known branches are " + sortedKnown + ); + } + + return targetUpperBoundNames; + } + + private void resetAllUpperBounds(TransportVersionResourcesService resources) throws IOException { + for (TransportVersionUpperBound upperBound : resources.getUpperBoundsFromUpstream()) { + resources.writeUpperBound(upperBound); + } + } + + private void removeUnusedNamedDefinitions( + TransportVersionResourcesService resources, + Set referencedNames, + List changedDefinitions + ) throws IOException { + for (String definitionName : changedDefinitions) { + if (referencedNames.contains(definitionName) == false) { + // we added this definition file, but it's now unreferenced, so delete it + getLogger().lifecycle("Deleting unreferenced named transport version definition [" + definitionName + "]"); + resources.deleteReferableDefinition(definitionName); + } + } + } + + private TransportVersionId maybeGetExistingId( + TransportVersionUpperBound upperBound, + TransportVersionDefinition existingDefinition, + String name + ) { + if (existingDefinition == null) { + // the name doesn't yet exist, so there is no id to return + return null; + } + if (upperBound.definitionName().equals(name)) { + // the name exists and this upper bound already points at it + return upperBound.definitionId(); + } + if (upperBound.name().equals(getCurrentUpperBoundName().get())) { + // this is the upper bound of the current branch, so use the primary id + return existingDefinition.ids().get(0); + } + // the upper bound is for a non-current branch, so find the id with the same base + for (TransportVersionId id : existingDefinition.ids()) { + if (id.base() == upperBound.definitionId().base()) { + return id; + } + } + return null; // no existing id for this upper bound + } + +} diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java index 407c3bd511f09..7b90907b9c23f 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java @@ -9,15 +9,17 @@ package org.elasticsearch.gradle.internal.transport; -record TransportVersionId(int complete, int major, int server, int subsidiary, int patch) implements Comparable { +record TransportVersionId(int complete, int base, int patch) implements Comparable { + + public static TransportVersionId fromInt(int complete) { + int patch = complete % 1000; + int base = complete - patch; + + return new TransportVersionId(complete, base, patch); + } static TransportVersionId fromString(String s) { - int complete = Integer.parseInt(s); - int patch = complete % 100; - int subsidiary = (complete / 100) % 10; - int server = (complete / 1000) % 1000; - int major = complete / 1000000; - return new TransportVersionId(complete, major, server, subsidiary, patch); + return fromInt(Integer.parseInt(s)); } @Override @@ -30,8 +32,4 @@ public int compareTo(TransportVersionId o) { public String toString() { return Integer.toString(complete); } - - public int base() { - return (complete / 1000) * 1000; - } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReference.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReference.java index f94f4fc6d9b6b..eabdf8370eb17 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReference.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReference.java @@ -29,7 +29,7 @@ record TransportVersionReference(String name, String location) { private static final Attribute REFERENCES_ATTRIBUTE = Attribute.of("transport-version-references", Boolean.class); static List listFromFile(Path file) throws IOException { - assert file.endsWith(".csv"); + assert file.toString().endsWith(".csv") : file + " does not end in .csv"; List results = new ArrayList<>(); for (String line : Files.readAllLines(file, StandardCharsets.UTF_8)) { String[] parts = line.split(",", 2); 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 5ee561764b5a4..ca6189e22e357 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 @@ -36,7 +36,7 @@ public void apply(Project project) { t.setDescription("Collects all TransportVersion references used throughout the project"); SourceSet mainSourceSet = GradleUtils.getJavaSourceSets(project).findByName(SourceSet.MAIN_SOURCE_SET_NAME); t.getClassPath().setFrom(mainSourceSet.getOutput()); - t.getOutputFile().set(project.getLayout().getBuildDirectory().file("transport-version/references.txt")); + t.getOutputFile().set(project.getLayout().getBuildDirectory().file("transport-version/references.csv")); }); var tvReferencesConfig = project.getConfigurations().consumable("transportVersionReferences", c -> { 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 2b1c61ecd75d4..931680eb96f5d 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,6 +9,8 @@ package org.elasticsearch.gradle.internal.transport; +import org.elasticsearch.gradle.Version; +import org.elasticsearch.gradle.VersionProperties; import org.elasticsearch.gradle.internal.ProjectSubscribeServicePlugin; import org.gradle.api.Plugin; import org.gradle.api.Project; @@ -29,6 +31,8 @@ public void apply(Project project) { var psService = project.getPlugins().apply(ProjectSubscribeServicePlugin.class).getService(); var resourceRoot = getResourceRoot(project); + String taskGroup = "Transport Versions"; + project.getGradle() .getSharedServices() .registerIfAbsent("transportVersionResources", TransportVersionResourcesService.class, spec -> { @@ -51,21 +55,35 @@ public void apply(Project project) { var validateTask = project.getTasks() .register("validateTransportVersionResources", ValidateTransportVersionResourcesTask.class, t -> { - t.setGroup("Transport Versions"); + t.setGroup(taskGroup); t.setDescription("Validates that all transport version resources are internally consistent with each other"); t.getReferencesFiles().setFrom(tvReferencesConfig); + t.getShouldValidateDensity().convention(true); + t.getShouldValidatePrimaryIdNotPatch().convention(true); }); project.getTasks().named(LifecycleBasePlugin.CHECK_TASK_NAME).configure(t -> t.dependsOn(validateTask)); var generateManifestTask = project.getTasks() .register("generateTransportVersionManifest", GenerateTransportVersionManifestTask.class, t -> { - t.setGroup("Transport Versions"); + t.setGroup(taskGroup); t.setDescription("Generate a manifest resource for all transport version definitions"); t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt")); }); project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> { t.into("transport/definitions", c -> c.from(generateManifestTask)); }); + + var generateDefinitionsTask = project.getTasks() + .register("generateTransportVersionDefinition", GenerateTransportVersionDefinitionTask.class, t -> { + t.setGroup(taskGroup); + t.setDescription("(Re)generates a transport version definition file"); + t.getReferencesFiles().setFrom(tvReferencesConfig); + t.getIncrement().convention(1000); + Version esVersion = VersionProperties.getElasticsearchVersion(); + t.getCurrentUpperBoundName().convention(esVersion.getMajor() + "." + esVersion.getMinor()); + }); + + validateTask.configure(t -> t.mustRunAfter(generateDefinitionsTask)); } private static String getResourceRoot(Project project) { 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 239ab0c9ab493..85590cb3cb196 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 @@ -10,6 +10,8 @@ package org.elasticsearch.gradle.internal.transport; import org.gradle.api.file.DirectoryProperty; +import org.gradle.api.logging.Logger; +import org.gradle.api.logging.Logging; import org.gradle.api.services.BuildService; import org.gradle.api.services.BuildServiceParameters; import org.gradle.process.ExecOperations; @@ -29,6 +31,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiFunction; +import java.util.stream.Collectors; import javax.inject.Inject; @@ -45,11 +48,13 @@ *
  • /transport/definitions/unreferable/ * - Definitions which contain ids that are known at runtime, but cannot be looked up by name.
    • *
  • /transport/upper_bounds/ - * - The maximum transport version definition that will be loaded for each release branch.
    • + * - The maximum transport version definition that will be loaded on a branch. * */ public abstract class TransportVersionResourcesService implements BuildService { + private static final Logger logger = Logging.getLogger(TransportVersionResourcesService.class); + public interface Parameters extends BuildServiceParameters { DirectoryProperty getTransportResourcesDirectory(); @@ -66,7 +71,8 @@ public interface Parameters extends BuildServiceParameters { private final Path transportResourcesDir; private final Path rootDir; - private final AtomicReference> mainResources = new AtomicReference<>(null); + private final AtomicReference upstreamRefName = new AtomicReference<>(); + private final AtomicReference> upstreamResources = new AtomicReference<>(null); private final AtomicReference> changedResources = new AtomicReference<>(null); @Inject @@ -101,10 +107,25 @@ Map getReferableDefinitions() throws IOExcep return readDefinitions(transportResourcesDir.resolve(REFERABLE_DIR)); } - /** Get a referable definition from main if it exists there, or null otherwise */ - TransportVersionDefinition getReferableDefinitionFromMain(String name) { + /** Get a referable definition from upstream if it exists there, or null otherwise */ + TransportVersionDefinition getReferableDefinitionFromUpstream(String name) { Path resourcePath = getReferableDefinitionRelativePath(name); - return getMainFile(resourcePath, TransportVersionDefinition::fromString); + return getUpstreamFile(resourcePath, TransportVersionDefinition::fromString); + } + + /** Get the definition names which have local changes relative to upstream */ + List getChangedReferableDefinitionNames() { + List changedDefinitions = new ArrayList<>(); + String referablePrefix = REFERABLE_DIR.toString(); + for (String changedPath : getChangedResources()) { + if (changedPath.contains(referablePrefix) == false) { + continue; + } + int lastSlashNdx = changedPath.lastIndexOf('/'); + String name = changedPath.substring(lastSlashNdx + 1, changedPath.length() - 4 /* .csv */); + changedDefinitions.add(name); + } + return changedDefinitions; } /** Test whether the given referable definition exists */ @@ -117,6 +138,21 @@ Path getReferableDefinitionRepositoryPath(TransportVersionDefinition definition) return rootDir.relativize(transportResourcesDir.resolve(getReferableDefinitionRelativePath(definition.name()))); } + void writeReferableDefinition(TransportVersionDefinition definition) throws IOException { + Path path = transportResourcesDir.resolve(getReferableDefinitionRelativePath(definition.name())); + logger.debug("Writing referable definition [" + definition + "] to [" + path + "]"); + Files.writeString( + path, + definition.ids().stream().map(Object::toString).collect(Collectors.joining(",")) + "\n", + StandardCharsets.UTF_8 + ); + } + + void deleteReferableDefinition(String name) throws IOException { + Path path = transportResourcesDir.resolve(getReferableDefinitionRelativePath(name)); + Files.deleteIfExists(path); + } + // return the path, relative to the resources dir, of an unreferable definition private Path getUnreferableDefinitionRelativePath(String name) { return UNREFERABLE_DIR.resolve(name + ".csv"); @@ -127,10 +163,10 @@ Map getUnreferableDefinitions() throws IOExc return readDefinitions(transportResourcesDir.resolve(UNREFERABLE_DIR)); } - /** Get a referable definition from main if it exists there, or null otherwise */ - TransportVersionDefinition getUnreferableDefinitionFromMain(String name) { + /** Get a referable definition from upstream if it exists there, or null otherwise */ + TransportVersionDefinition getUnreferableDefinitionFromUpstream(String name) { Path resourcePath = getUnreferableDefinitionRelativePath(name); - return getMainFile(resourcePath, TransportVersionDefinition::fromString); + return getUpstreamFile(resourcePath, TransportVersionDefinition::fromString); } /** Return the path within the repository of the given referable definition */ @@ -138,70 +174,140 @@ Path getUnreferableDefinitionRepositoryPath(TransportVersionDefinition definitio return rootDir.relativize(transportResourcesDir.resolve(getUnreferableDefinitionRelativePath(definition.name()))); } - /** Read all upper bound files and return them mapped by their release branch */ + /** Read all upper bound files and return them mapped by their release name */ Map getUpperBounds() throws IOException { Map upperBounds = new HashMap<>(); try (var stream = Files.list(transportResourcesDir.resolve(UPPER_BOUNDS_DIR))) { for (var latestFile : stream.toList()) { String contents = Files.readString(latestFile, StandardCharsets.UTF_8).strip(); var upperBound = TransportVersionUpperBound.fromString(latestFile, contents); - upperBounds.put(upperBound.branch(), upperBound); + upperBounds.put(upperBound.name(), upperBound); + } + } + return upperBounds; + } + + /** Retrieve an upper bound from upstream by name */ + TransportVersionUpperBound getUpperBoundFromUpstream(String name) { + Path resourcePath = getUpperBoundRelativePath(name); + return getUpstreamFile(resourcePath, TransportVersionUpperBound::fromString); + } + + /** Retrieve all upper bounds that exist in upstream */ + List getUpperBoundsFromUpstream() throws IOException { + List upperBounds = new ArrayList<>(); + for (String upstreamPathString : getUpstreamResources()) { + Path upstreamPath = Path.of(upstreamPathString); + if (upstreamPath.startsWith(UPPER_BOUNDS_DIR) == false) { + continue; } + TransportVersionUpperBound upperBound = getUpstreamFile(upstreamPath, TransportVersionUpperBound::fromString); + upperBounds.add(upperBound); } return upperBounds; } - /** Retrieve the latest transport version for the given release branch on main */ - TransportVersionUpperBound getUpperBoundFromMain(String releaseBranch) { - Path resourcePath = getUpperBoundRelativePath(releaseBranch); - return getMainFile(resourcePath, TransportVersionUpperBound::fromString); + /** Write the given upper bound to a file in the transport resources */ + void writeUpperBound(TransportVersionUpperBound upperBound) 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); } /** Return the path within the repository of the given latest */ Path getUpperBoundRepositoryPath(TransportVersionUpperBound latest) { - return rootDir.relativize(transportResourcesDir.resolve(getUpperBoundRelativePath(latest.branch()))); + return rootDir.relativize(transportResourcesDir.resolve(getUpperBoundRelativePath(latest.name()))); } - private Path getUpperBoundRelativePath(String releaseBranch) { - return UPPER_BOUNDS_DIR.resolve(releaseBranch + ".csv"); + private Path getUpperBoundRelativePath(String name) { + return UPPER_BOUNDS_DIR.resolve(name + ".csv"); } - // Return the transport version resources paths that exist in main - private Set getMainResources() { - if (mainResources.get() == null) { - synchronized (mainResources) { - String output = gitCommand("ls-tree", "--name-only", "-r", "main", "."); + private String getUpstreamRefName() { + if (upstreamRefName.get() == null) { + synchronized (upstreamRefName) { + String remotesOutput = gitCommand("remote").strip(); + + String refName; + if (remotesOutput.isEmpty()) { + refName = "main"; // fallback to local main if no remotes, this happens in tests + } else { + List remoteNames = List.of(remotesOutput.split("\n")); + String transportVersionRemoteName = "transport-version-resources-upstream"; + if (remoteNames.contains(transportVersionRemoteName) == false) { + // our special remote doesn't exist yet, so create it + String upstreamUrl = null; + for (String remoteName : remoteNames) { + String getUrlOutput = gitCommand("remote", "get-url", remoteName).strip(); + if (getUrlOutput.startsWith("git@github.com:elastic/") + || getUrlOutput.startsWith("https://github.com/elastic/")) { + upstreamUrl = getUrlOutput; + } + } + + if (upstreamUrl != null) { + gitCommand("remote", "add", transportVersionRemoteName, upstreamUrl); + } else { + throw new RuntimeException("No elastic github remotes found to copy"); + } + } + + // make sure the remote main ref is up to date + gitCommand("fetch", transportVersionRemoteName, "main"); + + refName = transportVersionRemoteName + "/main"; + } + upstreamRefName.set(refName); + + } + } + return upstreamRefName.get(); + } + + // Return the transport version resources paths that exist in upstream + private Set getUpstreamResources() { + if (upstreamResources.get() == null) { + synchronized (upstreamResources) { + String output = gitCommand("ls-tree", "--name-only", "-r", getUpstreamRefName(), "."); HashSet resources = new HashSet<>(); Collections.addAll(resources, output.split("\n")); // git always outputs LF - mainResources.set(resources); + upstreamResources.set(resources); } } - return mainResources.get(); + return upstreamResources.get(); } - // Return the transport version resources paths that have been changed relative to main + // Return the transport version resources paths that have been changed relative to upstream private Set getChangedResources() { if (changedResources.get() == null) { synchronized (changedResources) { - String output = gitCommand("diff", "--name-only", "main", "."); - HashSet resources = new HashSet<>(); - Collections.addAll(resources, output.split("\n")); // git always outputs LF + + String diffOutput = gitCommand("diff", "--name-only", getUpstreamRefName(), "."); + if (diffOutput.strip().isEmpty() == false) { + Collections.addAll(resources, diffOutput.split("\n")); // git always outputs LF + } + + String untrackedOutput = gitCommand("ls-files", "--others", "--exclude-standard"); + if (untrackedOutput.strip().isEmpty() == false) { + Collections.addAll(resources, untrackedOutput.split("\n")); // git always outputs LF + } + changedResources.set(resources); } } return changedResources.get(); } - // Read a transport version resource from the main branch, or return null if it doesn't exist on main - private T getMainFile(Path resourcePath, BiFunction parser) { + // Read a transport version resource from the upstream, or return null if it doesn't exist there + private T getUpstreamFile(Path resourcePath, BiFunction parser) { String pathString = resourcePath.toString().replace('\\', '/'); // normalize to forward slash that git uses - if (getMainResources().contains(pathString) == false) { + if (getUpstreamResources().contains(pathString) == false) { return null; } - String content = gitCommand("show", "main:./" + pathString).strip(); + String content = gitCommand("show", getUpstreamRefName() + ":./" + pathString).strip(); return parser.apply(resourcePath, content); } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUpperBound.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUpperBound.java index 104a51ab79f70..d3b914647f0fa 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUpperBound.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUpperBound.java @@ -14,13 +14,14 @@ /** * An object to represent the loaded version of a transport version upper bound. * - * An upper bound is the maximum transport version id that should be loaded for a given release branch. + * An upper bound is the maximum transport version definitionId that should be loaded for a given release branch. */ -record TransportVersionUpperBound(String branch, String name, TransportVersionId id) { +record TransportVersionUpperBound(String name, String definitionName, TransportVersionId definitionId) { public static TransportVersionUpperBound fromString(Path file, String contents) { String filename = file.getFileName().toString(); assert filename.endsWith(".csv"); - String branch = filename.substring(0, filename.length() - 4); + int slashIndex = filename.lastIndexOf('/'); + String branch = filename.substring(slashIndex == -1 ? 0 : (slashIndex + 1), filename.length() - 4); String[] parts = contents.split(","); if (parts.length != 2) { 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 d806cc0484097..afbb8d1d38579 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 @@ -16,6 +16,7 @@ import org.gradle.api.provider.Property; import org.gradle.api.services.ServiceReference; import org.gradle.api.tasks.CacheableTask; +import org.gradle.api.tasks.Input; import org.gradle.api.tasks.InputDirectory; import org.gradle.api.tasks.InputFiles; import org.gradle.api.tasks.Optional; @@ -53,6 +54,12 @@ public Path getResourcesDir() { @PathSensitive(PathSensitivity.RELATIVE) public abstract ConfigurableFileCollection getReferencesFiles(); + @Input + public abstract Property getShouldValidateDensity(); + + @Input + public abstract Property getShouldValidatePrimaryIdNotPatch(); + private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definition) {} private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+"); @@ -75,7 +82,7 @@ public void validateTransportVersions() throws IOException { } for (var definition : unreferableDefinitions.values()) { - validateUnreferencedDefinition(definition); + validateUnreferableDefinition(definition); } for (var entry : idsByBase.entrySet()) { @@ -127,7 +134,7 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set< // validate any modifications Map existingIdsByBase = new HashMap<>(); - TransportVersionDefinition originalDefinition = getResources().get().getReferableDefinitionFromMain(definition.name()); + TransportVersionDefinition originalDefinition = getResources().get().getReferableDefinitionFromUpstream(definition.name()); if (originalDefinition != null) { validateIdenticalPrimaryId(definition, originalDefinition); originalDefinition.ids().forEach(id -> existingIdsByBase.put(id.base(), id)); @@ -149,7 +156,7 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set< TransportVersionId id = definition.ids().get(ndx); if (ndx == 0) { - if (id.patch() != 0) { + if (getShouldValidatePrimaryIdNotPatch().get() && id.patch() != 0) { throwDefinitionFailure(definition, "has patch version " + id.complete() + " as primary id"); } } else { @@ -158,7 +165,7 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set< } } - // check modifications of ids on same branch, ie sharing same base + // check modifications of ids on same name, ie sharing same base TransportVersionId maybeModifiedId = existingIdsByBase.get(id.base()); if (maybeModifiedId != null && maybeModifiedId.complete() != id.complete()) { throwDefinitionFailure(definition, "modifies existing patch id from " + maybeModifiedId + " to " + id); @@ -166,8 +173,8 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set< } } - private void validateUnreferencedDefinition(TransportVersionDefinition definition) { - TransportVersionDefinition originalDefinition = getResources().get().getUnreferableDefinitionFromMain(definition.name()); + private void validateUnreferableDefinition(TransportVersionDefinition definition) { + TransportVersionDefinition originalDefinition = getResources().get().getUnreferableDefinitionFromUpstream(definition.name()); if (originalDefinition != null) { validateIdenticalPrimaryId(definition, originalDefinition); } @@ -195,26 +202,32 @@ private void validateUpperBound( Map definitions, Map> idsByBase ) { - TransportVersionDefinition upperBoundDefinition = definitions.get(upperBound.name()); + TransportVersionDefinition upperBoundDefinition = definitions.get(upperBound.definitionName()); if (upperBoundDefinition == null) { - throwUpperBoundFailure(upperBound, "contains transport version name [" + upperBound.name() + "] which is not defined"); + throwUpperBoundFailure( + upperBound, + "contains transport version name [" + upperBound.definitionName() + "] which is not defined" + ); } - if (upperBoundDefinition.ids().contains(upperBound.id()) == false) { + if (upperBoundDefinition.ids().contains(upperBound.definitionId()) == false) { Path relativePath = getResources().get().getReferableDefinitionRepositoryPath(upperBoundDefinition); - throwUpperBoundFailure(upperBound, "has id " + upperBound.id() + " which is not in definition [" + relativePath + "]"); + throwUpperBoundFailure( + upperBound, + "has id " + upperBound.definitionId() + " which is not in definition [" + relativePath + "]" + ); } - List baseIds = idsByBase.get(upperBound.id().base()); + List baseIds = idsByBase.get(upperBound.definitionId().base()); IdAndDefinition lastId = baseIds.get(baseIds.size() - 1); - if (lastId.id().complete() != upperBound.id().complete()) { + if (lastId.id().complete() != upperBound.definitionId().complete()) { throwUpperBoundFailure( upperBound, "has id " - + upperBound.id() + + upperBound.definitionId() + " from [" - + upperBound.name() + + upperBound.definitionName() + "] with base " - + upperBound.id().base() + + upperBound.definitionId().base() + " but another id " + lastId.id().complete() + " from [" @@ -223,12 +236,12 @@ private void validateUpperBound( ); } - TransportVersionUpperBound existingUpperBound = getResources().get().getUpperBoundFromMain(upperBound.branch()); + TransportVersionUpperBound existingUpperBound = getResources().get().getUpperBoundFromUpstream(upperBound.name()); if (existingUpperBound != null) { - if (upperBound.id().patch() != 0 && upperBound.id().base() != existingUpperBound.id().base()) { + if (upperBound.definitionId().patch() != 0 && upperBound.definitionId().base() != existingUpperBound.definitionId().base()) { throwUpperBoundFailure( upperBound, - "modifies base id from " + existingUpperBound.id().base() + " to " + upperBound.id().base() + "modifies base id from " + existingUpperBound.definitionId().base() + " to " + upperBound.definitionId().base() ); } } @@ -248,7 +261,7 @@ private void validateBase(int base, List ids) { ); } - if (previous.id().complete() - 1 != current.id().complete()) { + if (getShouldValidateDensity().get() && previous.id().complete() - 1 != current.id().complete()) { throw new IllegalStateException( "Transport version base id " + base + " is missing patch ids between " + current.id() + " and " + previous.id() ); @@ -267,7 +280,7 @@ private void validateLargestIdIsUsed( var highestId = highestDefinition.ids().get(0); for (var upperBound : upperBounds.values()) { - if (upperBound.id().equals(highestId)) { + if (upperBound.definitionId().equals(highestId)) { return; } } diff --git a/build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/AbstractGradleFuncTest.groovy b/build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/AbstractGradleFuncTest.groovy index 447f0b8af496f..df14753375c64 100644 --- a/build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/AbstractGradleFuncTest.groovy +++ b/build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/AbstractGradleFuncTest.groovy @@ -20,6 +20,7 @@ import org.elasticsearch.gradle.internal.test.NormalizeOutputGradleRunner import org.elasticsearch.gradle.internal.test.TestResultExtension import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.GradleRunner +import org.gradle.tooling.BuildException import org.junit.Rule import org.junit.rules.TemporaryFolder @@ -214,8 +215,11 @@ abstract class AbstractGradleFuncTest extends Specification { def proc = command.execute(Collections.emptyList(), workingDir) proc.waitFor() if (proc.exitValue()) { - System.err.println("Error running command ${command}:") - System.err.println("Syserr: " + proc.errorStream.text) + String msg = """Error running command ${command}: + Sysout: ${proc.inputStream.text} + Syserr: ${proc.errorStream.text} + """ + throw new RuntimeException(msg) } }