opendev/gerrit
Code Issues Proposed changes

MergeSuperSet: Fix transitive expansion of topics

Browse Source 
Expanding topics on ancestor changes (when submit whole topic is
enabled) was broken by change I8ea6fc18a7. By first expanding the topic
and then checking for predecessor change we missed topics on the
predecessor changes and aborted the loop too early.

It was unfortunate that this wasn't caught by the tests since this case
was actually tested by SubmittedTogetherIT#topicChaining. However in
this test all changes have topics and the existence of the
unrelated-topic on the third change made us not hit this issue.

To fix this problem this change basically reverts change I8ea6fc18a7,
but adds another topic expansion right before we start with the loop.
This way we do one topic expansion before we start to lookup predecessor
changes, which was the intention of change I8ea6fc18a7.

To make sure that topic expansion works properly add a new test
(respectTopicsOnAncestors) that requires at least 2 iterations of topic
expansion.

Change-Id: Ic034eb5ac0cee3bf54e9741aa56f0b8ac8e83b4f
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-10-18 08:46:56 +02:00
parent 98f2664177
commit 985725ad9f
2 changed files with 51 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
View File

@@ -177,6 +177,53 @@ public class SubmittedTogetherIT extends AbstractDaemonTest {
}
}
@Test
public void respectTopicsOnAncestors() throws Exception {
RevCommit initialHead = getRemoteHead();
RevCommit c1_1 = commitBuilder().add("a.txt", "1").message("subject: 1").create();
String id1 = getChangeId(c1_1);
pushHead(testRepo, "refs/for/master/" + name("connectingTopic"), false);
testRepo.reset(initialHead);
RevCommit c2_1 = commitBuilder().add("b.txt", "2").message("subject: 2").create();
String id2 = getChangeId(c2_1);
pushHead(testRepo, "refs/for/master/" + name("otherConnectingTopic"), false);
RevCommit c3_1 = commitBuilder().add("b.txt", "3").message("subject: 3").create();
String id3 = getChangeId(c3_1);
pushHead(testRepo, "refs/for/master/" + name("connectingTopic"), false);
RevCommit c4_1 = commitBuilder().add("b.txt", "4").message("subject: 4").create();
String id4 = getChangeId(c4_1);
pushHead(testRepo, "refs/for/master", false);
testRepo.reset(initialHead);
RevCommit c5_1 = commitBuilder().add("c.txt", "5").message("subject: 5").create();
String id5 = getChangeId(c5_1);
pushHead(testRepo, "refs/for/master", false);
RevCommit c6_1 = commitBuilder().add("c.txt", "6").message("subject: 6").create();
String id6 = getChangeId(c6_1);
pushHead(testRepo, "refs/for/master/" + name("otherConnectingTopic"), false);
if (isSubmitWholeTopicEnabled()) {
assertSubmittedTogether(id1, id6, id5, id3, id2, id1);
assertSubmittedTogether(id2, id6, id5, id2);
assertSubmittedTogether(id3, id6, id5, id3, id2, id1);
assertSubmittedTogether(id4, id6, id5, id4, id3, id2, id1);
assertSubmittedTogether(id5);
assertSubmittedTogether(id6, id6, id5, id2);
} else {
assertSubmittedTogether(id1);
assertSubmittedTogether(id2);
assertSubmittedTogether(id3, id3, id2);
assertSubmittedTogether(id4, id4, id3, id2);
assertSubmittedTogether(id5);
assertSubmittedTogether(id6, id6, id5);
}
}
@Test
public void newBranchTwoChangesTogether() throws Exception {
Project.NameKey p1 = createProject("a-new-project", null, false);

7
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
View File

@@ -194,12 +194,13 @@ public class MergeSuperSet {
int oldSeen;
int seen = 0;
changeSet = topicClosure(db, changeSet, user, topicsSeen, visibleTopicsSeen);
seen = topicsSeen.size() + visibleTopicsSeen.size();
do {
oldSeen = seen;
changeSet = topicClosure(db, changeSet, user, topicsSeen, visibleTopicsSeen);
changeSet = mergeSuperSetComputation.get().completeWithoutTopic(db, orm, changeSet, user);
changeSet = topicClosure(db, changeSet, user, topicsSeen, visibleTopicsSeen);
seen = topicsSeen.size() + visibleTopicsSeen.size();
} while (seen != oldSeen);
return changeSet;