Merge "WorkQueue: Skip `isReadyToStart` invocation for CANCELLED tasks" into stable-3.11
diff --git a/java/com/google/gerrit/extensions/api/changes/RevertInput.java b/java/com/google/gerrit/extensions/api/changes/RevertInput.java
index 613e48e..b16c75e 100644
--- a/java/com/google/gerrit/extensions/api/changes/RevertInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/RevertInput.java
@@ -35,7 +35,11 @@
* Mark the change as work-in-progress. This will also override the {@link #notify} value to
* {@link NotifyHandling#OWNER}
*/
- public boolean workInProgress;
+ public Boolean workInProgress;
public Map<String, String> validationOptions;
+
+ public boolean getWorkInProgress() {
+ return workInProgress != null ? workInProgress : false;
+ }
}
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index 8a7840b..4a5e1b0 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -315,7 +315,7 @@
throws IOException, RestApiException, UpdateException, ConfigInvalidException {
RevCommit revertCommit = revWalk.parseCommit(revertCommitId);
Change.Id changeId = Change.id(seq.nextChangeId());
- if (input.workInProgress) {
+ if (input.getWorkInProgress()) {
input.notify = firstNonNull(input.notify, NotifyHandling.NONE);
}
NotifyResolver.Result notify =
@@ -340,13 +340,13 @@
ccs.remove(user.getAccountId());
ins.setReviewersAndCcsIgnoreVisibility(reviewers, ccs);
ins.setRevertOf(notes.getChangeId());
- ins.setWorkInProgress(input.workInProgress);
+ ins.setWorkInProgress(input.getWorkInProgress());
try (BatchUpdate bu = updateFactory.create(notes.getProjectName(), user, ts)) {
bu.setRepository(git, revWalk, oi);
bu.setNotify(notify);
bu.insertChange(ins);
- if (!input.workInProgress) {
+ if (!input.getWorkInProgress()) {
addChangeRevertedNotificationOps(
bu, changeToRevert.getId(), changeId, generatedChangeId.name());
}
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java b/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
index 4e05420..be1bced 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
@@ -56,6 +56,7 @@
import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.patch.PatchApplier;
+import org.eclipse.jgit.patch.PatchApplier.Result.Error;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
@@ -86,8 +87,13 @@
@Override
public Response<EditInfo> apply(
RevisionResource revisionResource, ApplyProvidedFixInput applyProvidedFixInput)
- throws AuthException, BadRequestException, ResourceConflictException, IOException,
- ResourceNotFoundException, PermissionBackendException, RestApiException {
+ throws AuthException,
+ BadRequestException,
+ ResourceConflictException,
+ IOException,
+ ResourceNotFoundException,
+ PermissionBackendException,
+ RestApiException {
if (applyProvidedFixInput == null) {
throw new BadRequestException("applyProvidedFixInput is required");
}
@@ -173,7 +179,8 @@
String targetCommitMessage = targetCommitMessageFile.modifiableContent();
if (!originCommitMessage.equals(targetCommitMessage)) {
throw new ResourceConflictException(
- "The fix attempts to modify commit message of an older patchset, but commit message has been updated in a newer patchset. The fix can't be applied.");
+ "The fix attempts to modify commit message of an older patchset, but commit message"
+ + " has been updated in a newer patchset. The fix can't be applied.");
}
resultBuilder.newCommitMessage(originCommitModification.newCommitMessage().get());
}
@@ -185,9 +192,20 @@
try (ObjectInserter oi = repository.newObjectInserter()) {
ApplyPatchInput inp = new ApplyPatchInput();
inp.patch = patch;
- inp.allowConflicts = false;
+ // Allow conflicts for showing more precise error message.
+ inp.allowConflicts = true;
result =
ApplyPatchUtil.applyPatch(repository, oi, inp, repository.parseCommit(targetCommit));
+ if (!result.getErrors().isEmpty()) {
+ String errorMessage =
+ (result.getErrors().stream().anyMatch(Error::isGitConflict)
+ ? "Merge conflict while applying a fix:\n"
+ : "Error while applying a fix:\n")
+ + result.getErrors().stream()
+ .map(Error::toString)
+ .collect(Collectors.joining("\n"));
+ throw new ResourceConflictException(errorMessage);
+ }
oi.flush();
for (String path : result.getPaths()) {
try (TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), path, result.getTreeId())) {
diff --git a/java/com/google/gerrit/server/restapi/change/Revert.java b/java/com/google/gerrit/server/restapi/change/Revert.java
index f9ac958..2add133 100644
--- a/java/com/google/gerrit/server/restapi/change/Revert.java
+++ b/java/com/google/gerrit/server/restapi/change/Revert.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.change;
+import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
import static com.google.gerrit.server.permissions.ChangePermission.REVERT;
import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE;
@@ -91,10 +92,9 @@
contributorAgreements.check(rsrc.getProject(), rsrc.getUser());
permissionBackend.user(rsrc.getUser()).ref(change.getDest()).check(CREATE_CHANGE);
- projectCache
- .get(rsrc.getProject())
- .orElseThrow(illegalState(rsrc.getProject()))
- .checkStatePermitsWrite();
+ ProjectState projectState =
+ projectCache.get(rsrc.getProject()).orElseThrow(illegalState(rsrc.getProject()));
+ projectState.checkStatePermitsWrite();
rsrc.permissions().check(REVERT);
ChangeNotes notes = rsrc.getNotes();
Change.Id changeIdToRevert = notes.getChangeId();
@@ -103,6 +103,18 @@
if (patch == null) {
throw new ResourceNotFoundException(changeIdToRevert.toString());
}
+
+ if (input.workInProgress == null) {
+ input.workInProgress =
+ firstNonNull(
+ rsrc.getUser()
+ .asIdentifiedUser()
+ .state()
+ .generalPreferences()
+ .workInProgressByDefault,
+ false);
+ }
+
return Response.ok(
json.noOptions()
.format(
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index f04042c..9cd7908 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -169,8 +169,13 @@
@Override
public Response<RevertSubmissionInfo> apply(ChangeResource changeResource, RevertInput input)
- throws RestApiException, IOException, UpdateException, PermissionBackendException,
- NoSuchProjectException, ConfigInvalidException, StorageException {
+ throws RestApiException,
+ IOException,
+ UpdateException,
+ PermissionBackendException,
+ NoSuchProjectException,
+ ConfigInvalidException,
+ StorageException {
if (!changeResource.getChange().isMerged()) {
throw new ResourceConflictException(
@@ -232,8 +237,12 @@
private RevertSubmissionInfo revertSubmission(
List<ChangeData> changeData, RevertInput revertInput)
- throws RestApiException, IOException, UpdateException, ConfigInvalidException,
- StorageException, PermissionBackendException {
+ throws RestApiException,
+ IOException,
+ UpdateException,
+ ConfigInvalidException,
+ StorageException,
+ PermissionBackendException {
ListMultimap<BranchNameKey, ChangeData> changesPerProjectAndBranch = ArrayListMultimap.create();
changeData.stream().forEach(c -> changesPerProjectAndBranch.put(c.change().getDest(), c));
@@ -278,7 +287,10 @@
Iterator<PatchSetData> sortedChangesInProjectAndBranch,
Set<ObjectId> commitIdsInProjectAndBranch,
Instant timestamp)
- throws IOException, RestApiException, UpdateException, ConfigInvalidException,
+ throws IOException,
+ RestApiException,
+ UpdateException,
+ ConfigInvalidException,
PermissionBackendException {
while (sortedChangesInProjectAndBranch.hasNext()) {
ChangeNotes changeNotes = sortedChangesInProjectAndBranch.next().data().notes();
@@ -329,9 +341,9 @@
generatedChangeId,
cherryPickRevertChangeId,
timestamp,
- revertInput.workInProgress,
+ revertInput.getWorkInProgress(),
baseCommit));
- if (!revertInput.workInProgress) {
+ if (!revertInput.getWorkInProgress()) {
commitUtil.addChangeRevertedNotificationOps(
bu, changeNotes.getChangeId(), cherryPickRevertChangeId, generatedChangeId.name());
}
@@ -361,7 +373,7 @@
// change is created for the cherry-picked commit. Notifications are sent only for this change,
// but not for the intermediately created revert commit.
cherryPickInput.notify = revertInput.notify;
- if (revertInput.workInProgress) {
+ if (revertInput.getWorkInProgress()) {
cherryPickInput.notify = firstNonNull(cherryPickInput.notify, NotifyHandling.NONE);
}
cherryPickInput.notifyDetails = revertInput.notifyDetails;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index 10dd5e3..1f3a6eb 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -40,6 +40,7 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -203,6 +204,7 @@
String.format("Created a revert of this change as %s", revertChange.changeId);
assertThat(sourceMessages.get(3).message).isEqualTo(expectedMessage);
+ assertThat(revertChange.workInProgress).isNull();
assertThat(revertChange.messages).hasSize(1);
assertThat(revertChange.messages.iterator().next().message).isEqualTo("Uploaded patch set 1.");
assertThat(revertChange.revertOf).isEqualTo(gApi.changes().id(r.getChangeId()).get()._number);
@@ -235,6 +237,36 @@
}
@Test
+ public void revertChangeWithWipByUserPreference() throws Exception {
+ GeneralPreferencesInfo generalPreferencesInfo = new GeneralPreferencesInfo();
+ generalPreferencesInfo.workInProgressByDefault = true;
+ gApi.accounts().id(admin.id().get()).setPreferences(generalPreferencesInfo);
+ requestScopeOperations.resetCurrentApiUser();
+
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit();
+ ChangeInfo revertChange = gApi.changes().id(r.getChangeId()).revert().get();
+ assertThat(revertChange.workInProgress).isTrue();
+ }
+
+ @Test
+ public void revertChangeOverrideWipByUserPreference() throws Exception {
+ GeneralPreferencesInfo generalPreferencesInfo = new GeneralPreferencesInfo();
+ generalPreferencesInfo.workInProgressByDefault = true;
+ gApi.accounts().id(admin.id().get()).setPreferences(generalPreferencesInfo);
+ requestScopeOperations.resetCurrentApiUser();
+
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit();
+ RevertInput revertInput = new RevertInput();
+ revertInput.workInProgress = false;
+ ChangeInfo revertChange = gApi.changes().id(r.getChangeId()).revert(revertInput).get();
+ assertThat(revertChange.workInProgress).isNull();
+ }
+
+ @Test
public void revertWithDefaultTopic() throws Exception {
PushOneCommit.Result result = createChange();
gApi.changes().id(result.getChangeId()).current().review(ReviewInput.approve());
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
index d66a0a5..effd96f 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
@@ -378,8 +378,17 @@
.value()
.asString()
.isEqualTo(
- "New line at the start\nFirst line\nSecond line\nTModified contentrd line\nFourth line\nFifth line\n"
- + "Sixth line\nSeventh line\nEighth line\nNinth line\nTenth line\n");
+ "New line at the start\n"
+ + "First line\n"
+ + "Second line\n"
+ + "TModified contentrd line\n"
+ + "Fourth line\n"
+ + "Fifth line\n"
+ + "Sixth line\n"
+ + "Seventh line\n"
+ + "Eighth line\n"
+ + "Ninth line\n"
+ + "Tenth line\n");
applyProvidedFixInput = createApplyProvidedFixInput(FILE_NAME, "(1st)", 1, 5, 1, 5);
applyProvidedFixInput.originalPatchsetForFix = previousRevision;
@@ -390,8 +399,17 @@
.value()
.asString()
.isEqualTo(
- "New line at the start\nFirst(1st) line\nSecond line\nTModified contentrd line\nFourth line\nFifth line\n"
- + "Sixth line\nSeventh line\nEighth line\nNinth line\nTenth line\n");
+ "New line at the start\n"
+ + "First(1st) line\n"
+ + "Second line\n"
+ + "TModified contentrd line\n"
+ + "Fourth line\n"
+ + "Fifth line\n"
+ + "Sixth line\n"
+ + "Seventh line\n"
+ + "Eighth line\n"
+ + "Ninth line\n"
+ + "Tenth line\n");
}
@Test
@@ -440,8 +458,15 @@
.value()
.asString()
.isEqualTo(
- "Third line\nFourth line\nFifth line\n"
- + "Sixth line\nSeventh line\nEighth line\nNinth line\nFirst modification\nTenth line\n");
+ "Third line\n"
+ + "Fourth line\n"
+ + "Fifth line\n"
+ + "Sixth line\n"
+ + "Seventh line\n"
+ + "Eighth line\n"
+ + "Ninth line\n"
+ + "First modification\n"
+ + "Tenth line\n");
Optional<BinaryResult> file2 = gApi.changes().id(changeId).edit().getFile(FILE_NAME2);
BinaryResultSubject.assertThat(file2)
.value()
@@ -450,6 +475,44 @@
}
@Test
+ public void applyProvidedFixOnNewerPatchset_mergeConflictThrowsResourceConflictException()
+ throws Exception {
+ // Remember patch set and add another one.
+ int previousRevision = gApi.changes().id(changeId).get().currentRevisionNumber;
+ // Change first 2 lines;
+ String modifiedContent =
+ "abc\ndef\n"
+ + FILE_CONTENT.substring(
+ FILE_CONTENT.indexOf("\n", FILE_CONTENT.indexOf("\n") + 1) + 1);
+ amendChange(
+ changeId,
+ "refs/for/master",
+ admin,
+ testRepo,
+ PushOneCommit.SUBJECT,
+ FILE_NAME,
+ modifiedContent);
+ FixReplacementInfo fixReplacementInfo1 = new FixReplacementInfo();
+ fixReplacementInfo1.path = FILE_NAME;
+ fixReplacementInfo1.range = createRange(2, 0, 2, 0);
+ fixReplacementInfo1.replacement = "First modification\n";
+
+ ApplyProvidedFixInput applyProvidedFixInput = new ApplyProvidedFixInput();
+
+ applyProvidedFixInput.fixReplacementInfos = Arrays.asList(fixReplacementInfo1);
+ applyProvidedFixInput.originalPatchsetForFix = previousRevision;
+
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(changeId).current().applyFix(applyProvidedFixInput));
+
+ assertThat(thrown.getMessage()).contains("Merge conflict");
+ // Change edit must not be created
+ assertThat(gApi.changes().id(changeId).edit().get()).isEmpty();
+ }
+
+ @Test
public void applyProvidedFixOnCommitMessageCanBeAppliedToNewerPatchset() throws Exception {
// Set a dedicated commit message.
String footer = "\nChange-Id: " + changeId + "\n";
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
index ac2e7cc..beec0b5 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
@@ -110,6 +110,9 @@
.hide {
display: none;
}
+ #messageInput {
+ min-width: calc(72ch + 2px + 2 * var(--spacing-m) + 0.4px);
+ }
@media only screen and (max-width: 40em) {
.value {
width: 29em;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index f1ef362..72e8667 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -683,7 +683,7 @@
);
subscribe(
this,
- () => this.getChangeModel().latestRevision$,
+ () => this.getChangeModel().latestRevisionWithEdit$,
revision => {
this.latestCommitMessage = this.prepareCommitMsgForLinkify(
revision?.commit?.message ?? ''
diff --git a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
index 136979d..7c33764 100644
--- a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
+++ b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
@@ -130,6 +130,14 @@
);
subscribe(
this,
+ () => this.getChangeModel().revisions$,
+ revisions =>
+ (this.hasEdit = Object.values(revisions).some(
+ info => info._number === EDIT
+ ))
+ );
+ subscribe(
+ this,
() => this.getChangeModel().latestPatchNum$,
x => (this.latestPatchNum = x)
);
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index 04fa0d6..dd292e6 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -353,6 +353,10 @@
public readonly latestRevision$ = this.selectRevision(this.latestPatchNum$);
+ public readonly latestRevisionWithEdit$ = this.selectRevision(
+ this.latestPatchNumWithEdit$
+ );
+
public readonly isOwner$: Observable<boolean> = select(
combineLatest([this.change$, this.userModel.account$]),
([change, account]) => isOwner(change, account)
diff --git a/resources/com/google/gerrit/pgm/init/gerrit.sh b/resources/com/google/gerrit/pgm/init/gerrit.sh
index dc56fae..4dc8881 100755
--- a/resources/com/google/gerrit/pgm/init/gerrit.sh
+++ b/resources/com/google/gerrit/pgm/init/gerrit.sh
@@ -371,7 +371,7 @@
GERRIT_USER=`get_config --get container.user`
-if test "$(uname -s)" == "Darwin" ; then
+if test "$(uname -s)" = "Darwin" ; then
JAVA_OPTIONS="$JAVA_OPTIONS -XX:-MaxFDLimit"
fi
diff --git a/tools/run_gjf.sh b/tools/run_gjf.sh
index c6eea0f..2e10dc8 100755
--- a/tools/run_gjf.sh
+++ b/tools/run_gjf.sh
@@ -16,10 +16,10 @@
set -eu
-GJF_VERSION=$(grep -o "^VERSION=.*$" tools/setup_gjf.sh | grep -o "[0-9][0-9]*\.[0-9][0-9]*")
-GJF=$(find 'tools/format' -regex '.*/google-java-format-[0-9][0-9]*\.[0-9][0-9]*')
+GJF_VERSION=$(grep -o "^VERSION=.*$" tools/setup_gjf.sh | grep -o '[0-9][0-9]*\.[0-9][0-9]*[\.0-9]*')
+GJF="tools/format/google-java-format-$GJF_VERSION"
if [ ! -f "$GJF" ]; then
- ./setup_gjf.sh
+ tools/setup_gjf.sh
GJF=$(find 'tools/format' -regex '.*/google-java-format-[0-9][0-9]*\.[0-9][0-9]*')
fi
echo 'Running google-java-format check...'
