Skip to content

Commit 3c1654d

Browse files
leemthompoleemthompo
authored
Merge branch '9.1' into backport/9.1/pr-135600
2 parents 1954663 + 8dc3838 commit 3c1654d

File tree

111 files changed

+1885
-802
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

111 files changed

+1885
-802
lines changed

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,6 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest {
115115
include ':myserver'
116116
include ':myplugin'
117117
"""
118-
propertiesFile << """
119-
org.elasticsearch.transports.upstreamRef=main
120-
"""
121118
versionPropertiesFile.text = versionPropertiesFile.text.replace("9.1.0", "9.2.0")
122119

123120
file("myserver/build.gradle") << """

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,35 @@ class ResolveTransportVersionConflictFuncTest extends AbstractTransportVersionFu
9696
assertResolveAndValidateSuccess(result)
9797
assertUpperBound("9.2", "existing_92,8123000")
9898
}
99+
100+
def "upstream changes don't affect merge"() {
101+
given:
102+
// setup main with 2 commits, but we will only merge in the first one
103+
execute("git checkout main")
104+
referableAndReferencedTransportVersion("upstream_new_tv1", "8124000")
105+
transportVersionUpperBound("9.2", "upstream_new_tv1", "8124000")
106+
execute("git add .")
107+
execute("git commit -m update1")
108+
String toMerge = execute("git rev-parse HEAD")
109+
referableAndReferencedTransportVersion("upstream_new_tv2", "8125000")
110+
transportVersionUpperBound("9.2", "upstream_new_tv2", "8125000")
111+
execute("git add .")
112+
execute("git commit -m update2")
113+
execute("git checkout test")
114+
// now commit a conflict on the test branch, a new TV
115+
referableAndReferencedTransportVersion("branch_new_tv", "8124000")
116+
transportVersionUpperBound("9.2", "branch_new_tv", "8124000")
117+
execute("git add .")
118+
execute("git commit -m branch")
119+
// and finally initiate the merge
120+
System.out.println("Merging commit " + toMerge);
121+
execute("git merge " + toMerge, testProjectDir.root, true);
122+
123+
when:
124+
def result = runResolveAndValidateTask().build()
125+
126+
then:
127+
assertResolveAndValidateSuccess(result)
128+
assertUpperBound("9.2", "branch_new_tv,8125000")
129+
}
99130
}

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,6 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
9090
assertUpperBound("9.2", "new_tv,8124000")
9191
}
9292

93-
/*
94-
temporarily muted, see https://github.com/elastic/elasticsearch/pull/135226
95-
9693
def "invalid changes to a upper bounds should be reverted"() {
9794
given:
9895
transportVersionUpperBound("9.2", "modification", "9000000")
@@ -147,7 +144,7 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
147144
assertReferableDefinitionDoesNotExist("test_tv")
148145
assertUpperBound("9.2", "existing_92,8123000")
149146
assertUpperBound("9.1", "existing_92,8012001")
150-
}*/
147+
}
151148

152149
def "a reference can be renamed"() {
153150
given:
@@ -245,11 +242,8 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
245242
def "unreferenced definitions are removed"() {
246243
given:
247244
referableTransportVersion("test_tv", "8124000,8012002")
248-
/*
249-
TODO: reset of upper bounds
250245
transportVersionUpperBound("9.2", "test_tv", "8124000")
251246
transportVersionUpperBound("9.1", "test_tv", "8012002")
252-
*/
253247

254248
when:
255249
def result = runGenerateAndValidateTask().build()
@@ -412,8 +406,6 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
412406
assertUpperBound("9.2", "new_tv,8124000")
413407
}
414408

415-
/*
416-
TODO: reset of upper bounds
417409
def "deleted upper bounds files are restored"() {
418410
given:
419411
file("myserver/src/main/resources/transport/upper_bounds/9.2.csv").delete()
@@ -424,7 +416,7 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
424416
then:
425417
assertGenerateAndValidateSuccess(result)
426418
assertUpperBound("9.2", "existing_92,8123000")
427-
}*/
419+
}
428420

429421
def "upper bounds files must exist for backport branches"() {
430422
when:
@@ -504,4 +496,21 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
504496
assertUpperBound("9.2", "new_tv,8124000")
505497
assertReferableDefinition("new_tv", "8124000")
506498
}
499+
500+
def "generation is idempotent on upstream changes"() {
501+
given:
502+
execute("git checkout main")
503+
referableAndReferencedTransportVersion("new_tv", "8124000")
504+
transportVersionUpperBound("9.2", "new_tv", "8124000")
505+
execute("git add .")
506+
execute("git commit -m update")
507+
execute("git checkout test")
508+
509+
when:
510+
def result = runGenerateAndValidateTask().build()
511+
512+
then:
513+
assertGenerateAndValidateSuccess(result)
514+
assertUpperBound("9.2", "existing_92,8123000")
515+
}
507516
}

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/DockerBase.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public enum DockerBase {
1717
DEFAULT("redhat/ubi9-minimal:latest", "", "microdnf", "dockerfiles/default/Dockerfile"),
1818

1919
// The Iron Bank base image is UBI (albeit hardened), but we are required to parameterize the Docker build
20-
IRON_BANK("${BASE_REGISTRY}/${BASE_IMAGE}:${BASE_TAG}", "-ironbank", "yum", "Dockerfile"),
20+
IRON_BANK("${BASE_REGISTRY}/${BASE_IMAGE}:${BASE_TAG}", "-ironbank", "yum", "dockerfiles/ironbank/Dockerfile"),
2121

2222
// Chainguard based wolfi image with latest jdk
2323
WOLFI(

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@ public void visitLineNumber(int line, Label start) {
101101
lineNumber = line;
102102
}
103103

104+
@Override
105+
public void visitLabel(Label label) {
106+
// asm uses many debug labels that we do not want to consider
107+
// so we ignore labels so they do not become part of the instructions list
108+
}
109+
104110
@Override
105111
public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) {
106112
if (owner.equals(TRANSPORT_VERSION_SET_CLASS) && name.equals(TRANSPORT_VERSION_SET_METHOD_NAME)) {

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,31 +37,31 @@ public void run() throws IOException {
3737
Version releaseVersion = Version.fromString(getReleaseVersion().get());
3838
String upperBoundName = getUpperBoundName(releaseVersion);
3939
TransportVersionResourcesService resources = getResourceService().get();
40-
TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromUpstream(upperBoundName);
40+
TransportVersionUpperBound baseUpperBound = resources.getUpperBoundFromGitBase(upperBoundName);
4141
String initialDefinitionName = "initial_" + releaseVersion;
42-
TransportVersionDefinition existingDefinition = resources.getUnreferableDefinitionFromUpstream(initialDefinitionName);
42+
TransportVersionDefinition existingDefinition = resources.getUnreferableDefinitionFromGitBase(initialDefinitionName);
4343

44-
if (existingDefinition != null) {
45-
// this initial version has already been created upstream
46-
return;
47-
}
44+
// This task runs on main and release branches. In release branches we will generate the exact same
45+
// upper bound result because we always look at the base branch (ie upstream/main).
46+
if (existingDefinition == null) {
47+
if (baseUpperBound == null) {
48+
throw new RuntimeException("Missing upper bound " + upperBoundName + " for release version " + releaseVersion);
49+
}
4850

49-
if (upstreamUpperBound == null) {
50-
throw new RuntimeException("Missing upper bound " + upperBoundName + " for release version " + releaseVersion);
51-
}
52-
// minors increment by 1000 to create a unique base, patches increment by 1 as other patches do
53-
int increment = releaseVersion.getRevision() == 0 ? 1000 : 1;
54-
var id = TransportVersionId.fromInt(upstreamUpperBound.definitionId().complete() + increment);
55-
var definition = new TransportVersionDefinition(initialDefinitionName, List.of(id), false);
56-
resources.writeDefinition(definition);
57-
var newUpperBound = new TransportVersionUpperBound(upperBoundName, initialDefinitionName, id);
58-
resources.writeUpperBound(newUpperBound, false);
51+
// minors increment by 1000 to create a unique base, patches increment by 1 as other patches do
52+
int increment = releaseVersion.getRevision() == 0 ? 1000 : 1;
53+
var id = TransportVersionId.fromInt(baseUpperBound.definitionId().complete() + increment);
54+
var definition = new TransportVersionDefinition(initialDefinitionName, List.of(id), false);
55+
resources.writeDefinition(definition);
56+
var newUpperBound = new TransportVersionUpperBound(upperBoundName, initialDefinitionName, id);
57+
resources.writeUpperBound(newUpperBound, false);
5958

60-
if (releaseVersion.getRevision() == 0) {
61-
Version currentVersion = getCurrentVersion().get();
62-
String currentUpperBoundName = getUpperBoundName(currentVersion);
63-
var currentUpperBound = new TransportVersionUpperBound(currentUpperBoundName, initialDefinitionName, id);
64-
resources.writeUpperBound(currentUpperBound, false);
59+
if (releaseVersion.getRevision() == 0) {
60+
Version currentVersion = getCurrentVersion().get();
61+
String currentUpperBoundName = getUpperBoundName(currentVersion);
62+
var currentUpperBound = new TransportVersionUpperBound(currentUpperBoundName, initialDefinitionName, id);
63+
resources.writeUpperBound(currentUpperBound, false);
64+
}
6565
}
6666
}
6767

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

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package org.elasticsearch.gradle.internal.transport;
1111

12+
import org.elasticsearch.gradle.internal.transport.TransportVersionResourcesService.IdAndDefinition;
1213
import org.gradle.api.DefaultTask;
1314
import org.gradle.api.file.ConfigurableFileCollection;
1415
import org.gradle.api.file.RegularFileProperty;
@@ -31,6 +32,7 @@
3132
import java.util.Collections;
3233
import java.util.HashSet;
3334
import java.util.List;
35+
import java.util.Map;
3436
import java.util.Set;
3537

3638
/**
@@ -95,29 +97,39 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask
9597
public void run() throws IOException {
9698
TransportVersionResourcesService resources = getResourceService().get();
9799
Set<String> referencedNames = TransportVersionReference.collectNames(getReferencesFiles());
98-
List<String> changedDefinitionNames = resources.getChangedReferableDefinitionNames();
100+
Set<String> changedDefinitionNames = resources.getChangedReferableDefinitionNames();
99101
String targetDefinitionName = getTargetDefinitionName(resources, referencedNames, changedDefinitionNames);
100102

101-
getLogger().lifecycle("Generating transport version name: " + targetDefinitionName);
103+
// First check for any unused definitions. This later generation to not get confused by a definition that can't be used.
104+
removeUnusedNamedDefinitions(resources, referencedNames, changedDefinitionNames);
105+
106+
Map<Integer, List<IdAndDefinition>> idsByBase = resources.getIdsByBase();
102107
if (targetDefinitionName.isEmpty()) {
103-
// TODO: resetting upper bounds needs to be done locally, otherwise it pulls in some (incomplete) changes from upstream main
104-
// resetAllUpperBounds(resources);
108+
getLogger().lifecycle("No transport version name detected, resetting upper bounds");
109+
resetAllUpperBounds(resources, idsByBase);
105110
} else {
106-
List<TransportVersionUpperBound> upstreamUpperBounds = resources.getUpperBoundsFromUpstream();
111+
getLogger().lifecycle("Generating transport version name: " + targetDefinitionName);
112+
List<TransportVersionUpperBound> upstreamUpperBounds = resources.getUpperBoundsFromGitBase();
107113
Set<String> targetUpperBoundNames = getTargetUpperBoundNames(resources, upstreamUpperBounds, targetDefinitionName);
108114

109-
List<TransportVersionId> ids = updateUpperBounds(resources, upstreamUpperBounds, targetUpperBoundNames, targetDefinitionName);
115+
List<TransportVersionId> ids = updateUpperBounds(
116+
resources,
117+
upstreamUpperBounds,
118+
targetUpperBoundNames,
119+
idsByBase,
120+
targetDefinitionName
121+
);
110122
// (Re)write the definition file.
111123
resources.writeDefinition(new TransportVersionDefinition(targetDefinitionName, ids, true));
112124
}
113125

114-
removeUnusedNamedDefinitions(resources, referencedNames, changedDefinitionNames);
115126
}
116127

117128
private List<TransportVersionId> updateUpperBounds(
118129
TransportVersionResourcesService resources,
119130
List<TransportVersionUpperBound> existingUpperBounds,
120131
Set<String> targetUpperBoundNames,
132+
Map<Integer, List<IdAndDefinition>> idsByBase,
121133
String definitionName
122134
) throws IOException {
123135
String currentUpperBoundName = getCurrentUpperBoundName().get();
@@ -131,7 +143,7 @@ private List<TransportVersionId> updateUpperBounds(
131143
List<TransportVersionId> ids = new ArrayList<>();
132144
boolean stageInGit = getResolveConflict().getOrElse(false);
133145

134-
TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromUpstream(definitionName);
146+
TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromGitBase(definitionName);
135147
for (TransportVersionUpperBound existingUpperBound : existingUpperBounds) {
136148
String upperBoundName = existingUpperBound.name();
137149

@@ -146,9 +158,9 @@ private List<TransportVersionId> updateUpperBounds(
146158
resources.writeUpperBound(newUpperBound, stageInGit);
147159
}
148160
ids.add(targetId);
149-
} else {
161+
} else if (resources.getChangedUpperBoundNames().contains(upperBoundName)) {
150162
// Default case: we're not targeting this branch so reset it
151-
resources.writeUpperBound(existingUpperBound, false);
163+
resetUpperBound(resources, existingUpperBound, idsByBase, definitionName);
152164
}
153165
}
154166

@@ -159,7 +171,7 @@ private List<TransportVersionId> updateUpperBounds(
159171
private String getTargetDefinitionName(
160172
TransportVersionResourcesService resources,
161173
Set<String> referencedNames,
162-
List<String> changedDefinitions
174+
Set<String> changedDefinitions
163175
) {
164176
if (getDefinitionName().isPresent()) {
165177
// an explicit name was passed in, so use it
@@ -180,7 +192,7 @@ private String getTargetDefinitionName(
180192
if (changedDefinitions.isEmpty()) {
181193
return "";
182194
} else {
183-
String changedDefinitionName = changedDefinitions.getFirst();
195+
String changedDefinitionName = changedDefinitions.iterator().next();
184196
if (referencedNames.contains(changedDefinitionName)) {
185197
return changedDefinitionName;
186198
} else {
@@ -248,16 +260,38 @@ private Set<String> getUpperBoundNamesFromDefinition(
248260
return upperBoundNames;
249261
}
250262

251-
private void resetAllUpperBounds(TransportVersionResourcesService resources) throws IOException {
252-
for (TransportVersionUpperBound upperBound : resources.getUpperBoundsFromUpstream()) {
253-
resources.writeUpperBound(upperBound, false);
263+
private void resetAllUpperBounds(TransportVersionResourcesService resources, Map<Integer, List<IdAndDefinition>> idsByBase)
264+
throws IOException {
265+
for (String upperBoundName : resources.getChangedUpperBoundNames()) {
266+
TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromGitBase(upperBoundName);
267+
resetUpperBound(resources, upstreamUpperBound, idsByBase, null);
268+
}
269+
}
270+
271+
private void resetUpperBound(
272+
TransportVersionResourcesService resources,
273+
TransportVersionUpperBound upperBound,
274+
Map<Integer, List<IdAndDefinition>> idsByBase,
275+
String ignoreDefinitionName
276+
) throws IOException {
277+
List<IdAndDefinition> idsForUpperBound = idsByBase.get(upperBound.definitionId().base());
278+
if (idsForUpperBound == null) {
279+
throw new RuntimeException("Could not find base id: " + upperBound.definitionId().base());
280+
}
281+
IdAndDefinition resetValue = idsForUpperBound.getLast();
282+
if (resetValue.definition().name().equals(ignoreDefinitionName)) {
283+
// there must be another definition in this base since the ignored definition is new
284+
assert idsForUpperBound.size() >= 2;
285+
resetValue = idsForUpperBound.get(idsForUpperBound.size() - 2);
254286
}
287+
var resetUpperBound = new TransportVersionUpperBound(upperBound.name(), resetValue.definition().name(), resetValue.id());
288+
resources.writeUpperBound(resetUpperBound, false);
255289
}
256290

257291
private void removeUnusedNamedDefinitions(
258292
TransportVersionResourcesService resources,
259293
Set<String> referencedNames,
260-
List<String> changedDefinitions
294+
Set<String> changedDefinitions
261295
) throws IOException {
262296
for (String definitionName : changedDefinitions) {
263297
if (referencedNames.contains(definitionName) == false) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ public void apply(Project project) {
5050
Directory transportResources = project.getLayout().getProjectDirectory().dir("src/main/resources/" + resourceRoot);
5151
spec.getParameters().getTransportResourcesDirectory().set(transportResources);
5252
spec.getParameters().getRootDirectory().set(project.getLayout().getSettingsDirectory().getAsFile());
53-
Provider<String> upstreamRef = project.getProviders().gradleProperty("org.elasticsearch.transport.upstreamRef");
53+
Provider<String> upstreamRef = project.getProviders().gradleProperty("org.elasticsearch.transport.baseRef");
5454
if (upstreamRef.isPresent()) {
55-
spec.getParameters().getUpstreamRefOverride().set(upstreamRef.get());
55+
spec.getParameters().getBaseRefOverride().set(upstreamRef.get());
5656
}
5757
});
5858

0 commit comments

Comments
 (0)