Stacked changes continue being stacked downstream.
Previously, if change A and B were uplaoded, where Bs first parent was
A, then the first parents of both A and B were HEAD of the downstream
branch.
Now, if changes A and B are uploaded, where Bs first parent is A, then for
downstream A' and B', the first parent of B' should be A'.
Change-Id: I09a9abc6320129f64958859efc2e379214903e54
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 b12a0f3..2045762 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
@@ -365,7 +365,7 @@
String upstreamRevision, String topic, String downstreamBranch)
throws RestApiException, InvalidQueryParameterException {
List<Integer> downstreamChangeNumbers = new ArrayList<Integer>();
- List<ChangeInfo> changes = getChangesInTopic(topic, downstreamBranch);
+ List<ChangeInfo> changes = getChangesInTopicAndBranch(topic, downstreamBranch);
for (ChangeInfo change : changes) {
String changeRevision = change.currentRevision;
@@ -415,6 +415,11 @@
downstreamChangeInput.merge = mergeInput;
downstreamChangeInput.notify = NotifyHandling.NONE;
+ downstreamChangeInput.baseChange =
+ getBaseChangeId(
+ getChangeParents(sdsMergeInput.changeNumber, sdsMergeInput.currentRevision),
+ sdsMergeInput.downstreamBranch);
+
if (!sdsMergeInput.doMerge) {
mergeInput.strategy = "ours";
downstreamChangeInput.subject =
@@ -442,6 +447,42 @@
return topic;
}
+ /**
+ * Get the base change ID that the downstream change should be based off of, given the parents.
+ *
+ * <p>Given changes A and B where A is the first parent of B, and where A' is the change whose
+ * second parent is A, and B' is the change whose second parent is B, the first parent of B'
+ * should be A'.
+ *
+ * @param parents Parent commit SHAs of the change
+ * @return The base change ID that the change should be based off of, null if there is none.
+ * @throws InvalidQueryParameterException
+ * @throws RestApiException
+ */
+ private String getBaseChangeId(List<String> parents, String branch)
+ throws InvalidQueryParameterException, RestApiException {
+ if (parents.size() == 0) {
+ log.info("No base change id for change with no parents.");
+ return null;
+ }
+ // 1) Get topic of first parent
+ String firstParentTopic = getTopic(parents.get(0));
+ if (firstParentTopic == null) {
+ return null;
+ }
+ // 2) query that topic and use that to find A'
+ List<ChangeInfo> changesInTopic = getChangesInTopicAndBranch(firstParentTopic, branch);
+ String firstParent = parents.get(0);
+ for (ChangeInfo change : changesInTopic) {
+ List<CommitInfo> topicChangeParents =
+ change.revisions.get(change.currentRevision).commit.parents;
+ if (topicChangeParents.size() > 1 && topicChangeParents.get(1).commit.equals(firstParent)) {
+ return String.valueOf(change._number);
+ }
+ }
+ return null;
+ }
+
private void automergeChanges(ChangeInfo change, RevisionInfo revisionInfo)
throws RestApiException, IOException, ConfigInvalidException, InvalidQueryParameterException {
if (revisionInfo.draft != null && revisionInfo.draft) {
@@ -476,7 +517,7 @@
mdsMergeInput.changeNumber = change._number;
mdsMergeInput.patchsetNumber = revisionInfo._number;
mdsMergeInput.project = change.project;
- mdsMergeInput.topic = change.topic;
+ mdsMergeInput.topic = getOrSetTopic(change._number, change.topic);
mdsMergeInput.subject = change.subject;
mdsMergeInput.obsoleteRevision = previousRevision;
mdsMergeInput.currentRevision = currentRevision;
@@ -566,6 +607,20 @@
return previousRevision;
}
+ private List<String> getChangeParents(int changeNumber, String currentRevision)
+ throws RestApiException {
+ ChangeApi change = gApi.changes().id(changeNumber);
+ List<String> parents = new ArrayList<>();
+ Map<String, RevisionInfo> revisionMap =
+ change.get(EnumSet.of(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT))
+ .revisions;
+ List<CommitInfo> changeParents = revisionMap.get(currentRevision).commit.parents;
+ for (CommitInfo commit : changeParents) {
+ parents.add(commit.commit);
+ }
+ return parents;
+ }
+
private void abandonChange(Integer changeNumber) throws RestApiException {
log.debug("Abandoning change: {}", changeNumber);
AbandonInput abandonInput = new AbandonInput();
@@ -574,7 +629,25 @@
gApi.changes().id(changeNumber).abandon(abandonInput);
}
- private List<ChangeInfo> getChangesInTopic(String topic, String downstreamBranch)
+ private String getTopic(String revision) throws InvalidQueryParameterException, RestApiException {
+ QueryBuilder queryBuilder = new QueryBuilder();
+ queryBuilder.addParameter("commit", revision);
+ List<ChangeInfo> changes =
+ gApi.changes()
+ .query(queryBuilder.get())
+ .withOption(ListChangesOption.CURRENT_REVISION)
+ .get();
+ if (changes.size() > 0) {
+ for (ChangeInfo change : changes) {
+ if (change.currentRevision.equals(revision) && change.topic != "") {
+ return change.topic;
+ }
+ }
+ }
+ return null;
+ }
+
+ private List<ChangeInfo> getChangesInTopicAndBranch(String topic, String downstreamBranch)
throws InvalidQueryParameterException, RestApiException {
QueryBuilder queryBuilder = new QueryBuilder();
queryBuilder.addParameter("topic", topic);
@@ -589,7 +662,8 @@
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);
+ List<ChangeInfo> changes =
+ getChangesInTopicAndBranch(currentTopic, sdsMergeInput.downstreamBranch);
for (ChangeInfo change : changes) {
if (change.branch.equals(sdsMergeInput.downstreamBranch)) {
List<CommitInfo> parents = change.revisions.get(change.currentRevision).commit.parents;
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 1368a3f..38f8d08 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
@@ -224,6 +224,81 @@
}
@Test
+ public void testChangeStack() throws Exception {
+ Project.NameKey manifestNameKey = defaultSetup();
+ // Create initial change
+ PushOneCommit.Result result = createChange("subject", "filename", "content", "testtopic");
+ // Project name is scoped by test, so we need to get it from our initial change
+ String projectName = result.getChange().project().get();
+ createBranch(new Branch.NameKey(projectName, "ds_one"));
+ createBranch(new Branch.NameKey(projectName, "ds_two"));
+ pushConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two");
+ // After we upload our config, we upload a new patchset to create the downstreams
+ amendChange(result.getChangeId());
+ result.assertOkStatus();
+ PushOneCommit.Result result2 = createChange("subject2", "filename2", "content2", "testtopic");
+ result2.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())
+ .withOptions(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT)
+ .get();
+ assertThat(changesInTopic).hasSize(6);
+ List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic);
+
+ // Change A
+ ChangeInfo dsOneChangeInfo = sortedChanges.get(0);
+ assertThat(dsOneChangeInfo.branch).isEqualTo("ds_one");
+ // Change B
+ ChangeInfo dsOneChangeInfo2 = sortedChanges.get(1);
+ assertThat(dsOneChangeInfo2.branch).isEqualTo("ds_one");
+ String dsOneChangeInfo2FirstParentSha =
+ dsOneChangeInfo2
+ .revisions
+ .get(dsOneChangeInfo2.currentRevision)
+ .commit
+ .parents
+ .get(0)
+ .commit;
+ assertThat(dsOneChangeInfo.currentRevision).isEqualTo(dsOneChangeInfo2FirstParentSha);
+
+ // Change A'
+ ChangeInfo dsTwoChangeInfo = sortedChanges.get(2);
+ assertThat(dsTwoChangeInfo.branch).isEqualTo("ds_two");
+ // Change B'
+ ChangeInfo dsTwoChangeInfo2 = sortedChanges.get(3);
+ assertThat(dsTwoChangeInfo2.branch).isEqualTo("ds_two");
+ String dsTwoChangeInfo2FirstParentSha =
+ dsTwoChangeInfo2
+ .revisions
+ .get(dsTwoChangeInfo2.currentRevision)
+ .commit
+ .parents
+ .get(0)
+ .commit;
+ // Check that first parent of B' is A'
+ assertThat(dsTwoChangeInfo.currentRevision).isEqualTo(dsTwoChangeInfo2FirstParentSha);
+
+ // Change A''
+ ChangeInfo masterChangeInfo = sortedChanges.get(4);
+ assertThat(masterChangeInfo.branch).isEqualTo("master");
+ // Change B''
+ ChangeInfo masterChangeInfo2 = sortedChanges.get(5);
+ assertThat(masterChangeInfo2.branch).isEqualTo("master");
+ String masterChangeInfo2FirstParentSha =
+ masterChangeInfo2
+ .revisions
+ .get(masterChangeInfo2.currentRevision)
+ .commit
+ .parents
+ .get(0)
+ .commit;
+ // Check that first parent of B'' is A''
+ assertThat(masterChangeInfo.currentRevision).isEqualTo(masterChangeInfo2FirstParentSha);
+ }
+
+ @Test
public void testBlankMerge() throws Exception {
Project.NameKey manifestNameKey = defaultSetup();
// Create initial change
@@ -605,7 +680,11 @@
new Comparator<ChangeInfo>() {
@Override
public int compare(ChangeInfo c1, ChangeInfo c2) {
- return c1.branch.compareTo(c2.branch);
+ int compareResult = c1.branch.compareTo(c2.branch);
+ if (compareResult == 0) {
+ return Integer.compare(c1._number, c2._number);
+ }
+ return compareResult;
}
});
return listCopy;
