SubmoduleOp: Fix merging multiple changes to superprojects
Under certain conditions the SubmoduleOp did not behave correctly. Instead
of merging the changes from a submodule to a superproject, there was a
null pointer exception instead when parsing the commit messages of the
changes.
This is fixing the issue as well as having a regression test to prevent
such behavior in the future.
A new file in the acceptance test suite is introduced, because we need to
test with a non default configuration, and we don't want to change the
SubmoduleSubscriptionsIT.java to have the non default configuration on
top as that would run all the test twice with and without the new config.
Bug: issue 3197
Change-Id: I168cb2feeaba072987a9376352d289f6acccc08a
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK
index 1ad9c8c..446a183 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK
@@ -5,6 +5,7 @@
'DraftChangeBlockedIT.java',
'ForcePushIT.java',
'SubmitOnPushIT.java',
+ 'SubmoduleSubscriptionsWholeTopicMergeIT.java',
'SubmoduleSubscriptionsIT.java',
'VisibleRefFilterIT.java',
],
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java
new file mode 100644
index 0000000..85037f1
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java
@@ -0,0 +1,94 @@
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.git;
+
+import static com.google.gerrit.acceptance.GitUtil.getChangeId;
+
+import com.google.gerrit.acceptance.git.AbstractSubmoduleSubscription;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.testutil.ConfigSuite;
+
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.transport.RefSpec;
+import org.junit.Test;
+
+@NoHttpd
+public class SubmoduleSubscriptionsWholeTopicMergeIT
+ extends AbstractSubmoduleSubscription {
+
+ @ConfigSuite.Default
+ public static Config submitWholeTopicEnabled() {
+ return submitWholeTopicEnabledConfig();
+ }
+
+ @Test
+ public void testSubscriptionUpdateOfManyChanges() throws Exception {
+ TestRepository<?> superRepo = createProjectWithPush("super-project");
+ TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+ createSubscription(superRepo, "master", "subscribed-to-project", "master");
+
+ ObjectId subHEAD = subRepo.branch("HEAD").commit().insertChangeId()
+ .message("some change")
+ .add("a.txt", "a contents ")
+ .create();
+ subRepo.git().push().setRemote("origin").setRefSpecs(
+ new RefSpec("HEAD:refs/heads/master")).call();
+
+ RevCommit c = subRepo.getRevWalk().parseCommit(subHEAD);
+
+ RevCommit c1 = subRepo.branch("HEAD").commit().insertChangeId()
+ .message("first change")
+ .add("asdf", "asdf\n")
+ .parent(c)
+ .create();
+ subRepo.git().push().setRemote("origin")
+ .setRefSpecs(new RefSpec("HEAD:refs/for/master/" + name("topic-foo")))
+ .call();
+
+ subRepo.reset(c.getId());
+ RevCommit c2 = subRepo.branch("HEAD").commit().insertChangeId()
+ .message("qwerty")
+ .add("qwerty", "qwerty")
+ .parent(c)
+ .create();
+
+ RevCommit c3 = subRepo.branch("HEAD").commit().insertChangeId()
+ .message("qwerty followup")
+ .add("qwerty", "qwerty\nqwerty\n")
+ .parent(c2)
+ .create();
+ subRepo.git().push().setRemote("origin")
+ .setRefSpecs(new RefSpec("HEAD:refs/for/master/" + name("topic-foo")))
+ .call();
+
+ String id1 = getChangeId(subRepo, c1).get();
+ String id2 = getChangeId(subRepo, c2).get();
+ String id3 = getChangeId(subRepo, c3).get();
+ gApi.changes().id(id1).current().review(ReviewInput.approve());
+ gApi.changes().id(id2).current().review(ReviewInput.approve());
+ gApi.changes().id(id3).current().review(ReviewInput.approve());
+
+ gApi.changes().id(id1).current().submit();
+ ObjectId subRepoId = subRepo.git().fetch().setRemote("origin").call()
+ .getAdvertisedRef("refs/heads/master").getObjectId();
+
+ expectToHaveSubmoduleState(superRepo, "master",
+ "subscribed-to-project", subRepoId);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
index f05a1d4..773d665 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
@@ -137,7 +137,7 @@
updateSubmoduleSubscriptions();
updateSuperProjects(destBranch, rw, mergeTip.getId().toObjectId(), null);
- } catch (OrmException e) {
+ } catch (OrmException | IOException e) {
throw new SubmoduleException("Cannot open database", e);
} finally {
if (schema != null) {
@@ -210,7 +210,8 @@
}
private void updateSuperProjects(final Branch.NameKey updatedBranch, RevWalk myRw,
- final ObjectId mergedCommit, final String msg) throws SubmoduleException {
+ final ObjectId mergedCommit, final String msg) throws SubmoduleException,
+ IOException {
try {
final List<SubmoduleSubscription> subscribers =
schema.submoduleSubscriptions().bySubmodule(updatedBranch).toList();
@@ -236,6 +237,7 @@
&& (c.getStatusCode() == CommitMergeStatus.CLEAN_MERGE
|| c.getStatusCode() == CommitMergeStatus.CLEAN_PICK
|| c.getStatusCode() == CommitMergeStatus.CLEAN_REBASE)) {
+ myRw.parseBody(c);
sb.append("\n")
.append(c.getFullMessage());
}
