Handle diamond merges correctly.
If we have branches top, left, right, and bottom, and left == right,
then creating a change on top should create two changes with the same
sha on left and right, and only a single change on bottom.
top -> left -> bottom, and top -> right -> bottom simultaneously.
Change-Id: I8813cefb7cb7052b435dbef4e1af422c516e13a0
diff --git a/BUILD b/BUILD
index 0914e7b..1c3873a 100644
--- a/BUILD
+++ b/BUILD
@@ -23,5 +23,6 @@
tags = ["automerger"],
deps = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
":automerger__plugin",
+ "@commons_net//jar",
],
)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
index 766529d..b12a0f3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
@@ -365,16 +365,7 @@
String upstreamRevision, String topic, String downstreamBranch)
throws RestApiException, InvalidQueryParameterException {
List<Integer> downstreamChangeNumbers = new ArrayList<Integer>();
- QueryBuilder queryBuilder = new QueryBuilder();
- queryBuilder.addParameter("topic", topic);
- queryBuilder.addParameter("branch", downstreamBranch);
- queryBuilder.addParameter("status", "open");
- // get changes in same topic and check if their parent is upstreamRevision
- List<ChangeInfo> changes =
- gApi.changes()
- .query(queryBuilder.get())
- .withOptions(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT)
- .get();
+ List<ChangeInfo> changes = getChangesInTopic(topic, downstreamBranch);
for (ChangeInfo change : changes) {
String changeRevision = change.currentRevision;
@@ -396,11 +387,20 @@
* @param sdsMergeInput Input containing metadata for the merge.
* @throws RestApiException
* @throws ConfigInvalidException
+ * @throws InvalidQueryParameterException
*/
public void createSingleDownstreamMerge(SingleDownstreamMergeInput sdsMergeInput)
- throws RestApiException, ConfigInvalidException {
+ throws RestApiException, ConfigInvalidException, InvalidQueryParameterException {
String currentTopic = getOrSetTopic(sdsMergeInput.changeNumber, sdsMergeInput.topic);
+ if (isAlreadyMerged(sdsMergeInput, currentTopic)) {
+ log.info(
+ "Commit {} already merged into {}, not automerging again.",
+ sdsMergeInput.currentRevision,
+ sdsMergeInput.downstreamBranch);
+ return;
+ }
+
MergeInput mergeInput = new MergeInput();
mergeInput.source = sdsMergeInput.currentRevision;
mergeInput.strategy = "recursive";
@@ -573,4 +573,34 @@
abandonInput.message = "Merge parent updated; abandoning due to upstream conflict.";
gApi.changes().id(changeNumber).abandon(abandonInput);
}
+
+ private List<ChangeInfo> getChangesInTopic(String topic, String downstreamBranch)
+ throws InvalidQueryParameterException, RestApiException {
+ QueryBuilder queryBuilder = new QueryBuilder();
+ queryBuilder.addParameter("topic", topic);
+ queryBuilder.addParameter("branch", downstreamBranch);
+ queryBuilder.addParameter("status", "open");
+ return gApi.changes()
+ .query(queryBuilder.get())
+ .withOptions(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT)
+ .get();
+ }
+
+ private boolean isAlreadyMerged(SingleDownstreamMergeInput sdsMergeInput, String currentTopic)
+ throws InvalidQueryParameterException, RestApiException {
+ // If we've already merged this commit to this branch, don't do it again.
+ List<ChangeInfo> changes = getChangesInTopic(currentTopic, sdsMergeInput.downstreamBranch);
+ for (ChangeInfo change : changes) {
+ if (change.branch.equals(sdsMergeInput.downstreamBranch)) {
+ List<CommitInfo> parents = change.revisions.get(change.currentRevision).commit.parents;
+ if (parents.size() > 1) {
+ String secondParent = parents.get(1).commit;
+ if (secondParent.equals(sdsMergeInput.currentRevision)) {
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
index 69893c5..1368a3f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
@@ -32,13 +32,16 @@
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.testutil.TestTimeUtil;
import java.io.InputStream;
import java.io.InputStreamReader;
+import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.EnumSet;
import java.util.List;
+import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
@@ -91,6 +94,136 @@
}
@Test
+ public void testDiamondMerge() throws Exception {
+ Project.NameKey manifestNameKey = defaultSetup();
+ // Create initial change
+ PushOneCommit.Result initialResult = createChange("subject", "filename", "echo Hello");
+ // Project name is scoped by test, so we need to get it from our initial change
+ String projectName = initialResult.getChange().project().get();
+ createBranch(new Branch.NameKey(projectName, "left"));
+ createBranch(new Branch.NameKey(projectName, "right"));
+ initialResult.assertOkStatus();
+ merge(initialResult);
+ // Reset to create a sibling
+ ObjectId initial = repo().exactRef("HEAD").getLeaf().getObjectId();
+ testRepo.reset(initial);
+ // Make left != right
+ PushOneCommit.Result left =
+ createChange(
+ testRepo, "left", "subject", "filename", "echo \"Hello asdfsd World\"", "randtopic");
+ left.assertOkStatus();
+ merge(left);
+
+ String leftRevision = gApi.projects().name(projectName).branch("left").get().revision;
+ String rightRevision = gApi.projects().name(projectName).branch("right").get().revision;
+ // For this test, right != left
+ assertThat(leftRevision).isNotEqualTo(rightRevision);
+ createBranch(new Branch.NameKey(projectName, "bottom"));
+ pushDiamondConfig(manifestNameKey.get(), projectName);
+ // After we upload our config, we upload a new patchset to create the downstreams
+ PushOneCommit.Result result = createChange("subject", "filename2", "echo Hello", "sometopic");
+ result.assertOkStatus();
+ // Check that there are the correct number of changes in the topic
+ List<ChangeInfo> changesInTopic =
+ gApi.changes().query("topic: " + gApi.changes().id(result.getChangeId()).topic()).get();
+ assertThat(changesInTopic).hasSize(5);
+ // +2 and submit
+ merge(result);
+ List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic);
+
+ // Should create two changes on bottom, since left and right are different.
+ ChangeInfo bottomChangeInfoA = sortedChanges.get(0);
+ assertThat(bottomChangeInfoA.branch).isEqualTo("bottom");
+ ChangeApi bottomChangeA = gApi.changes().id(bottomChangeInfoA._number);
+ assertThat(getVote(bottomChangeA, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(bottomChangeA, "Code-Review").tag).isEqualTo("autogenerated:Automerger");
+
+ ChangeInfo bottomChangeInfoB = sortedChanges.get(1);
+ assertThat(bottomChangeInfoB.branch).isEqualTo("bottom");
+ ChangeApi bottomChangeB = gApi.changes().id(bottomChangeInfoB._number);
+ assertThat(getVote(bottomChangeB, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(bottomChangeB, "Code-Review").tag).isEqualTo("autogenerated:Automerger");
+
+ ChangeInfo leftChangeInfo = sortedChanges.get(2);
+ assertThat(leftChangeInfo.branch).isEqualTo("left");
+ ChangeApi leftChange = gApi.changes().id(leftChangeInfo._number);
+ assertThat(getVote(leftChange, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(leftChange, "Code-Review").tag).isEqualTo("autogenerated:Automerger");
+
+ ChangeInfo masterChangeInfo = sortedChanges.get(3);
+ assertThat(masterChangeInfo.branch).isEqualTo("master");
+ ChangeApi masterChange = gApi.changes().id(masterChangeInfo._number);
+ assertThat(getVote(masterChange, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(masterChange, "Code-Review").tag).isEqualTo(null);
+
+ ChangeInfo rightChangeInfo = sortedChanges.get(4);
+ assertThat(rightChangeInfo.branch).isEqualTo("right");
+ ChangeApi rightChange = gApi.changes().id(rightChangeInfo._number);
+ assertThat(getVote(rightChange, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(rightChange, "Code-Review").tag).isEqualTo("autogenerated:Automerger");
+ }
+
+ @Test
+ public void testDiamondMerge_identicalDiamondSides() throws Exception {
+ Project.NameKey manifestNameKey = defaultSetup();
+ // Create initial change
+ PushOneCommit.Result initialResult = createChange("subject", "filename", "echo Hello");
+ // Project name is scoped by test, so we need to get it from our initial change
+ String projectName = initialResult.getChange().project().get();
+ createBranch(new Branch.NameKey(projectName, "left"));
+ createBranch(new Branch.NameKey(projectName, "right"));
+ initialResult.assertOkStatus();
+ merge(initialResult);
+
+ String leftRevision = gApi.projects().name(projectName).branch("left").get().revision;
+ String rightRevision = gApi.projects().name(projectName).branch("right").get().revision;
+ // For this test, right == left
+ assertThat(leftRevision).isEqualTo(rightRevision);
+ createBranch(new Branch.NameKey(projectName, "bottom"));
+ pushDiamondConfig(manifestNameKey.get(), projectName);
+
+ // Freeze time so that the merge commit from left->bottom and right->bottom have same SHA
+ TestTimeUtil.resetWithClockStep(0, TimeUnit.MILLISECONDS);
+ TestTimeUtil.setClock(new Timestamp(TestTimeUtil.START.getMillis()));
+ // After we upload our config, we upload a new patchset to create the downstreams.
+ PushOneCommit.Result result = createChange("subject", "filename2", "echo Hello", "sometopic");
+ TestTimeUtil.useSystemTime();
+ result.assertOkStatus();
+
+ List<ChangeInfo> changesInTopic =
+ gApi.changes().query("topic: " + gApi.changes().id(result.getChangeId()).topic()).get();
+ assertThat(changesInTopic).hasSize(4);
+ // +2 and submit
+ merge(result);
+ List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic);
+
+ // Should create two changes on bottom, since left and right are different.
+ ChangeInfo bottomChangeInfo = sortedChanges.get(0);
+ assertThat(bottomChangeInfo.branch).isEqualTo("bottom");
+ ChangeApi bottomChange = gApi.changes().id(bottomChangeInfo._number);
+ assertThat(getVote(bottomChange, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(bottomChange, "Code-Review").tag).isEqualTo("autogenerated:Automerger");
+
+ ChangeInfo leftChangeInfo = sortedChanges.get(1);
+ assertThat(leftChangeInfo.branch).isEqualTo("left");
+ ChangeApi leftChange = gApi.changes().id(leftChangeInfo._number);
+ assertThat(getVote(leftChange, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(leftChange, "Code-Review").tag).isEqualTo("autogenerated:Automerger");
+
+ ChangeInfo masterChangeInfo = sortedChanges.get(2);
+ assertThat(masterChangeInfo.branch).isEqualTo("master");
+ ChangeApi masterChange = gApi.changes().id(masterChangeInfo._number);
+ assertThat(getVote(masterChange, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(masterChange, "Code-Review").tag).isEqualTo(null);
+
+ ChangeInfo rightChangeInfo = sortedChanges.get(3);
+ assertThat(rightChangeInfo.branch).isEqualTo("right");
+ ChangeApi rightChange = gApi.changes().id(rightChangeInfo._number);
+ assertThat(getVote(rightChange, "Code-Review").value).isEqualTo(2);
+ assertThat(getVote(rightChange, "Code-Review").tag).isEqualTo("autogenerated:Automerger");
+ }
+
+ @Test
public void testBlankMerge() throws Exception {
Project.NameKey manifestNameKey = defaultSetup();
// Create initial change
@@ -439,6 +572,28 @@
}
}
+ private void pushDiamondConfig(String manifestName, String project) throws Exception {
+ TestRepository<InMemoryRepository> allProjectRepo = cloneProject(allProjects, admin);
+ GitUtil.fetch(allProjectRepo, RefNames.REFS_CONFIG + ":config");
+ allProjectRepo.reset("config");
+ try (InputStream in = getClass().getResourceAsStream("diamond.config")) {
+ String resourceString = CharStreams.toString(new InputStreamReader(in, Charsets.UTF_8));
+ Config cfg = new Config();
+ cfg.fromText(resourceString);
+ // Update manifest project path to the result of createProject(resourceName), since it is
+ // scoped to the test method
+ cfg.setString("global", null, "manifestProject", manifestName);
+ cfg.setString("automerger", "master:left", "setProjects", project);
+ cfg.setString("automerger", "master:right", "setProjects", project);
+ cfg.setString("automerger", "left:bottom", "setProjects", project);
+ cfg.setString("automerger", "right:bottom", "setProjects", project);
+ PushOneCommit push =
+ pushFactory.create(
+ db, admin.getIdent(), allProjectRepo, "Subject", "automerger.config", cfg.toText());
+ push.to(RefNames.REFS_CONFIG).assertOkStatus();
+ }
+ }
+
private ApprovalInfo getVote(ChangeApi change, String label) throws RestApiException {
return change.get(EnumSet.of(ListChangesOption.DETAILED_LABELS)).labels.get(label).all.get(0);
}
diff --git a/src/test/resources/com/googlesource/gerrit/plugins/automerger/diamond.config b/src/test/resources/com/googlesource/gerrit/plugins/automerger/diamond.config
new file mode 100644
index 0000000..901c731
--- /dev/null
+++ b/src/test/resources/com/googlesource/gerrit/plugins/automerger/diamond.config
@@ -0,0 +1,11 @@
+[automerger "master:left"]
+ setProjects = platform/some/project
+[automerger "master:right"]
+ setProjects = platform/some/project
+[automerger "left:bottom"]
+ setProjects = platform/some/project
+[automerger "right:bottom"]
+ setProjects = platform/some/project
+[global]
+ manifestFile = default.xml
+ manifestProject = platform/manifest
\ No newline at end of file
