Skip to content

Commit 4f52323

Browse files
committed
Change upstreamRef override to baseRef override (elastic#135807)
When calculating the base git reference to read transport version resources from the upstream main ref must be known. Before we calculated the merge base this was overridden with a gradle property by tests and release tooling. However, in the case of release tooling we actually want to override the base ref directly so that it can be run on non-main branches. This commit renames the upstreamRef gradle property to baseRef. It also renames the resources service methods to make clearer what they are reading: from the git (merge) base.
1 parent 9dfd8e9 commit 4f52323

File tree

6 files changed

+45
-51
lines changed

6 files changed

+45
-51
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.transport.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/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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public void run() throws IOException {
109109
resetAllUpperBounds(resources, idsByBase);
110110
} else {
111111
getLogger().lifecycle("Generating transport version name: " + targetDefinitionName);
112-
List<TransportVersionUpperBound> upstreamUpperBounds = resources.getUpperBoundsFromUpstream();
112+
List<TransportVersionUpperBound> upstreamUpperBounds = resources.getUpperBoundsFromGitBase();
113113
Set<String> targetUpperBoundNames = getTargetUpperBoundNames(resources, upstreamUpperBounds, targetDefinitionName);
114114

115115
List<TransportVersionId> ids = updateUpperBounds(
@@ -143,7 +143,7 @@ private List<TransportVersionId> updateUpperBounds(
143143
List<TransportVersionId> ids = new ArrayList<>();
144144
boolean stageInGit = getResolveConflict().getOrElse(false);
145145

146-
TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromUpstream(definitionName);
146+
TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromGitBase(definitionName);
147147
for (TransportVersionUpperBound existingUpperBound : existingUpperBounds) {
148148
String upperBoundName = existingUpperBound.name();
149149

@@ -263,7 +263,7 @@ private Set<String> getUpperBoundNamesFromDefinition(
263263
private void resetAllUpperBounds(TransportVersionResourcesService resources, Map<Integer, List<IdAndDefinition>> idsByBase)
264264
throws IOException {
265265
for (String upperBoundName : resources.getChangedUpperBoundNames()) {
266-
TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromUpstream(upperBoundName);
266+
TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromGitBase(upperBoundName);
267267
resetUpperBound(resources, upstreamUpperBound, idsByBase, null);
268268
}
269269
}

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

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

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public interface Parameters extends BuildServiceParameters {
6666
DirectoryProperty getRootDirectory();
6767

6868
@Optional
69-
Property<String> getUpstreamRefOverride();
69+
Property<String> getBaseRefOverride();
7070
}
7171

7272
record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definition) {}
@@ -81,7 +81,6 @@ record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definit
8181

8282
private final Path transportResourcesDir;
8383
private final Path rootDir;
84-
private final String upstreamRefOverride;
8584
private final AtomicReference<String> baseRefName = new AtomicReference<>();
8685
private final AtomicReference<Set<String>> upstreamResources = new AtomicReference<>(null);
8786
private final AtomicReference<Set<String>> changedResources = new AtomicReference<>(null);
@@ -90,7 +89,9 @@ record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definit
9089
public TransportVersionResourcesService(Parameters params) {
9190
this.transportResourcesDir = params.getTransportResourcesDirectory().get().getAsFile().toPath();
9291
this.rootDir = params.getRootDirectory().get().getAsFile().toPath();
93-
upstreamRefOverride = params.getUpstreamRefOverride().getOrNull();
92+
if (params.getBaseRefOverride().isPresent()) {
93+
this.baseRefName.set(params.getBaseRefOverride().get());
94+
}
9495
}
9596

9697
/**
@@ -126,8 +127,8 @@ TransportVersionDefinition getReferableDefinition(String name) throws IOExceptio
126127
return TransportVersionDefinition.fromString(resourcePath, Files.readString(resourcePath, StandardCharsets.UTF_8), true);
127128
}
128129

129-
/** Get a referable definition from upstream if it exists there, or null otherwise */
130-
TransportVersionDefinition getReferableDefinitionFromUpstream(String name) {
130+
/** Get a referable definition from the merge base ref in git if it exists there, or null otherwise */
131+
TransportVersionDefinition getReferableDefinitionFromGitBase(String name) {
131132
Path resourcePath = getDefinitionRelativePath(name, true);
132133
return getUpstreamFile(resourcePath, (path, contents) -> TransportVersionDefinition.fromString(path, contents, true));
133134
}
@@ -174,8 +175,8 @@ Map<String, TransportVersionDefinition> getUnreferableDefinitions() throws IOExc
174175
return readDefinitions(transportResourcesDir.resolve(UNREFERABLE_DIR), false);
175176
}
176177

177-
/** Get a referable definition from upstream if it exists there, or null otherwise */
178-
TransportVersionDefinition getUnreferableDefinitionFromUpstream(String name) {
178+
/** Get a referable definition from the merge base ref in git if it exists there, or null otherwise */
179+
TransportVersionDefinition getUnreferableDefinitionFromGitBase(String name) {
179180
Path resourcePath = getDefinitionRelativePath(name, false);
180181
return getUpstreamFile(resourcePath, (path, contents) -> TransportVersionDefinition.fromString(path, contents, false));
181182
}
@@ -214,14 +215,14 @@ Map<String, TransportVersionUpperBound> getUpperBounds() throws IOException {
214215
return upperBounds;
215216
}
216217

217-
/** Retrieve an upper bound from upstream by name */
218-
TransportVersionUpperBound getUpperBoundFromUpstream(String name) {
218+
/** Retrieve an upper bound from the merge base ref in git by name */
219+
TransportVersionUpperBound getUpperBoundFromGitBase(String name) {
219220
Path resourcePath = getUpperBoundRelativePath(name);
220221
return getUpstreamFile(resourcePath, TransportVersionUpperBound::fromString);
221222
}
222223

223-
/** Retrieve all upper bounds that exist in upstream */
224-
List<TransportVersionUpperBound> getUpperBoundsFromUpstream() throws IOException {
224+
/** Retrieve all upper bounds that exist in the merge base ref in git */
225+
List<TransportVersionUpperBound> getUpperBoundsFromGitBase() throws IOException {
225226
List<TransportVersionUpperBound> upperBounds = new ArrayList<>();
226227
for (String upstreamPathString : getUpstreamResources()) {
227228
Path upstreamPath = Path.of(upstreamPathString);
@@ -278,15 +279,10 @@ private String getBaseRefName() {
278279
}
279280

280281
private String findUpstreamRef() {
281-
if (upstreamRefOverride != null) {
282-
return upstreamRefOverride;
283-
}
284-
285282
String remotesOutput = gitCommand("remote").strip();
286283
if (remotesOutput.isEmpty()) {
287-
throw new RuntimeException(
288-
"No remotes found. If this is a test set gradle property " + "org.elasticsearch.transport.upstreamRef"
289-
);
284+
logger.warn("No remotes found. Using 'main' branch as upstream ref for transport version resources");
285+
return "main";
290286
}
291287
List<String> remoteNames = List.of(remotesOutput.split("\n"));
292288
if (remoteNames.contains(UPSTREAM_REMOTE_NAME) == false) {
@@ -302,7 +298,8 @@ private String findUpstreamRef() {
302298
if (upstreamUrl != null) {
303299
gitCommand("remote", "add", UPSTREAM_REMOTE_NAME, upstreamUrl);
304300
} else {
305-
throw new RuntimeException("No elastic github remotes found to copy");
301+
logger.warn("No elastic github remotes found to copy. Using 'main' branch as upstream ref for transport version resources");
302+
return "main";
306303
}
307304
}
308305

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set<
153153
}
154154
}
155155
// validate any modifications
156-
TransportVersionDefinition originalDefinition = getResources().get().getReferableDefinitionFromUpstream(definition.name());
156+
TransportVersionDefinition originalDefinition = getResources().get().getReferableDefinitionFromGitBase(definition.name());
157157
if (originalDefinition != null) {
158158
validateIdenticalPrimaryId(definition, originalDefinition);
159159
for (int i = 1; i < originalDefinition.ids().size(); ++i) {
@@ -178,7 +178,7 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set<
178178
}
179179

180180
private void validateUnreferableDefinition(TransportVersionDefinition definition) {
181-
TransportVersionDefinition originalDefinition = getResources().get().getUnreferableDefinitionFromUpstream(definition.name());
181+
TransportVersionDefinition originalDefinition = getResources().get().getUnreferableDefinitionFromGitBase(definition.name());
182182
if (originalDefinition != null) {
183183
validateIdenticalPrimaryId(definition, originalDefinition);
184184
}
@@ -240,7 +240,7 @@ private void validateUpperBound(
240240
);
241241
}
242242

243-
TransportVersionUpperBound existingUpperBound = getResources().get().getUpperBoundFromUpstream(upperBound.name());
243+
TransportVersionUpperBound existingUpperBound = getResources().get().getUpperBoundFromGitBase(upperBound.name());
244244
if (existingUpperBound != null && getShouldValidatePrimaryIdNotPatch().get()) {
245245
if (upperBound.definitionId().patch() != 0 && upperBound.definitionId().base() != existingUpperBound.definitionId().base()) {
246246
throwUpperBoundFailure(

0 commit comments

Comments
 (0)