From 5af9b3bfb7224cc7e3e2338d69c7df597657519f Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Wed, 25 Mar 2020 16:30:33 +0100 Subject: [PATCH 01/33] Add a warning if submitting a change with an open change edit Bug: Issue 12287 Change-Id: I25aa799a69d0fcce1db55d9d1ed87675a6d3f1fb (cherry picked from commit a36f08348aaab175cab001d6f50be1db903a6d7b) --- .../gr-confirm-submit-dialog.html | 5 ++++ .../gr-confirm-submit-dialog.js | 5 ++++ .../gr-confirm-submit-dialog_test.html | 27 ++++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html index 1036b7fbcc..b9c32ed570 100644 --- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html +++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html @@ -53,6 +53,11 @@ limitations under the License. + diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js index 93d38df33d..55c5c672e8 100644 --- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js +++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js @@ -54,6 +54,11 @@ this.$.dialog.resetFocus(); }, + _computeHasChangeEdit(change) { + return !!change.revisions && + Object.values(change.revisions).some(rev => rev._number == 'edit'); + }, + _handleConfirmTap(e) { e.preventDefault(); this.dispatchEvent(new CustomEvent('confirm', {bubbles: false})); diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html index 86c15f6a6e..dd61d1c1c9 100644 --- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html +++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html @@ -50,7 +50,10 @@ limitations under the License. test('display', () => { element.action = {label: 'my-label'}; - element.change = {subject: 'my-subject'}; + element.change = { + subject: 'my-subject', + revisions: {}, + }; flushAsynchronousOperations(); const header = element.$$('.header'); assert.equal(header.textContent.trim(), 'my-label'); @@ -59,5 +62,27 @@ limitations under the License. assert.notEqual(message.textContent.length, 0); assert.notEqual(message.textContent.indexOf('my-subject'), -1); }); + + test('_computeHasChangeEdit', () => { + const change = { + revisions: { + d442ff05d6c4f2a3af0eeca1f67374b39f9dc3d8: { + _number: 'edit', + }, + }, + unresolved_comment_count: 0, + }; + + assert.equal(element._computeHasChangeEdit(change), true); + + const change2 = { + revisions: { + d442ff05d6c4f2a3af0eeca1f67374b39f9dc3d8: { + _number: 2, + }, + }, + }; + assert.equal(element._computeHasChangeEdit(change2), false); + }); }); From 5d6ca055210944851312d935aa405a38282229a6 Mon Sep 17 00:00:00 2001 From: Paladox none Date: Thu, 19 Nov 2020 22:21:32 +0000 Subject: [PATCH 02/33] Use strict equality This was done in another commit [1]. [1] https://gerrit-review.googlesource.com/c/gerrit/+/281526/3/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts#71 Change-Id: I2ae7435922b55a4e5f5422b73a65bc83c44cdf94 --- .../change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js index 55c5c672e8..282b394b50 100644 --- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js +++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js @@ -56,7 +56,7 @@ _computeHasChangeEdit(change) { return !!change.revisions && - Object.values(change.revisions).some(rev => rev._number == 'edit'); + Object.values(change.revisions).some(rev => rev._number === 'edit'); }, _handleConfirmTap(e) { From 7e22ca2291656d1949f56ed1cdc00f0a54267656 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 10 Nov 2020 09:48:10 -0800 Subject: [PATCH 03/33] Fix bazel run_shell usage for newer versions The Bazel option `--incompatible_run_shell_command_string` is going to be flipped to true in upcoming Bazel 4.0 release per default, see: [1] for more details. Test Plan: bazel build :release [1] https://github.com/bazelbuild/bazel/issues/5903 Bug: Issue 13612 Change-Id: Icc9589906198386b1e4805ceeabbb420a7ea1afb (cherry picked from commit c1f4e91406b9da411dd2f5eab4ee92bfc761e1f4) --- tools/bzl/asciidoc.bzl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/bzl/asciidoc.bzl b/tools/bzl/asciidoc.bzl index 1e7ec96ce3..7977cf051d 100644 --- a/tools/bzl/asciidoc.bzl +++ b/tools/bzl/asciidoc.bzl @@ -18,8 +18,7 @@ def documentation_attributes(): ] def _replace_macros_impl(ctx): - cmd = [ - ctx.file._exe.path, + args = [ "--suffix", ctx.attr.suffix, "-s", @@ -28,13 +27,14 @@ def _replace_macros_impl(ctx): ctx.outputs.out.path, ] if ctx.attr.searchbox: - cmd.append("--searchbox") + args.append("--searchbox") else: - cmd.append("--no-searchbox") - ctx.actions.run_shell( + args.append("--no-searchbox") + ctx.actions.run( inputs = [ctx.file._exe, ctx.file.src], outputs = [ctx.outputs.out], - command = cmd, + executable = ctx.file._exe.path, + arguments = args, use_default_shell_env = True, progress_message = "Replacing macros in %s" % ctx.file.src.short_path, ) From e59e2b3d25ee37b307202667ed5636d06e58814a Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Tue, 17 Nov 2020 13:14:13 +0100 Subject: [PATCH 04/33] ProjectConfig: Write resolved groups to file project.config references group names from the groups file. These names are local to the project. The groups file links the local name to a global groups UUID. Renaming a group makes Gerrit go through all projects and try to rename the local group names. This commit fixes a bug that would make Gerrit only change the groups file, but not the project.config file. This lead to half-baked renames that can break ACLs. Change-Id: I46993b4838ddfa700acb79911713aa42cc026590 (cherry picked from commit bf9be01299cb33847b2a5cf09494c5f8caa9dfe4) --- java/com/google/gerrit/server/project/ProjectConfig.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java index 54d9176be4..89038e241d 100644 --- a/java/com/google/gerrit/server/project/ProjectConfig.java +++ b/java/com/google/gerrit/server/project/ProjectConfig.java @@ -1425,7 +1425,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. if (group.getUUID() != null) { keepGroups.add(group.getUUID()); } - rules.add(rule.asString(needRange)); + rules.add(rule.toBuilder().setGroup(group).build().asString(needRange)); } rc.setStringList(CAPABILITY, null, permission.getName(), rules); } @@ -1470,7 +1470,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. if (group.getUUID() != null) { keepGroups.add(group.getUUID()); } - rules.add(rule.asString(needRange)); + rules.add(rule.toBuilder().setGroup(group).build().asString(needRange)); } rc.setStringList(ACCESS, refName, permission.getName(), rules); } From 0134ba4854cf13222bace12b65e448c86b79b4a4 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Wed, 25 Nov 2020 13:52:36 +0100 Subject: [PATCH 05/33] Fix flaky test for group renaming in ProjectConfig The groups API supports renaming a group. This will start a background thread that goes over all projects that have this group name in their configs and perform a local rename. The test was flaky because of that asynchronous behavior. With this commit, we test the rename on the project config instead wich allows us to skip the asynchronous behavior that has caused the flakyness. The new test would also have caught the original bug. Change-Id: I2f26ff5d01176240e1368acd8389bfac6ee2b494 (cherry picked from commit afb082c13b48723d7be1f8f365125675b88be412) --- .../acceptance/api/project/ProjectIT.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java index e99a6f576e..d5fc1c1af4 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.project; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow; import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block; import static com.google.gerrit.server.project.ProjectState.INHERITED_FROM_GLOBAL; @@ -39,7 +40,9 @@ import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.config.GerritConfig; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations; +import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.GroupReference; import com.google.gerrit.entities.Permission; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; @@ -55,6 +58,7 @@ import com.google.gerrit.extensions.api.projects.ProjectInput; import com.google.gerrit.extensions.client.InheritableBoolean; import com.google.gerrit.extensions.client.ProjectState; import com.google.gerrit.extensions.client.SubmitType; +import com.google.gerrit.extensions.common.GroupInfo; import com.google.gerrit.extensions.common.ProjectInfo; import com.google.gerrit.extensions.events.ChangeIndexedListener; import com.google.gerrit.extensions.events.ProjectIndexedListener; @@ -63,6 +67,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.server.config.ProjectConfigEntry; +import com.google.gerrit.server.git.meta.MetaDataUpdate; import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.project.ProjectConfig; import com.google.inject.AbstractModule; @@ -70,6 +75,7 @@ import com.google.inject.Inject; import com.google.inject.Module; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; @@ -939,6 +945,42 @@ public class ProjectIT extends AbstractDaemonTest { projectOperations.project(allProjects).getHead(RefNames.REFS_CONFIG).name())); } + @Test + public void renamingGroupGetsPersisted() throws Exception { + String name = name("Name1"); + GroupInfo group = gApi.groups().create(name).get(); + + // Use group in a permission + projectOperations + .project(project) + .forUpdate() + .add(allow(Permission.READ).ref(RefNames.REFS_CONFIG).group(AccountGroup.uuid(group.id))) + .update(); + Optional beforeRename = + projectCache.get(project).get().getLocalGroups().stream() + .filter(g -> g.getUUID().get().equals(group.id)) + .map(GroupReference::getName) + .findAny(); + // Groups created with ProjectOperations always have their UUID as local name + assertThat(beforeRename).hasValue(group.id); + + // Rename the group directly on the project config + String newName = name("Name2"); + try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) { + ProjectConfig config = projectConfigFactory.read(md); + config.renameGroup(AccountGroup.uuid(group.id), newName); + config.commit(md); + projectCache.evict(config.getProject()); + } + + Optional afterRename = + projectCache.get(project).get().getLocalGroups().stream() + .filter(g -> g.getUUID().get().equals(group.id)) + .map(GroupReference::getName) + .findAny(); + assertThat(afterRename).hasValue(newName); + } + private CommentLinkInfo commentLinkInfo(String name, String match, String link) { CommentLinkInfo info = new CommentLinkInfo(); info.name = name; From ac1f09139cd665e29f4d4b2a50e3004c76f92eee Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 26 Nov 2020 13:09:13 +0100 Subject: [PATCH 06/33] Upgrade JGit to 5.1.14.202011251942-r This version contains the following fix: Ensure that GC#deleteOrphans respects pack lock If pack or index files are guarded by a pack lock (.keep file) deleteOrphans() should not touch the respective files protected by the lock file. Otherwise it may interfere with PackInserter concurrently inserting a new pack file and its index. Release Notes: https://projects.eclipse.org/projects/technology.jgit/releases/5.1.14 Bug: Issue 13544 Change-Id: Ieeb5a883bcb487a4d45f299aec5b31475002cdd3 --- lib/jgit/jgit.bzl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl index 997879edf1..841b13e231 100644 --- a/lib/jgit/jgit.bzl +++ b/lib/jgit/jgit.bzl @@ -1,6 +1,6 @@ load("//tools/bzl:maven_jar.bzl", "MAVEN_CENTRAL", "maven_jar") -_JGIT_VERS = "5.1.13.202002110435-r" +_JGIT_VERS = "5.1.14.202011251942-r" _DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot @@ -40,28 +40,28 @@ def jgit_maven_repos(): name = "jgit-lib", artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "dc99e6ef37038090561bd5253c71b150791bea58", - src_sha1 = "882b62cac802a26e5fe383cd1a16f8b2d557e2fe", + sha1 = "f962f10e9aac2c476eac02e6d37cf8ee9a101958", + src_sha1 = "afa2ff384db5b4a3dc75b0ef2f127d5ef474d635", unsign = True, ) maven_jar( name = "jgit-servlet", artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "5860d7828839db9a41b664488540704f783d0280", + sha1 = "b7c6e973495c75f2de8f837fac213c6b1d9a9860", unsign = True, ) maven_jar( name = "jgit-archive", artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "fb8538a090daa5dfb6674cdf860dd34d6a4078ed", + sha1 = "4331d91515347e416d914a90299f6d6070599efc", ) maven_jar( name = "jgit-junit", artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "1e45351d6c19f65135ee4b693c0346772307671d", + sha1 = "5fd2a105454dd8b4834a37667ec12111d691167a", unsign = True, ) From 21425d0d4d4bae0dfae42f545ec512b88b6e6d69 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 21 Oct 2020 12:45:12 +0200 Subject: [PATCH 07/33] Add fatal to consistency and validation error enums The code-owners plugin would like to differentiate between normal errors and fatal errors. Signed-off-by: Edwin Kempin Change-Id: Ide55a2ba9ef84fa71604d42caf8cf02c66618ec7 (cherry picked from commit 50017186401bd4ae812e78d05ba8760865206dd9) --- Documentation/rest-api-config.txt | 2 +- .../gerrit/extensions/api/config/ConsistencyCheckInfo.java | 1 + .../google/gerrit/server/git/validators/ValidationMessage.java | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt index 4473a8d968..a62ed47b72 100644 --- a/Documentation/rest-api-config.txt +++ b/Documentation/rest-api-config.txt @@ -1701,7 +1701,7 @@ consistency problem. |====================== |Field Name|Description |`status` |The status of the consistency problem. + -Possible values are `ERROR` and `WARNING`. +Possible values are `FATAL`, `ERROR` and `WARNING`. |`message` |Message describing the consistency problem. |====================== diff --git a/java/com/google/gerrit/extensions/api/config/ConsistencyCheckInfo.java b/java/com/google/gerrit/extensions/api/config/ConsistencyCheckInfo.java index 2c166d08cb..e582f1bde1 100644 --- a/java/com/google/gerrit/extensions/api/config/ConsistencyCheckInfo.java +++ b/java/com/google/gerrit/extensions/api/config/ConsistencyCheckInfo.java @@ -48,6 +48,7 @@ public class ConsistencyCheckInfo { public static class ConsistencyProblemInfo { public enum Status { + FATAL, ERROR, WARNING, } diff --git a/java/com/google/gerrit/server/git/validators/ValidationMessage.java b/java/com/google/gerrit/server/git/validators/ValidationMessage.java index 372fc17a0d..b5d7eb1919 100644 --- a/java/com/google/gerrit/server/git/validators/ValidationMessage.java +++ b/java/com/google/gerrit/server/git/validators/ValidationMessage.java @@ -22,6 +22,7 @@ import java.util.Objects; */ public class ValidationMessage { public enum Type { + FATAL("FATAL: "), ERROR("ERROR: "), WARNING("WARNING: "), HINT("hint: "), @@ -70,7 +71,7 @@ public class ValidationMessage { * Returns {@true} if this message is an error. Used to decide if the operation should be aborted. */ public boolean isError() { - return type == Type.ERROR; + return type == Type.FATAL || type == Type.ERROR; } @Override From dc7ad8f2998154946ddcf37af03b875625d8fd11 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 6 Oct 2020 15:57:13 +0200 Subject: [PATCH 08/33] MergeValidationListener: Also provide RevWalk to the onPreMerge method This way implementors that need a RevWalk do not need to create a new instance. The callers have the RevWalk anyway available, so it's no extra effort for them to provide it. This makes the objects that are available to implementors of MergeValidationListener more consistent with the objects that are available to implementors of CommitValidationListener, which already contains the RevWalk inside of CommitReceivedEvent. Signed-off-by: Edwin Kempin Change-Id: Id93e378be10c91d3dc3365aec0bab9847ee8ef6d (cherry picked from commit f57cdd0b5d12c4ceed25980c81d988978496e8b5) --- .../validators/MergeValidationListener.java | 3 +++ .../git/validators/MergeValidators.java | 26 ++++++++++++------- .../google/gerrit/server/submit/MergeOp.java | 3 ++- plugins/hooks | 2 +- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/java/com/google/gerrit/server/git/validators/MergeValidationListener.java b/java/com/google/gerrit/server/git/validators/MergeValidationListener.java index b47d7d656b..79d53ac453 100644 --- a/java/com/google/gerrit/server/git/validators/MergeValidationListener.java +++ b/java/com/google/gerrit/server/git/validators/MergeValidationListener.java @@ -19,6 +19,7 @@ import com.google.gerrit.entities.PatchSet; import com.google.gerrit.extensions.annotations.ExtensionPoint; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.CodeReviewCommit; +import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.project.ProjectState; import org.eclipse.jgit.lib.Repository; @@ -33,6 +34,7 @@ public interface MergeValidationListener { * Validate a commit before it is merged. * * @param repo the repository + * @param revWalk the rev walk * @param commit commit details * @param destProject the destination project * @param destBranch the destination branch @@ -42,6 +44,7 @@ public interface MergeValidationListener { */ void onPreMerge( Repository repo, + CodeReviewRevWalk revWalk, CodeReviewCommit commit, ProjectState destProject, BranchNameKey destBranch, diff --git a/java/com/google/gerrit/server/git/validators/MergeValidators.java b/java/com/google/gerrit/server/git/validators/MergeValidators.java index 4e5ce0c60b..cbaa121029 100644 --- a/java/com/google/gerrit/server/git/validators/MergeValidators.java +++ b/java/com/google/gerrit/server/git/validators/MergeValidators.java @@ -35,6 +35,7 @@ import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.ProjectConfigEntry; import com.google.gerrit.server.git.CodeReviewCommit; +import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -51,7 +52,6 @@ import java.util.Objects; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.revwalk.RevWalk; /** * Collection of validators that run inside Gerrit before a change is submitted. The main purpose is @@ -92,6 +92,7 @@ public class MergeValidators { */ public void validatePreMerge( Repository repo, + CodeReviewRevWalk revWalk, CodeReviewCommit commit, ProjectState destProject, BranchNameKey destBranch, @@ -106,7 +107,7 @@ public class MergeValidators { groupValidatorFactory.create()); for (MergeValidationListener validator : validators) { - validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId, caller); + validator.onPreMerge(repo, revWalk, commit, destProject, destBranch, patchSetId, caller); } } @@ -168,11 +169,12 @@ public class MergeValidators { @Override public void onPreMerge( - final Repository repo, - final CodeReviewCommit commit, - final ProjectState destProject, - final BranchNameKey destBranch, - final PatchSet.Id patchSetId, + Repository repo, + CodeReviewRevWalk revWalk, + CodeReviewCommit commit, + ProjectState destProject, + BranchNameKey destBranch, + PatchSet.Id patchSetId, IdentifiedUser caller) throws MergeValidationException { if (RefNames.REFS_CONFIG.equals(destBranch.branch())) { @@ -260,6 +262,7 @@ public class MergeValidators { @Override public void onPreMerge( Repository repo, + CodeReviewRevWalk revWalk, CodeReviewCommit commit, ProjectState destProject, BranchNameKey destBranch, @@ -267,7 +270,7 @@ public class MergeValidators { IdentifiedUser caller) throws MergeValidationException { mergeValidationListeners.runEach( - l -> l.onPreMerge(repo, commit, destProject, destBranch, patchSetId, caller), + l -> l.onPreMerge(repo, revWalk, commit, destProject, destBranch, patchSetId, caller), MergeValidationException.class); } } @@ -294,6 +297,7 @@ public class MergeValidators { @Override public void onPreMerge( Repository repo, + CodeReviewRevWalk revWalk, CodeReviewCommit commit, ProjectState destProject, BranchNameKey destBranch, @@ -316,8 +320,9 @@ public class MergeValidators { throw new MergeValidationException("account validation unavailable"); } - try (RevWalk rw = new RevWalk(repo)) { - List errorMessages = accountValidator.validate(accountId, repo, rw, null, commit); + try { + List errorMessages = + accountValidator.validate(accountId, repo, revWalk, null, commit); if (!errorMessages.isEmpty()) { throw new MergeValidationException( "invalid account configuration: " + Joiner.on("; ").join(errorMessages)); @@ -345,6 +350,7 @@ public class MergeValidators { @Override public void onPreMerge( Repository repo, + CodeReviewRevWalk revWalk, CodeReviewCommit commit, ProjectState destProject, BranchNameKey destBranch, diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java index 58b0c8efb3..8a293a4d12 100644 --- a/java/com/google/gerrit/server/submit/MergeOp.java +++ b/java/com/google/gerrit/server/submit/MergeOp.java @@ -864,7 +864,8 @@ public class MergeOp implements AutoCloseable { MergeValidators mergeValidators = mergeValidatorsFactory.create(); try { - mergeValidators.validatePreMerge(or.repo, commit, or.project, destBranch, ps.id(), caller); + mergeValidators.validatePreMerge( + or.repo, or.rw, commit, or.project, destBranch, ps.id(), caller); } catch (MergeValidationException mve) { commitStatus.problem(changeId, mve.getMessage()); continue; diff --git a/plugins/hooks b/plugins/hooks index 7ed555fe88..ad4f877749 160000 --- a/plugins/hooks +++ b/plugins/hooks @@ -1 +1 @@ -Subproject commit 7ed555fe88f4be028acbfd5c245ac78537ac3666 +Subproject commit ad4f877749928b69ef94b62176c5797f6648887d From 34f8614cbc72377bde07078dcb57340ccecca276 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 6 Oct 2020 16:24:30 +0200 Subject: [PATCH 09/33] Include repo config into CommitReceivedEvent Some implementors of CommitValidationListener may need the repository config (e.g. in order to use DiffFormatter). At the moment they need to reopen the repository for this, but since callers have it already available they can simply provide it. This makes the objects that are available to implementors of CommitValidationListener more consistent with the objects that are available to implementors of MergeValidationListener, which already gets the repository and hence its config. We do not want to provide the Repository instance as it is intentionally write protected when batch updates are done. Signed-off-by: Edwin Kempin Change-Id: Ie9439e4196c894ff261f98ba38a9db9d57e209d8 (cherry picked from commit c54963cffba2998b07e89745a572a0a5d4c246cf) --- .../gerrit/server/change/ChangeInserter.java | 1 + .../gerrit/server/change/PatchSetInserter.java | 1 + .../server/events/CommitReceivedEvent.java | 4 ++++ .../git/receive/BranchCommitValidator.java | 18 ++++++++++++++++-- .../server/git/receive/ReceiveCommits.java | 3 ++- 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java index a086cb18fc..6091091ed9 100644 --- a/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/java/com/google/gerrit/server/change/ChangeInserter.java @@ -546,6 +546,7 @@ public class ChangeInserter implements InsertChangeOp { cmd, projectState.getProject(), change.getDest().branch(), + ctx.getRepoView().getConfig(), ctx.getRevWalk().getObjectReader(), commitId, ctx.getIdentifiedUser())) { diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java index 882352d70b..ef06ea1f65 100644 --- a/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -342,6 +342,7 @@ public class PatchSetInserter implements BatchUpdateOp { .orElseThrow(illegalState(origNotes.getProjectName())) .getProject(), origNotes.getChange().getDest().branch(), + ctx.getRepoView().getConfig(), ctx.getRevWalk().getObjectReader(), commitId, ctx.getIdentifiedUser())) { diff --git a/java/com/google/gerrit/server/events/CommitReceivedEvent.java b/java/com/google/gerrit/server/events/CommitReceivedEvent.java index 6e43621c6e..eb4d9ee301 100644 --- a/java/com/google/gerrit/server/events/CommitReceivedEvent.java +++ b/java/com/google/gerrit/server/events/CommitReceivedEvent.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.events; import com.google.gerrit.entities.Project; import com.google.gerrit.server.IdentifiedUser; import java.io.IOException; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.revwalk.RevCommit; @@ -28,6 +29,7 @@ public class CommitReceivedEvent extends RefEvent implements AutoCloseable { public ReceiveCommand command; public Project project; public String refName; + public Config repoConfig; public RevWalk revWalk; public RevCommit commit; public IdentifiedUser user; @@ -40,6 +42,7 @@ public class CommitReceivedEvent extends RefEvent implements AutoCloseable { ReceiveCommand command, Project project, String refName, + Config repoConfig, ObjectReader reader, ObjectId commitId, IdentifiedUser user) @@ -48,6 +51,7 @@ public class CommitReceivedEvent extends RefEvent implements AutoCloseable { this.command = command; this.project = project; this.refName = refName; + this.repoConfig = repoConfig; this.revWalk = new RevWalk(reader); this.commit = revWalk.parseCommit(commitId); this.user = user; diff --git a/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java b/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java index 7b5f90bdc0..5526122345 100644 --- a/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java +++ b/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java @@ -37,7 +37,9 @@ import com.google.gerrit.server.ssh.SshInfo; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; import java.io.IOException; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectReader; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.notes.NoteMap; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.transport.ReceiveCommand; @@ -94,6 +96,7 @@ public class BranchCommitValidator { /** * Validates a single commit. If the commit does not validate, the command is rejected. * + * @param repository the repository * @param objectReader the object reader to use. * @param cmd the ReceiveCommand executing the push. * @param commit the commit being validated. @@ -102,6 +105,7 @@ public class BranchCommitValidator { * @return The validation {@link Result}. */ Result validateCommit( + Repository repository, ObjectReader objectReader, ReceiveCommand cmd, RevCommit commit, @@ -109,12 +113,14 @@ public class BranchCommitValidator { NoteMap rejectCommits, @Nullable Change change) throws IOException { - return validateCommit(objectReader, cmd, commit, isMerged, rejectCommits, change, false); + return validateCommit( + repository, objectReader, cmd, commit, isMerged, rejectCommits, change, false); } /** * Validates a single commit. If the commit does not validate, the command is rejected. * + * @param repository the repository * @param objectReader the object reader to use. * @param cmd the ReceiveCommand executing the push. * @param commit the commit being validated. @@ -124,6 +130,7 @@ public class BranchCommitValidator { * @return The validation {@link Result}. */ Result validateCommit( + Repository repository, ObjectReader objectReader, ReceiveCommand cmd, RevCommit commit, @@ -135,7 +142,14 @@ public class BranchCommitValidator { try (TraceTimer traceTimer = TraceContext.newTimer("BranchCommitValidator#validateCommit")) { ImmutableList.Builder messages = new ImmutableList.Builder<>(); try (CommitReceivedEvent receiveEvent = - new CommitReceivedEvent(cmd, project, branch.branch(), objectReader, commit, user)) { + new CommitReceivedEvent( + cmd, + project, + branch.branch(), + new Config(repository.getConfig()), + objectReader, + commit, + user)) { CommitValidators validators; if (isMerged) { validators = diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 4347f8f156..6e1f8d3120 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -2212,6 +2212,7 @@ class ReceiveCommits { BranchCommitValidator.Result validationResult = validator.validateCommit( + repo, receivePack.getRevWalk().getObjectReader(), magicBranch.cmd, c, @@ -3231,7 +3232,7 @@ class ReceiveCommits { BranchCommitValidator.Result validationResult = validator.validateCommit( - walk.getObjectReader(), cmd, c, false, rejectCommits, null, skipValidation); + repo, walk.getObjectReader(), cmd, c, false, rejectCommits, null, skipValidation); messages.addAll(validationResult.messages()); if (!validationResult.isValid()) { break; From 401e01fd0605689129a159dc8188c0ebf91cdf31 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 26 Nov 2020 23:57:30 +0100 Subject: [PATCH 10/33] Upgrade JGit to 5.3.8.202011260953-r This version contains the following fix: Ensure that GC#deleteOrphans respects pack lock If pack or index files are guarded by a pack lock (.keep file) deleteOrphans() should not touch the respective files protected by the lock file. Otherwise it may interfere with PackInserter concurrently inserting a new pack file and its index. Release Notes: https://projects.eclipse.org/projects/technology.jgit/releases/5.1.14 Bug: Issue 13544 Change-Id: I81272f4cac9923b63b0966bcf227325efbf7d0e9 --- lib/jgit/jgit.bzl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl index db0e1e153f..47fc643fe1 100644 --- a/lib/jgit/jgit.bzl +++ b/lib/jgit/jgit.bzl @@ -1,6 +1,6 @@ load("//tools/bzl:maven_jar.bzl", "MAVEN_CENTRAL", "maven_jar") -_JGIT_VERS = "5.3.7.202002110540-r" +_JGIT_VERS = "5.3.8.202011260953-r" _DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot @@ -40,25 +40,25 @@ def jgit_maven_repos(): name = "jgit-lib", artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "b1714d4917750d6fad0d19d3b0e258b373db819a", + sha1 = "f34c7c9e0ffaf8ba9e5af00e299e51f70931a833", ) maven_jar( name = "jgit-servlet", artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "cf61e6e00a758a6f33995e53883aede76d3b2400", + sha1 = "f9517712c741660cd199311a4eb27584dd8d03f6", ) maven_jar( name = "jgit-archive", artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "3c0b259040d3bc3a9e884a301055cf4f2e1bb1e2", + sha1 = "9adac724af047dfaa1e9061eeb34fbb14ebaefa9", ) maven_jar( name = "jgit-junit", artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "f78409fb808c5a108c629ec3cba74cc6c14ebff2", + sha1 = "1c2dc9a93b30e3a6fb1fadb589f01f29343ec7d8", ) def jgit_dep(name): From a60b03950903bc7a644e86225102c9e4c3972ede Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 27 Nov 2020 12:23:12 +0100 Subject: [PATCH 11/33] Upgrade JGit to 5cd485e5 This version contains the following fix: Ensure that GC#deleteOrphans respects pack lock If pack or index files are guarded by a pack lock (.keep file) deleteOrphans() should not touch the respective files protected by the lock file. Otherwise it may interfere with PackInserter concurrently inserting a new pack file and its index. Bug: Issue 13544 Change-Id: I5a86a2bf615e6defe7b64119155affc988b96218 --- modules/jgit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jgit b/modules/jgit index e2663a8b85..5cd485e5dd 160000 --- a/modules/jgit +++ b/modules/jgit @@ -1 +1 @@ -Subproject commit e2663a8b85cf92f6a84d72834257243a84066e9d +Subproject commit 5cd485e5dda41d2ef06226a692c64f1aa221eb25 From 3deba8778f9bb416e36a1e769a07b054530d56cf Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 27 Nov 2020 10:25:46 +0100 Subject: [PATCH 12/33] Upgrade JGit to ad902087 This version contains the following fix: Ensure that GC#deleteOrphans respects pack lock If pack or index files are guarded by a pack lock (.keep file) deleteOrphans() should not touch the respective files protected by the lock file. Otherwise it may interfere with PackInserter concurrently inserting a new pack file and its index. Bug: Issue 13544 Change-Id: I7266f7b0c164826140726b939a647489902633b9 --- modules/jgit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jgit b/modules/jgit index a79c5b1f10..ad90208782 160000 --- a/modules/jgit +++ b/modules/jgit @@ -1 +1 @@ -Subproject commit a79c5b1f1046f4b41928bc40ab433155168dacc6 +Subproject commit ad902087820c1b2b3147b45ffbac4e3804274f9c From e78e7154a1c2c14954cbc1adbb583815385c1eaa Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 27 Nov 2020 11:08:19 +0100 Subject: [PATCH 13/33] Upgrade JGit to d1801402 This version contains the following fix: Ensure that GC#deleteOrphans respects pack lock If pack or index files are guarded by a pack lock (.keep file) deleteOrphans() should not touch the respective files protected by the lock file. Otherwise it may interfere with PackInserter concurrently inserting a new pack file and its index. Bug: Issue 13544 Change-Id: Ib543e9b8f094b8065e9ba4174953ab52be6eb43e --- modules/jgit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jgit b/modules/jgit index 9f3616dcb4..d1801402fe 160000 --- a/modules/jgit +++ b/modules/jgit @@ -1 +1 @@ -Subproject commit 9f3616dcb43246b883f9943af0de7cf67836c443 +Subproject commit d1801402fee871610e983846225afd6339ce8ffd From 2763b9d4d79369740aa251796333c53f325e08eb Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Thu, 22 Oct 2020 14:30:46 +0200 Subject: [PATCH 14/33] Improve message if submit fails due to non-fulfilled submit requirements If a submit request fails due to non-fulfilled submit requirements, the response only contains the fallback text of the submit requirement. Without letting the user know that this message is about a non-fulfilled submit requirement, the message may be unclear to the caller. The fallback text is usually kept as short as possible, since the frontend is displaying it in the UI. Hence it cannot be made more verbose without affecting the UI. Signed-off-by: Edwin Kempin Change-Id: Iecd9304aa98efa6f404da9a50d2e002c6aefc6f3 (cherry picked from commit f4099b6c72ecdd8480e75c2bc16613b476cc3ca5) --- java/com/google/gerrit/server/submit/MergeOp.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java index 8a293a4d12..fdf366409d 100644 --- a/java/com/google/gerrit/server/submit/MergeOp.java +++ b/java/com/google/gerrit/server/submit/MergeOp.java @@ -352,7 +352,7 @@ public class MergeOp implements AutoCloseable { } if (record.requirements != null) { record.requirements.stream() - .map(SubmitRequirement::fallbackText) + .map(MergeOp::describeSubmitRequirement) .forEach(blockingConditions::add); } return Joiner.on("; ").join(blockingConditions); @@ -388,6 +388,10 @@ public class MergeOp implements AutoCloseable { return Joiner.on("; ").join(labelResults); } + private static String describeSubmitRequirement(SubmitRequirement submitRequirement) { + return String.format("Submit requirement not fulfilled: %s", submitRequirement.fallbackText()); + } + private void checkSubmitRulesAndState(ChangeSet cs, boolean allowMerged) throws ResourceConflictException { checkArgument( From ba8dfadee25378e1d86b4d087518cbc19356c840 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 27 Nov 2020 12:59:07 +0000 Subject: [PATCH 15/33] Update zero-downtime documentation in config-gerrit.txt Zero-downtime upgrade instructions associated with gerrit.experimentalRollingUpgrade are updated to the v3.2 to v3.3 upgrade process. Change-Id: I2f583e1f2fb906e3556e0814bb206c32a980902f --- Documentation/config-gerrit.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 6491b5490a..7ee2a7c4be 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2325,8 +2325,8 @@ Defaults to the full hostname of the Gerrit server. [[gerrit.experimentalRollingUpgrade]]gerrit.experimentalRollingUpgrade:: + Enable Gerrit rolling upgrade to the next version. -For example if Gerrit v3.1 is version N (All-Projects:refs/meta/version=181) -then its next version N+1 is v3.2 (All-Projects:refs/meta/version=183). +For example if Gerrit v3.2 is version N (All-Projects:refs/meta/version=183) +then its next version N+1 is v3.3 (All-Projects:refs/meta/version=184). Allow Gerrit to start even if the underlying schema version has been bumped to the next Gerrit version. + @@ -2343,7 +2343,7 @@ of the following steps: 1. Set gerrit.experimentalRollingUpgrade to true on all Gerrit masters 2. Set the first master unhealthy 3. Shutdown the first master and [upgrade](install.html#init) to the next version -4. Startup the first master, wait for the online reindex to complete +4. Startup the first master, wait for the online reindex to complete (where applicable) 5. Verify the the first master upgrade is successful and online reindex is complete 6. Set the first master healthy 7. Repeat steps 2. to 6. for all the other Gerrit nodes From 9b076ae734a9417a43f2fa367e584a4076aaf434 Mon Sep 17 00:00:00 2001 From: Antonio Barone Date: Fri, 27 Nov 2020 14:17:32 +0100 Subject: [PATCH 16/33] Bump up jetty version to 9.4.33.v20201020 This version, in particular, fixes the bug: "Request without Host header fails with NullPointerException in ForwardedRequestCustomizer" [1] This bug caused Gerrit to throw a NullPointerException when serving forwarded http/1.0 requests having no `Host` header set. [1] https://github.com/eclipse/jetty.project/issues/5443 Bug: Issue 13752 Change-Id: I9f9f7df74f6d6c3996e044ba9883b2aa8951c209 --- WORKSPACE | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index e6d16c4688..454423a161 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -856,54 +856,54 @@ maven_jar( sha1 = "3e127311a86fc2e8f550ef8ee4abe094bbcf7e7e", ) -JETTY_VERS = "9.4.32.v20200930" +JETTY_VERS = "9.4.33.v20201020" maven_jar( name = "jetty-servlet", artifact = "org.eclipse.jetty:jetty-servlet:" + JETTY_VERS, - sha1 = "4253dd46c099e0bca4dd763fc1e10774e10de00a", + sha1 = "101609e8e5365c4406e4448099459eb605ac551f", ) maven_jar( name = "jetty-security", artifact = "org.eclipse.jetty:jetty-security:" + JETTY_VERS, - sha1 = "16a6110fa40e49050146de5f597ab3a3a3fa83b5", + sha1 = "c150bf2aca6cb1636e7195f844a2bb156546e50e", ) maven_jar( name = "jetty-server", artifact = "org.eclipse.jetty:jetty-server:" + JETTY_VERS, - sha1 = "d2d89099be5237cf68254bc943a7d800d3ee1945", + sha1 = "f586ff2ee048ad2575866c1833d854288f402307", ) maven_jar( name = "jetty-jmx", artifact = "org.eclipse.jetty:jetty-jmx:" + JETTY_VERS, - sha1 = "5e8e87a6f89b8eabf5b5b1765e3d758209001570", + sha1 = "56b723070eeafc51b943cd9bf1a064a037e806a7", ) maven_jar( name = "jetty-continuation", artifact = "org.eclipse.jetty:jetty-continuation:" + JETTY_VERS, - sha1 = "b46713a1b8b2baf951f6514dd621c5a546254d6c", + sha1 = "f672e58d528fc83060558ab4fc6a797c8137dfcb", ) maven_jar( name = "jetty-http", artifact = "org.eclipse.jetty:jetty-http:" + JETTY_VERS, - sha1 = "5fdcefd82178d11f895690f4fe6e843be69394b3", + sha1 = "ad28940f89ffde6ec1bd1656fe3f8493b01ba3c2", ) maven_jar( name = "jetty-io", artifact = "org.eclipse.jetty:jetty-io:" + JETTY_VERS, - sha1 = "0d0f32c3b511d6b3a542787f95ed229731588810", + sha1 = "9e4b0048285b71f4769908780f957a470eca11da", ) maven_jar( name = "jetty-util", artifact = "org.eclipse.jetty:jetty-util:" + JETTY_VERS, - sha1 = "efefd29006dcc9c9960a679263504287ce4e6896", + sha1 = "c88807f210ab216aa831b48569ef50bd797384bc", ) maven_jar( From 53528f49a5e47e1e67c74afeb54ce20d2e319fec Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 23 Nov 2020 16:23:50 +0100 Subject: [PATCH 17/33] Auto-disable auto gc in jgit By JGit's default, git-receive-pack will run auto gc after receiving data from git-push and updating refs. Disable this behavior in Gerrit init to avoid the additional load it creates: gc should be configured in gc config section or run as a separate process. Change-Id: I9450f8edbade7b6e789387645cfe70daa0949aff --- Documentation/config-gerrit.txt | 4 +- .../gerrit/pgm/init/InitJGitConfig.java | 71 +++++++++++++++++++ .../google/gerrit/pgm/init/InitModule.java | 1 + 3 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 java/com/google/gerrit/pgm/init/InitJGitConfig.java diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 7ee2a7c4be..3461774226 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -5604,10 +5604,12 @@ Supported versions: [[receive.autogc]]receive.autogc:: + -By default, `git-receive-pack` will run auto gc after receiving data from git-push and updating refs. +By default, up to Gerrit 3.2 `git-receive-pack` will run auto gc after receiving data from git-push and updating refs. You can stop it by setting this variable to `false`. This is recommended in gerrit to avoid the additional load this creates. Instead schedule gc using link:cmd-gc.html#gc.startTime[gc.startTime] and link:cmd-gc.html#gc.interval[gc.interval] or e.g. in a cron job that runs gc in a separate process. +Since Gerrit 3.3 the init command will auto-configure `git-receive-pack = false` in `etc/jgit.config` if +it wasn't set manually and show a warning if it was set to `true` manually. GERRIT ------ diff --git a/java/com/google/gerrit/pgm/init/InitJGitConfig.java b/java/com/google/gerrit/pgm/init/InitJGitConfig.java new file mode 100644 index 0000000000..2fb03cc98d --- /dev/null +++ b/java/com/google/gerrit/pgm/init/InitJGitConfig.java @@ -0,0 +1,71 @@ +// Copyright (C) 2020 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.pgm.init; + +import static com.google.gerrit.pgm.init.api.InitUtil.die; + +import com.google.gerrit.pgm.init.api.ConsoleUI; +import com.google.gerrit.pgm.init.api.InitStep; +import com.google.gerrit.server.config.SitePaths; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import java.io.IOException; +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.ConfigConstants; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.util.FS; + +/** Initialize the JGit configuration. */ +@Singleton +class InitJGitConfig implements InitStep { + private final ConsoleUI ui; + private final SitePaths sitePaths; + + @Inject + InitJGitConfig(ConsoleUI ui, SitePaths sitePaths) { + this.ui = ui; + this.sitePaths = sitePaths; + } + + @Override + public void run() { + ui.header("JGit Configuration"); + FileBasedConfig jgitConfig = new FileBasedConfig(sitePaths.jgit_config.toFile(), FS.DETECTED); + try { + jgitConfig.load(); + if (!jgitConfig + .getNames(ConfigConstants.CONFIG_RECEIVE_SECTION) + .contains(ConfigConstants.CONFIG_KEY_AUTOGC)) { + jgitConfig.setBoolean( + ConfigConstants.CONFIG_RECEIVE_SECTION, null, ConfigConstants.CONFIG_KEY_AUTOGC, false); + jgitConfig.save(); + ui.error( + "Auto-configured \"receive.autogc = false\" to disable auto-gc after git-receive-pack."); + } else if (jgitConfig.getBoolean( + ConfigConstants.CONFIG_RECEIVE_SECTION, ConfigConstants.CONFIG_KEY_AUTOGC, true)) { + ui.error( + "WARNING: JGit option \"receive.autogc = true\". This is not recommended in Gerrit.\n" + + "git-receive-pack will run auto gc after receiving data from " + + "git-push and updating refs.\n" + + "Disable this behavior to avoid the additional load it creates: " + + "gc should be configured in gc config section or run as a separate process."); + } + } catch (IOException e) { + throw die(String.format("Handling JGit configuration %s failed", sitePaths.jgit_config), e); + } catch (ConfigInvalidException e) { + throw die(String.format("Invalid JGit configuration %s", sitePaths.jgit_config), e); + } + } +} diff --git a/java/com/google/gerrit/pgm/init/InitModule.java b/java/com/google/gerrit/pgm/init/InitModule.java index b65867538b..32c66978fe 100644 --- a/java/com/google/gerrit/pgm/init/InitModule.java +++ b/java/com/google/gerrit/pgm/init/InitModule.java @@ -40,6 +40,7 @@ public class InitModule extends FactoryModule { // Steps are executed in the order listed here. // step().to(InitGitManager.class); + step().to(InitJGitConfig.class); step().to(InitLogging.class); step().to(InitIndex.class); step().to(InitAuth.class); From 99c83aaaeb61d3959ca99999f8dc28a6c123fa83 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 27 Nov 2020 22:13:50 +0000 Subject: [PATCH 18/33] Fix NPE with StoredCommentLinkInfoSerializer when enabled is null The 'enabled' field in the StoredCommentLinkInfo AutoValue is optional and, when null, its default value is 'true'. Implement the logic into the serialization of the AutoValue, avoiding NPE that would fail to store the data to the persistent cache. Bug: Issue 13754 Change-Id: If30030b66912f4c1ea96d957dc9cc24349867bd0 --- .../entities/StoredCommentLinkInfoSerializer.java | 3 ++- .../StoredCommentLinkInfoSerializerTest.java | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/java/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializer.java b/java/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializer.java index d7bd373238..a7a84f7579 100644 --- a/java/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializer.java +++ b/java/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializer.java @@ -19,6 +19,7 @@ import static com.google.common.base.Strings.nullToEmpty; import com.google.gerrit.entities.StoredCommentLinkInfo; import com.google.gerrit.server.cache.proto.Cache; +import java.util.Optional; /** Helper to (de)serialize values for caches. */ public class StoredCommentLinkInfoSerializer { @@ -38,7 +39,7 @@ public class StoredCommentLinkInfoSerializer { .setMatch(nullToEmpty(autoValue.getMatch())) .setLink(nullToEmpty(autoValue.getLink())) .setHtml(nullToEmpty(autoValue.getHtml())) - .setEnabled(autoValue.getEnabled()) + .setEnabled(Optional.ofNullable(autoValue.getEnabled()).orElse(true)) .setOverrideOnly(autoValue.getOverrideOnly()) .build(); } diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializerTest.java index 3a51b70a01..e293493f8a 100644 --- a/javatests/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializerTest.java +++ b/javatests/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializerTest.java @@ -56,4 +56,19 @@ public class StoredCommentLinkInfoSerializerTest { .build(); assertThat(deserialize(serialize(autoValue))).isEqualTo(autoValue); } + + @Test + public void nullEnabled_roundTrip() { + StoredCommentLinkInfo sourceAutoValue = + StoredCommentLinkInfo.builder("name").setLink("

html").setMatch("*").build(); + + StoredCommentLinkInfo storedAutoValue = + StoredCommentLinkInfo.builder("name") + .setLink("

html") + .setMatch("*") + .setEnabled(true) + .build(); + + assertThat(deserialize(serialize(sourceAutoValue))).isEqualTo(storedAutoValue); + } } From 4f43759cdf4df22ea1033d68617dae6dfeca3894 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sun, 29 Nov 2020 16:49:18 +0100 Subject: [PATCH 19/33] Auto-enable git wire protocol v2 in jgit By JGit's default, git wire protocol version 2 is disabled. The attempt to activate it per default in JGit wasn't approved yet: [1],[2]. Given, that git wire protocol version 2 on server side is considered to be now very stable, activate it per default in init site program. Update JGit module to include the following change: 23389a632 Add constants for parsing git wire protocol version [1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=563145 [2] https://git.eclipse.org/r/c/jgit/jgit/+/163216 Change-Id: I15da7c5e889bc222da08d473a4c41e349a2ee6a6 --- .../gerrit/pgm/init/InitJGitConfig.java | 31 +++++++++++++++++++ modules/jgit | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/java/com/google/gerrit/pgm/init/InitJGitConfig.java b/java/com/google/gerrit/pgm/init/InitJGitConfig.java index 2fb03cc98d..6e37f7f909 100644 --- a/java/com/google/gerrit/pgm/init/InitJGitConfig.java +++ b/java/com/google/gerrit/pgm/init/InitJGitConfig.java @@ -25,6 +25,7 @@ import java.io.IOException; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.transport.TransferConfig; import org.eclipse.jgit.util.FS; /** Initialize the JGit configuration. */ @@ -62,6 +63,36 @@ class InitJGitConfig implements InitStep { + "Disable this behavior to avoid the additional load it creates: " + "gc should be configured in gc config section or run as a separate process."); } + + if (!jgitConfig + .getNames(ConfigConstants.CONFIG_PROTOCOL_SECTION) + .contains(ConfigConstants.CONFIG_KEY_VERSION)) { + jgitConfig.setString( + ConfigConstants.CONFIG_PROTOCOL_SECTION, + null, + ConfigConstants.CONFIG_KEY_VERSION, + TransferConfig.ProtocolVersion.V2.version()); + jgitConfig.save(); + ui.error( + String.format( + "Auto-configured \"%s.%s = %s\" to activate git wire protocol version 2.", + ConfigConstants.CONFIG_PROTOCOL_SECTION, + ConfigConstants.CONFIG_KEY_VERSION, + TransferConfig.ProtocolVersion.V2.version())); + } else { + String version = + jgitConfig.getString( + ConfigConstants.CONFIG_PROTOCOL_SECTION, null, ConfigConstants.CONFIG_KEY_VERSION); + if (!TransferConfig.ProtocolVersion.V2.version().equals(version)) { + ui.error( + String.format( + "HINT: JGit option \"%s.%s = %s\". It's recommended to activate git\n" + + "wire protocol version 2 to improve git fetch performance.", + ConfigConstants.CONFIG_PROTOCOL_SECTION, + ConfigConstants.CONFIG_KEY_VERSION, + version)); + } + } } catch (IOException e) { throw die(String.format("Handling JGit configuration %s failed", sitePaths.jgit_config), e); } catch (ConfigInvalidException e) { diff --git a/modules/jgit b/modules/jgit index 5cd485e5dd..23389a6323 160000 --- a/modules/jgit +++ b/modules/jgit @@ -1 +1 @@ -Subproject commit 5cd485e5dda41d2ef06226a692c64f1aa221eb25 +Subproject commit 23389a63238c50daf0d8d1e3fd2b1d29f4171645 From 9f1e47da87e2ce3a46a3f60e213f1114f65e6973 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Tue, 1 Dec 2020 09:09:10 -0500 Subject: [PATCH 20/33] Set version to 3.3.0 Change-Id: Ieb0becee7e2c949ed6301c36900ef982c4c372ad --- tools/maven/gerrit-acceptance-framework_pom.xml | 2 +- tools/maven/gerrit-extension-api_pom.xml | 2 +- tools/maven/gerrit-plugin-api_pom.xml | 2 +- tools/maven/gerrit-war_pom.xml | 2 +- version.bzl | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml index 14c726ea4f..55b57a42aa 100644 --- a/tools/maven/gerrit-acceptance-framework_pom.xml +++ b/tools/maven/gerrit-acceptance-framework_pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-acceptance-framework - 3.3.0-SNAPSHOT + 3.3.0 jar Gerrit Code Review - Acceptance Test Framework Framework for Gerrit's acceptance tests diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml index bd323ba8b1..7bd1a0fcf6 100644 --- a/tools/maven/gerrit-extension-api_pom.xml +++ b/tools/maven/gerrit-extension-api_pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-extension-api - 3.3.0-SNAPSHOT + 3.3.0 jar Gerrit Code Review - Extension API API for Gerrit Extensions diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml index 3b059e5402..82eed06db1 100644 --- a/tools/maven/gerrit-plugin-api_pom.xml +++ b/tools/maven/gerrit-plugin-api_pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-api - 3.3.0-SNAPSHOT + 3.3.0 jar Gerrit Code Review - Plugin API API for Gerrit Plugins diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml index b8fa13227f..d3ba4c17ea 100644 --- a/tools/maven/gerrit-war_pom.xml +++ b/tools/maven/gerrit-war_pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-war - 3.3.0-SNAPSHOT + 3.3.0 war Gerrit Code Review - WAR Gerrit WAR diff --git a/version.bzl b/version.bzl index 78b286b556..a5d5361241 100644 --- a/version.bzl +++ b/version.bzl @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = "3.3.0-SNAPSHOT" +GERRIT_VERSION = "3.3.0" From e41f4bd3a5636f7f35c1c4e1f89fb5f1b4123cf7 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Tue, 1 Dec 2020 09:47:48 -0500 Subject: [PATCH 21/33] Set version to 3.3.1-SNAPSHOT Change-Id: I61f8896f32b12a0364fa9001d0bbf98d0dbe685e --- tools/maven/gerrit-acceptance-framework_pom.xml | 2 +- tools/maven/gerrit-extension-api_pom.xml | 2 +- tools/maven/gerrit-plugin-api_pom.xml | 2 +- tools/maven/gerrit-war_pom.xml | 2 +- version.bzl | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml index 55b57a42aa..e9272bf15b 100644 --- a/tools/maven/gerrit-acceptance-framework_pom.xml +++ b/tools/maven/gerrit-acceptance-framework_pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-acceptance-framework - 3.3.0 + 3.3.1-SNAPSHOT jar Gerrit Code Review - Acceptance Test Framework Framework for Gerrit's acceptance tests diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml index 7bd1a0fcf6..f1b46e4807 100644 --- a/tools/maven/gerrit-extension-api_pom.xml +++ b/tools/maven/gerrit-extension-api_pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-extension-api - 3.3.0 + 3.3.1-SNAPSHOT jar Gerrit Code Review - Extension API API for Gerrit Extensions diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml index 82eed06db1..526edeee1d 100644 --- a/tools/maven/gerrit-plugin-api_pom.xml +++ b/tools/maven/gerrit-plugin-api_pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-api - 3.3.0 + 3.3.1-SNAPSHOT jar Gerrit Code Review - Plugin API API for Gerrit Plugins diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml index d3ba4c17ea..dda0a73b68 100644 --- a/tools/maven/gerrit-war_pom.xml +++ b/tools/maven/gerrit-war_pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-war - 3.3.0 + 3.3.1-SNAPSHOT war Gerrit Code Review - WAR Gerrit WAR diff --git a/version.bzl b/version.bzl index a5d5361241..d41a334444 100644 --- a/version.bzl +++ b/version.bzl @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = "3.3.0" +GERRIT_VERSION = "3.3.1-SNAPSHOT" From e6c0142c176da2b81ca77f52def86699f8aa055e Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 2 Dec 2020 12:23:42 +0100 Subject: [PATCH 22/33] Update JGit to 5.1.15.202012011955-r This version fixes a bug occurring when processing a fetch request and running gc concurrently. Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=569349 Change-Id: I605749727d39822683371b98d996f5afdf1604e9 --- lib/jgit/jgit.bzl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl index 841b13e231..eafb37b18e 100644 --- a/lib/jgit/jgit.bzl +++ b/lib/jgit/jgit.bzl @@ -1,6 +1,6 @@ load("//tools/bzl:maven_jar.bzl", "MAVEN_CENTRAL", "maven_jar") -_JGIT_VERS = "5.1.14.202011251942-r" +_JGIT_VERS = "5.1.15.202012011955-r" _DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot @@ -40,28 +40,28 @@ def jgit_maven_repos(): name = "jgit-lib", artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "f962f10e9aac2c476eac02e6d37cf8ee9a101958", - src_sha1 = "afa2ff384db5b4a3dc75b0ef2f127d5ef474d635", + sha1 = "ca4a5fbd38cfad3ad386b46464b0e753eca3baea", + src_sha1 = "39a4180b70ac3024d68ee9781198e88abf567ccf", unsign = True, ) maven_jar( name = "jgit-servlet", artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "b7c6e973495c75f2de8f837fac213c6b1d9a9860", + sha1 = "698fa84e35787eab380efadc2df52706dbac5268", unsign = True, ) maven_jar( name = "jgit-archive", artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "4331d91515347e416d914a90299f6d6070599efc", + sha1 = "27c731c41c8cb45e6bccdaac14311bda1d724437", ) maven_jar( name = "jgit-junit", artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "5fd2a105454dd8b4834a37667ec12111d691167a", + sha1 = "6f961edbfecf5928c87d46b08acf2e1625a53089", unsign = True, ) From 7ca02ac596ce4857a9e5d1701b3be921f88e8dd9 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 2 Dec 2020 13:53:11 +0100 Subject: [PATCH 23/33] Update JGit to 5.3.9.202012012026-r This version fixes a bug occurring when processing a fetch request and running gc concurrently. Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=569349 Change-Id: I6aa23a9ac75a059156ee26b5a4e72bab676b7655 --- lib/jgit/jgit.bzl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl index 47fc643fe1..519d6edb35 100644 --- a/lib/jgit/jgit.bzl +++ b/lib/jgit/jgit.bzl @@ -1,6 +1,6 @@ load("//tools/bzl:maven_jar.bzl", "MAVEN_CENTRAL", "maven_jar") -_JGIT_VERS = "5.3.8.202011260953-r" +_JGIT_VERS = "5.3.9.202012012026-r" _DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot @@ -40,25 +40,25 @@ def jgit_maven_repos(): name = "jgit-lib", artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "f34c7c9e0ffaf8ba9e5af00e299e51f70931a833", + sha1 = "b6d3af64d2538db1c25ee8cf9f2346fd8663321b", ) maven_jar( name = "jgit-servlet", artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "f9517712c741660cd199311a4eb27584dd8d03f6", + sha1 = "1f68350b98cbbd9a5219ad5827b3aa6c46a15dea", ) maven_jar( name = "jgit-archive", artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "9adac724af047dfaa1e9061eeb34fbb14ebaefa9", + sha1 = "7a4bf3ac728274129acf8c13c11b66199808eb20", ) maven_jar( name = "jgit-junit", artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "1c2dc9a93b30e3a6fb1fadb589f01f29343ec7d8", + sha1 = "dd8a2b2cd0b65ee00c260d1a1e7ed33aed951c69", ) def jgit_dep(name): From 7a7b57641bff078c8c76a448ac812cd3fb3ee494 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 2 Dec 2020 23:41:21 +0100 Subject: [PATCH 24/33] Update JGit to c9d871f15 This version fixes a bug occurring when processing a fetch request and running gc concurrently. Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=569349 Change-Id: If9262d80bb50e107d6ba478b781160adc51cacdc --- modules/jgit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jgit b/modules/jgit index ad90208782..c9d871f15d 160000 --- a/modules/jgit +++ b/modules/jgit @@ -1 +1 @@ -Subproject commit ad902087820c1b2b3147b45ffbac4e3804274f9c +Subproject commit c9d871f15d4daa4ab959b34d2a0759016e415643 From 1a72faf1781578c375d5ec12d93d0cc8eae79800 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 3 Dec 2020 00:01:37 +0100 Subject: [PATCH 25/33] Update JGit to 8f422e9a9 This version fixes a bug occurring when processing a fetch request and running gc concurrently. Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=569349 Change-Id: I5e948970c8f92cf9e7639611715951af5f883805 --- modules/jgit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jgit b/modules/jgit index d1801402fe..8f422e9a9a 160000 --- a/modules/jgit +++ b/modules/jgit @@ -1 +1 @@ -Subproject commit d1801402fee871610e983846225afd6339ce8ffd +Subproject commit 8f422e9a9add4a7a7d972e8aa1dfa5e15fccdf99 From 227056c995ae8370ae0c4c5c2833bdffd1e60ed8 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 3 Dec 2020 00:17:38 +0100 Subject: [PATCH 26/33] Update JGit to 9a1065afe This version fixes a bug occurring when processing a fetch request and running gc concurrently. Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=569349 Change-Id: I8f1c19fb25096efa11d833c1f8f3f6db8029116f --- modules/jgit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jgit b/modules/jgit index 23389a6323..9a1065afec 160000 --- a/modules/jgit +++ b/modules/jgit @@ -1 +1 @@ -Subproject commit 23389a63238c50daf0d8d1e3fd2b1d29f4171645 +Subproject commit 9a1065afec18c5a76d3a95e77aa70d7a24252056 From 8c27f62954b07c41455ac684e95efdcd87c2eb32 Mon Sep 17 00:00:00 2001 From: Sven Selberg Date: Thu, 3 Dec 2020 13:40:11 +0100 Subject: [PATCH 27/33] Honor toogleWipState permission for %ready %wip push options When toogleWipState permission was implemented these push options were forgotten: * 6def400 Add new change permission: Toggle Work In Progress state Bug: Issue 13775 Change-Id: I98fc845b6f93b4a6eeff9eba86c3f246b724400d --- .../gerrit/server/git/receive/ReceiveCommits.java | 6 +++--- .../server/git/receive/ReceiveConstants.java | 4 ++-- .../acceptance/git/AbstractPushForReview.java | 14 +++++++++++++- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index b0eac616dd..3b5b080009 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -25,7 +25,7 @@ import static com.google.gerrit.git.ObjectIds.abbreviateName; import static com.google.gerrit.server.change.HashtagsUtil.cleanupHashtag; import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN; import static com.google.gerrit.server.git.receive.ReceiveConstants.COMMAND_REJECTION_MESSAGE_FOOTER; -import static com.google.gerrit.server.git.receive.ReceiveConstants.ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP; +import static com.google.gerrit.server.git.receive.ReceiveConstants.ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP; import static com.google.gerrit.server.git.receive.ReceiveConstants.PUSH_OPTION_SKIP_VALIDATION; import static com.google.gerrit.server.git.receive.ReceiveConstants.SAME_CHANGE_ID_IN_MULTIPLE_CHANGES; import static com.google.gerrit.server.git.validators.CommitValidators.NEW_PATCHSET_PATTERN; @@ -2852,9 +2852,9 @@ class ReceiveCommits { if (!hasWriteConfigPermission) { try { - permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER); + permissions.change(notes).check(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE); } catch (AuthException e1) { - reject(inputCommand, ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP); + reject(inputCommand, ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP); } } } diff --git a/java/com/google/gerrit/server/git/receive/ReceiveConstants.java b/java/com/google/gerrit/server/git/receive/ReceiveConstants.java index 03a1b33bbe..df1888be3f 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveConstants.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveConstants.java @@ -20,8 +20,8 @@ public final class ReceiveConstants { public static final String PUSH_OPTION_SKIP_VALIDATION = "skip-validation"; @VisibleForTesting - public static final String ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP = - "only change owner or project owner can modify Work-in-Progress"; + public static final String ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP = + "only users with Toogle-Wip-State permission can modify Work-in-Progress"; static final String COMMAND_REJECTION_MESSAGE_FOOTER = "Contact an administrator to fix the permissions"; diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java index cab12b3d87..ea82013877 100644 --- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -900,7 +900,19 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { GitUtil.fetch(user2Repo, r.getPatchSet().refName() + ":ps"); user2Repo.reset("ps"); r = amendChange(r.getChangeId(), "refs/for/master%ready", user2, user2Repo); - r.assertErrorStatus(ReceiveConstants.ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP); + r.assertErrorStatus(ReceiveConstants.ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP); + + // Non owner, non admin and non project owner with toggleWipState should succeed. + projectOperations + .project(project) + .forUpdate() + .add( + allow(Permission.TOGGLE_WORK_IN_PROGRESS_STATE) + .ref(RefNames.REFS_HEADS + "*") + .group(REGISTERED_USERS)) + .update(); + r = amendChange(r.getChangeId(), "refs/for/master%ready", user2, user2Repo); + r.assertOkStatus(); // Project owner trying to move from WIP to ready should succeed. projectOperations From e871d659b3d57194270846947e32742484bf9563 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Thu, 3 Dec 2020 22:08:25 +0100 Subject: [PATCH 28/33] Update git submodules * Update plugins/replication from branch 'stable-3.3' to a6ca76650e4befab43f9fac288e6f27a38708dd1 - Remove dependency on commons-io library This dependency was added in Id12780948a4 to support remoteNameStyle "basenameOnly". The only reason for this dependency is to translate project name from "foo/bar/myrepo" to myrepo. It seems to be overkill to add 169 KB to the plugin distribution for one single method. Another disadvantage is that the version of common-io library used for this is 2.2 from 2012. If gerrit installation site is using some other plugins in addition to replication plugin, then it can easily lead to classpath collision, when different versions of commons-io libraries are included as transitive dependencies of different plugins. To rectify, use a replacement method from guava library. Change-Id: Id254dc38831832a9855bd204e4c2129ec64b88ae --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index 2b6d691d5f..a6ca76650e 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 2b6d691d5fb6f918cd15dbff2432e228e457aa0f +Subproject commit a6ca76650e4befab43f9fac288e6f27a38708dd1 From 59d05c7b55b305947ef85bbbd7dbd26daafda4e3 Mon Sep 17 00:00:00 2001 From: Ben Rohlfs Date: Mon, 30 Nov 2020 10:42:23 +0100 Subject: [PATCH 29/33] Remove requesting DETAILED_LABELS for the dashboard This is following up on change 288347. The backend was changed such that it is not required anymore for the attention set dashboard to request DETAILED_LABELS. Just LABELS are enough. Change-Id: I9e82c94fbecd3fff821e7a8e67281c3cc395751c (cherry picked from commit 56f7e79153f0ee17d7ac3192d75481015e83e773) --- java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java | 4 +--- .../shared/gr-rest-api-interface/gr-rest-api-interface.ts | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java index c17cd97bc3..f1da6b7190 100644 --- a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java +++ b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java @@ -123,9 +123,7 @@ public class IndexPreloadingUtil { options.add(ListChangesOption.LABELS); options.add(ListChangesOption.DETAILED_ACCOUNTS); - if (isEnabledAttentionSet(serverApi)) { - options.add(ListChangesOption.DETAILED_LABELS); - } else { + if (!isEnabledAttentionSet(serverApi)) { options.add(ListChangesOption.REVIEWED); } return ListOption.toHex(options); diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts index 7d08560920..b3596c7586 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts @@ -1422,9 +1422,7 @@ export class GrRestApiInterface ListChangesOption.LABELS, ListChangesOption.DETAILED_ACCOUNTS, ]; - if (config?.change && config.change.enable_attention_set) { - options.push(ListChangesOption.DETAILED_LABELS); - } else { + if (!config?.change?.enable_attention_set) { options.push(ListChangesOption.REVIEWED); } From 9c09efbc48038c3b03d5f388f95c845ae8b409c1 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 4 Dec 2020 10:16:30 +0000 Subject: [PATCH 30/33] Compact the REST-API output JSON unconditionally The output JSON was initially compacted only when the Accept header was set to 'application/json', from the very initial Change-Id: I2014a2209. The ability to get a pretty-printed JSON is still available using the 'pp=1' query string, therefore it is best to always compress the JSON output by default. Bug: Issue 13781 Change-Id: I2f1cb91e95e10b27b328764834d58d0799e01ed4 --- .../gerrit/httpd/restapi/RestApiServlet.java | 23 ++---- .../acceptance/rest/RestApiServletIT.java | 76 +++++++++++++++++++ 2 files changed, 82 insertions(+), 17 deletions(-) create mode 100644 javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 665cc33e46..95338505db 100644 --- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -1328,7 +1328,7 @@ public class RestApiServlet extends HttpServlet { TemporaryBuffer.Heap buf = heap(HEAP_EST_SIZE, Integer.MAX_VALUE); buf.write(JSON_MAGIC); Writer w = new BufferedWriter(new OutputStreamWriter(buf, UTF_8)); - Gson gson = newGson(config, req); + Gson gson = newGson(config); if (result instanceof JsonElement) { gson.toJson((JsonElement) result, w); } else { @@ -1355,25 +1355,18 @@ public class RestApiServlet extends HttpServlet { req, res, asBinaryResult(buf).setContentType(JSON_TYPE).setCharacterEncoding(UTF_8)); } - private static Gson newGson( - ListMultimap config, @Nullable HttpServletRequest req) { + private static Gson newGson(ListMultimap config) { GsonBuilder gb = OutputFormat.JSON_COMPACT.newGsonBuilder(); - enablePrettyPrint(gb, config, req); + enablePrettyPrint(gb, config); enablePartialGetFields(gb, config); return gb.create(); } - private static void enablePrettyPrint( - GsonBuilder gb, ListMultimap config, @Nullable HttpServletRequest req) { - String pp = Iterables.getFirst(config.get("pp"), null); - if (pp == null) { - pp = Iterables.getFirst(config.get("prettyPrint"), null); - if (pp == null && req != null) { - pp = acceptsJson(req) ? "0" : "1"; - } - } + private static void enablePrettyPrint(GsonBuilder gb, ListMultimap config) { + String pp = + Iterables.getFirst(config.get("pp"), Iterables.getFirst(config.get("prettyPrint"), "0")); if ("1".equals(pp) || "true".equals(pp)) { gb.setPrettyPrinting(); } @@ -1886,10 +1879,6 @@ public class RestApiServlet extends HttpServlet { return CharMatcher.anyOf("<&").matchesAnyOf(text); } - private static boolean acceptsJson(HttpServletRequest req) { - return req != null && isType(JSON_TYPE, req.getHeader(HttpHeaders.ACCEPT)); - } - private static boolean acceptsGzip(HttpServletRequest req) { if (req != null) { String accepts = req.getHeader(HttpHeaders.ACCEPT_ENCODING); diff --git a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java new file mode 100644 index 0000000000..bc454605ab --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java @@ -0,0 +1,76 @@ +// Copyright (C) 2020 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.acceptance.rest; + +import static com.google.common.truth.Truth.assertThat; +import static org.apache.http.HttpStatus.SC_OK; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.httpd.restapi.RestApiServlet; +import java.io.IOException; +import java.util.regex.Pattern; +import org.apache.http.message.BasicHeader; +import org.junit.Test; + +public class RestApiServletIT extends AbstractDaemonTest { + private static String ANY_REST_API = "/accounts/self/capabilities"; + private static BasicHeader ACCEPT_STAR_HEADER = new BasicHeader("Accept", "*/*"); + private static Pattern ANY_SPACE = Pattern.compile("\\s"); + + @Test + public void restResponseBodyShouldBeCompactWithoutSpaces() throws Exception { + RestResponse response = adminRestSession.getWithHeader(ANY_REST_API, ACCEPT_STAR_HEADER); + assertThat(response.getStatusCode()).isEqualTo(SC_OK); + + assertThat(contentWithoutMagicJson(response)).doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBeCompactWithoutSpacesWhenPPIsZero() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("prettyPrint", 0))) + .doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBeCompactWithoutSpacesWhenPrerryPrintIsZero() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("pp", 0))) + .doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBePrettyfiedWhenPPIsOne() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("pp", 1))).containsMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBePrettyfiedWhenPrettyPrintIsOne() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("prettyPrint", 1))) + .containsMatch(ANY_SPACE); + } + + private RestResponse prettyJsonRestResponse(String ppArgument, int ppValue) throws Exception { + RestResponse response = + adminRestSession.getWithHeader( + ANY_REST_API + "?" + ppArgument + "=" + ppValue, ACCEPT_STAR_HEADER); + assertThat(response.getStatusCode()).isEqualTo(SC_OK); + + return response; + } + + private String contentWithoutMagicJson(RestResponse response) throws IOException { + return response.getEntityContent().substring(RestApiServlet.JSON_MAGIC.length); + } +} From f97056cd4ad303db151acefbb76099cdb96240e9 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 4 Dec 2020 23:20:19 +0000 Subject: [PATCH 31/33] Add z-index to gr-main-header The Gerrit header like the footer needs to be always in foreground, as they may also include some box shadows that otherwise would be hidden behind the content. Bug: Issue 13785 Change-Id: Ic6fe3f359a7da1e51a015b4303169f19ba33d98a --- polygerrit-ui/app/elements/gr-app-element_html.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/polygerrit-ui/app/elements/gr-app-element_html.ts b/polygerrit-ui/app/elements/gr-app-element_html.ts index 88cd7f5c88..7dc6833932 100644 --- a/polygerrit-ui/app/elements/gr-app-element_html.ts +++ b/polygerrit-ui/app/elements/gr-app-element_html.ts @@ -40,6 +40,8 @@ export const htmlTemplate = html` border-left: 0; border-top: 0; box-shadow: var(--header-box-shadow); + /* Make sure the header is above the main content, to preserve box-shadow visibility */ + z-index: 1; } footer { background: var( From a2e15d5cd4fa6afe1f9d391e19a80cfde4695d94 Mon Sep 17 00:00:00 2001 From: Takashi Kudo Date: Mon, 7 Dec 2020 20:42:22 +0900 Subject: [PATCH 32/33] Update git submodules * Update plugins/gitiles from branch 'stable-3.2' to b8ea789876c0661e1aef9fa3f54a6ac3b8abd62c - Fix error handling when project state is empty Before this change, an unexpected exception java.util.NoSuchElementException was thrown when the project state is empty, then Resolver could not handle this exception and this causes a Server Error. To fix this issue, a present check was added before the project state check. Bug: Issue 13492 Change-Id: I40a128bba8a0794b7daeb019ef0f75cefedef342 (cherry picked from commit a33c8b8d61b778f8ca84196b1a7cc1fd4fe24946) --- plugins/gitiles | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/gitiles b/plugins/gitiles index 641476e153..b8ea789876 160000 --- a/plugins/gitiles +++ b/plugins/gitiles @@ -1 +1 @@ -Subproject commit 641476e153143c2b67e334b35626beb9b2534956 +Subproject commit b8ea789876c0661e1aef9fa3f54a6ac3b8abd62c From 81cd8fff69286f76b6b6d013149c37fc387811f4 Mon Sep 17 00:00:00 2001 From: Sven Selberg Date: Tue, 8 Dec 2020 12:05:47 +0100 Subject: [PATCH 33/33] Update git submodules * Update plugins/gitiles from branch 'stable-3.3' to 584360d050f5adf8317ebc078a874da9c4316bd0 - Merge branch 'stable-3.2' into stable-3.3 * stable-3.2: Fix error handling when project state is empty Change-Id: Id657bfab45f3d0c06f5d7a6878f3986edf00d208 --- plugins/gitiles | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/gitiles b/plugins/gitiles index b8ea789876..584360d050 160000 --- a/plugins/gitiles +++ b/plugins/gitiles @@ -1 +1 @@ -Subproject commit b8ea789876c0661e1aef9fa3f54a6ac3b8abd62c +Subproject commit 584360d050f5adf8317ebc078a874da9c4316bd0