Browse Source

Change upstreamRef override to baseRef override (#135807) (#135816)

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.
Ryan Ernst 1 week ago
parent
commit
0d78dc0dec

+ 0 - 3
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") << """

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

@@ -37,31 +37,31 @@ public abstract class GenerateInitialTransportVersionTask extends DefaultTask {
         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);
+            }
         }
     }
 

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

@@ -109,7 +109,7 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask
             resetAllUpperBounds(resources, idsByBase);
         } else {
             getLogger().lifecycle("Generating transport version name: " + targetDefinitionName);
-            List<TransportVersionUpperBound> upstreamUpperBounds = resources.getUpperBoundsFromUpstream();
+            List<TransportVersionUpperBound> upstreamUpperBounds = resources.getUpperBoundsFromGitBase();
             Set<String> targetUpperBoundNames = getTargetUpperBoundNames(resources, upstreamUpperBounds, targetDefinitionName);
 
             List<TransportVersionId> ids = updateUpperBounds(
@@ -143,7 +143,7 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask
         List<TransportVersionId> 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 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask
     private void resetAllUpperBounds(TransportVersionResourcesService resources, Map<Integer, List<IdAndDefinition>> idsByBase)
         throws IOException {
         for (String upperBoundName : resources.getChangedUpperBoundNames()) {
-            TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromUpstream(upperBoundName);
+            TransportVersionUpperBound upstreamUpperBound = resources.getUpperBoundFromGitBase(upperBoundName);
             resetUpperBound(resources, upstreamUpperBound, idsByBase, null);
         }
     }

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

@@ -50,9 +50,9 @@ public class TransportVersionResourcesPlugin implements Plugin<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<String> upstreamRef = project.getProviders().gradleProperty("org.elasticsearch.transport.upstreamRef");
+                Provider<String> upstreamRef = project.getProviders().gradleProperty("org.elasticsearch.transport.baseRef");
                 if (upstreamRef.isPresent()) {
-                    spec.getParameters().getUpstreamRefOverride().set(upstreamRef.get());
+                    spec.getParameters().getBaseRefOverride().set(upstreamRef.get());
                 }
             });
 

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

@@ -66,7 +66,7 @@ public abstract class TransportVersionResourcesService implements BuildService<T
         DirectoryProperty getRootDirectory();
 
         @Optional
-        Property<String> getUpstreamRefOverride();
+        Property<String> getBaseRefOverride();
     }
 
     record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definition) {}
@@ -81,7 +81,6 @@ public abstract class TransportVersionResourcesService implements BuildService<T
 
     private final Path transportResourcesDir;
     private final Path rootDir;
-    private final String upstreamRefOverride;
     private final AtomicReference<String> baseRefName = new AtomicReference<>();
     private final AtomicReference<Set<String>> upstreamResources = new AtomicReference<>(null);
     private final AtomicReference<Set<String>> changedResources = new AtomicReference<>(null);
@@ -90,7 +89,9 @@ public abstract class TransportVersionResourcesService implements BuildService<T
     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 @@ public abstract class TransportVersionResourcesService implements BuildService<T
         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 @@ public abstract class TransportVersionResourcesService implements BuildService<T
         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 @@ public abstract class TransportVersionResourcesService implements BuildService<T
         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<TransportVersionUpperBound> getUpperBoundsFromUpstream() throws IOException {
+    /** Retrieve all upper bounds that exist in the merge base ref in git */
+    List<TransportVersionUpperBound> getUpperBoundsFromGitBase() throws IOException {
         List<TransportVersionUpperBound> upperBounds = new ArrayList<>();
         for (String upstreamPathString : getUpstreamResources()) {
             Path upstreamPath = Path.of(upstreamPathString);
@@ -278,15 +279,10 @@ public abstract class TransportVersionResourcesService implements BuildService<T
     }
 
     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<String> remoteNames = List.of(remotesOutput.split("\n"));
         if (remoteNames.contains(UPSTREAM_REMOTE_NAME) == false) {
@@ -302,7 +298,8 @@ public abstract class TransportVersionResourcesService implements BuildService<T
             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";
             }
         }
 

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

@@ -153,7 +153,7 @@ public abstract class ValidateTransportVersionResourcesTask extends DefaultTask
             }
         }
         // 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 @@ public abstract class ValidateTransportVersionResourcesTask extends DefaultTask
     }
 
     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 @@ public abstract class ValidateTransportVersionResourcesTask extends DefaultTask
             );
         }
 
-        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(