Preserve author of submodule commits for super repo commit
If a super-repo is subscribed to 2 submodules and both submodules get
updated together by submitting a topic, a single commit in the
super-repo is created that updates both submodules at once. If the
commits in the submodules had the same author it was intended to
preserve this author for the super-repo commit. However when the authors
of the submodule commits were compared the author timestamp was taken
into account and as result the authors were (almost) never equal. Due to
this we were falling back to using the Gerrit server identity as author
for the commit in the super-repo. This is now fixed by ignoring the
author timestamp when comparing the authors.
Change-Id: I3130d29cb50abaf82e808523519d8743227e09d5
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index 7984c76e..7be1307 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -423,7 +423,8 @@
if (newCommit != null) {
if (author == null) {
author = newCommit.getAuthorIdent();
- } else if (!author.equals(newCommit.getAuthorIdent())) {
+ } else if (!author.getName().equals(newCommit.getAuthorIdent().getName())
+ || !author.getEmailAddress().equals(newCommit.getAuthorIdent().getEmailAddress())) {
author = myIdent;
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
index 62138ca..ce73875 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
@@ -32,6 +32,7 @@
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
@@ -436,4 +437,22 @@
RevCommit c = rw.parseCommit(commitId);
assertThat(c.getFullMessage()).isEqualTo(expectedMessage);
}
+
+ protected void expectToHaveAuthor(
+ TestRepository<?> repo, String branch, String expectedAuthorName, String expectedAuthorEmail)
+ throws Exception {
+ ObjectId commitId =
+ repo.git()
+ .fetch()
+ .setRemote("origin")
+ .call()
+ .getAdvertisedRef("refs/heads/" + branch)
+ .getObjectId();
+
+ RevWalk rw = repo.getRevWalk();
+ RevCommit c = rw.parseCommit(commitId);
+ PersonIdent authorIdent = c.getAuthorIdent();
+ assertThat(authorIdent.getName()).isEqualTo(expectedAuthorName);
+ assertThat(authorIdent.getEmailAddress()).isEqualTo(expectedAuthorEmail);
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
index 81ee3a0..4381ed137 100644
--- a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
@@ -20,6 +20,7 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.testing.ConfigSuite;
import org.eclipse.jgit.junit.TestRepository;
@@ -520,6 +521,87 @@
testSubmoduleSubjectCommitMessageAndExpectTruncation();
}
+ @Test
+ public void superRepoCommitHasSameAuthorAsSubmoduleCommits() throws Exception {
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+
+ TestRepository<?> superRepo = createProjectWithPush("super-project");
+ TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+ TestRepository<?> subRepo2 = createProjectWithPush("subscribed-to-project-2");
+
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+
+ Config config = new Config();
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ pushSubmoduleConfig(superRepo, "master", config);
+
+ String topic = "foo";
+
+ subRepo.tick(1); // Make sure that both changes have different timestamps.
+ String changeId1 =
+ createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic)
+ .getChangeId();
+ approve(changeId1);
+
+ subRepo2.tick(2); // Make sure that both changes have different timestamps.
+ String changeId2 =
+ createChange(subRepo2, "refs/heads/master", "Change 2", "b.txt", "other content", topic)
+ .getChangeId();
+ approve(changeId2);
+
+ // Submit the topic, 2 changes with the same author.
+ gApi.changes().id(changeId1).current().submit();
+
+ // Expect that the author is preserved for the superRepo commit.
+ expectToHaveAuthor(superRepo, "master", admin.fullName, admin.email);
+ }
+
+ @Test
+ public void superRepoCommitHasGerritAsAuthorIfAuthorsOfSubmoduleCommitsDiffer() throws Exception {
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+
+ TestRepository<?> superRepo = createProjectWithPush("super-project");
+ TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+
+ TestRepository<?> subRepo2 = createProjectWithPush("subscribed-to-project-2");
+ subRepo2 = cloneProject(new Project.NameKey(name("subscribed-to-project-2")), user);
+
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+
+ Config config = new Config();
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ pushSubmoduleConfig(superRepo, "master", config);
+
+ String topic = "foo";
+
+ // Create change as admin.
+ String changeId1 =
+ createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic)
+ .getChangeId();
+ approve(changeId1);
+
+ // Create change as user.
+ PushOneCommit push =
+ pushFactory.create(db, user.getIdent(), subRepo2, "Change 2", "b.txt", "other content");
+ String changeId2 = push.to("refs/for/master/" + name(topic)).getChangeId();
+ approve(changeId2);
+
+ // Submit the topic, 2 changes with the different author.
+ gApi.changes().id(changeId1).current().submit();
+
+ // Expect that the Gerrit server identity is chosen as author for the superRepo commit.
+ expectToHaveAuthor(
+ superRepo, "master", serverIdent.get().getName(), serverIdent.get().getEmailAddress());
+ }
+
private void testSubmoduleSubjectCommitMessageAndExpectTruncation() throws Exception {
TestRepository<?> superRepo = createProjectWithPush("super-project");
TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
