InlineEdit: Allow to upload change edit with git push

Edits can already be pulled with download commands. This change allows
edit to be pushed to gerrit by using magic branch %edit option.

Change must already exist for edit to be created/replaced. One use case
where edit might need to be replaced with git operations is rebasing of
edits on new patch set, in which case two refs are affected: old ref is
deleted and new ref is created:

  $ git push g HEAD:refs/for/master%e
  Counting objects: 14, done.
  [...]
  remote: Processing changes: updated: 1, refs: 2, done
  remote:
  remote: Updated Changes:
  remote:   http://localhost:8080/42 Fix typo in the doc [EDIT]

Change-Id: I3dbf66f1700c212ab90acfa77a5bd1e774b94555
This commit is contained in:
David Ostrovsky 2015-01-19 07:43:44 +01:00
parent ba5c957fae
commit d07bb33996
5 changed files with 153 additions and 10 deletions

View File

@ -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

View File

@ -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<ListChangesOption> s = EnumSet.noneOf(ListChangesOption.class);

View File

@ -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 {

View File

@ -91,12 +91,26 @@ public class ChangeEditUtil {
*/
public Optional<ChangeEdit> 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<ChangeEdit> 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<String, Ref> 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();
}

View File

@ -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<ProjectConfigEntry> pluginConfigEntries;
private final NotesMigration notesMigration;
private final ChangeEditUtil editUtil;
private final List<CommitValidationMessage> messages = new ArrayList<>();
private ListMultimap<Error, String> errors = LinkedListMultimap.create();
@ -375,7 +379,8 @@ public class ReceiveCommits {
final MergeQueue mergeQueue,
final ChangeKindCache changeKindCache,
final DynamicMap<ProjectConfigEntry> 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<RevCommit, PatchSet.Id> 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<ChangeEdit> 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<PatchSet.Id, InsertException> 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<FooterLine> footerLines = newCommit.getFooterLines();