Merge "Add refs/for/<branch>%submit to auto-merge during push"
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 5cc053f..e38df75 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -652,6 +652,10 @@
above) must enable submit, and also must not block it. See above for
details on each label.
+To link:user-upload.html#auto_merge[immediately submit a change on push]
+the caller needs to have the Submit permission on `refs/for/<ref>`
+(e.g. on `refs/for/refs/heads/master`).
+
[[category_view_drafts]]
View Drafts
diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt
index 55ab895..83ce6ce 100644
--- a/Documentation/user-upload.txt
+++ b/Documentation/user-upload.txt
@@ -333,6 +333,26 @@
make undesired changes to the public repository.
+[[auto_merge]]
+Auto-Merge during Push
+~~~~~~~~~~~~~~~~~~~~~~
+
+Changes can be directly submitted on push. This is primarily useful
+for teams that don't want to do code review but want to use Gerrit's
+submit strategies to handle contention on busy branches. Using
+`%submit` creates a change and submits it immediately, if the caller
+has link:access-control.html#category_submit[Submit] permission on
+`refs/for/<ref>` (e.g. on `refs/for/refs/heads/master`).
+
+====
+ git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%submit
+====
+
+On auto-merge of a change neither labels nor submit rules are checked.
+If the merge fails the change stays open, but when pushing a new patch
+set the merge can be reattempted by using `%submit` again.
+
+
repo upload
-----------
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitUtil.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitUtil.java
index 9faf32a..c8158c07 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitUtil.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitUtil.java
@@ -107,11 +107,17 @@
public static String createCommit(Git git, PersonIdent i, String msg)
throws GitAPIException, IOException {
- return createCommit(git, i, msg, true);
+ return createCommit(git, i, msg, true, false);
}
- public static String createCommit(Git git, PersonIdent i, String msg,
- boolean insertChangeId) throws GitAPIException, IOException {
+ public static void amendCommit(Git git, PersonIdent i, String msg, String changeId)
+ throws GitAPIException, IOException {
+ msg = ChangeIdUtil.insertId(msg, ObjectId.fromString(changeId.substring(1)));
+ createCommit(git, i, msg, false, true);
+ }
+
+ private static String createCommit(Git git, PersonIdent i, String msg,
+ boolean insertChangeId, boolean amend) throws GitAPIException, IOException {
ObjectId changeId = null;
if (insertChangeId) {
changeId = computeChangeId(git, i, msg);
@@ -119,6 +125,7 @@
}
final CommitCommand commitCmd = git.commit();
+ commitCmd.setAmend(amend);
commitCmd.setAuthor(i);
commitCmd.setCommitter(i);
commitCmd.setMessage(msg);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/PushForReviewIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/PushForReviewIT.java
index f905350..7af4c55 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/PushForReviewIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/PushForReviewIT.java
@@ -14,27 +14,16 @@
package com.google.gerrit.acceptance.git;
-import static com.google.gerrit.acceptance.git.GitUtil.add;
import static com.google.gerrit.acceptance.git.GitUtil.cloneProject;
-import static com.google.gerrit.acceptance.git.GitUtil.createCommit;
import static com.google.gerrit.acceptance.git.GitUtil.createProject;
import static com.google.gerrit.acceptance.git.GitUtil.initSsh;
-import static com.google.gerrit.acceptance.git.GitUtil.pushHead;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import com.google.common.base.Function;
-import com.google.common.base.Strings;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AccountCreator;
import com.google.gerrit.acceptance.SshSession;
import com.google.gerrit.acceptance.TestAccount;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmException;
@@ -45,9 +34,6 @@
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
-import org.eclipse.jgit.transport.PushResult;
-import org.eclipse.jgit.transport.RemoteRefUpdate;
-import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -56,9 +42,7 @@
import org.junit.runners.Parameterized.Parameters;
import java.io.IOException;
-import java.util.Arrays;
import java.util.List;
-import java.util.Set;
@RunWith(Parameterized.class)
public class PushForReviewIT extends AbstractDaemonTest {
@@ -127,30 +111,24 @@
@Test
public void testPushForMaster() throws GitAPIException, OrmException,
IOException {
- PushOneCommit push = new PushOneCommit();
- String ref = "refs/for/master";
- PushResult r = push.to(ref);
- assertOkStatus(r, ref);
- assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, null);
+ PushOneCommit.Result r = pushTo("refs/for/master");
+ r.assertOkStatus();
+ r.assertChange(Change.Status.NEW, null);
}
@Test
public void testPushForMasterWithTopic() throws GitAPIException,
OrmException, IOException {
// specify topic in ref
- PushOneCommit push = new PushOneCommit();
String topic = "my/topic";
- String ref = "refs/for/master/" + topic;
- PushResult r = push.to(ref);
- assertOkStatus(r, ref);
- assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, topic);
+ PushOneCommit.Result r = pushTo("refs/for/master/" + topic);
+ r.assertOkStatus();
+ r.assertChange(Change.Status.NEW, topic);
// specify topic as option
- push = new PushOneCommit();
- ref = "refs/for/master%topic=" + topic;
- r = push.to(ref);
- assertOkStatus(r, ref);
- assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, topic);
+ r = pushTo("refs/for/master%topic=" + topic);
+ r.assertOkStatus();
+ r.assertChange(Change.Status.NEW, topic);
}
@Test
@@ -158,30 +136,24 @@
IOException, JSchException {
// cc one user
TestAccount user = accounts.create("user", "user@example.com", "User");
- PushOneCommit push = new PushOneCommit();
String topic = "my/topic";
- String ref = "refs/for/master/" + topic + "%cc=" + user.email;
- PushResult r = push.to(ref);
- assertOkStatus(r, ref);
- assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, topic);
+ PushOneCommit.Result r = pushTo("refs/for/master/" + topic + "%cc=" + user.email);
+ r.assertOkStatus();
+ r.assertChange(Change.Status.NEW, topic);
// cc several users
TestAccount user2 =
accounts.create("another-user", "another.user@example.com", "Another User");
- push = new PushOneCommit();
- ref = "refs/for/master/" + topic + "%cc=" + admin.email + ",cc=" + user.email
- + ",cc=" + user2.email;
- r = push.to(ref);
- assertOkStatus(r, ref);
- assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, topic);
+ r = pushTo("refs/for/master/" + topic + "%cc=" + admin.email + ",cc="
+ + user.email + ",cc=" + user2.email);
+ r.assertOkStatus();
+ r.assertChange(Change.Status.NEW, topic);
// cc non-existing user
String nonExistingEmail = "non.existing@example.com";
- push = new PushOneCommit();
- ref = "refs/for/master/" + topic + "%cc=" + admin.email + ",cc="
- + nonExistingEmail + ",cc=" + user.email;
- r = push.to(ref);
- assertErrorStatus(r, "user \"" + nonExistingEmail + "\" not found", ref);
+ r = pushTo("refs/for/master/" + topic + "%cc=" + admin.email + ",cc="
+ + nonExistingEmail + ",cc=" + user.email);
+ r.assertErrorStatus("user \"" + nonExistingEmail + "\" not found");
}
@Test
@@ -189,121 +161,52 @@
OrmException, IOException, JSchException {
// add one reviewer
TestAccount user = accounts.create("user", "user@example.com", "User");
- PushOneCommit push = new PushOneCommit();
String topic = "my/topic";
- String ref = "refs/for/master/" + topic + "%r=" + user.email;
- PushResult r = push.to(ref);
- assertOkStatus(r, ref);
- assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT,
- topic, user);
+ PushOneCommit.Result r = pushTo("refs/for/master/" + topic + "%r=" + user.email);
+ r.assertOkStatus();
+ r.assertChange(Change.Status.NEW, topic, user);
// add several reviewers
TestAccount user2 =
accounts.create("another-user", "another.user@example.com", "Another User");
- push = new PushOneCommit();
- ref = "refs/for/master/" + topic + "%r=" + admin.email + ",r=" + user.email
- + ",r=" + user2.email;
- r = push.to(ref);
- assertOkStatus(r, ref);
+ r = pushTo("refs/for/master/" + topic + "%r=" + admin.email + ",r=" + user.email
+ + ",r=" + user2.email);
+ r.assertOkStatus();
// admin is the owner of the change and should not appear as reviewer
- assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT,
- topic, user, user2);
+ r.assertChange(Change.Status.NEW, topic, user, user2);
// add non-existing user as reviewer
String nonExistingEmail = "non.existing@example.com";
- push = new PushOneCommit();
- ref = "refs/for/master/" + topic + "%r=" + admin.email + ",r="
- + nonExistingEmail + ",r=" + user.email;
- r = push.to(ref);
- assertErrorStatus(r, "user \"" + nonExistingEmail + "\" not found", ref);
+ r = pushTo("refs/for/master/" + topic + "%r=" + admin.email + ",r="
+ + nonExistingEmail + ",r=" + user.email);
+ r.assertErrorStatus("user \"" + nonExistingEmail + "\" not found");
}
@Test
public void testPushForMasterAsDraft() throws GitAPIException, OrmException,
IOException {
// create draft by pushing to 'refs/drafts/'
- PushOneCommit push = new PushOneCommit();
- String ref = "refs/drafts/master";
- PushResult r = push.to(ref);
- assertOkStatus(r, ref);
- assertChange(push.changeId, Change.Status.DRAFT, PushOneCommit.SUBJECT, null);
+ PushOneCommit.Result r = pushTo("refs/drafts/master");
+ r.assertOkStatus();
+ r.assertChange(Change.Status.DRAFT, null);
// create draft by using 'draft' option
- push = new PushOneCommit();
- ref = "refs/for/master%draft";
- r = push.to(ref);
- assertOkStatus(r, ref);
- assertChange(push.changeId, Change.Status.DRAFT, PushOneCommit.SUBJECT, null);
+ r = pushTo("refs/for/master%draft");
+ r.assertOkStatus();
+ r.assertChange(Change.Status.DRAFT, null);
}
@Test
public void testPushForNonExistingBranch() throws GitAPIException,
OrmException, IOException {
- PushOneCommit push = new PushOneCommit();
String branchName = "non-existing";
- String ref = "refs/for/" + branchName;
- PushResult r = push.to(ref);
- assertErrorStatus(r, "branch " + branchName + " not found", ref);
+ PushOneCommit.Result r = pushTo("refs/for/" + branchName);
+ r.assertErrorStatus("branch " + branchName + " not found");
}
- private void assertChange(String changeId, Change.Status expectedStatus,
- String expectedSubject, String expectedTopic,
- TestAccount... expectedReviewers) throws OrmException {
- Change c =
- Iterables.getOnlyElement(db.changes().byKey(new Change.Key(changeId)).toList());
- assertEquals(expectedSubject, c.getSubject());
- assertEquals(expectedStatus, c.getStatus());
- assertEquals(expectedTopic, Strings.emptyToNull(c.getTopic()));
- assertReviewers(c, expectedReviewers);
- }
-
- private void assertReviewers(Change c, TestAccount... expectedReviewers)
- throws OrmException {
- Set<Account.Id> expectedReviewerIds =
- Sets.newHashSet(Lists.transform(Arrays.asList(expectedReviewers),
- new Function<TestAccount, Account.Id>() {
- @Override
- public Account.Id apply(TestAccount a) {
- return a.id;
- }
- }));
-
- for (PatchSetApproval psa : db.patchSetApprovals().byPatchSet(
- c.currentPatchSetId())) {
- assertTrue("unexpected reviewer " + psa.getAccountId(),
- expectedReviewerIds.remove(psa.getAccountId()));
- }
- assertTrue("missing reviewers: " + expectedReviewerIds,
- expectedReviewerIds.isEmpty());
- }
-
- private static void assertOkStatus(PushResult result, String ref) {
- assertStatus(Status.OK, null, result, ref);
- }
-
- private static void assertErrorStatus(PushResult result,
- String expectedMessage, String ref) {
- assertStatus(Status.REJECTED_OTHER_REASON, expectedMessage, result, ref);
- }
-
- private static void assertStatus(Status expectedStatus,
- String expectedMessage, PushResult result, String ref) {
- RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref);
- assertEquals(refUpdate.getMessage() + "\n" + result.getMessages(),
- expectedStatus, refUpdate.getStatus());
- assertEquals(expectedMessage, refUpdate.getMessage());
- }
-
- private class PushOneCommit {
- final static String FILE_NAME = "a.txt";
- final static String FILE_CONTENT = "some content";
- final static String SUBJECT = "test commit";
- String changeId;
-
- public PushResult to(String ref) throws GitAPIException, IOException {
- add(git, FILE_NAME, FILE_CONTENT);
- changeId = createCommit(git, admin.getIdent(), SUBJECT);
- return pushHead(git, ref);
- }
+ private PushOneCommit.Result pushTo(String ref) throws GitAPIException,
+ IOException {
+ PushOneCommit push = new PushOneCommit(db, admin.getIdent());
+ return push.to(git, ref);
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/PushOneCommit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/PushOneCommit.java
new file mode 100644
index 0000000..fb1b592
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/PushOneCommit.java
@@ -0,0 +1,179 @@
+// Copyright (C) 2013 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.git.GitUtil.add;
+import static com.google.gerrit.acceptance.git.GitUtil.amendCommit;
+import static com.google.gerrit.acceptance.git.GitUtil.createCommit;
+import static com.google.gerrit.acceptance.git.GitUtil.pushHead;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gwtorm.server.OrmException;
+
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
+import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Set;
+
+public class PushOneCommit {
+ public final static String SUBJECT = "test commit";
+
+ private final static String FILE_NAME = "a.txt";
+ private final static String FILE_CONTENT = "some content";
+
+ private final ReviewDb db;
+ private final PersonIdent i;
+
+ private final String subject;
+ private final String fileName;
+ private final String content;
+ private String changeId;
+
+ public PushOneCommit(ReviewDb db, PersonIdent i) {
+ this(db, i, SUBJECT, FILE_NAME, FILE_CONTENT);
+ }
+
+ public PushOneCommit(ReviewDb db, PersonIdent i, String subject,
+ String fileName, String content) {
+ this(db, i, subject, fileName, content, null);
+ }
+
+ public PushOneCommit(ReviewDb db, PersonIdent i, String subject,
+ String fileName, String content, String changeId) {
+ this.db = db;
+ this.i = i;
+ this.subject = subject;
+ this.fileName = fileName;
+ this.content = content;
+ this.changeId = changeId;
+ }
+
+ public Result to(Git git, String ref)
+ throws GitAPIException, IOException {
+ add(git, fileName, content);
+ if (changeId != null) {
+ amendCommit(git, i, subject, changeId);
+ } else {
+ changeId = createCommit(git, i, subject);
+ }
+ return new Result(db, ref, pushHead(git, ref), changeId, subject);
+ }
+
+ public static class Result {
+ private final ReviewDb db;
+ private final String ref;
+ private final PushResult result;
+ private final String changeId;
+ private final String subject;
+
+ private Result(ReviewDb db, String ref, PushResult result, String changeId,
+ String subject) {
+ this.db = db;
+ this.ref = ref;
+ this.result = result;
+ this.changeId = changeId;
+ this.subject = subject;
+ }
+
+ public PatchSet.Id getPatchSetId() throws OrmException {
+ return Iterables.getOnlyElement(
+ db.changes().byKey(new Change.Key(changeId))).currentPatchSetId();
+ }
+
+ public String getChangeId() {
+ return changeId;
+ }
+
+ public void assertChange(Change.Status expectedStatus,
+ String expectedTopic, TestAccount... expectedReviewers)
+ throws OrmException {
+ Change c =
+ Iterables.getOnlyElement(db.changes().byKey(new Change.Key(changeId)).toList());
+ assertEquals(subject, c.getSubject());
+ assertEquals(expectedStatus, c.getStatus());
+ assertEquals(expectedTopic, Strings.emptyToNull(c.getTopic()));
+ assertReviewers(c, expectedReviewers);
+ }
+
+ private void assertReviewers(Change c, TestAccount... expectedReviewers)
+ throws OrmException {
+ Set<Account.Id> expectedReviewerIds =
+ Sets.newHashSet(Lists.transform(Arrays.asList(expectedReviewers),
+ new Function<TestAccount, Account.Id>() {
+ @Override
+ public Account.Id apply(TestAccount a) {
+ return a.id;
+ }
+ }));
+
+ for (PatchSetApproval psa : db.patchSetApprovals().byPatchSet(
+ c.currentPatchSetId())) {
+ assertTrue("unexpected reviewer " + psa.getAccountId(),
+ expectedReviewerIds.remove(psa.getAccountId()));
+ }
+ assertTrue("missing reviewers: " + expectedReviewerIds,
+ expectedReviewerIds.isEmpty());
+ }
+
+ public void assertOkStatus() {
+ assertStatus(Status.OK, null);
+ }
+
+ public void assertErrorStatus(String expectedMessage) {
+ assertStatus(Status.REJECTED_OTHER_REASON, expectedMessage);
+ }
+
+ private void assertStatus(Status expectedStatus, String expectedMessage) {
+ RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref);
+ assertEquals(message(refUpdate),
+ expectedStatus, refUpdate.getStatus());
+ assertEquals(expectedMessage, refUpdate.getMessage());
+ }
+
+ public void assertMessage(String expectedMessage) {
+ RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref);
+ assertTrue(message(refUpdate), message(refUpdate).toLowerCase().contains(
+ expectedMessage.toLowerCase()));
+ }
+
+ private String message(RemoteRefUpdate refUpdate) {
+ StringBuilder b = new StringBuilder();
+ if (refUpdate.getMessage() != null) {
+ b.append(refUpdate.getMessage());
+ b.append("\n");
+ }
+ b.append(result.getMessages());
+ return b.toString();
+ }
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
new file mode 100644
index 0000000..fa85927
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
@@ -0,0 +1,300 @@
+// Copyright (C) 2013 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.git.GitUtil.cloneProject;
+import static com.google.gerrit.acceptance.git.GitUtil.createProject;
+import static com.google.gerrit.acceptance.git.GitUtil.initSsh;
+import static org.junit.Assert.assertEquals;
+
+import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.AccountCreator;
+import com.google.gerrit.acceptance.SshSession;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.git.CommitMergeStatus;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Inject;
+
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.RefSpec;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+public class SubmitOnPushIT extends AbstractDaemonTest {
+
+ @Inject
+ private AccountCreator accounts;
+
+ @Inject
+ private SchemaFactory<ReviewDb> reviewDbProvider;
+
+ @Inject
+ private GitRepositoryManager repoManager;
+
+ @Inject
+ private MetaDataUpdate.Server metaDataUpdateFactory;
+
+ @Inject
+ private ProjectCache projectCache;
+
+ @Inject
+ private GroupCache groupCache;
+
+ @Inject
+ private @GerritPersonIdent PersonIdent serverIdent;
+
+ private TestAccount admin;
+ private Project.NameKey project;
+ private Git git;
+ private ReviewDb db;
+
+ @Before
+ public void setUp() throws Exception {
+ admin =
+ accounts.create("admin", "admin@example.com", "Administrator",
+ "Administrators");
+
+ project = new Project.NameKey("p");
+ initSsh(admin);
+ SshSession sshSession = new SshSession(admin);
+ createProject(sshSession, project.get());
+ git = cloneProject(sshSession.getUrl() + "/" + project.get());
+ sshSession.close();
+
+ db = reviewDbProvider.open();
+ }
+
+ @After
+ public void cleanup() {
+ db.close();
+ }
+
+ @Test
+ public void submitOnPush() throws GitAPIException, OrmException,
+ IOException, ConfigInvalidException {
+ grantSubmit(project, "refs/for/refs/heads/master");
+ PushOneCommit.Result r = pushTo("refs/for/master%submit");
+ r.assertOkStatus();
+ r.assertChange(Change.Status.MERGED, null, admin);
+ assertSubmitApproval(r.getPatchSetId());
+ assertCommit(project, "refs/heads/master");
+ }
+
+ @Test
+ public void submitOnPushToRefsMetaConfig() throws GitAPIException,
+ OrmException, IOException, ConfigInvalidException {
+ grantSubmit(project, "refs/for/refs/meta/config");
+
+ git.fetch().setRefSpecs(new RefSpec("refs/meta/config:refs/meta/config")).call();
+ ObjectId objectId = git.getRepository().getRef("refs/meta/config").getObjectId();
+ git.checkout().setName(objectId.getName()).call();
+
+ PushOneCommit.Result r = pushTo("refs/for/refs/meta/config%submit");
+ r.assertOkStatus();
+ r.assertChange(Change.Status.MERGED, null, admin);
+ assertSubmitApproval(r.getPatchSetId());
+ assertCommit(project, "refs/meta/config");
+ }
+
+ @Test
+ public void submitOnPushMergeConflict() throws GitAPIException, OrmException,
+ IOException, ConfigInvalidException {
+ String master = "refs/heads/master";
+ ObjectId objectId = git.getRepository().getRef(master).getObjectId();
+ push(master, "one change", "a.txt", "some content");
+ git.checkout().setName(objectId.getName()).call();
+
+ grantSubmit(project, "refs/for/refs/heads/master");
+ PushOneCommit.Result r =
+ push("refs/for/master%submit", "other change", "a.txt", "other content");
+ r.assertOkStatus();
+ r.assertChange(Change.Status.NEW, null, admin);
+ r.assertMessage(CommitMergeStatus.PATH_CONFLICT.getMessage());
+ }
+
+ @Test
+ public void submitOnPushSuccessfulMerge() throws GitAPIException, OrmException,
+ IOException, ConfigInvalidException {
+ String master = "refs/heads/master";
+ ObjectId objectId = git.getRepository().getRef(master).getObjectId();
+ push(master, "one change", "a.txt", "some content");
+ git.checkout().setName(objectId.getName()).call();
+
+ grantSubmit(project, "refs/for/refs/heads/master");
+ PushOneCommit.Result r =
+ push("refs/for/master%submit", "other change", "b.txt", "other content");
+ r.assertOkStatus();
+ r.assertChange(Change.Status.MERGED, null, admin);
+ assertMergeCommit(master, "other change");
+ }
+
+ @Test
+ public void submitOnPushNewPatchSet() throws GitAPIException,
+ OrmException, IOException, ConfigInvalidException {
+ PushOneCommit.Result r =
+ push("refs/for/master", PushOneCommit.SUBJECT, "a.txt", "some content");
+
+ grantSubmit(project, "refs/for/refs/heads/master");
+ r = push("refs/for/master%submit", PushOneCommit.SUBJECT, "a.txt",
+ "other content", r.getChangeId());
+ r.assertOkStatus();
+ r.assertChange(Change.Status.MERGED, null, admin);
+ Change c = Iterables.getOnlyElement(db.changes().byKey(
+ new Change.Key(r.getChangeId())).toList());
+ assertEquals(2, db.patchSets().byChange(c.getId()).toList().size());
+ assertSubmitApproval(r.getPatchSetId());
+ assertCommit(project, "refs/heads/master");
+ }
+
+ @Test
+ public void submitOnPushNotAllowed_Error() throws GitAPIException,
+ OrmException, IOException {
+ PushOneCommit.Result r = pushTo("refs/for/master%submit");
+ r.assertErrorStatus("submit not allowed");
+ }
+
+ @Test
+ public void submitOnPushNewPatchSetNotAllowed_Error() throws GitAPIException,
+ OrmException, IOException, ConfigInvalidException {
+ PushOneCommit.Result r =
+ push("refs/for/master", PushOneCommit.SUBJECT, "a.txt", "some content");
+
+ r = push("refs/for/master%submit", PushOneCommit.SUBJECT, "a.txt",
+ "other content", r.getChangeId());
+ r.assertErrorStatus("submit not allowed");
+ }
+
+ @Test
+ public void submitOnPushingDraft_Error() throws GitAPIException,
+ OrmException, IOException {
+ PushOneCommit.Result r = pushTo("refs/for/master%draft,submit");
+ r.assertErrorStatus("cannot submit draft");
+ }
+
+ @Test
+ public void submitOnPushToNonExistingBranch_Error() throws GitAPIException,
+ OrmException, IOException {
+ String branchName = "non-existing";
+ PushOneCommit.Result r = pushTo("refs/for/" + branchName + "%submit");
+ r.assertErrorStatus("branch " + branchName + " not found");
+ }
+
+ private void grantSubmit(Project.NameKey project, String ref)
+ throws RepositoryNotFoundException, IOException, ConfigInvalidException {
+ MetaDataUpdate md = metaDataUpdateFactory.create(project);
+ md.setMessage("Grant submit on " + ref);
+ ProjectConfig config = ProjectConfig.read(md);
+ AccessSection s = config.getAccessSection(ref, true);
+ Permission p = s.getPermission(Permission.SUBMIT, true);
+ AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
+ p.add(new PermissionRule(config.resolve(adminGroup)));
+ config.commit(md);
+ projectCache.evict(config.getProject());
+ }
+
+ private void assertSubmitApproval(PatchSet.Id patchSetId) throws OrmException {
+ List<PatchSetApproval> approvals = db.patchSetApprovals().byPatchSet(patchSetId).toList();
+ assertEquals(1, approvals.size());
+ PatchSetApproval a = approvals.get(0);
+ assertEquals(PatchSetApproval.LabelId.SUBMIT.get(), a.getLabel());
+ assertEquals(1, a.getValue());
+ assertEquals(admin.id, a.getAccountId());
+ }
+
+ private void assertCommit(Project.NameKey project, String branch) throws IOException {
+ Repository r = repoManager.openRepository(project);
+ try {
+ RevWalk rw = new RevWalk(r);
+ try {
+ RevCommit c = rw.parseCommit(r.getRef(branch).getObjectId());
+ assertEquals(PushOneCommit.SUBJECT, c.getShortMessage());
+ assertEquals(admin.email, c.getAuthorIdent().getEmailAddress());
+ assertEquals(admin.email, c.getCommitterIdent().getEmailAddress());
+ } finally {
+ rw.release();
+ }
+ } finally {
+ r.close();
+ }
+ }
+
+ private void assertMergeCommit(String branch, String subject) throws IOException {
+ Repository r = repoManager.openRepository(project);
+ try {
+ RevWalk rw = new RevWalk(r);
+ try {
+ RevCommit c = rw.parseCommit(r.getRef(branch).getObjectId());
+ assertEquals(2, c.getParentCount());
+ assertEquals("Merge \"" + subject + "\"", c.getShortMessage());
+ assertEquals(admin.email, c.getAuthorIdent().getEmailAddress());
+ assertEquals(serverIdent.getEmailAddress(), c.getCommitterIdent().getEmailAddress());
+ } finally {
+ rw.release();
+ }
+ } finally {
+ r.close();
+ }
+ }
+
+ private PushOneCommit.Result pushTo(String ref) throws GitAPIException,
+ IOException {
+ PushOneCommit push = new PushOneCommit(db, admin.getIdent());
+ return push.to(git, ref);
+ }
+
+ private PushOneCommit.Result push(String ref, String subject,
+ String fileName, String content) throws GitAPIException, IOException {
+ PushOneCommit push =
+ new PushOneCommit(db, admin.getIdent(), subject, fileName, content);
+ return push.to(git, ref);
+ }
+
+ private PushOneCommit.Result push(String ref, String subject,
+ String fileName, String content, String changeId) throws GitAPIException,
+ IOException {
+ PushOneCommit push = new PushOneCommit(db, admin.getIdent(), subject,
+ fileName, content, changeId);
+ return push.to(git, ref);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
index c63bf5d..2b51b0a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
@@ -106,6 +106,10 @@
checkSubmitRule(rsrc);
change = submit(rsrc, caller);
+ if (change == null) {
+ throw new ResourceConflictException("change is "
+ + status(dbProvider.get().changes().get(rsrc.getChange().getId())));
+ }
if (input.waitForMerge) {
mergeQueue.merge(change.getDest());
@@ -123,21 +127,7 @@
case MERGED:
return new Output(Status.MERGED, change);
case NEW:
- // If the merge was attempted and it failed the system usually
- // writes a comment as a ChangeMessage and sets status to NEW.
- // Find the relevant message and report that as the conflict.
- final Timestamp before = rsrc.getChange().getLastUpdatedOn();
- ChangeMessage msg = Iterables.getFirst(Iterables.filter(
- Lists.reverse(dbProvider.get().changeMessages()
- .byChange(change.getId())
- .toList()),
- new Predicate<ChangeMessage>() {
- @Override
- public boolean apply(ChangeMessage input) {
- return input.getAuthor() == null
- && input.getWrittenOn().getTime() >= before.getTime();
- }
- }), null);
+ ChangeMessage msg = getConflictMessage(rsrc);
if (msg != null) {
throw new ResourceConflictException(msg.getMessage());
}
@@ -146,8 +136,30 @@
}
}
- private Change submit(RevisionResource rsrc, IdentifiedUser caller)
- throws OrmException, ResourceConflictException {
+ /**
+ * If the merge was attempted and it failed the system usually writes a
+ * comment as a ChangeMessage and sets status to NEW. Find the relevant
+ * message and return it.
+ */
+ public ChangeMessage getConflictMessage(RevisionResource rsrc)
+ throws OrmException {
+ final Timestamp before = rsrc.getChange().getLastUpdatedOn();
+ ChangeMessage msg = Iterables.getFirst(Iterables.filter(
+ Lists.reverse(dbProvider.get().changeMessages()
+ .byChange(rsrc.getChange().getId())
+ .toList()),
+ new Predicate<ChangeMessage>() {
+ @Override
+ public boolean apply(ChangeMessage input) {
+ return input.getAuthor() == null
+ && input.getWrittenOn().getTime() >= before.getTime();
+ }
+ }), null);
+ return msg;
+ }
+
+ public Change submit(RevisionResource rsrc, IdentifiedUser caller)
+ throws OrmException {
final Timestamp timestamp = new Timestamp(System.currentTimeMillis());
Change change = rsrc.getChange();
ReviewDb db = dbProvider.get();
@@ -169,8 +181,7 @@
}
});
if (change == null) {
- throw new ResourceConflictException("change is "
- + status(db.changes().get(rsrc.getChange().getId())));
+ return null;
}
db.commit();
} finally {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
index 69dcb15..8595472 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
@@ -14,7 +14,7 @@
package com.google.gerrit.server.git;
-enum CommitMergeStatus {
+public enum CommitMergeStatus {
/** */
CLEAN_MERGE("Change has been successfully merged into the git repository."),
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 3199a14..45c6e5a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -59,6 +59,9 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeInserter;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.TrackingFooters;
@@ -87,6 +90,7 @@
import com.google.gwtorm.server.ResultSet;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -277,6 +281,8 @@
private Map<String, Ref> allRefs;
private final SubmoduleOp.Factory subOpFactory;
+ private final Provider<Submit> submitProvider;
+ private final MergeQueue mergeQueue;
private final List<CommitValidationMessage> messages = new ArrayList<CommitValidationMessage>();
private ListMultimap<Error, String> errors = LinkedListMultimap.create();
@@ -316,7 +322,9 @@
ReceiveConfig config,
@Assisted final ProjectControl projectControl,
@Assisted final Repository repo,
- final SubmoduleOp.Factory subOpFactory) throws IOException {
+ final SubmoduleOp.Factory subOpFactory,
+ final Provider<Submit> submitProvider,
+ final MergeQueue mergeQueue) throws IOException {
this.currentUser = (IdentifiedUser) projectControl.getCurrentUser();
this.db = db;
this.schemaFactory = schemaFactory;
@@ -351,6 +359,8 @@
this.rejectCommits = loadRejectCommitsMap();
this.subOpFactory = subOpFactory;
+ this.submitProvider = submitProvider;
+ this.mergeQueue = mergeQueue;
this.messageSender = new ReceivePackMessageSender();
@@ -1000,6 +1010,9 @@
@Option(name = "--draft", usage = "mark new/update changes as draft")
boolean draft;
+ @Option(name = "--submit", usage = "immediately submit the change")
+ boolean submit;
+
@Option(name = "-r", metaVar = "EMAIL", usage = "add reviewer to changes")
void reviewer(Account.Id id) {
reviewer.add(id);
@@ -1024,6 +1037,10 @@
return draft;
}
+ boolean isSubmit() {
+ return submit;
+ }
+
MailRecipients getMailRecipients() {
return new MailRecipients(reviewer, cc);
}
@@ -1120,6 +1137,16 @@
return;
}
+ if (magicBranch.isDraft() && magicBranch.isSubmit()) {
+ reject(cmd, "cannot submit draft");
+ return;
+ }
+
+ if (magicBranch.isSubmit() && !projectControl.controlForRef(
+ MagicBranch.NEW_CHANGE + ref).canSubmit()) {
+ reject(cmd, "submit not allowed");
+ }
+
// Validate that the new commits are connected with the target
// branch. If they aren't, we want to abort. We do this check by
// looking to see if we can compute a merge base between the new
@@ -1493,6 +1520,41 @@
return "send-email newchange";
}
}));
+
+ if (magicBranch != null && magicBranch.isSubmit()) {
+ submit(projectControl.controlFor(change), ps);
+ }
+ }
+ }
+
+ private void submit(ChangeControl changeCtl, PatchSet ps) throws OrmException {
+ Submit submit = submitProvider.get();
+ RevisionResource rsrc = new RevisionResource(new ChangeResource(changeCtl), ps);
+ Change c = submit.submit(rsrc, currentUser);
+ if (c == null) {
+ addError("Submitting change " + changeCtl.getChange().getChangeId()
+ + " failed.");
+ } else {
+ addMessage("");
+ mergeQueue.merge(c.getDest());
+ c = db.changes().get(c.getId());
+ switch (c.getStatus()) {
+ case SUBMITTED:
+ addMessage("Change " + c.getChangeId() + " submitted.");
+ break;
+ case MERGED:
+ addMessage("Change " + c.getChangeId() + " merged.");
+ break;
+ case NEW:
+ ChangeMessage msg = submit.getConflictMessage(rsrc);
+ if (msg != null) {
+ addMessage("Change " + c.getChangeId() + ": " + msg.getMessage());
+ break;
+ }
+ default:
+ addMessage("change " + c.getChangeId() + " is "
+ + c.getStatus().name().toLowerCase());
+ }
}
}
@@ -1865,6 +1927,11 @@
return "send-email newpatchset";
}
}));
+
+ if (magicBranch != null && magicBranch.isSubmit()) {
+ submit(changeCtl, newPatchSet);
+ }
+
return newPatchSet.getId();
}
}
