From 2ceb69f30be9d54cdece7484a40b74970cfc6255 Mon Sep 17 00:00:00 2001 From: Serge Bazanski Date: Sun, 8 Oct 2023 12:27:44 +0000 Subject: [PATCH] gerrit: bump to 3.7.5 This involved messing with both of our source-built plugins (owners and oauth). The main issue seems to have been the desync between Jackson as requested by different plugins. Jackson is split into multiple Maven packages, and they all have to be the same version to work together. The oauth plugin was requesting only part of it, and these parts were incompatible with the parts that the owners plugin requested. In addition, we have to make the owners plugin include more bits of Jackson. Without these changes, we would get runtime `java.lang.NoClassDefFoundError: com/fasterxml/jackson/...` errors, which were a symptom of Jackson either not being included fully into the plugin's JAR, or a mixup between Jackson component/package versions. While we're at it, we remove the broken theming attempt. Change-Id: I26531818a395de2a8bb6054d2583881fd1d5b806 Reviewed-on: https://gerrit.hackerspace.pl/c/hscloud/+/1642 Reviewed-by: q3k --- WORKSPACE | 46 +++---- devtools/gerrit/BUILD | 30 +---- devtools/gerrit/entrypoint.sh | 4 - devtools/gerrit/gerrit-oauth-provider/BUILD | 2 + .../external_plugin_deps.bzl | 20 ++- third_party/gerrit_plugins_owner.patch | 124 ++++++++++++++++++ 6 files changed, 159 insertions(+), 67 deletions(-) create mode 100644 third_party/gerrit_plugins_owner.patch diff --git a/WORKSPACE b/WORKSPACE index 82a3fde2..826c28d8 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -199,11 +199,11 @@ container_pull( ) container_pull( - name = "gerrit-3.3.2", - digest = "sha256:f7fc8b749706f38475f94191117091973914a8f084e69518deff7dbc9c2c557d", + name = "gerrit-3.7.5", + digest = "sha256:3705f3cb365bf53a8e310dd7e09c8fb44a073e1266dc0e713b6fdc04c91ab9f2", registry = "index.docker.io", repository = "gerritcodereview/gerrit", - tag = "3.3.2-ubuntu20", + tag = "3.7.5-ubuntu20", ) # third_party/factorio @@ -215,50 +215,34 @@ factorio_repositories() git_repository( name = "com_googlesource_gerrit_bazlets", - commit = "a511f3c90129d7de7ae67c0637001162980c08d5", + commit = "e68cc7a45d9ee2b100024b9b12533b50a4598585", remote = "https://gerrit.googlesource.com/bazlets", shallow_since = "1606931369 -0600", - patch_args = ["-p1"], - patches = [ - "//third_party:bazlets_py3.patch", - ], ) +# Force newer version of Gerrit plugin API. load("@com_googlesource_gerrit_bazlets//:gerrit_api.bzl", "gerrit_api") - -gerrit_api() +gerrit_api( + version = "3.7.5", + plugin_api_sha1 = "1ea41f95da74045c2cdb30d359041a79b61e72ff", + acceptance_framework_sha1 = "47019cf43ef7e6e8d2d5c0aeba0407d23c93bbbb", +) load("//devtools/gerrit/gerrit-oauth-provider:external_plugin_deps.bzl", gerrit_oauth_deps = "external_plugin_deps") gerrit_oauth_deps(omit_commons_codec = False) -# Gerrit 3.3.2 built by q3k, backported with fix for 'empty reviewers column' bug. -# See: https://bugs.chromium.org/p/gerrit/issues/detail?id=13899 -# Override can be removed once we update to > 3.3.2, as the fix for this is -# pending for the 3.3 branch. -# -# Built from v3.3.2 tag at df0507df5917fd78af01aee2495b2663530d52d1 -# Cherry-picked fix from 8731af3ae785efe9ecff7f3d04302b6b01c4fc0b -# Resulted in commit 5949bfb86e62a32a95293e339ed86bfe52a283e9 -# Built against Java 8: -# bazel build --java_toolchain //tools:error_prone_warnings_toolchain :release -# -http_file( - name = "org_q3k_gerrit_3_3_2_backport", - downloaded_file_path = "gerrit.war", - sha256 = "d1839d691a8534f4ccb27bed9a98281e45972fbebec50d004cecd4d5da2b15a6", - urls = [ - "https://object.ceph-waw3.hswaw.net/q3k-personal/d1839d691a8534f4ccb27bed9a98281e45972fbebec50d004cecd4d5da2b15a6.war", - ], -) - # Gerrit OWNERS plugins external repositories git_repository( name = "com_googlesource_gerrit_plugin_owners", - commit = "17817c9e319073c03513f9d5177b6142b8fd567b", + commit = "6db2d3f048f56fa49469d315b716787d21f8393f", remote = "https://gerrit.googlesource.com/plugins/owners/", shallow_since = "1593642470 +0200", + patch_args = ["-p1"], + patches = [ + "//third_party:gerrit_plugins_owner.patch", + ], ) load("@com_googlesource_gerrit_plugin_owners//:external_plugin_deps_standalone.bzl", gerrit_owners_deps = "external_plugin_deps_standalone") diff --git a/devtools/gerrit/BUILD b/devtools/gerrit/BUILD index 79cf6cb6..4c8b2400 100644 --- a/devtools/gerrit/BUILD +++ b/devtools/gerrit/BUILD @@ -2,7 +2,7 @@ load("@io_bazel_rules_docker//container:container.bzl", "container_image", "cont container_image( name = "with_plugins", - base = "@gerrit-3.3.2//image", + base = "@gerrit-3.7.5//image", # we cannot drop it directly in /var/gerrit/plugins as that changes the # directory owner to 0:0 and then breaks the gerrit installer that wants # to overwrite plugins. @@ -14,30 +14,8 @@ container_image( ) container_image( - name = "with_theme", + name = "3.7.5-r4", base = ":with_plugins", - directory = "/var/gerrit-theme", - files = [ - "theme/etc/GerritSite.css", - "theme/static/pepper-icon.png", - ], -) - -# Add gerrit 3.3.2 with backported fix. See org_q3k_gerrit_3_3_2_backport in -# WORKSPACE for more background. -# TODO(q3k): drop once gerrit > 3.3.2 lands. -container_image( - name = "with_gerrit_override", - base = "with_theme", - directory = "/var/gerrit/bin/", - files = [ - "@org_q3k_gerrit_3_3_2_backport//file:gerrit.war", - ], -) - -container_image( - name = "3.3.2-r4", - base = ":with_gerrit_override", directory = "/", entrypoint = ["/entrypoint.sh"], files = [":entrypoint.sh"], @@ -46,8 +24,8 @@ container_image( container_push( name = "push", format = "Docker", - image = ":3.3.2-r4", + image = ":3.7.5-r4", registry = "registry.k0.hswaw.net", repository = "q3k/gerrit", - tag = "3.3.2-r4", + tag = "3.7.5-r4", ) diff --git a/devtools/gerrit/entrypoint.sh b/devtools/gerrit/entrypoint.sh index fe903a07..49eb41e1 100755 --- a/devtools/gerrit/entrypoint.sh +++ b/devtools/gerrit/entrypoint.sh @@ -21,10 +21,6 @@ cp /var/gerrit-plugins/* /var/gerrit/plugins/ mkdir -p /var/gerrit/hooks/ cp /var/gerrit-hooks/* /var/gerrit/hooks/ -mkdir -p /var/gerrit/static -cp -r /var/gerrit-theme/*png /var/gerrit/static/ -cp -r /var/gerrit-theme/*css /var/gerrit/etc/ - echo "Starting config updater..." # Keep copying config over in background. We cannot run directly from # the configmap filesystem as gerrit really wants a read-write FS. diff --git a/devtools/gerrit/gerrit-oauth-provider/BUILD b/devtools/gerrit/gerrit-oauth-provider/BUILD index e19169f5..aa8f9236 100644 --- a/devtools/gerrit/gerrit-oauth-provider/BUILD +++ b/devtools/gerrit/gerrit-oauth-provider/BUILD @@ -20,7 +20,9 @@ gerrit_plugin( resources = glob(["src/main/resources/**/*"]), deps = [ "@commons-codec//jar:neverlink", + "@jackson-core//jar", "@jackson-databind//jar", + "@jackson-annotations//jar", "@scribejava-core//jar", ], ) diff --git a/devtools/gerrit/gerrit-oauth-provider/external_plugin_deps.bzl b/devtools/gerrit/gerrit-oauth-provider/external_plugin_deps.bzl index 52794350..ce8c30b3 100644 --- a/devtools/gerrit/gerrit-oauth-provider/external_plugin_deps.bzl +++ b/devtools/gerrit/gerrit-oauth-provider/external_plugin_deps.bzl @@ -1,6 +1,7 @@ load("//devtools/gerrit/gerrit-oauth-provider/tools/bzl:maven_jar.bzl", "maven_jar") def external_plugin_deps(omit_commons_codec = True): + # Keep in sync with gerrit owners plugin! JACKSON_VERS = "2.10.2" maven_jar( name = "scribejava-core", @@ -8,17 +9,24 @@ def external_plugin_deps(omit_commons_codec = True): sha1 = "ed761f450d8382f75787e8fee9ae52e7ec768747", ) maven_jar( - name = "jackson-annotations", - artifact = "com.fasterxml.jackson.core:jackson-annotations:" + JACKSON_VERS, - sha1 = "3a13b6105946541b8d4181a0506355b5fae63260", + name = "jackson-core", + artifact = "com.fasterxml.jackson.core:jackson-core:" + JACKSON_VERS, + sha1 = "73d4322a6bda684f676a2b5fe918361c4e5c7cca", ) maven_jar( name = "jackson-databind", artifact = "com.fasterxml.jackson.core:jackson-databind:" + JACKSON_VERS, sha1 = "0528de95f198afafbcfb0c09d2e43b6e0ea663ec", - deps = [ - "@jackson-annotations//jar", - ], + ) + maven_jar( + name = "jackson-annotations", + artifact = "com.fasterxml.jackson.core:jackson-annotations:" + JACKSON_VERS, + sha1 = "3a13b6105946541b8d4181a0506355b5fae63260", + ) + maven_jar( + name = "jackson-dataformat-yaml", + artifact = "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:" + JACKSON_VERS, + sha1 = "8a6a6ad573b48dc3b623414719428ecbfeb259a3", ) if not omit_commons_codec: maven_jar( diff --git a/third_party/gerrit_plugins_owner.patch b/third_party/gerrit_plugins_owner.patch new file mode 100644 index 00000000..257b6fb1 --- /dev/null +++ b/third_party/gerrit_plugins_owner.patch @@ -0,0 +1,124 @@ +Synchronzies Jackson version with oauth plugin, bumps API to 3.7. + +diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl +index 5ad1930..712e0d3 100644 +--- a/external_plugin_deps.bzl ++++ b/external_plugin_deps.bzl +@@ -1,30 +1,30 @@ + load("//tools/bzl:maven_jar.bzl", "maven_jar") + +-JACKSON_VER = "2.9.7" ++JACKSON_VER = "2.10.2" + + def external_plugin_deps(): + maven_jar( + name = "jackson-core", + artifact = "com.fasterxml.jackson.core:jackson-core:" + JACKSON_VER, +- sha1 = "4b7f0e0dc527fab032e9800ed231080fdc3ac015", ++ sha1 = "73d4322a6bda684f676a2b5fe918361c4e5c7cca", + ) + + maven_jar( + name = "jackson-databind", + artifact = "com.fasterxml.jackson.core:jackson-databind:" + JACKSON_VER, +- sha1 = "e6faad47abd3179666e89068485a1b88a195ceb7", ++ sha1 = "0528de95f198afafbcfb0c09d2e43b6e0ea663ec", + ) + + maven_jar( + name = "jackson-annotations", + artifact = "com.fasterxml.jackson.core:jackson-annotations:" + JACKSON_VER, +- sha1 = "4b838e5c4fc17ac02f3293e9a558bb781a51c46d", ++ sha1 = "3a13b6105946541b8d4181a0506355b5fae63260", + ) + + maven_jar( + name = "jackson-dataformat-yaml", + artifact = "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:" + JACKSON_VER, +- sha1 = "a428edc4bb34a2da98a50eb759c26941d4e85960", ++ sha1 = "8a6a6ad573b48dc3b623414719428ecbfeb259a3", + ) + + maven_jar( +diff --git a/external_plugin_deps_standalone.bzl b/external_plugin_deps_standalone.bzl +index b2964a5..9f56994 100644 +--- a/external_plugin_deps_standalone.bzl ++++ b/external_plugin_deps_standalone.bzl +@@ -5,7 +5,7 @@ PROLOG_VERS = "1.4.3" + PROLOG_REPO = GERRIT + + def external_plugin_deps_standalone(): +- external_plugin_deps(omit_jackson_core = False) ++ external_plugin_deps() + + maven_jar( + name = "prolog-runtime", +diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java +index b83d7a8..d15da8e 100644 +--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java ++++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java +@@ -18,7 +18,7 @@ package com.googlesource.gerrit.owners; + + import com.google.common.collect.ImmutableSet; + import com.google.gerrit.extensions.annotations.Listen; +-import com.google.gerrit.server.rules.prolog.PredicateProvider; ++import com.google.gerrit.server.rules.PredicateProvider; + import com.google.inject.Inject; + import com.googlesource.gerrit.owners.common.Accounts; + import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache; +diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java +index 81aef29..7d2c4d5 100644 +--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java ++++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java +@@ -17,7 +17,7 @@ package com.googlesource.gerrit.owners; + + import com.google.common.flogger.FluentLogger; + import com.google.gerrit.extensions.registration.DynamicSet; +-import com.google.gerrit.server.rules.prolog.PredicateProvider; ++import com.google.gerrit.server.rules.PredicateProvider; + import com.google.inject.AbstractModule; + import com.google.inject.Inject; + import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache; +diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java +index 9d5b3d9..c4d27e4 100644 +--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java ++++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java +@@ -21,8 +21,8 @@ import com.google.gerrit.metrics.Timer0; + import com.google.gerrit.server.git.GitRepositoryManager; + import com.google.gerrit.server.patch.filediff.FileDiffOutput; + import com.google.gerrit.server.project.ProjectState; +-import com.google.gerrit.server.rules.prolog.StoredValue; +-import com.google.gerrit.server.rules.prolog.StoredValues; ++import com.google.gerrit.server.rules.StoredValue; ++import com.google.gerrit.server.rules.StoredValues; + import com.googlecode.prolog_cafe.lang.Prolog; + import com.googlesource.gerrit.owners.common.Accounts; + import com.googlesource.gerrit.owners.common.PathOwners; +diff --git a/owners/src/main/java/gerrit_owners/PRED_code_review_user_1.java b/owners/src/main/java/gerrit_owners/PRED_code_review_user_1.java +index 92df174..a2dd06b 100644 +--- a/owners/src/main/java/gerrit_owners/PRED_code_review_user_1.java ++++ b/owners/src/main/java/gerrit_owners/PRED_code_review_user_1.java +@@ -20,7 +20,7 @@ import com.google.gerrit.entities.LabelId; + import com.google.gerrit.entities.LabelType; + import com.google.gerrit.entities.LabelValue; + import com.google.gerrit.server.query.change.ChangeData; +-import com.google.gerrit.server.rules.prolog.StoredValues; ++import com.google.gerrit.server.rules.StoredValues; + import com.googlecode.prolog_cafe.exceptions.PrologException; + import com.googlecode.prolog_cafe.lang.IntegerTerm; + import com.googlecode.prolog_cafe.lang.JavaObjectTerm; +diff --git a/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java b/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java +index 11f1a74..cbb23cf 100644 +--- a/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java ++++ b/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java +@@ -18,8 +18,8 @@ import static com.googlesource.gerrit.owners.common.StreamUtils.iteratorStream; + + import com.google.gerrit.entities.Account; + import com.google.gerrit.server.IdentifiedUser; +-import com.google.gerrit.server.rules.prolog.PrologEnvironment; +-import com.google.gerrit.server.rules.prolog.StoredValues; ++import com.google.gerrit.server.rules.PrologEnvironment; ++import com.google.gerrit.server.rules.StoredValues; + import com.googlecode.prolog_cafe.exceptions.PInstantiationException; + import com.googlecode.prolog_cafe.exceptions.PrologException; + import com.googlecode.prolog_cafe.lang.Operation;