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 d97c3747ce343..3d1f397406f3e 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 @@ -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") diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/ResolveTransportVersionConflictFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/ResolveTransportVersionConflictFuncTest.groovy index 974637020ea37..2f009ad5d9e2a 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/ResolveTransportVersionConflictFuncTest.groovy +++ b/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") + } } diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy index 7dcdf5f66d166..0f2571d7ede9d 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy +++ b/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") + } } 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 5be53829d06f8..89b4cf67ce6e5 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 @@ -58,6 +58,7 @@ public abstract class TransportVersionResourcesService implements BuildService { 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,7 +81,8 @@ record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definit private final Path transportResourcesDir; private final Path rootDir; - private final AtomicReference upstreamRefName = new AtomicReference<>(); + private final String upstreamRefOverride; + private final AtomicReference baseRefName = new AtomicReference<>(); private final AtomicReference> upstreamResources = new AtomicReference<>(null); private final AtomicReference> changedResources = new AtomicReference<>(null); @@ -88,9 +90,7 @@ record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definit 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 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("git@github.com: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 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("git@github.com: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 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 resources = new HashSet<>(); Collections.addAll(resources, output.split("\n")); // git always outputs LF @@ -319,7 +332,7 @@ private Set getChangedResources() { synchronized (changedResources) { HashSet 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 getUpstreamFile(Path resourcePath, BiFunction par return null; } - String content = gitCommand("show", getUpstreamRefName() + ":./" + pathString).strip(); + String content = gitCommand("show", getBaseRefName() + ":./" + pathString).strip(); return parser.apply(resourcePath, content); } diff --git a/build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/AbstractGradleFuncTest.groovy b/build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/AbstractGradleFuncTest.groovy index 553d5ea7d8d40..6109979df0331 100644 --- a/build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/AbstractGradleFuncTest.groovy +++ b/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}