Added support for changing parent revision during rebase
It is a common use-case of configuration managers (CMs) to restructure
a sequence of patches towards a target branch.
This patch allows restructuring of patches from the UI and REST API
using the rebase action.
It is now possible to:
- Introduce a new dependency towards another change.
- Remove dependency towards another change.
There is a non-obvious limitation regarding the parent revisions: It has
to be a valid patch set towards the same target branch.
Change-Id: I882b16a929b2ce0c66b1a6d9b64947220bb46d0b
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index f508a83..49ebb49 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -852,9 +852,17 @@
Rebases a change.
+Optionally, the parent revision can be changed to another patch set through the
+link:#rebase-input[RebaseInput] entity.
+
.Request
----
POST /changes/myProject~master~I3ea943139cb62e86071996f2480e58bf3eeb9dd2/rebase HTTP/1.0
+ Content-Type: application/json;charset=UTF-8
+
+ {
+ "base" : "1234",
+ }
----
As response a link:#change-info[ChangeInfo] entity is returned that
@@ -2200,9 +2208,17 @@
Rebases a revision.
+Optionally, the parent revision can be changed to another patch set through the
+link:#rebase-input[RebaseInput] entity.
+
.Request
----
POST /changes/myProject~master~I3ea943139cb62e86071996f2480e58bf3eeb9dd2/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/rebase HTTP/1.0
+ Content-Type: application/json;charset=UTF-8
+
+ {
+ "base" : "1234",
+ }
----
As response a link:#change-info[ChangeInfo] entity is returned that
@@ -3877,6 +3893,21 @@
outcome of the fix.
|===========================
+[[rebase-input]]
+=== RebaseInput
+The `RebaseInput` entity contains information for changing parent when rebasing.
+
+[options="header",width="50%",cols="1,^1,5"]
+|===========================
+|Field Name ||Description
+|`base` |optional|
+The new parent revision. This can be a ref or a SHA1 to a concrete patchset. +
+Alternatively, a change number can be specified, in which case the current
+patch set is inferred. +
+Empty string is used for rebasing directly on top of the target branch,
+which effectively breaks dependency towards a parent change.
+|===========================
+
[[related-change-and-commit-info]]
=== RelatedChangeAndCommitInfo
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
index 428a19c..74f9f80 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -215,6 +215,10 @@
queryProvider.get().byKeyPrefix(commit.getChangeId()));
}
+ public PatchSet getPatchSet() throws OrmException {
+ return getChange().currentPatchSet();
+ }
+
public PatchSet.Id getPatchSetId() throws OrmException {
return getChange().change().currentPatchSetId();
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 94d564d..86c2c1f 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -23,6 +23,7 @@
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.RebaseInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ListChangesOption;
@@ -32,6 +33,7 @@
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.PatchSet;
import org.eclipse.jgit.lib.Constants;
import org.junit.Test;
@@ -110,6 +112,49 @@
.rebase();
}
+ @Test
+ public void rebaseChangeBase() throws Exception {
+ PushOneCommit.Result r1 = createChange();
+ PushOneCommit.Result r2 = createChange();
+ PushOneCommit.Result r3 = createChange();
+ RebaseInput ri = new RebaseInput();
+
+ // rebase r3 directly onto master (break dep. towards r2)
+ ri.base = "";
+ gApi.changes()
+ .id(r3.getChangeId())
+ .revision(r3.getCommit().name())
+ .rebase(ri);
+ PatchSet ps3 = r3.getPatchSet();
+ assertThat(ps3.getId().get()).is(2);
+
+ // rebase r2 onto r3 (referenced by ref)
+ ri.base = ps3.getId().toRefName();
+ gApi.changes()
+ .id(r2.getChangeId())
+ .revision(r2.getCommit().name())
+ .rebase(ri);
+ PatchSet ps2 = r2.getPatchSet();
+ assertThat(ps2.getId().get()).is(2);
+
+ // rebase r1 onto r2 (referenced by commit)
+ ri.base = ps2.getRevision().get();
+ gApi.changes()
+ .id(r1.getChangeId())
+ .revision(r1.getCommit().name())
+ .rebase(ri);
+ PatchSet ps1 = r1.getPatchSet();
+ assertThat(ps1.getId().get()).is(2);
+
+ // rebase r1 onto r3 (referenced by change number)
+ ri.base = String.valueOf(r3.getChange().getId().get());
+ gApi.changes()
+ .id(r1.getChangeId())
+ .revision(ps1.getRevision().get())
+ .rebase(ri);
+ assertThat(r1.getPatchSetId().get()).is(3);
+ }
+
private Set<Account.Id> getReviewers(String changeId) throws Exception {
ChangeInfo ci = gApi.changes().id(changeId).get();
Set<Account.Id> result = Sets.newHashSet();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 4acec9a..afcd5d0 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -43,7 +43,8 @@
String changeId = createChangeWithTopic("foo1").getChangeId();
Map<String, ActionInfo> actions = getActions(changeId);
assertThat(actions).containsKey("cherrypick");
- assertThat(actions).hasSize(1);
+ assertThat(actions).containsKey("rebase");
+ assertThat(actions).hasSize(2);
}
@Test
@@ -51,8 +52,7 @@
String changeId = createChangeWithTopic("foo1").getChangeId();
approve(changeId);
Map<String, ActionInfo> actions = getActions(changeId);
- assertThat(actions).containsKey("cherrypick");
- assertThat(actions).containsKey("submit");
+ commonActionsAssertions(actions);
if (isSubmitWholeTopicEnabled()) {
ActionInfo info = actions.get("submit");
assertThat(info.enabled).isTrue();
@@ -62,8 +62,6 @@
} else {
noSubmitWholeTopicAssertions(actions);
}
- // no other actions
- assertThat(actions).hasSize(2);
}
@Test
@@ -73,10 +71,7 @@
// create another change with the same topic
createChangeWithTopic("foo2").getChangeId();
Map<String, ActionInfo> actions = getActions(changeId);
- assertThat(actions).containsKey("cherrypick");
- assertThat(actions).containsKey("submit");
- // no other actions:
- assertThat(actions).hasSize(2);
+ commonActionsAssertions(actions);
if (isSubmitWholeTopicEnabled()) {
ActionInfo info = actions.get("submit");
assertThat(info.enabled).isNull();
@@ -96,10 +91,7 @@
String changeId2 = createChangeWithTopic("foo2").getChangeId();
approve(changeId2);
Map<String, ActionInfo> actions = getActions(changeId);
- assertThat(actions).containsKey("cherrypick");
- assertThat(actions).containsKey("submit");
- // no other actions:
- assertThat(actions).hasSize(2);
+ commonActionsAssertions(actions);
if (isSubmitWholeTopicEnabled()) {
ActionInfo info = actions.get("submit");
assertThat(info.enabled).isTrue();
@@ -128,6 +120,13 @@
assertThat(info.title).isEqualTo("Submit patch set 1 into master");
}
+ private void commonActionsAssertions(Map<String, ActionInfo> actions) {
+ assertThat(actions).hasSize(3);
+ assertThat(actions).containsKey("cherrypick");
+ assertThat(actions).containsKey("submit");
+ assertThat(actions).containsKey("rebase");
+ }
+
private PushOneCommit.Result createChangeWithTopic(String topic) throws GitAPIException,
IOException {
PushOneCommit push = pushFactory.create(db, admin.getIdent());
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RebaseInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RebaseInput.java
new file mode 100644
index 0000000..5f4a014
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RebaseInput.java
@@ -0,0 +1,19 @@
+// Copyright (C) 2015 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.extensions.api.changes;
+
+public class RebaseInput {
+ public String base;
+}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 202699c..b940cc9 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -34,6 +34,7 @@
void publish() throws RestApiException;
ChangeApi cherryPick(CherryPickInput in) throws RestApiException;
ChangeApi rebase() throws RestApiException;
+ ChangeApi rebase(RebaseInput in) throws RestApiException;
boolean canRebase();
void setReviewed(String path, boolean reviewed) throws RestApiException;
@@ -94,6 +95,11 @@
}
@Override
+ public ChangeApi rebase(RebaseInput in) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public boolean canRebase() {
throw new NotImplementedException();
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
index c919c6b..2844b5e 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
@@ -158,6 +158,8 @@
String projectFilterLabel();
String projectFilterPanel();
String projectNameColumn();
+ String rebaseContentPanel();
+ String rebaseSuggestBox();
String registerScreenExplain();
String registerScreenNextLinks();
String registerScreenSection();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java
index 93881d2..37b90c4 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java
@@ -176,7 +176,7 @@
@UiHandler("rebase")
void onRebase(@SuppressWarnings("unused") ClickEvent e) {
- RebaseAction.call(changeId, revision);
+ RebaseAction.call(rebase, project, changeInfo.branch(), changeId, revision);
}
@UiHandler("submit")
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RebaseAction.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RebaseAction.java
index b1ad583..ffe9627 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RebaseAction.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RebaseAction.java
@@ -18,17 +18,42 @@
import com.google.gerrit.client.changes.ChangeApi;
import com.google.gerrit.client.changes.ChangeInfo;
import com.google.gerrit.client.rpc.GerritCallback;
+import com.google.gerrit.client.ui.RebaseDialog;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gwt.event.logical.shared.CloseEvent;
+import com.google.gwt.user.client.ui.Button;
+import com.google.gwt.user.client.ui.PopupPanel;
class RebaseAction {
- static void call(final Change.Id id, String revision) {
- ChangeApi.rebase(id.get(), revision,
- new GerritCallback<ChangeInfo>() {
- @Override
- public void onSuccess(ChangeInfo result) {
- Gerrit.display(PageLinks.toChange(id));
- }
- });
+ static void call(final Button b, final String project, final String branch,
+ final Change.Id id, final String revision) {
+ b.setEnabled(false);
+
+ new RebaseDialog(project, branch, id) {
+ @Override
+ public void onSend() {
+ ChangeApi.rebase(id.get(), revision, getBase(), new GerritCallback<ChangeInfo>() {
+ @Override
+ public void onSuccess(ChangeInfo result) {
+ sent = true;
+ hide();
+ Gerrit.display(PageLinks.toChange(id));
+ }
+
+ @Override
+ public void onFailure(Throwable caught) {
+ enableButtons(true);
+ super.onFailure(caught);
+ }
+ });
+ }
+
+ @Override
+ public void onClose(CloseEvent<PopupPanel> event) {
+ super.onClose(event);
+ b.setEnabled(true);
+ }
+ }.center();
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevertAction.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevertAction.java
index f9c18cd..3af97f5 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevertAction.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevertAction.java
@@ -19,7 +19,7 @@
import com.google.gerrit.client.changes.ChangeInfo;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.rpc.GerritCallback;
-import com.google.gerrit.client.ui.CommentedActionDialog;
+import com.google.gerrit.client.ui.TextAreaActionDialog;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gwt.user.client.ui.Button;
@@ -29,7 +29,7 @@
final String commitSubject) {
// TODO Replace ActionDialog with a nicer looking display.
b.setEnabled(false);
- new CommentedActionDialog(
+ new TextAreaActionDialog(
Util.C.revertChangeTitle(),
Util.C.headingRevertMessage()) {
{
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
index 0a29a00..1135491 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
@@ -216,10 +216,11 @@
change(id).view("edit:rebase").post(in, cb);
}
- /** Rebase a revision onto the branch tip. */
- public static void rebase(int id, String commit, AsyncCallback<ChangeInfo> cb) {
- JavaScriptObject in = JavaScriptObject.createObject();
- call(id, commit, "rebase").post(in, cb);
+ /** Rebase a revision onto the branch tip or another change. */
+ public static void rebase(int id, String commit, String base, AsyncCallback<ChangeInfo> cb) {
+ RebaseInput rebaseInput = RebaseInput.create();
+ rebaseInput.setBase(base);
+ call(id, commit, "rebase").post(rebaseInput, cb);
}
private static class Input extends JavaScriptObject {
@@ -260,6 +261,17 @@
}
}
+ private static class RebaseInput extends JavaScriptObject {
+ final native void setBase(String b) /*-{ this.base = b; }-*/;
+
+ static RebaseInput create() {
+ return (RebaseInput) createObject();
+ }
+
+ protected RebaseInput() {
+ }
+ }
+
private static class SubmitInput extends JavaScriptObject {
final native void wait_for_merge(boolean b) /*-{ this.wait_for_merge=b; }-*/;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
index 2bbb429..00c4404 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
@@ -164,6 +164,11 @@
String cherryPickCommitMessage();
String cherryPickTitle();
+ String buttonRebaseChangeSend();
+ String rebaseConfirmMessage();
+ String rebasePlaceholderMessage();
+ String rebaseTitle();
+
String buttonAbandonChangeBegin();
String buttonAbandonChangeSend();
String headingAbandonMessage();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
index df1e2c5..9bf3f39 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
@@ -150,6 +150,11 @@
cherryPickCommitMessage = Cherry Pick Commit Message:
cherryPickTitle = Code Review - Cherry Pick Change to Another Branch
+buttonRebaseChangeSend = Rebase
+rebaseConfirmMessage = Change parent revision
+rebasePlaceholderMessage = (subject, change number, or leave empty)
+rebaseTitle = Code Review - Rebase Change
+
buttonRestoreChangeBegin = Restore Change
restoreChangeTitle = Code Review - Restore Change
headingRestoreMessage = Restore Message:
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css b/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
index 0d0f793..8d961ca 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
@@ -1162,6 +1162,16 @@
white-space: nowrap;
font-size: small;
}
+.commentedActionDialog .rebaseContentPanel {
+ margin-left: 10px;
+ background: trimColor;
+ padding: 5px 5px 5px 5px;
+ width: 300px;
+}
+.commentedActionDialog .rebaseContentPanel .rebaseSuggestBox {
+ font-size: small;
+ width: 100%;
+}
/** PatchBrowserPopup **/
.patchBrowserPopup {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CherryPickDialog.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CherryPickDialog.java
index 3813af2..00269f9 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CherryPickDialog.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CherryPickDialog.java
@@ -31,7 +31,7 @@
import java.util.LinkedList;
import java.util.List;
-public abstract class CherryPickDialog extends CommentedActionDialog {
+public abstract class CherryPickDialog extends TextAreaActionDialog {
private SuggestBox newBranch;
private List<BranchInfo> branches;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CommentedActionDialog.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CommentedActionDialog.java
index aa32394..60b5f93 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CommentedActionDialog.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CommentedActionDialog.java
@@ -24,16 +24,15 @@
import com.google.gwt.user.client.ui.FocusWidget;
import com.google.gwt.user.client.ui.PopupPanel;
import com.google.gwtexpui.globalkey.client.GlobalKey;
-import com.google.gwtexpui.globalkey.client.NpTextArea;
import com.google.gwtexpui.user.client.AutoCenterDialogBox;
public abstract class CommentedActionDialog extends AutoCenterDialogBox
implements CloseHandler<PopupPanel> {
protected final FlowPanel panel;
- protected final NpTextArea message;
protected final Button sendButton;
protected final Button cancelButton;
protected final FlowPanel buttonPanel;
+ protected final FlowPanel contentPanel;
protected FocusWidget focusOn;
protected boolean sent = false;
@@ -45,11 +44,6 @@
addStyleName(Gerrit.RESOURCES.css().commentedActionDialog());
- message = new NpTextArea();
- message.setCharacterWidth(60);
- message.setVisibleLines(10);
- message.getElement().setPropertyBoolean("spellcheck", true);
- setFocusOn(message);
sendButton = new Button(Util.C.commentedActionButtonSend());
sendButton.addClickHandler(new ClickHandler() {
@Override
@@ -68,9 +62,8 @@
}
});
- final FlowPanel mwrap = new FlowPanel();
- mwrap.setStyleName(Gerrit.RESOURCES.css().commentedActionMessage());
- mwrap.add(message);
+ contentPanel = new FlowPanel();
+ contentPanel.setStyleName(Gerrit.RESOURCES.css().commentedActionMessage());
buttonPanel = new FlowPanel();
buttonPanel.add(sendButton);
@@ -78,8 +71,10 @@
buttonPanel.getElement().getStyle().setProperty("marginTop", "4px");
panel = new FlowPanel();
- panel.add(new SmallHeading(heading));
- panel.add(mwrap);
+ if (heading != null) {
+ panel.add(new SmallHeading(heading));
+ }
+ panel.add(contentPanel);
panel.add(buttonPanel);
add(panel);
@@ -110,8 +105,4 @@
}
public abstract void onSend();
-
- public String getMessageText() {
- return message.getText().trim();
- }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CreateChangeDialog.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CreateChangeDialog.java
index 2b6a866..021f39c 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CreateChangeDialog.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CreateChangeDialog.java
@@ -31,7 +31,7 @@
import java.util.ArrayList;
import java.util.List;
-public abstract class CreateChangeDialog extends CommentedActionDialog {
+public abstract class CreateChangeDialog extends TextAreaActionDialog {
private SuggestBox newChange;
private List<BranchInfo> branches;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/RebaseDialog.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/RebaseDialog.java
new file mode 100644
index 0000000..58f806a
--- /dev/null
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/RebaseDialog.java
@@ -0,0 +1,119 @@
+// Copyright (C) 2015 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.client.ui;
+
+import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.client.changes.ChangeInfo;
+import com.google.gerrit.client.changes.ChangeList;
+import com.google.gerrit.client.changes.Util;
+import com.google.gerrit.client.rpc.GerritCallback;
+import com.google.gerrit.client.rpc.Natives;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gwt.event.dom.client.ClickHandler;
+import com.google.gwt.event.dom.client.ClickEvent;
+import com.google.gwt.user.client.ui.CheckBox;
+import com.google.gwt.user.client.ui.SuggestBox;
+import com.google.gwt.user.client.ui.SuggestOracle.Suggestion;
+import com.google.gwtexpui.safehtml.client.HighlightSuggestOracle;
+
+import java.util.LinkedList;
+import java.util.List;
+
+public abstract class RebaseDialog extends CommentedActionDialog {
+ private final SuggestBox base;
+ private final CheckBox cb;
+ private List<ChangeInfo> changes;
+
+ public RebaseDialog(final String project, final String branch,
+ final Change.Id changeId) {
+ super(Util.C.rebaseTitle(), null);
+ sendButton.setText(Util.C.buttonRebaseChangeSend());
+
+ // create the suggestion box
+ base = new SuggestBox(new HighlightSuggestOracle() {
+ @Override
+ protected void onRequestSuggestions(Request request, Callback done) {
+ String query = request.getQuery().toLowerCase();
+ LinkedList<ChangeSuggestion> suggestions = new LinkedList<>();
+ for (final ChangeInfo ci : changes) {
+ if (changeId.equals(ci.legacy_id())) {
+ continue; // do not suggest current change
+ }
+ String id = String.valueOf(ci.legacy_id().get());
+ if (id.contains(query) || ci.subject().toLowerCase().contains(query)) {
+ suggestions.add(new ChangeSuggestion(ci));
+ if (suggestions.size() >= 50) { // limit to 50 suggestions
+ break;
+ }
+ }
+ }
+ done.onSuggestionsReady(request, new Response(suggestions));
+ }
+ });
+ base.setEnabled(false);
+ base.getElement().setAttribute("placeholder",
+ Util.C.rebasePlaceholderMessage());
+ base.setStyleName(Gerrit.RESOURCES.css().rebaseSuggestBox());
+
+ // the checkbox which must be clicked before the change list is populated
+ cb = new CheckBox(Util.C.rebaseConfirmMessage());
+ cb.addClickHandler(new ClickHandler() {
+ @Override
+ public void onClick(ClickEvent event) {
+ boolean checked = ((CheckBox) event.getSource()).getValue();
+ if (checked) {
+ ChangeList.next("project:" + project + " AND branch:" + branch
+ + " AND is:open NOT age:90d", 0, 1000,
+ new GerritCallback<ChangeList>() {
+ @Override
+ public void onSuccess(ChangeList result) {
+ changes = Natives.asList(result);
+ base.setEnabled(true);
+ }
+ });
+ } else {
+ base.setEnabled(false);
+ }
+ }
+ });
+
+ // add the checkbox and suggestbox widgets to the content panel
+ contentPanel.add(cb);
+ contentPanel.add(base);
+ contentPanel.setStyleName(Gerrit.RESOURCES.css().rebaseContentPanel());
+ }
+
+ public String getBase() {
+ return cb.getValue() ? base.getText() : null;
+ }
+
+ private static class ChangeSuggestion implements Suggestion {
+ private ChangeInfo change;
+
+ public ChangeSuggestion(ChangeInfo change) {
+ this.change = change;
+ }
+
+ @Override
+ public String getDisplayString() {
+ return String.valueOf(change.legacy_id().get()) + ": " + change.subject();
+ }
+
+ @Override
+ public String getReplacementString() {
+ return String.valueOf(change.legacy_id().get());
+ }
+ }
+}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/TextAreaActionDialog.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/TextAreaActionDialog.java
new file mode 100644
index 0000000..d7d5d6a
--- /dev/null
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/TextAreaActionDialog.java
@@ -0,0 +1,40 @@
+// Copyright (C) 2015 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.client.ui;
+
+import com.google.gwt.event.logical.shared.CloseHandler;
+import com.google.gwt.user.client.ui.PopupPanel;
+import com.google.gwtexpui.globalkey.client.NpTextArea;
+
+public abstract class TextAreaActionDialog extends CommentedActionDialog
+ implements CloseHandler<PopupPanel> {
+ protected final NpTextArea message;
+
+ public TextAreaActionDialog(String title, String heading) {
+ super(title, heading);
+
+ message = new NpTextArea();
+ message.setCharacterWidth(60);
+ message.setVisibleLines(10);
+ message.getElement().setPropertyBoolean("spellcheck", true);
+ setFocusOn(message);
+
+ contentPanel.add(message);
+ }
+
+ public String getMessageText() {
+ return message.getText().trim();
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 6b4c1a1..c36faa2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.api.changes.DraftApi;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.FileApi;
+import com.google.gerrit.extensions.api.changes.RebaseInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.changes.SubmitInput;
@@ -179,8 +180,14 @@
@Override
public ChangeApi rebase() throws RestApiException {
+ RebaseInput in = new RebaseInput();
+ return rebase(in);
+ }
+
+ @Override
+ public ChangeApi rebase(RebaseInput in) throws RestApiException {
try {
- return changes.id(rebase.apply(revision, null)._number);
+ return changes.id(rebase.apply(revision, in)._number);
} catch (OrmException | EmailException e) {
throw new RestApiException("Cannot rebase ps", e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
index 331dfe5..3f61b01 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import com.google.gerrit.common.errors.EmailException;
+import com.google.gerrit.extensions.api.changes.RebaseInput;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -23,9 +24,10 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.change.Rebase.Input;
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
@@ -35,27 +37,34 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import java.io.IOException;
@Singleton
-public class Rebase implements RestModifyView<RevisionResource, Input>,
+public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
UiAction<RevisionResource> {
- public static class Input {
- }
+
+ private static final Logger log =
+ LoggerFactory.getLogger(Rebase.class);
private final Provider<RebaseChange> rebaseChange;
private final ChangeJson json;
+ private final Provider<ReviewDb> dbProvider;
@Inject
- public Rebase(Provider<RebaseChange> rebaseChange, ChangeJson json) {
+ public Rebase(Provider<RebaseChange> rebaseChange, ChangeJson json,
+ Provider<ReviewDb> dbProvider) {
this.rebaseChange = rebaseChange;
this.json = json
.addOption(ListChangesOption.CURRENT_REVISION)
.addOption(ListChangesOption.CURRENT_COMMIT);
+ this.dbProvider = dbProvider;
}
@Override
- public ChangeInfo apply(RevisionResource rsrc, Input input)
+ public ChangeInfo apply(RevisionResource rsrc, RebaseInput input)
throws AuthException, ResourceNotFoundException,
ResourceConflictException, EmailException, OrmException {
ChangeControl control = rsrc.getControl();
@@ -65,11 +74,52 @@
} else if (!change.getStatus().isOpen()) {
throw new ResourceConflictException("change is "
+ change.getStatus().name().toLowerCase());
+ } else if (!hasOneParent(rsrc.getPatchSet().getId())) {
+ throw new ResourceConflictException(
+ "cannot rebase merge commits or commit with no ancestor");
+ }
+
+ String baseRev = null;
+ if (input != null && input.base != null) {
+ String base = input.base.trim();
+ do {
+ if (base.equals("")) {
+ // remove existing dependency to other patch set
+ baseRev = change.getDest().get();
+ break;
+ }
+
+ ReviewDb db = dbProvider.get();
+ PatchSet basePatchSet = parseBase(base);
+ if (basePatchSet == null) {
+ throw new ResourceConflictException("base revision is missing: " + base);
+ } else if (!rsrc.getControl().isPatchVisible(basePatchSet, db)) {
+ throw new AuthException("base revision not accessible: " + base);
+ } else if (change.getId().equals(basePatchSet.getId().getParentKey())) {
+ throw new ResourceConflictException("cannot depend on self");
+ }
+
+ Change baseChange = db.changes().get(basePatchSet.getId().getParentKey());
+ if (baseChange != null) {
+ if (!baseChange.getProject().equals(change.getProject())) {
+ throw new ResourceConflictException("base change is in wrong project: "
+ + baseChange.getProject());
+ } else if (!baseChange.getDest().equals(change.getDest())) {
+ throw new ResourceConflictException("base change is targetting wrong branch: "
+ + baseChange.getDest());
+ } else if (baseChange.getStatus() == Status.ABANDONED) {
+ throw new ResourceConflictException("base change is abandoned: "
+ + baseChange.getKey());
+ }
+ baseRev = basePatchSet.getRevision().get();
+ break;
+ }
+ } while (false); // just wanted to use the break statement
}
try {
rebaseChange.get().rebase(rsrc.getChange(), rsrc.getPatchSet().getId(),
- rsrc.getUser());
+ rsrc.getUser(), baseRev);
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
} catch (IOException e) {
@@ -81,6 +131,53 @@
return json.format(change.getId());
}
+ private PatchSet parseBase(final String base) throws OrmException {
+ ReviewDb db = dbProvider.get();
+
+ PatchSet.Id basePatchSetId = PatchSet.Id.fromRef(base);
+ if (basePatchSetId != null) {
+ // try parsing the base as a ref string
+ return db.patchSets().get(basePatchSetId);
+ }
+
+ // try parsing base as a change number (assume current patch set)
+ PatchSet basePatchSet = null;
+ try {
+ Change.Id baseChangeId = Change.Id.parse(base);
+ if (baseChangeId != null) {
+ for (PatchSet ps : db.patchSets().byChange(baseChangeId)) {
+ if (basePatchSet == null || basePatchSet.getId().get() < ps.getId().get()){
+ basePatchSet = ps;
+ }
+ }
+ }
+ } catch (NumberFormatException e) { // probably a SHA1
+ }
+
+ // try parsing as SHA1
+ if (basePatchSet == null) {
+ for (PatchSet ps : db.patchSets().byRevision(new RevId(base))) {
+ if (basePatchSet == null || basePatchSet.getId().get() < ps.getId().get()) {
+ basePatchSet = ps;
+ }
+ }
+ }
+
+ return basePatchSet;
+ }
+
+ private boolean hasOneParent(final PatchSet.Id patchSetId) {
+ try {
+ // prevent rebase of exotic changes (merge commit, no ancestor).
+ return (dbProvider.get().patchSetAncestors()
+ .ancestorsOf(patchSetId).toList().size() == 1);
+ } catch (OrmException e) {
+ log.error("Failed to get ancestors of patch set "
+ + patchSetId.toRefName(), e);
+ return false;
+ }
+ }
+
@Override
public UiAction.Description getDescription(RevisionResource resource) {
return new UiAction.Description()
@@ -88,30 +185,28 @@
.setTitle("Rebase onto tip of branch or parent change")
.setVisible(resource.getChange().getStatus().isOpen()
&& resource.getControl().canRebase()
- && rebaseChange.get().canRebase(resource));
+ && hasOneParent(resource.getPatchSet().getId()));
}
public static class CurrentRevision implements
- RestModifyView<ChangeResource, Input> {
- private final Provider<ReviewDb> dbProvider;
+ RestModifyView<ChangeResource, RebaseInput> {
private final Rebase rebase;
@Inject
- CurrentRevision(Provider<ReviewDb> dbProvider, Rebase rebase) {
- this.dbProvider = dbProvider;
+ CurrentRevision(Rebase rebase) {
this.rebase = rebase;
}
@Override
- public ChangeInfo apply(ChangeResource rsrc, Input input)
+ public ChangeInfo apply(ChangeResource rsrc, RebaseInput input)
throws AuthException, ResourceNotFoundException,
ResourceConflictException, EmailException, OrmException {
PatchSet ps =
- dbProvider.get().patchSets()
+ rebase.dbProvider.get().patchSets()
.get(rsrc.getChange().currentPatchSetId());
if (ps == null) {
throw new ResourceConflictException("current revision is missing");
- } else if (!rsrc.getControl().isPatchVisible(ps, dbProvider.get())) {
+ } else if (!rsrc.getControl().isPatchVisible(ps, rebase.dbProvider.get())) {
throw new AuthException("current revision not accessible");
}
return rebase.apply(new RevisionResource(rsrc, ps), input);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java
index 9bb67ea..f80a474 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java
@@ -99,6 +99,7 @@
* @param change the change to perform the rebase for
* @param patchSetId the id of the patch set
* @param uploader the user that creates the rebased patch set
+ * @param newBaseRev the commit that should be the new base
* @throws NoSuchChangeException thrown if the change to which the patch set
* belongs does not exist or is not visible to the user
* @throws EmailException thrown if sending the e-mail to notify about the new
@@ -107,9 +108,9 @@
* @throws IOException thrown if rebase is not possible or not needed
* @throws InvalidChangeOperationException thrown if rebase is not allowed
*/
- public void rebase(Change change, PatchSet.Id patchSetId, final IdentifiedUser uploader)
- throws NoSuchChangeException, EmailException, OrmException, IOException,
- InvalidChangeOperationException {
+ public void rebase(Change change, PatchSet.Id patchSetId, final IdentifiedUser uploader,
+ final String newBaseRev) throws NoSuchChangeException, EmailException, OrmException,
+ IOException, InvalidChangeOperationException {
final Change.Id changeId = patchSetId.getParentKey();
final ChangeControl changeControl =
changeControlFactory.validateFor(change, uploader);
@@ -126,10 +127,17 @@
rw = new RevWalk(git);
inserter = git.newObjectInserter();
- final String baseRev = findBaseRevision(patchSetId, db.get(),
- change.getDest(), git, null, null, null);
- final RevCommit baseCommit =
- rw.parseCommit(ObjectId.fromString(baseRev));
+ String baseRev = newBaseRev;
+ if (baseRev == null) {
+ baseRev = findBaseRevision(patchSetId, db.get(),
+ change.getDest(), git, null, null, null);
+ }
+ ObjectId baseObjectId = git.resolve(baseRev);
+ if (baseObjectId == null) {
+ throw new InvalidChangeOperationException(
+ "Cannot rebase: Failed to resolve baseRev: " + baseRev);
+ }
+ final RevCommit baseCommit = rw.parseCommit(baseObjectId);
PersonIdent committerIdent =
uploader.newCommitterIdent(TimeUtil.nowTs(),
