From f2a7ee2331485974151e992ed0fa4acfa865ccf5 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 1 Oct 2025 15:28:43 -0700 Subject: [PATCH] Change upstreamRef override to baseRef override (#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. --- .../AbstractTransportVersionFuncTest.groovy | 3 -- .../GenerateInitialTransportVersionTask.java | 42 +++++++++---------- ...enerateTransportVersionDefinitionTask.java | 6 +-- .../TransportVersionResourcesPlugin.java | 4 +- .../TransportVersionResourcesService.java | 35 +++++++--------- ...ValidateTransportVersionResourcesTask.java | 6 +-- 6 files changed, 45 insertions(+), 51 deletions(-) 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 3d1f397406f3e..18ebfb6a314d7 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 @@ -115,9 +115,6 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest { include ':myserver' include ':myplugin' """ - propertiesFile << """ - org.elasticsearch.transport.upstreamRef=main - """ versionPropertiesFile.text = versionPropertiesFile.text.replace("9.1.0", "9.2.0") file("myserver/build.gradle") << """ diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateInitialTransportVersionTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateInitialTransportVersionTask.java index b285e18d80dbf..a0691ca6a057e 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateInitialTransportVersionTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateInitialTransportVersionTask.java @@ -37,31 +37,31 @@ public void run() throws IOException { Version releaseVersion = Version.fromString(getReleaseVersion().get()); String upperBoundName = getUpperBoundName(releaseVersion); TransportVersionResourcesService resources = getResourceService().get(); - TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromUpstream(upperBoundName); + TransportVersionUpperBound baseUpperBound = resources.getUpperBoundFromGitBase(upperBoundName); String initialDefinitionName = "initial_" + releaseVersion; - TransportVersionDefinition existingDefinition = resources.getUnreferableDefinitionFromUpstream(initialDefinitionName); + TransportVersionDefinition existingDefinition = resources.getUnreferableDefinitionFromGitBase(initialDefinitionName); - if (existingDefinition != null) { - // this initial version has already been created upstream - return; - } + // This task runs on main and release branches. In release branches we will generate the exact same + // upper bound result because we always look at the base branch (ie upstream/main). + if (existingDefinition == null) { + if (baseUpperBound == null) { + throw new RuntimeException("Missing upper bound " + upperBoundName + " for release version " + releaseVersion); + } - if (upstreamUpperBound == null) { - throw new RuntimeException("Missing upper bound " + upperBoundName + " for release version " + releaseVersion); - } - // minors increment by 1000 to create a unique base, patches increment by 1 as other patches do - int increment = releaseVersion.getRevision() == 0 ? 1000 : 1; - var id = TransportVersionId.fromInt(upstreamUpperBound.definitionId().complete() + increment); - var definition = new TransportVersionDefinition(initialDefinitionName, List.of(id), false); - resources.writeDefinition(definition); - var newUpperBound = new TransportVersionUpperBound(upperBoundName, initialDefinitionName, id); - resources.writeUpperBound(newUpperBound, false); + // minors increment by 1000 to create a unique base, patches increment by 1 as other patches do + int increment = releaseVersion.getRevision() == 0 ? 1000 : 1; + var id = TransportVersionId.fromInt(baseUpperBound.definitionId().complete() + increment); + var definition = new TransportVersionDefinition(initialDefinitionName, List.of(id), false); + resources.writeDefinition(definition); + var newUpperBound = new TransportVersionUpperBound(upperBoundName, initialDefinitionName, id); + resources.writeUpperBound(newUpperBound, false); - if (releaseVersion.getRevision() == 0) { - Version currentVersion = getCurrentVersion().get(); - String currentUpperBoundName = getUpperBoundName(currentVersion); - var currentUpperBound = new TransportVersionUpperBound(currentUpperBoundName, initialDefinitionName, id); - resources.writeUpperBound(currentUpperBound, false); + if (releaseVersion.getRevision() == 0) { + Version currentVersion = getCurrentVersion().get(); + String currentUpperBoundName = getUpperBoundName(currentVersion); + var currentUpperBound = new TransportVersionUpperBound(currentUpperBoundName, initialDefinitionName, id); + resources.writeUpperBound(currentUpperBound, false); + } } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java index 4ba262ee16a10..f75e4633972f3 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java @@ -109,7 +109,7 @@ public void run() throws IOException { resetAllUpperBounds(resources, idsByBase); } else { getLogger().lifecycle("Generating transport version name: " + targetDefinitionName); - List upstreamUpperBounds = resources.getUpperBoundsFromUpstream(); + List upstreamUpperBounds = resources.getUpperBoundsFromGitBase(); Set targetUpperBoundNames = getTargetUpperBoundNames(resources, upstreamUpperBounds, targetDefinitionName); List ids = updateUpperBounds( @@ -143,7 +143,7 @@ private List updateUpperBounds( List ids = new ArrayList<>(); boolean stageInGit = getResolveConflict().getOrElse(false); - TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromUpstream(definitionName); + TransportVersionDefinition existingDefinition = resources.getReferableDefinitionFromGitBase(definitionName); for (TransportVersionUpperBound existingUpperBound : existingUpperBounds) { String upperBoundName = existingUpperBound.name(); @@ -263,7 +263,7 @@ private Set getUpperBoundNamesFromDefinition( private void resetAllUpperBounds(TransportVersionResourcesService resources, Map> idsByBase) throws IOException { for (String upperBoundName : resources.getChangedUpperBoundNames()) { - TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromUpstream(upperBoundName); + TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromGitBase(upperBoundName); resetUpperBound(resources, upstreamUpperBound, idsByBase, null); } } 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 756627616fb3b..1e3c2ad2e3f82 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 @@ -50,9 +50,9 @@ public void apply(Project project) { Directory transportResources = project.getLayout().getProjectDirectory().dir("src/main/resources/" + resourceRoot); spec.getParameters().getTransportResourcesDirectory().set(transportResources); spec.getParameters().getRootDirectory().set(project.getLayout().getSettingsDirectory().getAsFile()); - Provider upstreamRef = project.getProviders().gradleProperty("org.elasticsearch.transport.upstreamRef"); + Provider upstreamRef = project.getProviders().gradleProperty("org.elasticsearch.transport.baseRef"); if (upstreamRef.isPresent()) { - spec.getParameters().getUpstreamRefOverride().set(upstreamRef.get()); + spec.getParameters().getBaseRefOverride().set(upstreamRef.get()); } }); 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 89b4cf67ce6e5..208d73b0a8a6f 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 @@ -66,7 +66,7 @@ public interface Parameters extends BuildServiceParameters { DirectoryProperty getRootDirectory(); @Optional - Property getUpstreamRefOverride(); + Property getBaseRefOverride(); } record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definition) {} @@ -81,7 +81,6 @@ record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definit private final Path transportResourcesDir; private final Path rootDir; - private final String upstreamRefOverride; private final AtomicReference baseRefName = new AtomicReference<>(); private final AtomicReference> upstreamResources = new AtomicReference<>(null); private final AtomicReference> changedResources = new AtomicReference<>(null); @@ -90,7 +89,9 @@ record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definit public TransportVersionResourcesService(Parameters params) { this.transportResourcesDir = params.getTransportResourcesDirectory().get().getAsFile().toPath(); this.rootDir = params.getRootDirectory().get().getAsFile().toPath(); - upstreamRefOverride = params.getUpstreamRefOverride().getOrNull(); + if (params.getBaseRefOverride().isPresent()) { + this.baseRefName.set(params.getBaseRefOverride().get()); + } } /** @@ -126,8 +127,8 @@ TransportVersionDefinition getReferableDefinition(String name) throws IOExceptio return TransportVersionDefinition.fromString(resourcePath, Files.readString(resourcePath, StandardCharsets.UTF_8), true); } - /** Get a referable definition from upstream if it exists there, or null otherwise */ - TransportVersionDefinition getReferableDefinitionFromUpstream(String name) { + /** Get a referable definition from the merge base ref in git if it exists there, or null otherwise */ + TransportVersionDefinition getReferableDefinitionFromGitBase(String name) { Path resourcePath = getDefinitionRelativePath(name, true); return getUpstreamFile(resourcePath, (path, contents) -> TransportVersionDefinition.fromString(path, contents, true)); } @@ -174,8 +175,8 @@ Map getUnreferableDefinitions() throws IOExc return readDefinitions(transportResourcesDir.resolve(UNREFERABLE_DIR), false); } - /** Get a referable definition from upstream if it exists there, or null otherwise */ - TransportVersionDefinition getUnreferableDefinitionFromUpstream(String name) { + /** Get a referable definition from the merge base ref in git if it exists there, or null otherwise */ + TransportVersionDefinition getUnreferableDefinitionFromGitBase(String name) { Path resourcePath = getDefinitionRelativePath(name, false); return getUpstreamFile(resourcePath, (path, contents) -> TransportVersionDefinition.fromString(path, contents, false)); } @@ -214,14 +215,14 @@ Map getUpperBounds() throws IOException { return upperBounds; } - /** Retrieve an upper bound from upstream by name */ - TransportVersionUpperBound getUpperBoundFromUpstream(String name) { + /** Retrieve an upper bound from the merge base ref in git by name */ + TransportVersionUpperBound getUpperBoundFromGitBase(String name) { Path resourcePath = getUpperBoundRelativePath(name); return getUpstreamFile(resourcePath, TransportVersionUpperBound::fromString); } - /** Retrieve all upper bounds that exist in upstream */ - List getUpperBoundsFromUpstream() throws IOException { + /** Retrieve all upper bounds that exist in the merge base ref in git */ + List getUpperBoundsFromGitBase() throws IOException { List upperBounds = new ArrayList<>(); for (String upstreamPathString : getUpstreamResources()) { Path upstreamPath = Path.of(upstreamPathString); @@ -278,15 +279,10 @@ private String getBaseRefName() { } private String findUpstreamRef() { - if (upstreamRefOverride != null) { - return upstreamRefOverride; - } - String remotesOutput = gitCommand("remote").strip(); if (remotesOutput.isEmpty()) { - throw new RuntimeException( - "No remotes found. If this is a test set gradle property " + "org.elasticsearch.transport.upstreamRef" - ); + logger.warn("No remotes found. Using 'main' branch as upstream ref for transport version resources"); + return "main"; } List remoteNames = List.of(remotesOutput.split("\n")); if (remoteNames.contains(UPSTREAM_REMOTE_NAME) == false) { @@ -302,7 +298,8 @@ private String findUpstreamRef() { if (upstreamUrl != null) { gitCommand("remote", "add", UPSTREAM_REMOTE_NAME, upstreamUrl); } else { - throw new RuntimeException("No elastic github remotes found to copy"); + logger.warn("No elastic github remotes found to copy. Using 'main' branch as upstream ref for transport version resources"); + return "main"; } } 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 02338d7f677f1..a344741d7ef88 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 @@ -153,7 +153,7 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set< } } // validate any modifications - TransportVersionDefinition originalDefinition = getResources().get().getReferableDefinitionFromUpstream(definition.name()); + TransportVersionDefinition originalDefinition = getResources().get().getReferableDefinitionFromGitBase(definition.name()); if (originalDefinition != null) { validateIdenticalPrimaryId(definition, originalDefinition); for (int i = 1; i < originalDefinition.ids().size(); ++i) { @@ -178,7 +178,7 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set< } private void validateUnreferableDefinition(TransportVersionDefinition definition) { - TransportVersionDefinition originalDefinition = getResources().get().getUnreferableDefinitionFromUpstream(definition.name()); + TransportVersionDefinition originalDefinition = getResources().get().getUnreferableDefinitionFromGitBase(definition.name()); if (originalDefinition != null) { validateIdenticalPrimaryId(definition, originalDefinition); } @@ -240,7 +240,7 @@ private void validateUpperBound( ); } - TransportVersionUpperBound existingUpperBound = getResources().get().getUpperBoundFromUpstream(upperBound.name()); + TransportVersionUpperBound existingUpperBound = getResources().get().getUpperBoundFromGitBase(upperBound.name()); if (existingUpperBound != null && getShouldValidatePrimaryIdNotPatch().get()) { if (upperBound.definitionId().patch() != 0 && upperBound.definitionId().base() != existingUpperBound.definitionId().base()) { throwUpperBoundFailure(