diff options
Diffstat (limited to 'gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java')
-rw-r--r-- | gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java | 303 |
1 files changed, 200 insertions, 103 deletions
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 bcc30144fc..3e28b07d87 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 @@ -18,7 +18,6 @@ import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID; import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; - import static org.eclipse.jgit.lib.Constants.R_HEADS; import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK; @@ -30,12 +29,14 @@ import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.base.Splitter; import com.google.common.base.Strings; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.BiMap; +import com.google.common.collect.HashBiMap; import com.google.common.collect.HashMultimap; import com.google.common.collect.Iterables; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.common.util.concurrent.CheckedFuture; @@ -63,6 +64,9 @@ import com.google.gerrit.server.GerritPersonIdent; 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; @@ -91,6 +95,7 @@ import com.google.gwtorm.server.OrmException; 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; @@ -130,6 +135,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -278,10 +284,13 @@ public class ReceiveCommits { new HashMap<RevCommit, ReplaceRequest>(); private final Set<RevCommit> validCommits = new HashSet<RevCommit>(); + private ListMultimap<Change.Id, Ref> refsByChange; private SetMultimap<ObjectId, Ref> refsById; 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(); @@ -321,7 +330,9 @@ public class ReceiveCommits { 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; @@ -356,6 +367,8 @@ public class ReceiveCommits { this.rejectCommits = loadRejectCommitsMap(); this.subOpFactory = subOpFactory; + this.submitProvider = submitProvider; + this.mergeQueue = mergeQueue; this.messageSender = new ReceivePackMessageSender(); @@ -998,6 +1011,10 @@ public class ReceiveCommits { RefControl ctl; Set<Account.Id> reviewer = Sets.newLinkedHashSet(); Set<Account.Id> cc = Sets.newLinkedHashSet(); + RevCommit baseCommit; + + @Option(name = "--base", metaVar = "BASE", usage = "merge base of changes") + ObjectId base; @Option(name = "--topic", metaVar = "NAME", usage = "attach topic to changes") String topic; @@ -1005,6 +1022,9 @@ public class ReceiveCommits { @Option(name = "--draft", usage = "mark new/updated 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); @@ -1029,6 +1049,10 @@ public class ReceiveCommits { return draft; } + boolean isSubmit() { + return submit; + } + MailRecipients getMailRecipients() { return new MailRecipients(reviewer, cc); } @@ -1125,13 +1149,41 @@ public class ReceiveCommits { 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"); + } + + RevWalk walk = rp.getRevWalk(); + if (magicBranch.base != null) { + try { + magicBranch.baseCommit = walk.parseCommit(magicBranch.base); + } catch (IncorrectObjectTypeException notCommit) { + reject(cmd, "base must be a commit"); + return; + } catch (MissingObjectException e) { + reject(cmd, "base not found"); + return; + } catch (IOException e) { + log.warn(String.format( + "Project %s cannot read %s", + project.getName(), magicBranch.base.name()), e); + reject(cmd, "internal server error"); + return; + } + } + // 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 // commits and the target branch head. // try { - final RevWalk walk = rp.getRevWalk(); final RevCommit tip = walk.parseCommit(magicBranch.cmd.getNewId()); Ref targetRef = rp.getAdvertisedRefs().get(magicBranch.ctl.getRefName()); if (targetRef == null || targetRef.getObjectId() == null) { @@ -1261,10 +1313,14 @@ public class ReceiveCommits { try { Set<ObjectId> existing = Sets.newHashSet(); walk.markStart(walk.parseCommit(magicBranch.cmd.getNewId())); - markHeadsAsUninteresting( - walk, - existing, - magicBranch.ctl != null ? magicBranch.ctl.getRefName() : null); + if (magicBranch.baseCommit != null) { + walk.markUninteresting(magicBranch.baseCommit); + } else { + markHeadsAsUninteresting( + walk, + existing, + magicBranch.ctl != null ? magicBranch.ctl.getRefName() : null); + } List<ChangeLookup> pending = Lists.newArrayList(); final Set<Change.Key> newChangeIds = new HashSet<Change.Key>(); @@ -1471,8 +1527,8 @@ public class ReceiveCommits { recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines)); recipients.remove(me); - changeInserter.insertChange(db, change, ps, commit, labelTypes, - footerLines, info, recipients.getReviewers()); + changeInserter.insertChange(db, change, ps, commit, labelTypes, info, + recipients.getReviewers()); created = true; @@ -1498,16 +1554,56 @@ public class ReceiveCommits { 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()); + } } } private void preparePatchSetsForReplace() { try { readChangesForReplace(); - readPatchSetsForReplace(); - for (ReplaceRequest req : replaceByChange.values()) { + for (Iterator<ReplaceRequest> itr = replaceByChange.values().iterator(); + itr.hasNext();) { + ReplaceRequest req = itr.next(); if (req.inputCommand.getResult() == NOT_ATTEMPTED) { req.validate(false); + if (req.skip && req.cmd == null) { + itr.remove(); + replaceByCommit.remove(req.newCommit); + } } } } catch (OrmException err) { @@ -1559,17 +1655,6 @@ public class ReceiveCommits { } } - private void readPatchSetsForReplace() throws OrmException { - Map<Change.Id, ResultSet<PatchSet>> results = Maps.newHashMap(); - for (ReplaceRequest request : replaceByChange.values()) { - Change.Id id = request.ontoChange; - results.put(id, db.patchSets().byChange(id)); - } - for (ReplaceRequest req : replaceByChange.values()) { - req.patchSets = results.get(req.ontoChange).toList(); - } - } - private class ReplaceRequest { final Change.Id ontoChange; final RevCommit newCommit; @@ -1577,12 +1662,13 @@ public class ReceiveCommits { final boolean checkMergedInto; Change change; ChangeControl changeCtl; - List<PatchSet> patchSets; + BiMap<RevCommit, PatchSet.Id> revisions; PatchSet newPatchSet; ReceiveCommand cmd; PatchSetInfo info; ChangeMessage msg; String mergedIntoRef; + boolean skip; private PatchSet.Id priorPatchSet; ReplaceRequest(final Change.Id toChange, final RevCommit newCommit, @@ -1591,20 +1677,39 @@ public class ReceiveCommits { this.newCommit = newCommit; this.inputCommand = cmd; this.checkMergedInto = checkMergedInto; + + revisions = HashBiMap.create(); + for (Ref ref : refs(toChange)) { + try { + revisions.forcePut( + rp.getRevWalk().parseCommit(ref.getObjectId()), + PatchSet.Id.fromRef(ref.getName())); + } catch (IOException err) { + log.warn(String.format( + "Project %s contains invalid change ref %s", + project.getName(), ref.getName()), err); + } + } } boolean validate(boolean autoClose) throws IOException { if (!autoClose && inputCommand.getResult() != NOT_ATTEMPTED) { return false; + } else if (change == null) { + reject(inputCommand, "change " + ontoChange + " not found"); + return false; } - if (change == null || patchSets == null) { - reject(inputCommand, "change " + ontoChange + " not found"); + priorPatchSet = change.currentPatchSetId(); + if (!revisions.containsValue(priorPatchSet)) { + reject(inputCommand, "change " + ontoChange + " missing revisions"); return false; } - if (change.getStatus().isClosed()) { - reject(inputCommand, "change " + ontoChange + " closed"); + RevCommit priorCommit = revisions.inverse().get(priorPatchSet); + if (newCommit == priorCommit) { + // Ignore requests to make the change its current state. + skip = true; return false; } @@ -1612,88 +1717,59 @@ public class ReceiveCommits { if (!changeCtl.canAddPatchSet()) { reject(inputCommand, "cannot replace " + ontoChange); return false; + } else if (change.getStatus().isClosed()) { + reject(inputCommand, "change " + ontoChange + " closed"); + return false; + } else if (revisions.containsKey(newCommit)) { + reject(inputCommand, "commit already exists"); + return false; } - rp.getRevWalk().parseBody(newCommit); + for (RevCommit prior : revisions.keySet()) { + // Don't allow a change to directly depend upon itself. This is a + // very common error due to users making a new commit rather than + // amending when trying to address review comments. + if (rp.getRevWalk().isMergedInto(prior, newCommit)) { + reject(inputCommand, "squash commits first"); + return false; + } + } + rp.getRevWalk().parseBody(newCommit); if (!validCommit(changeCtl.getRefControl(), inputCommand, newCommit)) { return false; } - priorPatchSet = change.currentPatchSetId(); - for (final PatchSet ps : patchSets) { - if (ps.getRevision() == null) { - log.warn("Patch set " + ps.getId() + " has no revision"); - reject(inputCommand, "change state corrupt"); - return false; - } + // Don't allow the same tree if the commit message is unmodified + // or no parents were updated (rebase), else warn that only part + // of the commit was modified. + if (newCommit.getTree() == priorCommit.getTree()) { + rp.getRevWalk().parseBody(priorCommit); + final boolean messageEq = + eq(newCommit.getFullMessage(), priorCommit.getFullMessage()); + final boolean parentsEq = parentsEqual(newCommit, priorCommit); + final boolean authorEq = authorEqual(newCommit, priorCommit); - final String revIdStr = ps.getRevision().get(); - final ObjectId commitId; - try { - commitId = ObjectId.fromString(revIdStr); - } catch (IllegalArgumentException e) { - log.warn("Invalid revision in " + ps.getId() + ": " + revIdStr); - reject(inputCommand, "change state corrupt"); + if (messageEq && parentsEq && authorEq && !autoClose) { + reject(inputCommand, "no changes made"); return false; - } - - try { - final RevCommit prior = rp.getRevWalk().parseCommit(commitId); - - // Don't allow the same commit to appear twice on the same change - // - if (newCommit == prior) { - reject(inputCommand, "commit already exists"); - return false; + } else { + ObjectReader reader = rp.getRevWalk().getObjectReader(); + StringBuilder msg = new StringBuilder(); + msg.append("(W) "); + msg.append(reader.abbreviate(newCommit).name()); + msg.append(":"); + msg.append(" no files changed"); + if (!authorEq) { + msg.append(", author changed"); } - - // Don't allow a change to directly depend upon itself. This is a - // very common error due to users making a new commit rather than - // amending when trying to address review comments. - // - if (rp.getRevWalk().isMergedInto(prior, newCommit)) { - reject(inputCommand, "squash commits first"); - return false; + if (!messageEq) { + msg.append(", message updated"); } - - // Don't allow the same tree if the commit message is unmodified - // or no parents were updated (rebase), else warn that only part - // of the commit was modified. - // - if (priorPatchSet.equals(ps.getId()) && newCommit.getTree() == prior.getTree()) { - rp.getRevWalk().parseBody(prior); - final boolean messageEq = - eq(newCommit.getFullMessage(), prior.getFullMessage()); - final boolean parentsEq = parentsEqual(newCommit, prior); - final boolean authorEq = authorEqual(newCommit, prior); - - if (messageEq && parentsEq && authorEq && !autoClose) { - reject(inputCommand, "no changes made"); - return false; - } else { - ObjectReader reader = rp.getRevWalk().getObjectReader(); - StringBuilder msg = new StringBuilder(); - msg.append("(W) "); - msg.append(reader.abbreviate(newCommit).name()); - msg.append(":"); - msg.append(" no files changed"); - if (!authorEq) { - msg.append(", author changed"); - } - if (!messageEq) { - msg.append(", message updated"); - } - if (!parentsEq) { - msg.append(", was rebased"); - } - addMessage(msg.toString()); - } + if (!parentsEq) { + msg.append(", was rebased"); } - } catch (IOException e) { - log.error("Change " + change.getId() + " missing " + revIdStr, e); - reject(inputCommand, "change state corrupt"); - return false; + addMessage(msg.toString()); } } @@ -1770,10 +1846,12 @@ public class ReceiveCommits { mergedIntoRef = mergedInto != null ? mergedInto.getName() : null; } - List<PatchSetApproval> patchSetApprovals = - approvalsUtil.copyVetosToPatchSet(db, labelTypes, newPatchSet.getId()); - final MailRecipients oldRecipients = - getRecipientsFromApprovals(patchSetApprovals); + List<PatchSetApproval> oldChangeApprovals = + db.patchSetApprovals().byChange(change.getId()).toList(); + final MailRecipients oldRecipients = getRecipientsFromApprovals( + oldChangeApprovals); + ApprovalsUtil.copyLabels(db, labelTypes, oldChangeApprovals, + priorPatchSet, newPatchSet.getId()); approvalsUtil.addReviewers(db, labelTypes, change, newPatchSet, info, recipients.getReviewers(), oldRecipients.getAll()); recipients.add(oldRecipients); @@ -1879,10 +1957,30 @@ public class ReceiveCommits { return "send-email newpatchset"; } })); + + if (magicBranch != null && magicBranch.isSubmit()) { + submit(changeCtl, newPatchSet); + } + return newPatchSet.getId(); } } + private List<Ref> refs(Change.Id changeId) { + if (refsByChange == null) { + int estRefsPerChange = 4; + refsByChange = ArrayListMultimap.create( + allRefs.size() / estRefsPerChange, + estRefsPerChange); + for (Ref ref : allRefs.values()) { + if (ref.getObjectId() != null && PatchSet.isRef(ref.getName())) { + refsByChange.put(Change.Id.fromRef(ref.getName()), ref); + } + } + } + return refsByChange.get(changeId); + } + static boolean parentsEqual(RevCommit a, RevCommit b) { if (a.getParentCount() != b.getParentCount()) { return false; @@ -2038,7 +2136,6 @@ public class ReceiveCommits { if (onto != null) { final ReplaceRequest req = new ReplaceRequest(onto, c, cmd, false); req.change = db.changes().get(onto); - req.patchSets = db.patchSets().byChange(onto).toList(); toClose.add(req); break; } |