Merge branch 'main' into esql/trivial-javadoc-fix

Author: romseygeek

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

@@ -116,7 +116,7 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest {
             include ':myplugin'
         """
         propertiesFile << """
-            org.elasticsearch.transports.upstreamRef=main
+            org.elasticsearch.transport.upstreamRef=main
         """
         versionPropertiesFile.text = versionPropertiesFile.text.replace("9.1.0", "9.2.0")
 

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

@@ -96,4 +96,35 @@ class ResolveTransportVersionConflictFuncTest extends AbstractTransportVersionFu
         assertResolveAndValidateSuccess(result)
         assertUpperBound("9.2", "existing_92,8123000")
     }
+
+    def "upstream changes don't affect merge"() {
+        given:
+        // setup main with 2 commits, but we will only merge in the first one
+        execute("git checkout main")
+        referableAndReferencedTransportVersion("upstream_new_tv1", "8124000")
+        transportVersionUpperBound("9.2", "upstream_new_tv1", "8124000")
+        execute("git add .")
+        execute("git commit -m update1")
+        String toMerge = execute("git rev-parse HEAD")
+        referableAndReferencedTransportVersion("upstream_new_tv2", "8125000")
+        transportVersionUpperBound("9.2", "upstream_new_tv2", "8125000")
+        execute("git add .")
+        execute("git commit -m update2")
+        execute("git checkout test")
+        // now commit a conflict on the test branch, a new TV
+        referableAndReferencedTransportVersion("branch_new_tv", "8124000")
+        transportVersionUpperBound("9.2", "branch_new_tv", "8124000")
+        execute("git add .")
+        execute("git commit -m branch")
+        // and finally initiate the merge
+        System.out.println("Merging commit " + toMerge);
+        execute("git merge " + toMerge, testProjectDir.root, true);
+
+        when:
+        def result = runResolveAndValidateTask().build()
+
+        then:
+        assertResolveAndValidateSuccess(result)
+        assertUpperBound("9.2", "branch_new_tv,8125000")
+    }
 }

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

@@ -496,4 +496,21 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
         assertUpperBound("9.2", "new_tv,8124000")
         assertReferableDefinition("new_tv", "8124000")
     }
+
+    def "generation is idempotent on upstream changes"() {
+        given:
+        execute("git checkout main")
+        referableAndReferencedTransportVersion("new_tv", "8124000")
+        transportVersionUpperBound("9.2", "new_tv", "8124000")
+        execute("git add .")
+        execute("git commit -m update")
+        execute("git checkout test")
+
+        when:
+        def result = runGenerateAndValidateTask().build()
+
+        then:
+        assertGenerateAndValidateSuccess(result)
+        assertUpperBound("9.2", "existing_92,8123000")
+    }
 }

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

@@ -17,7 +17,7 @@ public enum DockerBase {
     DEFAULT("redhat/ubi9-minimal:latest", "", "microdnf", "dockerfiles/default/Dockerfile"),
 
     // The Iron Bank base image is UBI (albeit hardened), but we are required to parameterize the Docker build
-    IRON_BANK("${BASE_REGISTRY}/${BASE_IMAGE}:${BASE_TAG}", "-ironbank", "yum", "Dockerfile"),
+    IRON_BANK("${BASE_REGISTRY}/${BASE_IMAGE}:${BASE_TAG}", "-ironbank", "yum", "dockerfiles/ironbank/Dockerfile"),
 
     // Chainguard based wolfi image with latest jdk
     WOLFI(

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

@@ -101,6 +101,12 @@ public void visitLineNumber(int line, Label start) {
                         lineNumber = line;
                     }
 
+                    @Override
+                    public void visitLabel(Label label) {
+                        // asm uses many debug labels that we do not want to consider
+                        // so we ignore labels so they do not become part of the instructions list
+                    }
+
                     @Override
                     public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) {
                         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/TransportVersionResourcesService.java

@@ -58,6 +58,7 @@
 public abstract class TransportVersionResourcesService implements BuildService<TransportVersionResourcesService.Parameters> {
 
     private static final Logger logger = Logging.getLogger(TransportVersionResourcesService.class);
+    private static final String UPSTREAM_REMOTE_NAME = "transport-version-resources-upstream";
 
     public interface Parameters extends BuildServiceParameters {
         DirectoryProperty getTransportResourcesDirectory();
@@ -80,17 +81,16 @@ record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definit
 
     private final Path transportResourcesDir;
     private final Path rootDir;
-    private final AtomicReference<String> upstreamRefName = new AtomicReference<>();
+    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);
 
     @Inject
     public TransportVersionResourcesService(Parameters params) {
         this.transportResourcesDir = params.getTransportResourcesDirectory().get().getAsFile().toPath();
         this.rootDir = params.getRootDirectory().get().getAsFile().toPath();
-        if (params.getUpstreamRefOverride().isPresent()) {
-            upstreamRefName.set(params.getUpstreamRefOverride().get());
-        }
+        upstreamRefOverride = params.getUpstreamRefOverride().getOrNull();
     }
 
     /**
@@ -258,52 +258,65 @@ private Path getUpperBoundRelativePath(String name) {
         return UPPER_BOUNDS_DIR.resolve(name + ".csv");
     }
 
-    private String getUpstreamRefName() {
-        if (upstreamRefName.get() == null) {
-            synchronized (upstreamRefName) {
-                String remotesOutput = gitCommand("remote").strip();
-
+    private String getBaseRefName() {
+        if (baseRefName.get() == null) {
+            synchronized (baseRefName) {
                 String refName;
-                if (remotesOutput.isEmpty()) {
-                    refName = "main"; // fallback to local main if no remotes, this happens in tests
+                // the existence of the MERGE_HEAD ref means we are in the middle of a merge, and should use that as our base
+                String gitDir = gitCommand("rev-parse", "--git-dir").strip();
+                if (Files.exists(Path.of(gitDir).resolve("MERGE_HEAD"))) {
+                    refName = gitCommand("rev-parse", "--verify", "MERGE_HEAD").strip();
                 } else {
-                    List<String> remoteNames = List.of(remotesOutput.split("\n"));
-                    String transportVersionRemoteName = "transport-version-resources-upstream";
-                    if (remoteNames.contains(transportVersionRemoteName) == false) {
-                        // our special remote doesn't exist yet, so create it
-                        String upstreamUrl = null;
-                        for (String remoteName : remoteNames) {
-                            String getUrlOutput = gitCommand("remote", "get-url", remoteName).strip();
-                            if (getUrlOutput.startsWith("[email protected]:elastic/")
-                                || getUrlOutput.startsWith("https://github.com/elastic/")) {
-                                upstreamUrl = getUrlOutput;
-                            }
-                        }
-
-                        if (upstreamUrl != null) {
-                            gitCommand("remote", "add", transportVersionRemoteName, upstreamUrl);
-                        } else {
-                            throw new RuntimeException("No elastic github remotes found to copy");
-                        }
-                    }
-
-                    // make sure the remote main ref is up to date
-                    gitCommand("fetch", transportVersionRemoteName, "main");
-
-                    refName = transportVersionRemoteName + "/main";
+                    String upstreamRef = findUpstreamRef();
+                    refName = gitCommand("merge-base", upstreamRef, "HEAD").strip();
+                }
+
+                baseRefName.set(refName);
+            }
+        }
+        return baseRefName.get();
+    }
+
+    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"
+            );
+        }
+        List<String> remoteNames = List.of(remotesOutput.split("\n"));
+        if (remoteNames.contains(UPSTREAM_REMOTE_NAME) == false) {
+            // our special remote doesn't exist yet, so create it
+            String upstreamUrl = null;
+            for (String remoteName : remoteNames) {
+                String getUrlOutput = gitCommand("remote", "get-url", remoteName).strip();
+                if (getUrlOutput.startsWith("[email protected]:elastic/") || getUrlOutput.startsWith("https://github.com/elastic/")) {
+                    upstreamUrl = getUrlOutput;
                 }
-                upstreamRefName.set(refName);
+            }
 
+            if (upstreamUrl != null) {
+                gitCommand("remote", "add", UPSTREAM_REMOTE_NAME, upstreamUrl);
+            } else {
+                throw new RuntimeException("No elastic github remotes found to copy");
             }
         }
-        return upstreamRefName.get();
+
+        // make sure the remote main ref is up to date
+        gitCommand("fetch", UPSTREAM_REMOTE_NAME, "main");
+
+        return UPSTREAM_REMOTE_NAME + "/main";
     }
 
     // Return the transport version resources paths that exist in upstream
     private Set<String> getUpstreamResources() {
         if (upstreamResources.get() == null) {
             synchronized (upstreamResources) {
-                String output = gitCommand("ls-tree", "--name-only", "-r", getUpstreamRefName(), ".");
+                String output = gitCommand("ls-tree", "--name-only", "-r", getBaseRefName(), ".");
 
                 HashSet<String> resources = new HashSet<>();
                 Collections.addAll(resources, output.split("\n")); // git always outputs LF
@@ -319,7 +332,7 @@ private Set<String> getChangedResources() {
             synchronized (changedResources) {
                 HashSet<String> resources = new HashSet<>();
 
-                String diffOutput = gitCommand("diff", "--name-only", "--relative", getUpstreamRefName(), ".");
+                String diffOutput = gitCommand("diff", "--name-only", "--relative", getBaseRefName(), ".");
                 if (diffOutput.strip().isEmpty() == false) {
                     Collections.addAll(resources, diffOutput.split("\n")); // git always outputs LF
                 }
@@ -357,7 +370,7 @@ private <T> T getUpstreamFile(Path resourcePath, BiFunction<Path, String, T> par
             return null;
         }
 
-        String content = gitCommand("show", getUpstreamRefName() + ":./" + pathString).strip();
+        String content = gitCommand("show", getBaseRefName() + ":./" + pathString).strip();
         return parser.apply(resourcePath, content);
     }
 

build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/AbstractGradleFuncTest.groovy

@@ -211,10 +211,10 @@ abstract class AbstractGradleFuncTest extends Specification {
         """
     }
 
-    String execute(String command, File workingDir = testProjectDir.root) {
+    String execute(String command, File workingDir = testProjectDir.root, boolean ignoreFailure = false) {
         def proc = command.execute(Collections.emptyList(), workingDir)
         proc.waitFor()
-        if (proc.exitValue()) {
+        if (proc.exitValue() && ignoreFailure == false) {
             String msg = """Error running command ${command}:
                 Sysout: ${proc.inputStream.text}
                 Syserr: ${proc.errorStream.text}

distribution/docker/README.md

@@ -134,8 +134,11 @@ custom behaviour in the
 
 We go to some lengths to try and make the Docker build resilient to transient
 network errors. This is why, when browsing the
-[Dockerfile](src/docker/Dockerfile), you'll see many commands wrapped in looping
-logic, so that if e.g. package installation fails, we try again. We also perform
+[Dockerfile](src/docker/dockerfiles/wolfi/Dockerfile), you'll see many commands wrapped in looping
+logic, so that if e.g. package installation fails, we try again.
+Our default docker image and the Iron Bank image do not have this retry logic,
+because Dockerhub and Iron Bank asked for as concise as possible Dockerfiles.
+We also perform
 explicit `docker pull` commands instead of relying on `docker run` to pull an
 image down automatically, so that we can wrap the `pull` part in a retry.