Merge branch 'stable-3.8' into stable-3.9

* stable-3.8:
  Make gr-comment-thread test reliable in stable-3.6
  Bazel: Remove left over --java_toolchain option
  Optionally skip submit rules evaluation on closed changes
  Optimise the way that `SubmitRuleEvaluator.evaluate` loads objects
  SubmitRuleEvaluator: Simply logic that checks that change and project exist
  Update Jetty to 9.4.53.v20231009 for security updates
  Bump JGit to v5.13.2.202306221912-r (5aa8a7e27)

Release-Notes: skip
Change-Id: Ib577d1452d5646341c93550763949ab54b127721

10 files changed, 185 insertions, 4 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 52d9da00da..67a16d66b0 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1707,6 +1707,21 @@ link:rest-api-changes.html#get-diff[file diff] request will fail.
 +
 If not set or set to zero, no limits are applied on file sizes.
 
+[[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/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 82d1dda087..21ae8e1586 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -63,6 +63,7 @@ import com.google.gerrit.server.config.EnablePeerIPInReflogRecordProvider;
 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.AttentionSetObserver;
 import com.google.gerrit.server.extensions.events.EventUtil;
@@ -215,6 +216,7 @@ public class BatchProgramModule extends FactoryModule {
     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 81d48ade5a..fe82a8852f 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -294,6 +294,7 @@ public class GerritGlobalModule extends FactoryModule {
     install(new ApprovalModule());
     install(new MailSoySauceModule());
     install(new VersionInfoModule());
+    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 0000000000..b812710ca1
--- /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 0000000000..9eaae021e0
--- /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 0000000000..bc25a3353f
--- /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/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 0c6277e6ba..0c02920cb0 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -78,6 +78,7 @@ import com.google.gerrit.server.change.PureRevert;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GerritServerId;
+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.MergeUtilFactory;
@@ -337,6 +338,7 @@ public class ChangeData {
             null,
             serverId,
             virtualIdAlgo,
+            false,
             project,
             id,
             null,
@@ -374,6 +376,7 @@ public class ChangeData {
   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;
@@ -470,7 +473,8 @@ public class ChangeData {
       SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
       @GerritServerId String gerritServerId,
       ChangeNumberVirtualIdAlgorithm virtualIdFunc,
-      @Assisted NameKey project,
+      @SkipCurrentRulesEvaluationOnClosedChanges Boolean skipCurrentRulesEvaluationOnClosedChange,
+      @Assisted Project.NameKey project,
       @Assisted Change.Id id,
       @Assisted @Nullable Change change,
       @Assisted @Nullable ChangeNotes notes) {
@@ -496,6 +500,7 @@ public class ChangeData {
     this.submitRequirementsEvaluator = submitRequirementsEvaluator;
     this.submitRequirementsUtil = submitRequirementsUtil;
     this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
+    this.skipCurrentRulesEvaluationOnClosedChanges = skipCurrentRulesEvaluationOnClosedChange;
 
     this.project = project;
     this.legacyId = id;
@@ -1194,6 +1199,9 @@ public class ChangeData {
             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 8bdd42fee0..d6874d87a1 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -32,6 +32,7 @@ import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.b
 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;
@@ -3630,6 +3631,74 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @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/tools/eclipse/project.py b/tools/eclipse/project.py
index 3ad9e0c260..9e54e7f5fe 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -96,8 +96,7 @@ def _build_bazel_cmd(*args):
             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 eacb02b9f4..b25656da7f 100644
--- a/tools/maven/package.bzl
+++ b/tools/maven/package.bzl
@@ -40,7 +40,7 @@ def maven_package(
         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 = []