Merge branch 'stable-3.10' * stable-3.10: Allow influence plugin load order from gerrit.config Extract plugin load order comparator into separate class gr-main-header: fix header to wrap the better Drop support for Java 11 Revert "Fix accidental bump to require java 17" WorkQueue: Refactor to call get() directly on Extension Set version to 3.10.0-SNAPSHOT Set version to 3.10.0-rc4 gr-settings-view: Move preference to its own module/element Add baseUrl to INITIAL_DATA in IndexHtmlUtil Add `await` Fix clearing cache in gr-rest-api Remove reload-diff-preference unneeded event Set version to 3.10.0-SNAPSHOT Set version to 3.10.0-rc3 Specify in docs that robot comments are deprecated Fix redirecting to login with a base url Save only previewed suggestions Fix inability to remove AI suggestions after adding user suggestions Fix comment widgets losing focus by stable sorting Prevent automatic expansion of attention section for large account list Show Edit button under commit message when in Edit mode Show Edit File Actions for commit message Enable applying fixes for drafts Do not disable apply fix when on Edit patchset Add tooltip for copy button in User Suggested Edit Rename ML-suggested edits to AI-suggested fixes Show Apply Fix Suggestion only to owner Fix buttons on header disappearing Release-Notes: skip Change-Id: I706510e1427d508bedf081b57267f93afcade077
diff --git a/.bazelproject b/.bazelproject index ad7b022..0f2ff90 100644 --- a/.bazelproject +++ b/.bazelproject
@@ -16,7 +16,7 @@ targets: //...:all -java_language_level: 11 +java_language_level: 17 workspace_type: java
diff --git a/.bazelrc b/.bazelrc index 9662078..480bea7 100644 --- a/.bazelrc +++ b/.bazelrc
@@ -32,25 +32,6 @@ 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 dbda410..b338148 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt
@@ -4648,6 +4648,37 @@ 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 04309e5..ef99d80 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 c04db2c..e824a95 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 d51ee5e..307ec5c 100644 --- a/java/com/google/gerrit/server/git/WorkQueue.java +++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -504,11 +504,11 @@ } 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 e122d8d..7c7d8d5 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.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.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.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 @@ private final MandatoryPluginsCollection mandatoryPlugins; private final UniversalServerPluginProvider serverPluginFactory; private final GerritRuntime gerritRuntime; + private final PluginOrderComparator pluginOrderComparator; @Inject public PluginLoader( @@ -122,6 +120,10 @@ 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 @@ 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 0000000..45ff9eb --- /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 0000000..4f7402a --- /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 71ad096..ed55a4f 100644 --- a/tools/BUILD +++ b/tools/BUILD
@@ -7,17 +7,6 @@ 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 ff207b3..672c6f9 100644 --- a/tools/defs.bzl +++ b/tools/defs.bzl
@@ -6,8 +6,6 @@ """ 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 a3f4d8f..9a28d9d 100755 --- a/tools/eclipse/project.py +++ b/tools/eclipse/project.py
@@ -49,7 +49,7 @@ 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'