Merge "Merge branch 'stable-3.10'"upstream/master

12 files changed, 283 insertions, 77 deletions

diff --git a/.bazelproject b/.bazelproject
index ad7b022328..0f2ff90381 100644
--- a/.bazelproject
+++ b/.bazelproject
@@ -16,7 +16,7 @@ directories:
 targets:
   //...:all
 
-java_language_level: 11
+java_language_level: 17
 
 workspace_type: java
 
diff --git a/.bazelrc b/.bazelrc
index 96620783d1..480bea79c6 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -32,25 +32,6 @@ build:remote_gcp --config=remote
 build:remote_bb --config=config_bb
 build:remote_bb --config=build_shared
 
-# Define configuration using remotejdk_11, executes using remotejdk_11 or local_jdk
-build:build_java11_shared --java_language_version=11
-build:build_java11_shared --java_runtime_version=remotejdk_11
-build:build_java11_shared --tool_java_language_version=11
-build:build_java11_shared --tool_java_runtime_version=remotejdk_11
-
-build:java11 --config=build_java11_shared
-
-# Builds and executes on Google GCP RBE using remotejdk_11
-build:remote11 --config=config_gcp
-build:remote11 --config=build_java11_shared
-
-# Define remote11 configuration alias
-build:remote11_gcp --config=remote11
-
-# Builds and executes on BuildBuddy RBE using remotejdk_11
-build:remote11_bb --config=config_bb
-build:remote11_bb --config=build_java11_shared
-
 # Builds using remotejdk_21, executes using remotejdk_21 or local_jdk
 build:build_java21_shared --java_language_version=21
 build:build_java21_shared --java_runtime_version=remotejdk_21
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index dbda4101d6..b338148173 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -4648,6 +4648,37 @@ options even if they are not registered by any plugin(e.g. "myplugin~foo").
 This config can be used when gerrit migrates from a deprecated plugin to the new one. The new plugin
 can (temporary) accept push options of the old plugin without registering such options.
 
+[[plugins.loadPriority]]plugins.loadPriority::
++
+List of `pluginName`s required to have a specific loading order during Gerrit startup.
++
+Each entry should contain a plugin name defined in the `MANIFEST.MF` under
+`Gerrit-PluginName` or a plugin JAR file name. During the Gerrit startup
+the `loadPriority` will influence the loading sequence of the JAR plugins.
++
+NOTE: Non-JAR plugins (e.g. Scripting, PolyGerrit plugins, ApiModule) are not
+influenced by this setting.
++
+Gerrit will always load plugins defining `Gerrit-ApiModule` in their
+`MANIFEST.MF` first. Then will load other plugins according to:
+
+ * the order of `plugins.loadPriority` in `gerrit.config`,
+ * the natural order of plugin JAR file names in `plugins/` directory.
++
+Example:
+Assuming we have three plugins: `a-plugin.jar`, `b-plugin.jar`, `c-plugin.jar`
+deployed to `plugins/` directory. By default Gerrit will load them in the same order
+as they are listed above, as that follows the _natural storing order_. Now assume
+the below configuration
+
+----
+[plugins]
+  loadPriority = c-plugin
+  loadPriority = a-plugin
+----
+
+Gerrit will load `c-plugin` first, followed-up by `a-plugin` and `b-plugin` last.
+
 [[receive]]
 === Section receive
 
diff --git a/Documentation/config-robot-comments.txt b/Documentation/config-robot-comments.txt
index 04309e5ffc..ef99d80213 100644
--- a/Documentation/config-robot-comments.txt
+++ b/Documentation/config-robot-comments.txt
@@ -1,5 +1,9 @@
 = Gerrit Code Review - Robot Comments
 
+[NOTE]
+Robot Comments are deprecated in favour of link:pg-plugin-checks-api.html[Checks API] and human
+comments.
+
 Gerrit has special support for inline comments that are generated by
 automated third-party systems, so called "robot comments". For example
 robot comments can be used to represent the results of code analyzers.
diff --git a/java/Main.java b/java/Main.java
index c04db2cb34..e824a95996 100644
--- a/java/Main.java
+++ b/java/Main.java
@@ -16,7 +16,7 @@
 public final class Main {
   private static final String FLOGGER_BACKEND_PROPERTY = "flogger.backend_factory";
   private static final String FLOGGER_LOGGING_CONTEXT = "flogger.logging_context";
-  private static final Runtime.Version MIN_JAVA_VERSION = Runtime.Version.parse("11.0.10");
+  private static final Runtime.Version MIN_JAVA_VERSION = Runtime.Version.parse("17.0.5");
 
   // We don't do any real work here because we need to import
   // the archive lookup code and we cannot import a class in
diff --git a/java/com/google/gerrit/server/git/WorkQueue.java b/java/com/google/gerrit/server/git/WorkQueue.java
index d51ee5eee0..307ec5c02f 100644
--- a/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -504,11 +504,11 @@ public class WorkQueue {
     }
 
     public void onStart(Task<?> task) {
-      listeners.runEach(extension -> extension.getProvider().get().onStart(task));
+      listeners.runEach(extension -> extension.get().onStart(task));
     }
 
     public void onStop(Task<?> task) {
-      listeners.runEach(extension -> extension.getProvider().get().onStop(task));
+      listeners.runEach(extension -> extension.get().onStop(task));
     }
   }
 
diff --git a/java/com/google/gerrit/server/plugins/PluginLoader.java b/java/com/google/gerrit/server/plugins/PluginLoader.java
index e122d8de32..7c7d8d5aca 100644
--- a/java/com/google/gerrit/server/plugins/PluginLoader.java
+++ b/java/com/google/gerrit/server/plugins/PluginLoader.java
@@ -18,7 +18,6 @@ import com.google.common.base.CharMatcher;
 import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
-import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
@@ -52,7 +51,6 @@ import java.util.AbstractMap;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Comparator;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -62,7 +60,6 @@ import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
-import java.util.jar.JarFile;
 import org.eclipse.jgit.internal.storage.file.FileSnapshot;
 import org.eclipse.jgit.lib.Config;
 
@@ -93,6 +90,7 @@ public class PluginLoader implements LifecycleListener {
   private final MandatoryPluginsCollection mandatoryPlugins;
   private final UniversalServerPluginProvider serverPluginFactory;
   private final GerritRuntime gerritRuntime;
+  private final PluginOrderComparator pluginOrderComparator;
 
   @Inject
   public PluginLoader(
@@ -122,6 +120,10 @@ public class PluginLoader implements LifecycleListener {
     mandatoryPlugins = mpc;
     this.gerritRuntime = gerritRuntime;
 
+    ImmutableList<String> pluginOrderOverrides =
+        ImmutableList.copyOf(cfg.getStringList("plugins", null, "loadPriority"));
+    pluginOrderComparator = new PluginOrderComparator(pluginOrderOverrides);
+
     long checkFrequency =
         ConfigUtil.getTimeUnit(
             cfg,
@@ -473,43 +475,7 @@ public class PluginLoader implements LifecycleListener {
 
   private TreeSet<Map.Entry<String, Path>> jarsApiFirstSortedPluginsSet(
       Map<String, Path> activePlugins) {
-    TreeSet<Map.Entry<String, Path>> sortedPlugins =
-        Sets.newTreeSet(
-            new Comparator<Map.Entry<String, Path>>() {
-              @Override
-              public int compare(Map.Entry<String, Path> e1, Map.Entry<String, Path> e2) {
-                Path n1 = e1.getValue().getFileName();
-                Path n2 = e2.getValue().getFileName();
-
-                try {
-                  boolean e1IsApi = isApi(e1.getValue());
-                  boolean e2IsApi = isApi(e2.getValue());
-                  return ComparisonChain.start()
-                      .compareTrueFirst(e1IsApi, e2IsApi)
-                      .compareTrueFirst(isJar(n1), isJar(n2))
-                      .compare(n1, n2)
-                      .result();
-                } catch (IOException ioe) {
-                  logger.atSevere().withCause(ioe).log("Unable to compare %s and %s", n1, n2);
-                  return 0;
-                }
-              }
-
-              private boolean isJar(Path pluginPath) {
-                return pluginPath.toString().endsWith(".jar");
-              }
-
-              private boolean isApi(Path pluginPath) throws IOException {
-                return isJar(pluginPath) && hasApiModuleEntryInManifest(pluginPath);
-              }
-
-              private boolean hasApiModuleEntryInManifest(Path pluginPath) throws IOException {
-                try (JarFile jarFile = new JarFile(pluginPath.toFile())) {
-                  return !Strings.isNullOrEmpty(
-                      jarFile.getManifest().getMainAttributes().getValue(ServerPlugin.API_MODULE));
-                }
-              }
-            });
+    TreeSet<Map.Entry<String, Path>> sortedPlugins = Sets.newTreeSet(pluginOrderComparator);
 
     addAllEntries(activePlugins, sortedPlugins);
     return sortedPlugins;
diff --git a/java/com/google/gerrit/server/plugins/PluginOrderComparator.java b/java/com/google/gerrit/server/plugins/PluginOrderComparator.java
new file mode 100644
index 0000000000..45ff9ebe1e
--- /dev/null
+++ b/java/com/google/gerrit/server/plugins/PluginOrderComparator.java
@@ -0,0 +1,95 @@
+// Copyright (C) 2024 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.plugins;
+
+import com.google.common.base.Strings;
+import com.google.common.collect.ComparisonChain;
+import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.Map;
+import java.util.jar.JarFile;
+import java.util.jar.Manifest;
+
+class PluginOrderComparator implements Comparator<Map.Entry<String, Path>> {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+  private static final ManifestLoader DEFAULT_LOADER =
+      pluginPath -> {
+        try (JarFile jarFile = new JarFile(pluginPath.toFile())) {
+          return jarFile.getManifest();
+        }
+      };
+
+  @FunctionalInterface
+  interface ManifestLoader {
+    Manifest load(Path pluginPath) throws IOException;
+  }
+
+  private final ManifestLoader manifestLoader;
+  private final ImmutableList<String> pluginLoadOrderOverrides;
+
+  PluginOrderComparator(ImmutableList<String> pluginLoadOrderOverrides) {
+    this(pluginLoadOrderOverrides, DEFAULT_LOADER);
+  }
+
+  PluginOrderComparator(
+      ImmutableList<String> pluginLoadOrderOverrides, ManifestLoader manifestLoader) {
+    this.manifestLoader = manifestLoader;
+    this.pluginLoadOrderOverrides = pluginLoadOrderOverrides;
+  }
+
+  @Override
+  public int compare(Map.Entry<String, Path> e1, Map.Entry<String, Path> e2) {
+    Path n1 = e1.getValue().getFileName();
+    Path n2 = e2.getValue().getFileName();
+
+    try {
+      boolean e1IsApi = isApi(e1.getValue());
+      boolean e2IsApi = isApi(e2.getValue());
+      return ComparisonChain.start()
+          .compareTrueFirst(e1IsApi, e2IsApi)
+          .compareTrueFirst(isJar(n1), isJar(n2))
+          .compare(loadOrderOverrides(e1.getKey()), loadOrderOverrides(e2.getKey()))
+          .compare(n1, n2)
+          .result();
+    } catch (IOException ioe) {
+      logger.atSevere().withCause(ioe).log("Unable to compare %s and %s", n1, n2);
+      return 0;
+    }
+  }
+
+  private boolean isJar(Path pluginPath) {
+    return pluginPath.toString().endsWith(".jar");
+  }
+
+  private boolean isApi(Path pluginPath) throws IOException {
+    return isJar(pluginPath) && hasApiModuleEntryInManifest(pluginPath);
+  }
+
+  private boolean hasApiModuleEntryInManifest(Path pluginPath) throws IOException {
+    return !Strings.isNullOrEmpty(
+        manifestLoader.load(pluginPath).getMainAttributes().getValue(ServerPlugin.API_MODULE));
+  }
+
+  private int loadOrderOverrides(String pluginName) throws IOException {
+    int pluginNameIndex = pluginLoadOrderOverrides.indexOf(pluginName);
+    if (pluginNameIndex > -1) {
+      return pluginNameIndex - pluginLoadOrderOverrides.size();
+    }
+    return 0;
+  }
+}
diff --git a/javatests/com/google/gerrit/server/plugins/PluginOrderComparatorTest.java b/javatests/com/google/gerrit/server/plugins/PluginOrderComparatorTest.java
new file mode 100644
index 0000000000..4f7402a347
--- /dev/null
+++ b/javatests/com/google/gerrit/server/plugins/PluginOrderComparatorTest.java
@@ -0,0 +1,142 @@
+// Copyright (C) 2024 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.plugins;
+
+import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.jar.Manifest;
+import org.junit.Test;
+
+public class PluginOrderComparatorTest {
+  private static final String API_MODULE = "Gerrit-ApiModule: com.google.gerrit.UnitTest";
+
+  private static final Path FIRST_PLUGIN_PATH = Paths.get("01-first.jar");
+  private static final Path SECOND_PLUGIN_PATH = Paths.get("20-second.jar");
+  private static final Path THIRD_PLUGIN_PATH = Paths.get("30-third.jar");
+  private static final Path LAST_PLUGIN_PATH = Paths.get("99-last.jar");
+
+  private static final Map.Entry<String, Path> FIRST_ENTRY = Map.entry("first", FIRST_PLUGIN_PATH);
+  private static final Map.Entry<String, Path> SECOND_ENTRY =
+      Map.entry("second", SECOND_PLUGIN_PATH);
+  private static final Map.Entry<String, Path> THIRD_ENTRY = Map.entry("third", THIRD_PLUGIN_PATH);
+  private static final Map.Entry<String, Path> LAST_ENTRY = Map.entry("last", LAST_PLUGIN_PATH);
+
+  private static final Manifest EMPTY_MANIFEST = newManifest("");
+  private static final Manifest API_MODULE_MANIFEST = newManifest(API_MODULE);
+
+  @Test
+  public void shouldOrderPluginsBasedOnFileName() {
+    PluginOrderComparator comparator =
+        new PluginOrderComparator(ImmutableList.of(), pluginPath -> EMPTY_MANIFEST);
+
+    assertOrder(comparator, List.of(FIRST_ENTRY, LAST_ENTRY), List.of(FIRST_ENTRY, LAST_ENTRY));
+  }
+
+  @Test
+  public void shouldReturnPluginWithApiModuleFirst() {
+    // return empty manifest for the first plugin and manifest with ApiModule for the last
+    PluginOrderComparator.ManifestLoader loader = customLoader(EMPTY_MANIFEST, API_MODULE_MANIFEST);
+
+    PluginOrderComparator comparator = new PluginOrderComparator(ImmutableList.of(), loader);
+
+    assertOrder(comparator, List.of(FIRST_ENTRY, LAST_ENTRY), List.of(LAST_ENTRY, FIRST_ENTRY));
+  }
+
+  @Test
+  public void shouldUseOrderFromOverrideListBasedOnFileName() {
+    ImmutableList<String> pluginOrderOverrides = ImmutableList.of("last", "first");
+
+    PluginOrderComparator comparator =
+        new PluginOrderComparator(pluginOrderOverrides, pluginPath -> EMPTY_MANIFEST);
+
+    assertOrder(comparator, List.of(FIRST_ENTRY, LAST_ENTRY), List.of(LAST_ENTRY, FIRST_ENTRY));
+  }
+
+  @Test
+  public void shouldCombineOrderOverridesAndNaturalFileNaming() {
+    ImmutableList<String> pluginOrderOverrides = ImmutableList.of("third", "second");
+
+    PluginOrderComparator comparator =
+        new PluginOrderComparator(pluginOrderOverrides, pluginPath -> EMPTY_MANIFEST);
+
+    assertOrder(
+        comparator,
+        List.of(FIRST_ENTRY, SECOND_ENTRY, THIRD_ENTRY, LAST_ENTRY),
+        List.of(THIRD_ENTRY, SECOND_ENTRY, FIRST_ENTRY, LAST_ENTRY));
+  }
+
+  @Test
+  public void shouldReturnApiModuleFirstWithOrderOverrides() {
+    ImmutableList<String> pluginOrderOverrides = ImmutableList.of("third", "second");
+    PluginOrderComparator.ManifestLoader loader =
+        pluginPath -> {
+          if (pluginPath.equals(LAST_PLUGIN_PATH)) {
+            return API_MODULE_MANIFEST;
+          }
+          return EMPTY_MANIFEST;
+        };
+
+    PluginOrderComparator comparator = new PluginOrderComparator(pluginOrderOverrides, loader);
+
+    assertOrder(
+        comparator,
+        List.of(FIRST_ENTRY, SECOND_ENTRY, THIRD_ENTRY, LAST_ENTRY),
+        List.of(LAST_ENTRY, THIRD_ENTRY, SECOND_ENTRY, FIRST_ENTRY));
+  }
+
+  private void assertOrder(
+      PluginOrderComparator comparator,
+      List<Map.Entry<String, Path>> input,
+      List<Map.Entry<String, Path>> expected) {
+    TreeSet<Map.Entry<String, Path>> actual = Sets.newTreeSet(comparator);
+    actual.addAll(input);
+
+    assertThat(List.copyOf(actual)).isEqualTo(expected);
+  }
+
+  private static Manifest newManifest(String content) {
+    String withEmptyLine = content + "\n";
+    try {
+      Manifest manifest = new Manifest();
+      manifest.read(new ByteArrayInputStream(withEmptyLine.getBytes(UTF_8)));
+      return manifest;
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  private PluginOrderComparator.ManifestLoader customLoader(
+      Manifest firstManifest, Manifest secondManifest) {
+    return pluginPath -> {
+      if (pluginPath.equals(FIRST_PLUGIN_PATH)) {
+        return firstManifest;
+      }
+      if (pluginPath.equals(LAST_PLUGIN_PATH)) {
+        return secondManifest;
+      }
+      throw new IllegalArgumentException("unsupported path: " + pluginPath);
+    };
+  }
+}
diff --git a/tools/BUILD b/tools/BUILD
index 71ad096e55..ed55a4f8e4 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -7,17 +7,6 @@ load("@rules_java//java:defs.bzl", "java_package_configuration")
 
 exports_files(["nongoogle.bzl"])
 
-default_java_toolchain(
-    name = "error_prone_warnings_toolchain_java11",
-    configuration = NONPREBUILT_TOOLCHAIN_CONFIGURATION,
-    package_configuration = [
-        ":error_prone",
-    ],
-    source_version = "11",
-    target_version = "11",
-    visibility = ["//visibility:public"],
-)
-
 [default_java_toolchain(
     name = "error_prone_warnings_toolchain_java" + VERSION,
     configuration = dict(),
diff --git a/tools/defs.bzl b/tools/defs.bzl
index ff207b38a7..672c6f9431 100644
--- a/tools/defs.bzl
+++ b/tools/defs.bzl
@@ -6,8 +6,6 @@ def gerrit_init():
     """
     protobuf_deps()
 
-    native.register_toolchains("//tools:error_prone_warnings_toolchain_java11_definition")
-
     native.register_toolchains("//tools:error_prone_warnings_toolchain_java17_definition")
 
     native.register_toolchains("//tools:error_prone_warnings_toolchain_java21_definition")
diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py
index a3f4d8fad3..9a28d9de1f 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -49,7 +49,7 @@ opts.add_argument('--name', help='name of the generated project',
 opts.add_argument('-b', '--batch', action='store_true',
                   dest='batch', help='Bazel batch option')
 opts.add_argument('-j', '--java', action='store',
-                  dest='java', help='Post Java 11')
+                  dest='java', help='Post Java 17')
 opts.add_argument('--bazel',
                   help=('name of the bazel executable. Defaults to using'
                         ' bazelisk if found, or bazel if bazelisk is not'