From a502d96a860456ec5e8c96761db70f7cabb74751 Mon Sep 17 00:00:00 2001
From: Paul Martin <paul@paulsputer.com>
Date: Sat, 30 Apr 2016 04:19:14 -0400
Subject: [PATCH] Merge pull request #1073 from gitblit/1062-DocEditorUpdates
---
src/main/java/com/gitblit/git/PatchsetReceivePack.java | 255 +++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 226 insertions(+), 29 deletions(-)
diff --git a/src/main/java/com/gitblit/git/PatchsetReceivePack.java b/src/main/java/com/gitblit/git/PatchsetReceivePack.java
index 2044f60..ef0b409 100644
--- a/src/main/java/com/gitblit/git/PatchsetReceivePack.java
+++ b/src/main/java/com/gitblit/git/PatchsetReceivePack.java
@@ -51,6 +51,7 @@
import com.gitblit.Constants;
import com.gitblit.Keys;
+import com.gitblit.extensions.PatchsetHook;
import com.gitblit.manager.IGitblit;
import com.gitblit.models.RepositoryModel;
import com.gitblit.models.TicketModel;
@@ -60,6 +61,7 @@
import com.gitblit.models.TicketModel.PatchsetType;
import com.gitblit.models.TicketModel.Status;
import com.gitblit.models.UserModel;
+import com.gitblit.tickets.BranchTicketService;
import com.gitblit.tickets.ITicketService;
import com.gitblit.tickets.TicketMilestone;
import com.gitblit.tickets.TicketNotifier;
@@ -71,6 +73,7 @@
import com.gitblit.utils.JGitUtils.MergeStatus;
import com.gitblit.utils.RefLogUtils;
import com.gitblit.utils.StringUtils;
+import com.google.common.collect.Lists;
/**
@@ -105,7 +108,7 @@
protected final TicketNotifier ticketNotifier;
- private boolean requireCleanMerge;
+ private boolean requireMergeablePatchset;
public PatchsetReceivePack(IGitblit gitblit, Repository db, RepositoryModel repository, UserModel user) {
super(gitblit, db, repository, user);
@@ -148,6 +151,11 @@
LOGGER.error("failed to determine default branch for " + repository.name, e);
}
+ if (!StringUtils.isEmpty(getRepositoryModel().mergeTo)) {
+ // repository settings specifies a default integration branch
+ defaultBranch = Repository.shortenRefName(getRepositoryModel().mergeTo);
+ }
+
long ticketId = 0L;
try {
ticketId = Long.parseLong(branch);
@@ -162,11 +170,11 @@
/** Extracts the ticket id from the ref name */
private long getTicketId(String refName) {
+ if (refName.indexOf('%') > -1) {
+ refName = refName.substring(0, refName.indexOf('%'));
+ }
if (refName.startsWith(Constants.R_FOR)) {
String ref = refName.substring(Constants.R_FOR.length());
- if (ref.indexOf('%') > -1) {
- ref = ref.substring(0, ref.indexOf('%'));
- }
try {
return Long.parseLong(ref);
} catch (Exception e) {
@@ -257,12 +265,26 @@
/** Execute commands to update references. */
@Override
protected void executeCommands() {
+ // we process patchsets unless the user is pushing something special
+ boolean processPatchsets = true;
+ for (ReceiveCommand cmd : filterCommands(Result.NOT_ATTEMPTED)) {
+ if (ticketService instanceof BranchTicketService
+ && BranchTicketService.BRANCH.equals(cmd.getRefName())) {
+ // the user is pushing an update to the BranchTicketService data
+ processPatchsets = false;
+ }
+ }
+
// workaround for JGit's awful scoping choices
//
// reset the patchset refs to NOT_ATTEMPTED (see validateCommands)
for (ReceiveCommand cmd : filterCommands(Result.OK)) {
if (isPatchsetRef(cmd.getRefName())) {
cmd.setResult(Result.NOT_ATTEMPTED);
+ } else if (ticketService instanceof BranchTicketService
+ && BranchTicketService.BRANCH.equals(cmd.getRefName())) {
+ // the user is pushing an update to the BranchTicketService data
+ processPatchsets = false;
}
}
@@ -292,7 +314,8 @@
continue;
}
- if (isPatchsetRef(cmd.getRefName())) {
+ if (isPatchsetRef(cmd.getRefName()) && processPatchsets) {
+
if (ticketService == null) {
sendRejection(cmd, "Sorry, the ticket service is unavailable and can not accept patchsets at this time.");
continue;
@@ -330,10 +353,27 @@
continue;
}
+ if (cmd.getNewId().equals(ObjectId.zeroId())) {
+ // ref deletion request
+ if (cmd.getRefName().startsWith(Constants.R_TICKET)) {
+ if (user.canDeleteRef(repository)) {
+ batch.addCommand(cmd);
+ } else {
+ sendRejection(cmd, "Sorry, you do not have permission to delete {}", cmd.getRefName());
+ }
+ } else {
+ sendRejection(cmd, "Sorry, you can not delete {}", cmd.getRefName());
+ }
+ continue;
+ }
+
if (patchsetRefCmd != null) {
sendRejection(cmd, "You may only push one patchset at a time.");
continue;
}
+
+ LOGGER.info(MessageFormat.format("Verifying {0} push ref \"{1}\" received from {2}",
+ repository.name, cmd.getRefName(), user.username));
// responsible verification
String responsible = PatchsetCommand.getSingleOption(cmd, PatchsetCommand.RESPONSIBLE);
@@ -365,13 +405,18 @@
// watcher verification
List<String> watchers = PatchsetCommand.getOptions(cmd, PatchsetCommand.WATCH);
if (!ArrayUtils.isEmpty(watchers)) {
+ boolean verified = true;
for (String watcher : watchers) {
UserModel user = gitblit.getUserModel(watcher);
if (user == null) {
// watcher does not exist
sendRejection(cmd, "Sorry, \"{0}\" is not a valid username for the watch list!", watcher);
- continue;
+ verified = false;
+ break;
}
+ }
+ if (!verified) {
+ continue;
}
}
@@ -393,6 +438,8 @@
for (ReceiveCommand cmd : toApply) {
if (cmd.getResult() == Result.NOT_ATTEMPTED) {
sendRejection(cmd, "lock error: {0}", err.getMessage());
+ LOGGER.error(MessageFormat.format("failed to lock {0}:{1}",
+ repository.name, cmd.getRefName()), err);
}
}
}
@@ -412,7 +459,10 @@
patchsetRefCmd.setResult(Result.OK);
// update the ticket branch ref
- RefUpdate ru = updateRef(patchsetCmd.getTicketBranch(), patchsetCmd.getNewId());
+ RefUpdate ru = updateRef(
+ patchsetCmd.getTicketBranch(),
+ patchsetCmd.getNewId(),
+ patchsetCmd.getPatchsetType());
updateReflog(ru);
TicketModel ticket = processPatchset(patchsetCmd);
@@ -492,8 +542,10 @@
break;
}
}
- sendError("Sorry, {0} already merged {1} from ticket {2,number,0} to {3}!",
+ if (mergeChange != null) {
+ sendError("Sorry, {0} already merged {1} from ticket {2,number,0} to {3}!",
mergeChange.author, mergeChange.patchset, number, ticket.mergeTo);
+ }
sendRejection(cmd, "Ticket {0,number,0} already resolved", number);
return null;
} else if (!StringUtils.isEmpty(ticket.mergeTo)) {
@@ -539,7 +591,7 @@
case MERGEABLE:
break;
default:
- if (ticket == null || requireCleanMerge) {
+ if (ticket == null || requireMergeablePatchset) {
sendError("");
sendError("Your patchset can not be cleanly merged into {0}.", forBranch);
sendError("Please rebase your patchset and push again.");
@@ -575,14 +627,102 @@
if (patchset.commits > 1) {
sendError("");
- sendError("To create a proposal ticket, please squash your commits and");
- sendError("provide a meaningful commit message with a short title &");
- sendError("an optional description/body.");
+ sendError("You may not create a ''{0}'' branch proposal ticket from {1} commits!",
+ forBranch, patchset.commits);
sendError("");
- sendError(minTitle);
- sendError(maxTitle);
+ // display an ellipsized log of the commits being pushed
+ RevWalk walk = getRevWalk();
+ walk.reset();
+ walk.sort(RevSort.TOPO);
+ int boundary = 3;
+ int count = 0;
+ try {
+ walk.markStart(tipCommit);
+ walk.markUninteresting(mergeBase);
+
+ for (;;) {
+
+ RevCommit c = walk.next();
+ if (c == null) {
+ break;
+ }
+
+ if (count < boundary || count >= (patchset.commits - boundary)) {
+
+ walk.parseBody(c);
+ sendError(" {0} {1}", c.getName().substring(0, shortCommitIdLen),
+ StringUtils.trimString(c.getShortMessage(), 60));
+
+ } else if (count == boundary) {
+
+ sendError(" ... more commits ...");
+
+ }
+
+ count++;
+ }
+
+ } catch (IOException e) {
+ // Should never happen, the core receive process would have
+ // identified the missing object earlier before we got control.
+ LOGGER.error("failed to get commit count", e);
+ } finally {
+ walk.close();
+ }
+
sendError("");
- sendRejection(cmd, "please squash to one commit");
+ sendError("Possible Solutions:");
+ sendError("");
+ int solution = 1;
+ String forSpec = cmd.getRefName().substring(Constants.R_FOR.length());
+ if (forSpec.equals("default") || forSpec.equals("new")) {
+ try {
+ // determine other possible integration targets
+ List<String> bases = Lists.newArrayList();
+ for (Ref ref : getRepository().getRefDatabase().getRefs(Constants.R_HEADS).values()) {
+ if (!ref.getName().startsWith(Constants.R_TICKET)
+ && !ref.getName().equals(forBranchRef.getName())) {
+ if (JGitUtils.isMergedInto(getRepository(), ref.getObjectId(), tipCommit)) {
+ bases.add(Repository.shortenRefName(ref.getName()));
+ }
+ }
+ }
+
+ if (!bases.isEmpty()) {
+
+ if (bases.size() == 1) {
+ // suggest possible integration targets
+ String base = bases.get(0);
+ sendError("{0}. Propose this change for the ''{1}'' branch.", solution++, base);
+ sendError("");
+ sendError(" git push origin HEAD:refs/for/{0}", base);
+ sendError(" pt propose {0}", base);
+ sendError("");
+ } else {
+ // suggest possible integration targets
+ sendError("{0}. Propose this change for a different branch.", solution++);
+ sendError("");
+ for (String base : bases) {
+ sendError(" git push origin HEAD:refs/for/{0}", base);
+ sendError(" pt propose {0}", base);
+ sendError("");
+ }
+ }
+
+ }
+ } catch (IOException e) {
+ LOGGER.error(null, e);
+ }
+ }
+ sendError("{0}. Squash your changes into a single commit with a meaningful message.", solution++);
+ sendError("");
+ sendError("{0}. Open a ticket for your changes and then push your {1} commits to the ticket.",
+ solution++, patchset.commits);
+ sendError("");
+ sendError(" git push origin HEAD:refs/for/{id}");
+ sendError(" pt propose {id}");
+ sendError("");
+ sendRejection(cmd, "too many commits");
return null;
}
@@ -647,8 +787,7 @@
sendError(" 1. you created the ticket");
sendError(" 2. you created the first patchset");
sendError(" 3. you are specified as responsible for the ticket");
- sendError(" 4. you are listed as a reviewer for the ticket");
- sendError(" 5. you have push (RW) permission to {0}", repository.name);
+ sendError(" 4. you have push (RW) permissions to {0}", repository.name);
sendError("");
sendRejection(cmd, "not permitted to push to ticket {0,number,0}", ticket.number);
return null;
@@ -689,6 +828,15 @@
RefLogUtils.updateRefLog(user, getRepository(),
Arrays.asList(new ReceiveCommand(cmd.getOldId(), cmd.getNewId(), cmd.getRefName())));
+ // call any patchset hooks
+ for (PatchsetHook hook : gitblit.getExtensions(PatchsetHook.class)) {
+ try {
+ hook.onNewPatchset(ticket);
+ } catch (Exception e) {
+ LOGGER.error("Failed to execute extension", e);
+ }
+ }
+
return ticket;
} else {
sendError("FAILED to create ticket");
@@ -715,6 +863,20 @@
// log the new patchset ref
RefLogUtils.updateRefLog(user, getRepository(),
Arrays.asList(new ReceiveCommand(cmd.getOldId(), cmd.getNewId(), cmd.getRefName())));
+
+ // call any patchset hooks
+ final boolean isNewPatchset = change.patchset.rev == 1;
+ for (PatchsetHook hook : gitblit.getExtensions(PatchsetHook.class)) {
+ try {
+ if (isNewPatchset) {
+ hook.onNewPatchset(ticket);
+ } else {
+ hook.onUpdatePatchset(ticket);
+ }
+ } catch (Exception e) {
+ LOGGER.error("Failed to execute extension", e);
+ }
+ }
// return the updated ticket
return ticket;
@@ -751,6 +913,9 @@
}
TicketModel ticket = ticketService.getTicket(repository, ticketNumber);
+ if (ticket == null) {
+ continue;
+ }
String integrationBranch;
if (StringUtils.isEmpty(ticket.mergeTo)) {
// unspecified integration branch
@@ -808,8 +973,8 @@
psCmd.updateTicket(c, mergeTo, ticket, null);
// create a ticket patchset ref
- updateRef(psCmd.getPatchsetBranch(), c.getId());
- RefUpdate ru = updateRef(psCmd.getTicketBranch(), c.getId());
+ updateRef(psCmd.getPatchsetBranch(), c.getId(), patchset.type);
+ RefUpdate ru = updateRef(psCmd.getTicketBranch(), c.getId(), patchset.type);
updateReflog(ru);
// create a change from the patchset command
@@ -870,11 +1035,20 @@
if (parseMessage) {
// parse commit message looking for fixes/closes #n
- Pattern p = Pattern.compile("(?:fixes|closes)[\\s-]+#?(\\d+)", Pattern.CASE_INSENSITIVE);
- Matcher m = p.matcher(commit.getFullMessage());
- while (m.find()) {
- String val = m.group();
- return Long.parseLong(val);
+ String dx = "(?:fixes|closes)[\\s-]+#?(\\d+)";
+ String x = settings.getString(Keys.tickets.closeOnPushCommitMessageRegex, dx);
+ if (StringUtils.isEmpty(x)) {
+ x = dx;
+ }
+ try {
+ Pattern p = Pattern.compile(x, Pattern.CASE_INSENSITIVE);
+ Matcher m = p.matcher(commit.getFullMessage());
+ while (m.find()) {
+ String val = m.group(1);
+ return Long.parseLong(val);
+ }
+ } catch (Exception e) {
+ LOGGER.error(String.format("Failed to parse \"%s\" in commit %s", x, commit.getName()), e);
}
}
return 0L;
@@ -904,7 +1078,7 @@
LOGGER.error("failed to get commit count", e);
return 0;
} finally {
- walk.release();
+ walk.close();
}
return count;
}
@@ -1016,7 +1190,7 @@
return newPatchset;
}
- private RefUpdate updateRef(String ref, ObjectId newId) {
+ private RefUpdate updateRef(String ref, ObjectId newId, PatchsetType type) {
ObjectId ticketRefId = ObjectId.zeroId();
try {
ticketRefId = getRepository().resolve(ref);
@@ -1027,7 +1201,17 @@
try {
RefUpdate ru = getRepository().updateRef(ref, false);
ru.setRefLogIdent(getRefLogIdent());
- ru.setForceUpdate(true);
+ switch (type) {
+ case Amend:
+ case Rebase:
+ case Rebase_Squash:
+ case Squash:
+ ru.setForceUpdate(true);
+ break;
+ default:
+ break;
+ }
+
ru.setExpectedOldObjectId(ticketRefId);
ru.setNewObjectId(newId);
RefUpdate.Result result = ru.update(getRevWalk());
@@ -1112,11 +1296,24 @@
if (ticket != null) {
ticketNotifier.queueMailing(ticket);
- // update the reflog with the merge
if (oldRef != null) {
ReceiveCommand cmd = new ReceiveCommand(oldRef.getObjectId(),
ObjectId.fromString(mergeResult.sha), oldRef.getName());
- RefLogUtils.updateRefLog(user, getRepository(), Arrays.asList(cmd));
+ cmd.setResult(Result.OK);
+ List<ReceiveCommand> commands = Arrays.asList(cmd);
+
+ logRefChange(commands);
+ updateIncrementalPushTags(commands);
+ updateGitblitRefLog(commands);
+ }
+
+ // call patchset hooks
+ for (PatchsetHook hook : gitblit.getExtensions(PatchsetHook.class)) {
+ try {
+ hook.onMergePatchset(ticket);
+ } catch (Exception e) {
+ LOGGER.error("Failed to execute extension", e);
+ }
}
return mergeResult.status;
} else {
--
Gitblit v1.9.1