Merge changes Ie0e40ee9,Ifcc30aaa into stable-3.6
* changes:
Document ChecksApi.updateResult()
Document `check-result-expanded` plugin endpoint
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 4b89c6b..524b171 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1662,6 +1662,21 @@
+
Default is 5 minutes.
+[[change.skipCurrentRulesEvaluationOnClosedChanges]]
++
+If false, Gerrit will always take latest project configuration to
+compute submit labels. This means that, closed changes (either merged
+or abandoned) will be evaluated against the latest configuration which
+may produce different results. Especially for merged changes, they may
+look like they didn't meet the submit requirements.
++
+When true, evaluation will be skipped and Gerrit will show the
+exact status of submit labels when change was submitted. Post-review
+votes will only be allowed on labels that were configured when change
+was closed.
++
+Default it false.
+
[[changeCleanup]]
=== Section changeCleanup
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index d1a5bcf..08529df 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -54,7 +54,7 @@
To build Gerrit with Java 11 language level, run:
```
- $ bazel build --java_toolchain=//tools:error_prone_warnings_toolchain_java11 :release
+ $ bazel build :release
```
[[java-17]]
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 78a162b..fe5966c 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -60,6 +60,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
+import com.google.gerrit.server.config.SkipCurrentRulesEvaluationOnClosedChangesModule;
import com.google.gerrit.server.config.SysExecutorModule;
import com.google.gerrit.server.extensions.events.EventUtil;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -209,6 +210,7 @@
modules.add(new PrologModule(getConfig()));
modules.add(new DefaultSubmitRuleModule());
modules.add(new IgnoreSelfApprovalRuleModule());
+ modules.add(new SkipCurrentRulesEvaluationOnClosedChangesModule());
// Global submit requirements
DynamicSet.setOf(binder(), SubmitRequirement.class);
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 6449155..d6baa7f 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -290,6 +290,7 @@
install(ThreadLocalRequestContext.module());
install(new ApprovalModule());
install(new MailSoySauceModule());
+ install(new SkipCurrentRulesEvaluationOnClosedChangesModule());
factory(CapabilityCollection.Factory.class);
factory(ChangeData.AssistedFactory.class);
diff --git a/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChanges.java b/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChanges.java
new file mode 100644
index 0000000..b812710
--- /dev/null
+++ b/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChanges.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2023 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.server.config;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+import java.lang.annotation.Retention;
+
+/** Marker on a {@link Boolean} holding the evaluation of current Prolog rules on closed changes. */
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface SkipCurrentRulesEvaluationOnClosedChanges {}
diff --git a/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChangesModule.java b/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChangesModule.java
new file mode 100644
index 0000000..9eaae02
--- /dev/null
+++ b/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChangesModule.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2023 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.server.config;
+
+import com.google.inject.AbstractModule;
+
+public class SkipCurrentRulesEvaluationOnClosedChangesModule extends AbstractModule {
+
+ @Override
+ protected void configure() {
+ bind(Boolean.class)
+ .annotatedWith(SkipCurrentRulesEvaluationOnClosedChanges.class)
+ .toProvider(SkipCurrentRulesEvaluationOnClosedChangesProvider.class);
+ }
+}
diff --git a/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChangesProvider.java b/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChangesProvider.java
new file mode 100644
index 0000000..bc25a33
--- /dev/null
+++ b/java/com/google/gerrit/server/config/SkipCurrentRulesEvaluationOnClosedChangesProvider.java
@@ -0,0 +1,35 @@
+// Copyright (C) 2023 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.server.config;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import org.eclipse.jgit.lib.Config;
+
+@Singleton
+public class SkipCurrentRulesEvaluationOnClosedChangesProvider implements Provider<Boolean> {
+ private final Boolean value;
+
+ @Inject
+ SkipCurrentRulesEvaluationOnClosedChangesProvider(@GerritServerConfig Config config) {
+ value = config.getBoolean("change", null, "skipCurrentRulesEvaluationOnClosedChanges", false);
+ }
+
+ @Override
+ public Boolean get() {
+ return value;
+ }
+}
diff --git a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 513303b..c5928f6 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -18,7 +18,6 @@
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.entities.SubmitTypeRecord;
@@ -118,25 +117,12 @@
"Evaluate submit rules for change %d (caller: %s)",
cd.change().getId().get(), callerFinder.findCallerLazy());
try (Timer0.Context ignored = metrics.submitRuleEvaluationLatency.start()) {
- Change change;
- ProjectState projectState;
- try {
- change = cd.change();
- if (change == null) {
- throw new StorageException("Change not found");
- }
-
- Project.NameKey name = cd.project();
- Optional<ProjectState> projectStateOptional = projectCache.get(name);
- if (!projectStateOptional.isPresent()) {
- throw new NoSuchProjectException(name);
- }
- projectState = projectStateOptional.get();
- } catch (NoSuchProjectException e) {
- throw new IllegalStateException("Unable to find project while evaluating submit rule", e);
+ if (cd.change() == null) {
+ throw new StorageException("Change not found");
}
- if (change.isClosed() && (!opts.recomputeOnClosedChanges() || OnlineReindexMode.isActive())) {
+ if (cd.change().isClosed()
+ && (!opts.recomputeOnClosedChanges() || OnlineReindexMode.isActive())) {
return cd.notes().getSubmitRecords().stream()
.map(
r -> {
@@ -150,6 +136,15 @@
.collect(toImmutableList());
}
+ ProjectState projectState =
+ projectCache
+ .get(cd.project())
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Unable to find project while evaluating submit rule",
+ new NoSuchProjectException(cd.project())));
+
// We evaluate all the plugin-defined evaluators,
// and then we collect the results in one list.
return Streams.stream(submitRules)
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index cc08254..3f5f63b 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -75,6 +75,7 @@
import com.google.gerrit.server.change.MergeabilityCache;
import com.google.gerrit.server.change.PureRevert;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.SkipCurrentRulesEvaluationOnClosedChanges;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
@@ -284,7 +285,7 @@
ChangeData cd =
new ChangeData(
null, null, null, null, null, null, null, null, null, null, null, null, null, null,
- null, null, null, project, id, null, null);
+ null, null, null, false, project, id, null, null);
cd.currentPatchSet =
PatchSet.builder()
.id(PatchSet.id(id, currentPatchSetId))
@@ -313,6 +314,7 @@
private final SubmitRequirementsEvaluator submitRequirementsEvaluator;
private final SubmitRequirementsUtil submitRequirementsUtil;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
+ private final boolean skipCurrentRulesEvaluationOnClosedChanges;
// Required assisted injected fields.
private final Project.NameKey project;
@@ -400,6 +402,7 @@
SubmitRequirementsEvaluator submitRequirementsEvaluator,
SubmitRequirementsUtil submitRequirementsUtil,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
+ @SkipCurrentRulesEvaluationOnClosedChanges Boolean skipCurrentRulesEvaluationOnClosedChange,
@Assisted Project.NameKey project,
@Assisted Change.Id id,
@Assisted @Nullable Change change,
@@ -421,6 +424,7 @@
this.submitRequirementsEvaluator = submitRequirementsEvaluator;
this.submitRequirementsUtil = submitRequirementsUtil;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
+ this.skipCurrentRulesEvaluationOnClosedChanges = skipCurrentRulesEvaluationOnClosedChange;
this.project = project;
this.legacyId = id;
@@ -1083,6 +1087,9 @@
project(), getId().get());
return Collections.emptyList();
}
+ if (skipCurrentRulesEvaluationOnClosedChanges && change().isClosed()) {
+ return notes().getSubmitRecords();
+ }
records = submitRuleEvaluatorFactory.create(options).evaluate(this);
submitRecords.put(options, records);
if (!change().isClosed() && submitRecords.size() == 1) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index c5f0d23..86691c6 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -31,6 +31,7 @@
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.labelPermissionKey;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.permissionKey;
import static com.google.gerrit.entities.RefNames.changeMetaRef;
+import static com.google.gerrit.extensions.client.ChangeStatus.ABANDONED;
import static com.google.gerrit.extensions.client.ChangeStatus.MERGED;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CHANGE_ACTIONS;
@@ -3806,6 +3807,74 @@
}
@Test
+ @GerritConfig(name = "change.skipCurrentRulesEvaluationOnClosedChanges", value = "true")
+ public void checkLabelsNotUpdatedForMergedChange() throws Exception {
+ 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 change = gApi.changes().id(r.getChangeId()).get();
+ assertThat(change.status).isEqualTo(MERGED);
+ assertThat(change.submissionId).isNotNull();
+ assertThat(change.labels.keySet()).containsExactly(LabelId.CODE_REVIEW);
+ assertThat(change.permittedLabels.keySet()).containsExactly(LabelId.CODE_REVIEW);
+ assertPermitted(change, LabelId.CODE_REVIEW, 2);
+
+ LabelType verified = TestLabels.verified();
+ AccountGroup.UUID registeredUsers = systemGroupBackend.getGroup(REGISTERED_USERS).getUUID();
+ String heads = RefNames.REFS_HEADS + "*";
+
+ // add new label and assert that it's returned for existing changes
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig().upsertLabelType(verified);
+ u.save();
+ }
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel(verified.getName()).ref(heads).group(registeredUsers).range(-1, 1))
+ .update();
+
+ change = gApi.changes().id(r.getChangeId()).get();
+ assertThat(change.labels.keySet()).containsExactly(LabelId.CODE_REVIEW);
+ assertThat(change.permittedLabels.keySet())
+ .containsExactly(LabelId.CODE_REVIEW, LabelId.VERIFIED);
+ assertPermitted(change, LabelId.CODE_REVIEW, 2);
+ }
+
+ @Test
+ @GerritConfig(name = "change.skipCurrentRulesEvaluationOnClosedChanges", value = "true")
+ public void checkLabelsNotUpdatedForAbandonedChange() throws Exception {
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).abandon();
+
+ ChangeInfo change = gApi.changes().id(r.getChangeId()).get();
+ assertThat(change.status).isEqualTo(ABANDONED);
+ assertThat(change.labels.keySet()).isEmpty();
+ assertThat(change.submitRecords).isEmpty();
+
+ LabelType verified = TestLabels.verified();
+ AccountGroup.UUID registeredUsers = systemGroupBackend.getGroup(REGISTERED_USERS).getUUID();
+ String heads = RefNames.REFS_HEADS + "*";
+
+ // add new label and assert that it's returned for existing changes
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig().upsertLabelType(verified);
+ u.save();
+ }
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel(verified.getName()).ref(heads).group(registeredUsers).range(-1, 1))
+ .update();
+
+ change = gApi.changes().id(r.getChangeId()).get();
+ assertThat(change.labels.keySet()).isEmpty();
+ assertThat(change.permittedLabels.keySet()).isEmpty();
+ assertThat(change.submitRecords).isEmpty();
+ }
+
+ @Test
public void notifyConfigForDirectoryTriggersEmail() throws Exception {
// Configure notifications on project level.
RevCommit oldHead = projectOperations.project(project).getHead("master");
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index 685151b..8cbbcee 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -953,7 +953,7 @@
commit?: CommitInfo;
files?: {[filename: string]: FileInfo};
reviewed?: boolean;
- commit_with_footers?: boolean;
+ commit_with_footers?: string;
push_certificate?: PushCertificateInfo;
description?: string;
basePatchNum?: BasePatchSetNum;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 0329a4e..3573197 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -278,27 +278,32 @@
element.showCommentContext = true;
element.thread = createThread(commentWithContext);
await element.updateComplete;
- expect(queryAndAssert(element, '.diff-container')).dom.to.equal(/* HTML */ `
- <div class="diff-container">
- <gr-diff
- class="disable-context-control-buttons hide-line-length-indicator no-left"
- id="diff"
- style="--line-limit-marker:100ch; --content-width:none; --diff-max-width:none; --font-size:12px;"
- >
- </gr-diff>
- <div class="view-diff-container">
- <a href="">
- <gr-button
- aria-disabled="false"
- class="view-diff-button"
- link=""
- role="button"
- tabindex="0"
- >
- View Diff
- </gr-button>
- </a>
- </div>
+
+ const gr_diff = queryAndAssert(element, '.diff-container gr-diff');
+ expect(gr_diff.id).to.equal('diff');
+ const gr_diff_classes = gr_diff.classList.value;
+ expect(gr_diff_classes).to.include('disable-context-control-buttons');
+ expect(gr_diff_classes).to.include('hide-line-length-indicator');
+ expect(gr_diff_classes).to.include('no-left');
+ // @ts-ignore
+ expect(gr_diff.getAttribute('style').replace(/\s+/g, '')).to.equal(
+ '--line-limit-marker:100ch;--content-width:none;--diff-max-width:none;--font-size:12px;'
+ );
+
+ expect(queryAndAssert(element, '.diff-container .view-diff-container')).dom
+ .to.equal(/* HTML */ `
+ <div class="view-diff-container">
+ <a href="">
+ <gr-button
+ aria-disabled="false"
+ class="view-diff-button"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ View Diff
+ </gr-button>
+ </a>
</div>
`);
});
diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py
index d574ecf..dc136ed 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -96,8 +96,7 @@
build = True
cmd.append(arg)
if custom_java:
- cmd.append('--host_java_toolchain=@bazel_tools//tools/jdk:toolchain_java%s' % custom_java)
- cmd.append('--java_toolchain=@bazel_tools//tools/jdk:toolchain_java%s' % custom_java)
+ cmd.append('--config=java%s' % custom_java)
return cmd
diff --git a/tools/maven/package.bzl b/tools/maven/package.bzl
index eacb02b..b25656d 100644
--- a/tools/maven/package.bzl
+++ b/tools/maven/package.bzl
@@ -40,7 +40,7 @@
src = {},
doc = {},
war = {}):
- build_cmd = ["bazel_cmd", "build", "--java_toolchain=//tools:error_prone_warnings_toolchain_java11"]
+ build_cmd = ["bazel_cmd", "build"]
mvn_cmd = ["python", "tools/maven/mvn.py", "-v", version]
api_cmd = mvn_cmd[:]
api_targets = []
