Skip to content

Commit 3bf3ed9

Browse files
authored
Only validate primary ids on release branches (#135044) (#135093)
Primary ids are only incremented on the main branch. For release branches only patch ids will be created. The primary id validation won't always work on release branches because only some of the primary ids will be backported. This commit skips primary id validation if we are sure we are on a release branch. We can tell this by looking at the current upper bound compared to other upper bounds. If there is a newer one, we know we are on a release branch. If there isn't a newer one, we _might_ be on a release branch, or on main, but in this case doing primary id validation is ok because there's effectively no difference, we are looking at the latest upper bound.
1 parent 197b6d5 commit 3bf3ed9

File tree

4 files changed

+59
-6
lines changed

4 files changed

+59
-6
lines changed

build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest {
125125
tasks.named('generateTransportVersion') {
126126
currentUpperBoundName = '9.2'
127127
}
128+
tasks.named('validateTransportVersionResources') {
129+
currentUpperBoundName = '9.2'
130+
}
128131
"""
129132
referableAndReferencedTransportVersion("existing_91", "8012000")
130133
referableAndReferencedTransportVersion("older_92", "8122000")

build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,21 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes
277277
"[myserver/src/main/resources/transport/definitions/referable/existing_92.csv]"
278278
)
279279
}
280+
281+
def "primary id checks skipped on release branch"() {
282+
given:
283+
file("myserver/build.gradle") << """
284+
tasks.named('validateTransportVersionResources') {
285+
currentUpperBoundName = '9.1'
286+
}
287+
"""
288+
referableAndReferencedTransportVersion("some_tv", "8125000")
289+
transportVersionUpperBound("9.2", "some_tv", "8125000")
290+
291+
when:
292+
def result = gradleRunner("validateTransportVersionResources").build()
293+
294+
then:
295+
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
296+
}
280297
}

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.gradle.internal.conventions.VersionPropertiesPlugin;
1515
import org.elasticsearch.gradle.internal.conventions.precommit.PrecommitPlugin;
1616
import org.elasticsearch.gradle.internal.conventions.precommit.PrecommitTaskPlugin;
17+
import org.gradle.api.Action;
1718
import org.gradle.api.Plugin;
1819
import org.gradle.api.Project;
1920
import org.gradle.api.file.Directory;
@@ -69,6 +70,7 @@ public void apply(Project project) {
6970
t.getReferencesFiles().setFrom(tvReferencesConfig);
7071
t.getShouldValidateDensity().convention(true);
7172
t.getShouldValidatePrimaryIdNotPatch().convention(true);
73+
t.getCurrentUpperBoundName().convention(currentVersion.getMajor() + "." + currentVersion.getMinor());
7274
});
7375
project.getTasks().named(PrecommitPlugin.PRECOMMIT_TASK_NAME).configure(t -> t.dependsOn(validateTask));
7476

@@ -79,19 +81,31 @@ public void apply(Project project) {
7981
t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt"));
8082
});
8183
project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> {
82-
t.into("transport/definitions", c -> c.from(generateManifestTask));
84+
t.into(resourceRoot + "/definitions", c -> c.from(generateManifestTask));
8385
});
8486

87+
Action<GenerateTransportVersionDefinitionTask> generationConfiguration = t -> {
88+
t.setGroup(taskGroup);
89+
t.getReferencesFiles().setFrom(tvReferencesConfig);
90+
t.getIncrement().convention(1000);
91+
t.getCurrentUpperBoundName().convention(currentVersion.getMajor() + "." + currentVersion.getMinor());
92+
};
93+
8594
var generateDefinitionsTask = project.getTasks()
8695
.register("generateTransportVersion", GenerateTransportVersionDefinitionTask.class, t -> {
87-
t.setGroup(taskGroup);
8896
t.setDescription("(Re)generates a transport version definition file");
89-
t.getReferencesFiles().setFrom(tvReferencesConfig);
90-
t.getIncrement().convention(1000);
91-
t.getCurrentUpperBoundName().convention(currentVersion.getMajor() + "." + currentVersion.getMinor());
9297
});
98+
generateDefinitionsTask.configure(generationConfiguration);
9399
validateTask.configure(t -> t.mustRunAfter(generateDefinitionsTask));
94100

101+
var resolveConflictTask = project.getTasks()
102+
.register("resolveTransportVersionConflict", GenerateTransportVersionDefinitionTask.class, t -> {
103+
t.setDescription("Resolve merge conflicts in transport version internal state files");
104+
t.getResolveConflict().set(true);
105+
});
106+
resolveConflictTask.configure(generationConfiguration);
107+
validateTask.configure(t -> t.mustRunAfter(resolveConflictTask));
108+
95109
var generateInitialTask = project.getTasks()
96110
.register("generateInitialTransportVersion", GenerateInitialTransportVersionTask.class, t -> {
97111
t.setGroup(taskGroup);

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ public Path getResourcesDir() {
6060
@Input
6161
public abstract Property<Boolean> getShouldValidatePrimaryIdNotPatch();
6262

63+
/**
64+
* The name of the upper bounds file which will be used at runtime on the current branch. Normally
65+
* this equates to VersionProperties.getElasticsearchVersion().
66+
*/
67+
@Input
68+
public abstract Property<String> getCurrentUpperBoundName();
69+
6370
private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definition) {}
6471

6572
private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+");
@@ -76,6 +83,7 @@ public void validateTransportVersions() throws IOException {
7683
Map<String, TransportVersionDefinition> allDefinitions = collectAllDefinitions(referableDefinitions, unreferableDefinitions);
7784
Map<Integer, List<IdAndDefinition>> idsByBase = collectIdsByBase(allDefinitions.values());
7885
Map<String, TransportVersionUpperBound> upperBounds = resources.getUpperBounds();
86+
boolean onReleaseBranch = checkIfDefinitelyOnReleaseBranch(upperBounds);
7987

8088
for (var definition : referableDefinitions.values()) {
8189
validateNamedDefinition(definition, referencedNames);
@@ -93,7 +101,9 @@ public void validateTransportVersions() throws IOException {
93101
validateUpperBound(upperBound, allDefinitions, idsByBase);
94102
}
95103

96-
validatePrimaryIds(resources, upperBounds, allDefinitions);
104+
if (onReleaseBranch == false) {
105+
validatePrimaryIds(resources, upperBounds, allDefinitions);
106+
}
97107
}
98108

99109
private Map<String, TransportVersionDefinition> collectAllDefinitions(
@@ -318,6 +328,15 @@ private void validatePrimaryIds(
318328
);
319329
}
320330

331+
private boolean checkIfDefinitelyOnReleaseBranch(Map<String, TransportVersionUpperBound> upperBounds) {
332+
// only want to look at definitions <= the current upper bound.
333+
// TODO: we should filter all of the upper bounds/definitions that are validated by this, not just in this method
334+
String currentUpperBoundName = getCurrentUpperBoundName().get();
335+
TransportVersionUpperBound currentUpperBound = upperBounds.get(currentUpperBoundName);
336+
337+
return upperBounds.values().stream().anyMatch(u -> u.definitionId().complete() > currentUpperBound.definitionId().complete());
338+
}
339+
321340
private void throwDefinitionFailure(TransportVersionDefinition definition, String message) {
322341
Path relativePath = getResources().get().getDefinitionPath(definition);
323342
throw new VerificationException("Transport version definition file [" + relativePath + "] " + message);

0 commit comments

Comments
 (0)