diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt index a9deba8541..46aa535a53 100644 --- a/Documentation/user-upload.txt +++ b/Documentation/user-upload.txt @@ -169,6 +169,23 @@ apply multiple review labels. The value is optional. If not specified, it defaults to +1 (if the label range allows it). +[[change_edit]] +A change edit can be pushed by specifying the `edit` (or `e`) option on +the reference: + +==== + git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%edit +==== + +There is at most one change edit per user and change. In order to push +a change edit the change must already exist. + +[NOTE] +When a change edit already exists for a change then pushing with +`%edit` replaces the existing change edit. This option is useful to +rebase a change edit on the newest patch set when the rebase of the +change edit in the web UI fails due to conflicts. + If you are frequently uploading changes to the same Gerrit server, consider adding an SSH host block in `~/.ssh/config` to remember your username, hostname and port number. This permits the use of diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 84c2674039..c59b3d9669 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -31,6 +31,7 @@ import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.changes.RevisionApi; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.EditInfo; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Project; @@ -222,11 +223,16 @@ public abstract class AbstractDaemonTest { Chars.asList(new char[]{'a','b','c','d','e','f','g','h'}); protected PushOneCommit.Result amendChange(String changeId) throws GitAPIException, IOException { + return amendChange(changeId, "refs/for/master"); + } + + protected PushOneCommit.Result amendChange(String changeId, String ref) + throws GitAPIException, IOException { Collections.shuffle(RANDOM); PushOneCommit push = pushFactory.create(db, admin.getIdent(), PushOneCommit.SUBJECT, PushOneCommit.FILE_NAME, new String(Chars.toArray(RANDOM)), changeId); - return push.to(git, "refs/for/master"); + return push.to(git, ref); } protected ChangeInfo getChange(String changeId, ListChangesOption... options) @@ -252,6 +258,11 @@ public abstract class AbstractDaemonTest { return gApi.changes().id(id).get(); } + protected EditInfo getEdit(String id) + throws RestApiException { + return gApi.changes().id(id).getEdit(); + } + protected ChangeInfo get(String id, ListChangesOption... options) throws RestApiException { EnumSet s = EnumSet.noneOf(ListChangesOption.class); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 9887c38b5a..1d21cad114 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -23,6 +23,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.EditInfo; import com.google.gerrit.extensions.common.LabelInfo; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Change; @@ -162,6 +163,21 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { r.assertChange(Change.Status.DRAFT, null); } + @Test + public void testPushForMasterAsEdit() throws GitAPIException, + IOException, RestApiException { + PushOneCommit.Result r = pushTo("refs/for/master"); + r.assertOkStatus(); + EditInfo edit = getEdit(r.getChangeId()); + assertThat(edit).isNull(); + + // specify edit as option + r = amendChange(r.getChangeId(), "refs/for/master%edit"); + r.assertOkStatus(); + edit = getEdit(r.getChangeId()); + assertThat(edit).isNotNull(); + } + @Test public void testPushForMasterWithApprovals() throws GitAPIException, IOException, RestApiException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 9a23293f14..b6c6122092 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -91,12 +91,26 @@ public class ChangeEditUtil { */ public Optional byChange(Change change) throws AuthException, IOException { - if (!user.get().isIdentifiedUser()) { + CurrentUser currentUser = user.get(); + if (!currentUser.isIdentifiedUser()) { throw new AuthException("Authentication required"); } + return byChange(change, (IdentifiedUser)currentUser); + } + + /** + * Retrieve edits for a change and user. Max. one change edit can + * exist per user and change. + * + * @param change + * @param user to retrieve change edits for + * @return edit for this change for this user, if present. + * @throws IOException + */ + public Optional byChange(Change change, IdentifiedUser me) + throws IOException { Repository repo = gitManager.openRepository(change.getProject()); try { - IdentifiedUser me = (IdentifiedUser) user.get(); String editRefPrefix = editRefPrefix(me.getAccountId(), change.getId()); Map refs = repo.getRefDatabase().getRefs(editRefPrefix); if (refs.isEmpty()) { @@ -201,7 +215,7 @@ public class ChangeEditUtil { * @param psId patch set number * @return reference for this change edit */ - static String editRefName(Account.Id accountId, Change.Id changeId, + public static String editRefName(Account.Id accountId, Change.Id changeId, PatchSet.Id psId) { return editRefPrefix(accountId, changeId) + psId.get(); } 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 cc2330b2c0..99d0542ab5 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 @@ -30,6 +30,7 @@ import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_RE import com.google.common.base.Function; import com.google.common.base.Joiner; +import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.base.Splitter; import com.google.common.base.Strings; @@ -90,6 +91,8 @@ import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.ProjectConfigEntry; +import com.google.gerrit.server.edit.ChangeEdit; +import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.events.CommitReceivedEvent; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.MultiProgressMonitor.Task; @@ -326,6 +329,7 @@ public class ReceiveCommits { private final MergeQueue mergeQueue; private final DynamicMap pluginConfigEntries; private final NotesMigration notesMigration; + private final ChangeEditUtil editUtil; private final List messages = new ArrayList<>(); private ListMultimap errors = LinkedListMultimap.create(); @@ -375,7 +379,8 @@ public class ReceiveCommits { final MergeQueue mergeQueue, final ChangeKindCache changeKindCache, final DynamicMap pluginConfigEntries, - final NotesMigration notesMigration) throws IOException { + final NotesMigration notesMigration, + final ChangeEditUtil editUtil) throws IOException { this.currentUser = (IdentifiedUser) projectControl.getCurrentUser(); this.db = db; this.queryProvider = queryProvider; @@ -422,6 +427,8 @@ public class ReceiveCommits { this.pluginConfigEntries = pluginConfigEntries; this.notesMigration = notesMigration; + this.editUtil = editUtil; + this.messageSender = new ReceivePackMessageSender(); ProjectState ps = projectControl.getProjectState(); @@ -560,6 +567,7 @@ public class ReceiveCommits { if (!batch.getCommands().isEmpty()) { try { + batch.setAllowNonFastForwards(magicBranch != null && magicBranch.edit); batch.execute(rp.getRevWalk(), commandProgress); } catch (IOException err) { int cnt = 0; @@ -658,7 +666,7 @@ public class ReceiveCommits { addMessage(""); addMessage("New Changes:"); for (CreateRequest c : created) { - addMessage(formatChangeUrl(canonicalWebUrl, c.change)); + addMessage(formatChangeUrl(canonicalWebUrl, c.change, false)); } addMessage(""); } @@ -681,14 +689,16 @@ public class ReceiveCommits { if (!updated.isEmpty()) { addMessage(""); addMessage("Updated Changes:"); + boolean edit = magicBranch != null && magicBranch.edit; for (ReplaceRequest u : updated) { - addMessage(formatChangeUrl(canonicalWebUrl, u.change)); + addMessage(formatChangeUrl(canonicalWebUrl, u.change, edit)); } addMessage(""); } } - private static String formatChangeUrl(String url, Change change) { + private static String formatChangeUrl(String url, Change change, + boolean edit) { StringBuilder m = new StringBuilder() .append(" ") .append(url) @@ -698,6 +708,9 @@ public class ReceiveCommits { if (change.getStatus() == Change.Status.DRAFT) { m.append(" [DRAFT]"); } + if (edit) { + m.append(" [EDIT]"); + } return m.toString(); } @@ -1118,6 +1131,9 @@ public class ReceiveCommits { @Option(name = "--draft", usage = "mark new/updated changes as draft") boolean draft; + @Option(name = "--edit", aliases = {"-e"}, usage = "upload as change edit") + boolean edit; + @Option(name = "--submit", usage = "immediately submit the change") boolean submit; @@ -1583,6 +1599,10 @@ public class ReceiveCommits { reject(magicBranch.cmd, "no new changes"); return; } + if (!newChanges.isEmpty() && magicBranch.edit) { + reject(magicBranch.cmd, "edit is not supported for new changes"); + return; + } for (CreateRequest create : newChanges) { batch.addCommand(create.cmd); } @@ -1776,6 +1796,9 @@ public class ReceiveCommits { for (ReplaceRequest req : replaceByChange.values()) { if (req.inputCommand.getResult() == NOT_ATTEMPTED && req.cmd != null) { + if (req.prev != null) { + batch.addCommand(req.prev); + } batch.addCommand(req.cmd); } } @@ -1816,6 +1839,7 @@ public class ReceiveCommits { ChangeControl changeCtl; BiMap revisions; PatchSet newPatchSet; + ReceiveCommand prev; ReceiveCommand cmd; PatchSetInfo info; ChangeMessage msg; @@ -1938,6 +1962,59 @@ public class ReceiveCommits { } } + if (magicBranch != null && magicBranch.edit) { + return newEdit(); + } + + newPatchSet(); + return true; + } + + private boolean newEdit() { + newPatchSet = new PatchSet(change.currentPatchSetId()); + Optional edit = null; + + try { + edit = editUtil.byChange(change, currentUser); + } catch (IOException e) { + log.error("Cannt retrieve edit", e); + return false; + } + + if (edit.isPresent()) { + if (edit.get().getBasePatchSet().getId().equals(newPatchSet.getId())) { + // replace edit + cmd = new ReceiveCommand( + edit.get().getRef().getObjectId(), + newCommit, + edit.get().getRefName()); + } else { + // delete old edit ref on rebase + prev = new ReceiveCommand( + edit.get().getRef().getObjectId(), + ObjectId.zeroId(), + edit.get().getRefName()); + createEditCommand(); + } + } else { + createEditCommand(); + } + + return true; + } + + private void createEditCommand() { + // create new edit + cmd = new ReceiveCommand( + ObjectId.zeroId(), + newCommit, + ChangeEditUtil.editRefName( + currentUser.getAccountId(), + change.getId(), + newPatchSet.getId())); + } + + private void newPatchSet() { PatchSet.Id id = ChangeUtil.nextPatchSetId(allRefs, change.currentPatchSetId()); newPatchSet = new PatchSet(id); @@ -1952,7 +2029,6 @@ public class ReceiveCommits { ObjectId.zeroId(), newCommit, newPatchSet.getRefName()); - return true; } CheckedFuture insertPatchSet() @@ -1965,7 +2041,9 @@ public class ReceiveCommits { @Override public PatchSet.Id call() throws OrmException, IOException, NoSuchChangeException { try { - if (caller == Thread.currentThread()) { + if (magicBranch.edit) { + return upsertEdit(); + } else if (caller == Thread.currentThread()) { return insertPatchSet(db); } else { ReviewDb db = schemaFactory.open(); @@ -2007,6 +2085,13 @@ public class ReceiveCommits { return msg; } + PatchSet.Id upsertEdit() { + if (cmd.getResult() == NOT_ATTEMPTED) { + cmd.execute(rp); + } + return newPatchSet.getId(); + } + PatchSet.Id insertPatchSet(ReviewDb db) throws OrmException, IOException { final Account.Id me = currentUser.getAccountId(); final List footerLines = newCommit.getFooterLines();