diff --git a/ReleaseNotes/ReleaseNotes-2.12.4.txt b/ReleaseNotes/ReleaseNotes-2.12.4.txt index ce2f3690c5..3ba17f4f5d 100644 --- a/ReleaseNotes/ReleaseNotes-2.12.4.txt +++ b/ReleaseNotes/ReleaseNotes-2.12.4.txt @@ -65,6 +65,26 @@ changes. Fix internal error when a query does not contain any token. * link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4241[Issue 4241]: -Fix 'Cannot format velocity template' error. +Fix 'Cannot format velocity template' error when sending notification emails. * Fix `sshd.idleTimeout` setting being ignored. ++ +Ths `sshd.idleTimeout` setting was not being correctly set on the SSHD +backend, causing idle sessions to not time out. + +* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4324[Issue 4324]: +Set the correct uploader on new patch sets created via the inline editor. + +* Log a warning instead of failing when invalid commentlinks are configured. + +* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4136[Issue 4136]: +Fix support for `HEAD` requests in the REST API. ++ +Sending a `HEAD` request failed with '404 Not Found'. + +* Return proper error response when trying to confirm an email that is already +used by another user. + +* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4318[Issue 4318] +Fix 'Rebase if Necessary' merge strategy to prevent introducing a duplicate +commit when submitting a merge commit. diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java index 872c91277f..390cae34a6 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java @@ -51,6 +51,16 @@ public class HttpResponse { return response.getStatusLine().getStatusCode(); } + public String getContentType() { + return response.getFirstHeader("X-FYI-Content-Type").getValue(); + } + + public boolean hasContent() { + Preconditions.checkNotNull(response, + "Response is not initialized."); + return response.getEntity() != null; + } + public String getEntityContent() throws IOException { Preconditions.checkNotNull(response, "Response is not initialized."); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java index 9533f3fc4f..ee2dbfe17b 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -27,8 +27,8 @@ import static org.junit.Assert.fail; import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; -import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; +import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.common.data.Permission; import com.google.gerrit.extensions.api.changes.ChangeApi; @@ -77,7 +77,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; -@NoHttpd public class RevisionIT extends AbstractDaemonTest { @Inject @@ -622,6 +621,20 @@ public class RevisionIT extends AbstractDaemonTest { assertThat(res).isEqualTo(FILE_CONTENT); } + @Test + public void contentType() throws Exception { + PushOneCommit.Result r = createChange(); + + String endPoint = "/changes/" + r.getChangeId() + + "/revisions/" + r.getCommit().name() + + "/files/" + FILE_NAME + + "/content"; + RestResponse response = adminRestSession.head(endPoint); + response.assertOK(); + assertThat(response.getContentType()).startsWith("text/plain"); + assertThat(response.hasContent()).isFalse(); + } + private void assertMergeable(String id, boolean expected) throws Exception { MergeableInfo m = gApi.changes().id(id).current().mergeable(); assertThat(m.mergeable).isEqualTo(expected); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java index 288e96efee..d72a940ce9 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java @@ -19,6 +19,7 @@ import static com.google.gerrit.acceptance.GitUtil.getChangeId; import static com.google.gerrit.acceptance.GitUtil.pushHead; import com.google.common.collect.Iterables; +import com.google.common.collect.ImmutableList; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.TestProjectInput; import com.google.gerrit.extensions.api.changes.SubmitInput; @@ -150,6 +151,57 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit { change4.getChangeId(), headAfterSecondSubmit.name()); } + @Test + public void submitWithRebaseMergeCommit() throws Exception { + /* + * (HEAD, origin/master, origin/HEAD) Merge changes X,Y + |\ + | * Merge branch 'master' into origin/master + | |\ + | | * SHA Added a + | |/ + * | Before + |/ + * Initial empty repository + */ + RevCommit initialHead = getRemoteHead(); + PushOneCommit.Result change1 = createChange("Added a", "a.txt", ""); + + PushOneCommit change2Push = pushFactory.create(db, admin.getIdent(), testRepo, + "Merge to master", "m.txt", ""); + change2Push.setParents(ImmutableList.of(initialHead, change1.getCommit())); + PushOneCommit.Result change2 = change2Push.to("refs/for/master"); + + testRepo.reset(initialHead); + PushOneCommit.Result change3 = createChange("Before", "b.txt", ""); + + approve(change3.getChangeId()); + submit(change3.getChangeId()); + + approve(change1.getChangeId()); + approve(change2.getChangeId()); + submit(change2.getChangeId()); + + RevCommit newHead = getRemoteHead(); + assertThat(newHead.getParentCount()).isEqualTo(2); + + RevCommit headParent1 = parse(newHead.getParent(0).getId()); + RevCommit headParent2 = parse(newHead.getParent(1).getId()); + + assertThat(headParent1.getId()).isEqualTo(change3.getCommit().getId()); + assertThat(headParent1.getParentCount()).isEqualTo(1); + assertThat(headParent1.getParent(0)).isEqualTo(initialHead); + + assertThat(headParent2.getId()).isEqualTo(change2.getCommit().getId()); + assertThat(headParent2.getParentCount()).isEqualTo(2); + + RevCommit headGrandparent1 = parse(headParent2.getParent(0).getId()); + RevCommit headGrandparent2 = parse(headParent2.getParent(1).getId()); + + assertThat(headGrandparent1.getId()).isEqualTo(initialHead.getId()); + assertThat(headGrandparent2.getId()).isEqualTo(change1.getCommit().getId()); + } + @Test @TestProjectInput(useContentMerge = InheritableBoolean.TRUE) public void submitWithContentMerge() throws Exception { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index bcd936ed74..49d30b4b14 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -326,8 +326,7 @@ public class RestApiServlet extends HttpServlet { return; } - if (viewData.view instanceof RestReadView - && "GET".equals(req.getMethod())) { + if (viewData.view instanceof RestReadView && isGetOrHead(req)) { result = ((RestReadView) viewData.view).apply(rsrc); } else if (viewData.view instanceof RestModifyView) { @SuppressWarnings("unchecked") diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java index cca7452c00..f183772d83 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java @@ -49,6 +49,17 @@ public class RebaseIfNecessary extends SubmitStrategy { List ops = new ArrayList<>(sorted.size()); boolean first = true; + for (CodeReviewCommit c : sorted) { + if (c.getParentCount() > 1) { + // Since there is a merge commit, sort and prune again using + // MERGE_IF_NECESSARY semantics to avoid creating duplicate + // commits. + // + sorted = args.mergeUtil.reduceToMinimalMerge(args.mergeSorter, sorted); + break; + } + } + while (!sorted.isEmpty()) { CodeReviewCommit n = sorted.remove(0); if (first && args.mergeTip.getInitialTip() == null) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/tools/ToolsCatalog.java b/gerrit-server/src/main/java/com/google/gerrit/server/tools/ToolsCatalog.java index 37383b75e6..9e48aadcaa 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/tools/ToolsCatalog.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/tools/ToolsCatalog.java @@ -16,6 +16,8 @@ package com.google.gerrit.server.tools; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.base.Strings; +import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Version; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -60,7 +62,11 @@ public class ToolsCatalog { * @param name path of the item, relative to the root of the catalog. * @return the entry; null if the item is not part of the catalog. */ - public Entry get(String name) { + @Nullable + public Entry get(@Nullable String name) { + if (Strings.isNullOrEmpty(name)) { + return null; + } if (name.startsWith("/")) { name = name.substring(1); } @@ -109,6 +115,7 @@ public class ToolsCatalog { return Collections.unmodifiableSortedMap(toc); } + @Nullable private static byte[] read(String path) { String name = "root/" + path; try (InputStream in = ToolsCatalog.class.getResourceAsStream(name)) { @@ -128,6 +135,7 @@ public class ToolsCatalog { } } + @Nullable private static String dirOf(String path) { final int s = path.lastIndexOf('/'); return s < 0 ? null : path.substring(0, s);